Repository navigation
Merge a Cyd Bluesky archive into the account that holds its identity - #115
Merged
Merged
Conversation
#97 asks for the entry point #93 removed and for what happens when a picked archive belongs to an identity this installation already has. Routing an archive and merging one are the same decision, so they land together: `chooseBlueskyArchiveImportDestination` reads the DID intake reported and says restore (#96), merge, or reconcile. Nothing looks at the file's name, and nothing is unpacked before intake has verified it. `services/archive-merge` is the merge. It is a union rather than a replacement, decided as a plan before anything is written: rows sharing a stable Bluesky identifier collapse into one, a flag either side set stays set, a deletion either side observed keeps the earlier time, and an archive that is quieter than the account never turns what the account knows back into absence. Because the plan only carries rows that would actually change, importing the same archive twice writes nothing at all, which is the acceptance criterion stated as a property of the code. The account's own columns survive an `INSERT OR REPLACE` because the merge names them: a preserved post stays preserved, a downloader's attempt count stays its own. A merge can bring back records somebody deleted from Cyd on purpose, so the preview names them before the commit does anything (ADR 0018). The commit re-reads the account rather than trusting the preview, so a save that ran in between is merged rather than overwritten. What the ports cannot do is the rest of the promise: there is no way from a merge to write settings, a schedule, a handle or a Bluesky connection, so an existing identity keeps its UUID and everything hanging off it. Duplicate DIDs are the one case with no unambiguous destination (ADR 0011). Mobile has enforced one account per DID since its first migration, so the state only arrives from outside — a database restored from an OS backup, say — and reconciliation shows what each account holds, takes the surviving UUID and the settings the person chose, unions the rest through the same rules, moves the media across, and puts the unique index back, which is also the check that the duplicates are gone. Retiring a local-account UUID retires a server-side reminder with it, so the import tells the server; a reminder still only opens a review. The interchange reader, the translation and the media pass are now shared with restore rather than copied, so the version 2 adapter stays one thing. merge.test.ts and duplicates.test.ts drive the committed real-data fixtures through intake, restore, and then a second import of the same archive, and assert through Mobile's own browse queries and the files on disk. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two of these were bugs the fixtures could not see, because the Node double was kinder than the device. Reconciliation removed a duplicate Bluesky local account by passing a null account id, which the device adapter read as "no row to delete" — so on a phone the losing account's rows survived while its storage went, leaving the identity still held twice and a listed account pointing at nothing. The port now removes by local-account UUID, which is what reconciliation actually knows, and takes the Bluesky connection with it. Putting the unique index back was the other half of the illusion: Mobile has carried that index since its first migration, so `CREATE UNIQUE INDEX IF NOT EXISTS` can only fail on the databases that lost it, and could never be the proof the docblock claimed. Reconciliation now counts the holders afterwards and refuses to report success while the identity is still held twice. Cancelling during a merge preview did not cancel. The preview cannot be called back, so it finished and reopened itself over staging that cancelling had just deleted; committing from there failed. Imports are now generational: walking away moves the count on, a step that lands afterwards reports to nobody, and intake is asked to stop between payloads rather than only ignored. Cancelling before intake even started used to leave a fully prepared staging directory behind, which the same counter now clears. An archive that never captured engagement counts translates them to zero, and a newer one would write those zeros over counts this device had seen — absence replacing a populated value, which is exactly what the union is not allowed to do. Counts get their own rule, and it is the safe direction: a count that really fell to zero is corrected by the next save. Reconciliation now names the records keeping one account would bring back, rather than only counting what each holds, so the choice between two accounts carries the same disclosure an archive merge does (ADR 0018). Where the survivor inherits a schedule the other account owned, Cyd registers the push token against the surviving UUID before sending the schedule, or the server would hold a reminder it cannot deliver; permission is only read, never asked for, because an import is the wrong moment for a notifications prompt. The rest is what the review called: the unavailable-asset rule and the content-addressed path prediction live in the shared media module rather than in two places, the merge ports lost a clock nothing used, the import hook has one seam for reminders instead of two, and a result type lost two fields nothing read. use-bluesky-archive-import.test.ts covers the entry point the acceptance criteria rest on: restoring without asking, stopping for a decision, and walking away leaving nothing on screen or in staging. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four things the fixtures could not see, found by restoring the committed archives onto a phone and following where they led. A restored Bluesky local account was showing a blank name and a "?" where its avatar goes. The archive is honest: an account that never set a display name exports an empty string, which is an absence rather than a name — the same thing the merge's own `populated` rule refuses to treat as an observation. Restore now stores it as nothing, and the fallbacks both account screens had their own broken copy of live in one place: an account is named by its handle when it has nothing better, and one with no avatar gets a person rather than a punctuation mark. A Cyd Bluesky archive never carries an avatar at all, so this is what a restored account always looks like until it is connected. Signing in from the import dialog authorized the account and left it asking to be authorized again. Whether Cyd can act on a Bluesky local account is cached in that account's own database, and the check returns the cached "signed out" without reading the stored Bluesky connection unless it is forced — which is exactly what an account that has never been connected holds. `connectBlueskyAccount` makes the forced check part of signing in rather than something each caller remembers, which matters most for the caller furthest from the account screen: an account restored from an archive is signed in from the import that created it. `add-account.tsx` still does this inline and could adopt it. Merging an archive that carried a file an earlier export could not described itself as changing nothing. The merge was right — the plan upgraded the failed download and the commit wrote it — but the sentence read three of the summary's nine tables and missed the one that changed. Both the preview and the report after it now ask `totalMergeChanges`, which groups every table into what somebody recognises: records, files, and the rows a record needs to render. Its map is exhaustive over the summary's own keys, so a tenth table will not compile until somebody says what it is. The restore dialog broke the account's handle across two lines mid-word. The title is now the verb alone with the handle under it, and a handle too long for one line breaks before a dot instead of wherever it runs out of room. Nothing is truncated: the handle is how somebody knows which account this is. The dialog also offers the Bluesky sign-in a restored account needs, instead of only "Done". merge.test.ts gains the recovery somebody actually performs — exporting again once a failed download finished, and importing that over the account the first export built. Every existing merge test restores and merges the same fixture, which is the idempotent path, so nothing exercised an upgrade at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Browse renders a record's author out of the account's own `profile` table, not out of the account row, so the two can disagree — and after a restore they do. A Cyd Bluesky archive cannot carry an avatar at all, and an account that never set a display name exports an empty one, so a restored account browsed its own posts under the name the archive carried while its header showed the name Bluesky had just given it. Saving posts would have corrected it eventually, as a side effect of saving their authors, but nobody should have to run a save to see their own name. `upsertProfileRow` comes out of `PostPersistence` as a function the class still delegates to, taking the four fields it stores rather than one shape of profile view, so the detailed profile an account fetches about itself fits as well as the basic one attached to a post. `refreshOwnProfile` writes it, and signing in calls it: that is the first moment Cyd has anything newer than the archive. Signing in now happens in one place. The previous commit left the account screen and the add-account screen doing the sequence by hand, and the account screen promptly proved why that does not hold: reauthenticating from it authorized the account and refreshed nothing, because refreshing was somewhere else. Both now call `connectBlueskyAccount`, which returns the status the account screen shows. A profile that will not refresh no longer takes the sign-in with it. Post cards showed an empty circle for an author with no avatar, which is every author an archive restored. They get the same person icon the account screens use. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The merge preview named every record coming back, under a heading warning that importing brings back anything deleted from Cyd on purpose. Mobile has no Bluesky local deletion — setting that state up for a device test took SQL — so the warning described a situation the app cannot produce, and the list under it was a wall of `posts: <text>` nobody could act on. Both are gone, and ADR 0018 is amended rather than left contradicted: the preview counts rather than names, and naming becomes worth its screen again if Mobile ever lets somebody remove Bluesky saved data without removing it from Bluesky. The plan still carries the records, so that would be a presentation change. It carries them for another reason too. Choosing between two Bluesky local accounts that hold one identity is a different question from what an archive would add, and there an example of what one account holds and the other does not is how somebody tells them apart. That screen keeps its example, with the reasoning it actually rests on. In place of the list, a line per kind: "3 posts will be added", "1 repost will be added", nothing at all for a kind with nothing to add. Mobile keeps posts, reposts, likes and bookmarks in one table and tells them apart by column, so the plan classifies the added rows exactly as `buildTotalCountQuery` does and takes the account DID to do it — a post is the account's own rather than somebody else's that it liked only if the DID says so. A row can land under two headings, which is what Browse shows: the fixture account's own post that it also reposted appears in the Posts tab and the Reposts tab, and now in both lines of the preview. Agreeing with the tabs somebody will go and look at beats adding up to a tidier total. The report after the merge uses the same headings in the past tense. "Your settings, schedule and Bluesky connection are left alone" is gone from the preview and from the report. A merge has no port that can reach any of them, so it was a sentence that was never news. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Importing an archive into an account, then opening that account, broke Browse
and the auth-status check with
Call to function 'NativeDatabase.prepareAsync' has been rejected.
→ Caused by: java.lang.NullPointerException
minutes after the import that caused it, on a controller the manager had
cached long before.
expo-sqlite keeps one native connection per database path and hands it to
everyone who opens it with the same options, but each JS handle also owns it:
`NativeDatabase` is a `SharedRef` whose `sharedObjectDidRelease` closes the
connection, and that runs when the handle is garbage collected regardless of
who else is still holding one. So opening an already-open database is not a
second connection. It is a promise that somebody's queries will start failing
at whatever later moment the collector runs, which is why this looked like an
unrelated screen breaking on its own.
`BaseAccountController` already knew this and kept a map of one handle per
file, refcounted. The archive adapters did not: restore and merge both called
`openDatabaseAsync` themselves and closed what they got, and restore also
closed the database it had just created for a new account. The map moves to
`database/account-db/shared-handles.ts`, the controller delegates to it, and
both adapters borrow — their port's `close` hands the handle back rather than
closing it. Acquiring registers the handle before the open resolves, so two
callers arriving together share one connection rather than racing into two. A
controller holding a handle that never came from the map still closes it
itself, since that one is nobody else's.
The Node test double has no shared-connection semantics, so no fixture could
have caught this. Seven tests cover the map directly.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reviewing the reconciliation screen on a device raised the obvious question: can Cyd Mobile actually hold one Bluesky identity in two Bluesky local accounts? It cannot, and never could. `bsky_account.did` has carried its unique index since the same migration that added the column — both in one statement list, so in one transaction, in the initial commit and still after `dc80de5 "Flatten migrations"` moved them into version 1 together. Every write path reuses the row the DID or handle already names: `findBlueskyLocalAccount` and `saveAuthenticatedBlueskyAccount` both match `did = ? OR handle = ?`, and archive restore refuses an identity this installation holds and merges instead. Restoring an OS backup — the justification this branch gave — restores the index along with everything else, so it does not produce the state either. Setting the state up to test it took `DROP INDEX` over adb. ADR 0011 said Mobile's migration detects duplicates and flags them. No migration does, and the ADR appears to describe the shared Cyd contract's history rather than Mobile's. It is marked superseded here and left standing for clients whose schema does permit the state. So the feature goes: `duplicates.ts` and its tests, the reconciling screen and hook state, the two ports that existed only to read and write another account's settings, the ports that removed an account and put the unique index back, and `scheduled-reminder-sync.ts`, which existed only to retire a local-account UUID that only reconciliation retired. `chooseBlueskyArchiveImportDestination` now refuses an archive it cannot place rather than choosing between two accounts: a database holding one identity twice is one Cyd did not build and cannot reason about, and saying so beats merging into whichever row came back first. What is reachable, and deliberately still unhandled, is an old account with no DID beside a newer one that has it. Nothing proves those are one identity, and guessing from a handle is what the DID exists to avoid. Reconciliation did not handle that case either — `findDuplicateBlueskyIdentities` skipped null DIDs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The merge preview counts what it would add rather than naming it, so a test called "names them before committing" says something the code no longer does. It asserts the plan, which still works the records out. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
micahflee
changed the title
The import modal had no `onRequestClose`, so the Android back button did nothing during an import while every other modal in the repo handles it. Back now does what the nearest button does: cancels a question, dismisses a report, and is ignored only while a merge is being written, for the same reason there is no button to press there. The modal was also rendered inside the account bar, which returns null on the account screen — so an import started from the accounts list went invisible if somebody opened the account while it ran, and carried on without them. It renders whether or not the bar does. `summary.restorations` lost its last reader when the preview stopped naming records, and was still working out and carrying up to fifty of them on every merge. The plan keeps the added rows it counts from, so ADR 0018's point — that naming them again would be a presentation change over rows already in hand — still holds. The test that asserted the named records now asserts the outcome instead: the three posts that come back are the three that went. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #97.
#93 removed the prototype import from the accounts-list menu, and #95 and #96 replaced its internals without restoring a call site, so nothing in the app could start an import. This adds the entry point back and the half of the import that #96 refused: an archive whose Bluesky identity this installation already holds.
Routing an archive and merging one are the same decision, so they land together. Intake reports the DID, and
chooseBlueskyArchiveImportDestinationsays restore or merge. Nothing reads the picked file's name, and nothing is unpacked before intake has verified it against the manifest.The merge
services/archive-mergeis a union rather than a replacement, decided as a plan before anything is written:A live account's own columns survive an
INSERT OR REPLACEbecause the merge names them: a preserved post stays preserved, a downloader's attempt count stays its own. The commit re-reads the account rather than trusting the preview, so a save that ran in between is merged rather than overwritten. What the ports cannot do is the rest of the promise: there is no way from a merge to write settings, a schedule, a handle, or a Bluesky connection.The preview says what it would add, a line per kind, in the words Browse uses — "3 posts will be added", "1 repost will be added", nothing at all for a kind with nothing to add. Posts, reposts, likes and bookmarks share one table and are told apart by column, so the plan classifies them exactly as
buildTotalCountQuerydoes and takes the account DID to do it. A row can land under two headings, which is what Browse shows: an account's own post that it also reposted appears in both tabs, and in both lines.What this branch stopped doing
Naming the records coming back. The preview used to list them under a warning about importing bringing back anything deleted from Cyd on purpose. Mobile has no Bluesky local deletion — producing that state for a device test took SQL — so the warning described a situation the app cannot reach. ADR 0018 is amended: the preview counts. The plan still works the records out, so naming them stays a presentation change if that ever changes.
Reconciling duplicate DIDs. ADR 0011 has Mobile detecting local accounts that share a Bluesky DID and reconciling them. No migration ever did, and none could:
bsky_account.didhas carried its unique index since the same migration that added the column, in the same statement list and so the same transaction. Every write path reuses the row the DID or handle already names, restore refuses an identity this installation holds, and restoring an OS backup restores the index with it. Setting the state up to test it tookDROP INDEXover adb. The feature is gone — 1,300 lines with its screen, its ports, its tests and the reminder sync that existed only to retire a UUID only it retired.chooseBlueskyArchiveImportDestinationnow refuses an archive it cannot place rather than choosing between two accounts. ADR 0011 is marked superseded here and left standing for clients whose schema permits the state.What is reachable, and deliberately still unhandled, is an old account with no DID beside a newer one that has it. Nothing proves those are one identity, and guessing from a handle is what the DID exists to avoid.
What running it on a phone found
Four things the fixtures could not see, in the order they turned up.
A restored account looked anonymous. An account that never set a display name exports an empty string, which is an absence rather than a name — the same thing the merge's own
populatedrule refuses to treat as an observation. Restore stores it as nothing now, and the fallbacks both account screens had their own broken copy of live in one place: an account is named by its handle when it has nothing better, and one with no avatar gets a person icon rather than a "?". A Cyd Bluesky archive cannot carry an avatar at all, so this is what a restored account always looks like until it is connected.Signing in did not take. The restore dialog offers a Bluesky sign-in, since a restored account holds somebody's data and no authorization to act on it. It authorized the account and left it asking to be authorized again: whether Cyd can act on an account is cached in that account's own database, and the check returns the cached "signed out" without reading the stored connection unless it is forced — exactly what an account that has never been connected holds.
connectBlueskyAccountmakes the forced check part of signing in, and is now the only way in; the account screen and the add-account screen had been doing the sequence by hand, and the account screen promptly proved why. It also writes the profile it just fetched into the account's own database, because Browse renders authors from there and a restored account's copy is whatever the archive carried.A merge that recovered a file said it would change nothing. The two committed fixtures differ by one asset, which is the recovery somebody actually performs: export again once a failed download finished. The merge was right and the sentence was wrong — it read three of the summary's nine tables. Both the preview and the report now ask
totalMergeChanges, whose category map is exhaustive over the summary's own keys, so a tenth table will not compile until somebody says what it is.Importing, then opening the account, broke Browse minutes later.
Call to function 'NativeDatabase.prepareAsync' has been rejected → java.lang.NullPointerException, on a controller cached long before. expo-sqlite hands every caller the same native connection per path, but each JS handle also owns it:NativeDatabaseis aSharedRefwhosesharedObjectDidReleasecloses the connection when the handle is garbage collected, regardless of who else holds one.BaseAccountControlleralready kept one refcounted handle per file for this reason; the archive adapters opened their own and closed it. The map moves todatabase/account-db/shared-handles.ts, the controller delegates to it, and both adapters borrow — their port'sclosehands the handle back. This is the one change here that is not about importing: it was latent for anything that opened an account database twice, and the import is simply the first thing that did.Tests
merge.test.tsdrives the committed real-data fixtures through intake, restore, and a second import of the same archive, asserting through Mobile's own browse queries and the files on disk — including the incomplete-to-complete upgrade, which nothing covered before, because every other merge test restores and merges the same fixture and so only ever exercises the idempotent path.merge-plan.test.tspins the union rules and the per-kind counting.use-bluesky-archive-import.test.tscovers the entry point: restoring without asking, stopping for a decision, and walking away leaving nothing on screen or in staging.shared-handles.test.tsandbluesky-sign-in.test.tscover the two device-only failures above, neither of which any fixture could reach: the Node double has no shared-connection semantics, and no cached auth status.The interchange reader, the translation, and the media pass are shared with restore rather than copied, so the version 2 adapter stays one thing.
(This was written by an LLM.)
🤖 Generated with Claude Code