From c22a75b7f0a068782f95076032d8867edd0d85e5 Mon Sep 17 00:00:00 2001 From: Joep Meindertsma Date: Fri, 25 Sep 2026 08:42:47 +0000 Subject: [PATCH 1/3] fix(lib): say what actually blocks disconnecting a workspace Turning workspace sync off answered all three of its preconditions with "Open this drive with local storage available before disconnecting". Two of them have nothing to do with local storage, so someone signed out, or on a server this client holds no live connection to, was told to do something they had already done. Each precondition now names itself. The local database is waited for rather than refused: it attaches a few hundred milliseconds after a page load and again after every sign-in, and a click inside that window was rejected outright. A websocket that exists but is not open is reported as such, instead of letting the inventory request fail on the raw "WebSocket is not open". Also normalize the local-only drive set on every write and on rehydrate. Every reader looks through normalizeSubject, so a trailing slash or a legacy did:ad: spelling could store a key nothing found again, and a resource inside a disconnected workspace could still try the server. --- browser/CHANGELOG.md | 14 +++ browser/lib/src/make-drive-local.test.ts | 111 +++++++++++++++++++++++ browser/lib/src/store.ts | 81 ++++++++++++++--- 3 files changed, 195 insertions(+), 11 deletions(-) create mode 100644 browser/lib/src/make-drive-local.test.ts diff --git a/browser/CHANGELOG.md b/browser/CHANGELOG.md index 211cfe819..aeff1020d 100644 --- a/browser/CHANGELOG.md +++ b/browser/CHANGELOG.md @@ -4,6 +4,20 @@ This changelog covers all five packages, as they are (for now) updated as a whol ## UNRELEASED +- Turning workspace sync off says what is actually in the way. All three of its + preconditions used to answer with "Open this drive with local storage + available before disconnecting", so someone signed out, or on a server this + client holds no live connection to, was told to do something they had already + done. Each one now names itself, and a socket that exists but is not open says + so rather than leaving the attempt to fail on "WebSocket is not open". The + local database is also waited for instead of refused: it attaches a few + hundred milliseconds after a page load and again after every sign-in, and a + click inside that window was rejected outright. +- A local-only workspace is recognised whatever spelling its subject is written + in. The set of disconnected workspaces is read through the canonical form but + was written raw, so a trailing slash or a legacy `did:ad:` identifier could + store a key nothing found again, and a resource inside such a workspace could + still try to reach the server. - A private workspace the app has to recreate is titled after whoever it belongs to, like the one onboarding and an accepted invitation already make. A returning account on a second device, and a sign-in whose cloud diff --git a/browser/lib/src/make-drive-local.test.ts b/browser/lib/src/make-drive-local.test.ts new file mode 100644 index 000000000..6d462474e --- /dev/null +++ b/browser/lib/src/make-drive-local.test.ts @@ -0,0 +1,111 @@ +import { describe, it } from 'vitest'; +import type { ClientDbWorker } from './client-db.js'; +import { Resource } from './resource.js'; +import { testStore } from './test-store.js'; + +const DRIVE = 'https://example.com/drive/abc'; + +/** Only the two members `makeDriveLocal`'s preconditions read. `setClientDb` + * touches nothing else on a store with no pending writes. */ +const dbStub = (initialized: boolean) => + ({ + isReady: false, + waitForInit: async () => initialized, + }) as unknown as ClientDbWorker; + +/** + * Turning workspace sync off has three preconditions, and they used to share + * one message: "Open this drive with local storage available before + * disconnecting". Signed out, or on a server this client holds no socket for, + * that told the user to do something they had already done. + */ +describe('makeDriveLocal preconditions', () => { + it('asks a signed-out user to sign in', async ({ expect }) => { + const { store } = await testStore(); + store.setAgent(undefined); + + await expect(store.makeDriveLocal(DRIVE)).rejects.toThrow(/Sign in/); + }); + + it('still asks for local storage when there is no database', async ({ + expect, + }) => { + // No `expectClientDb`, so this app has opted out and waiting is pointless. + const { store } = await testStore(); + + await expect(store.makeDriveLocal(DRIVE)).rejects.toThrow(/local storage/); + }); + + it('asks for local storage when the database never initializes', async ({ + expect, + }) => { + const { store } = await testStore(); + store.setClientDb(dbStub(false)); + + await expect(store.makeDriveLocal(DRIVE)).rejects.toThrow(/local storage/); + }); + + it('asks for a server once the database is there', async ({ expect }) => { + const { store } = await testStore(); + store.setClientDb(dbStub(true)); + + // Past the storage guard, and a test store holds no websocket. + await expect(store.makeDriveLocal(DRIVE)).rejects.toThrow( + /Connect to a server/, + ); + }); + + it('waits for a database that attaches after the click', async ({ + expect, + }) => { + // The attach lands a few hundred ms after boot and after every agent + // change. A click inside that window used to be refused outright. + const { store } = await testStore(); + store.expectClientDb(); + + const pending = store.makeDriveLocal(DRIVE); + store.setClientDb(dbStub(true)); + + // Reaching the socket check proves it waited instead of refusing. + await expect(pending).rejects.toThrow(/Connect to a server/); + }); +}); + +/** The set is read through `normalizeSubject`, so it has to be written + * through it too. */ +describe('local-only drive registration', () => { + it('matches a drive registered with a trailing slash', async ({ expect }) => { + const { store } = await testStore(); + store.registerLocalOnlyDrive(`${DRIVE}/`); + + expect(store.isLocalOnlyDrive(DRIVE)).toBe(true); + }); + + it('unregisters either spelling', async ({ expect }) => { + const { store } = await testStore(); + store.registerLocalOnlyDrive(DRIVE); + store.unregisterLocalOnlyDrive(`${DRIVE}/`); + + expect(store.isLocalOnlyDrive(DRIVE)).toBe(false); + }); + + it('matches a resource whose drive propval uses the legacy scheme', async ({ + expect, + }) => { + // A demo workspace registers `did:ad:` and the server writes the same + // spelling into each resource's `drive`. The set holds the canonical + // `atomic:` form, so this branch has to canonicalize before it looks. + const { store } = await testStore(); + store.registerLocalOnlyDrive('did:ad:legacy-drive'); + + const child = new Resource('did:ad:legacy-child'); + child.setStore(store); + child.applyHydratedValues([ + ['https://atomicdata.dev/properties/drive', 'did:ad:legacy-drive'], + ]); + child.loading = false; + store.addResource(child); + + expect(store.isLocalOnlySubject('did:ad:legacy-child')).toBe(true); + }); +}); diff --git a/browser/lib/src/store.ts b/browser/lib/src/store.ts index e7598225d..9df92cb08 100644 --- a/browser/lib/src/store.ts +++ b/browser/lib/src/store.ts @@ -763,7 +763,14 @@ export class Store { const rawLocalOnly = localStorage.getItem('atomic.localOnlyDrives'); if (rawLocalOnly) { - this.localOnlyDrives = new Set(JSON.parse(rawLocalOnly)); + // Normalized on the way back in: an earlier build stored whatever + // spelling the caller passed, and every reader looks with the + // canonical form. + this.localOnlyDrives = new Set( + (JSON.parse(rawLocalOnly) as string[]).map(subject => + this.normalizeSubject(subject), + ), + ); } } catch { // ignore corrupt value @@ -902,30 +909,70 @@ export class Store { private localOnlyDrives = new Set(); /** Mark a drive as local-only. Must be called BEFORE the drive's first - * `save()` — registration is what routes saves away from the outbox. */ + * `save()` — registration is what routes saves away from the outbox. + * + * Normalized on the way in, because every reader normalizes before it + * looks (`isLocalOnlyDrive`, `isLocalOnlySubject`): a caller's trailing + * slash or `did:ad:` spelling would otherwise store a key nothing finds. */ public registerLocalOnlyDrive(drive: string): void { + const normalized = this.normalizeSubject(drive); + if (typeof localStorage !== 'undefined') { localStorage.setItem( 'atomic.localOnlyDrives', - JSON.stringify([...new Set([...this.localOnlyDrives, drive])]), + JSON.stringify([...new Set([...this.localOnlyDrives, normalized])]), ); } - this.localOnlyDrives.add(drive); + this.localOnlyDrives.add(normalized); } /** Switch this client to browser-only sync after verifying its local copy. - * Does not delete data from the server or alter other devices' configuration. */ + * Does not delete data from the server or alter other devices' configuration. + * + * The three preconditions each get their own message. They used to share + * "Open this drive with local storage available before disconnecting", which + * is only true for one of them: someone signed out, or on a server this + * client holds no socket for, was told to do something they had already done + * and given nothing to act on. + */ public async makeDriveLocal(drive: string): Promise { - const db = this.getClientDb(); + const normalized = this.normalizeSubject(drive); const agent = this.getAgent(); - const serverUrl = this.serverUrl; - const ws = this.getDefaultWebSocket(); - if (!db?.isReady || !agent || !ws) + + if (!agent) throw new Error('Sign in before disconnecting this workspace.'); + + // The local database attaches a few hundred ms after boot and again after + // every agent change, so a click inside that window found no database at + // all. Wait for the attach rather than refusing. `waitForInit` and not + // `isReady`, because the latter also demands the bootstrap seed and + // nothing below reads a bootstrap resource. + await this.waitForClientDb(); + const db = this.getClientDb(); + + if (!db || !(await db.waitForInit())) throw new Error( 'Open this drive with local storage available before disconnecting.', ); + const serverUrl = this.serverUrl; + // Sockets are registered under whatever string opened them, which is not + // always `serverUrl`, so fall back to the drive's origin exactly as + // `promoteLocalDrive` does. An open one, at that: the inventory below is a + // live request, and a socket that merely exists left `rbsrItems` to fail + // with "WebSocket is not open", which is not something a user can act on. + const open = (candidate: WSClient | undefined) => + candidate?.readyState === WebSocket.OPEN ? candidate : undefined; + const ws = + open(this.getDefaultWebSocket()) ?? + open(this.getWebSocketForSubject(normalized)); + + if (!ws) + throw new AtomicError( + 'Connect to a server before disconnecting this workspace.', + ErrorType.Server, + ); + const current = () => { const status = this.getSyncStatus(); if ( @@ -942,6 +989,11 @@ export class Store { }; current(); + // The caller's spelling goes to the socket and to the verification, not the + // normalized one: `rbsrItems` echoes the subjects it was asked about, + // `verifyLocalDriveCopy` matches the drive against them, and the drive-wide + // SUB this later drops was sent under `getDrive()`'s own spelling, which is + // not normalized either. `registerLocalOnlyDrive` normalizes for itself. const inventory = await ws.rbsrItems(drive, ''); await verifyLocalDriveCopy(db, drive, inventory); // A second inventory catches changes made while attachment verification ran. @@ -955,7 +1007,8 @@ export class Store { /** Forget a local-only drive (e.g. after deleting a demo workspace), * keeping the persisted registration set bounded. */ public unregisterLocalOnlyDrive(drive: string): void { - if (!this.localOnlyDrives.delete(drive)) return; + // Normalized to match what `registerLocalOnlyDrive` stored. + if (!this.localOnlyDrives.delete(this.normalizeSubject(drive))) return; if (typeof localStorage !== 'undefined') { localStorage.setItem( @@ -1054,7 +1107,13 @@ export class Store { ); const drive = resource?.get('https://atomicdata.dev/properties/drive'); - if (typeof drive === 'string' && this.localOnlyDrives.has(drive)) { + // Normalized, because the set holds normalized keys and a propval is + // whatever the server wrote: a `did:ad:` spelling of an `atomic:` drive + // would otherwise miss here and let a local-only resource try a POST. + if ( + typeof drive === 'string' && + this.localOnlyDrives.has(this.normalizeSubject(drive)) + ) { return true; } From 1aeed34b89da98bc9327f2ef04b50ececa74b4cc Mon Sep 17 00:00:00 2001 From: Joep Meindertsma Date: Fri, 25 Sep 2026 08:45:13 +0000 Subject: [PATCH 2/3] chore: gitignore server/integrations_assets_tmp build.rs stages the plugin assets there before embedding them, exactly as it does with the already-ignored server/assets_tmp, so a built checkout reports the staging directory as untracked. --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index ba6de0002..a1aa9ebf3 100644 --- a/.gitignore +++ b/.gitignore @@ -16,6 +16,8 @@ artifact /.e2e-store /.e2e-runs server/assets_tmp +# Same, for the plugin assets `build.rs` stages before embedding them. +server/integrations_assets_tmp server/js-build.log .netlify scratchpad From 2a8efee5234847f8d09ae06f6fbb38b2d2e2d218 Mon Sep 17 00:00:00 2001 From: Joep Meindertsma Date: Fri, 25 Sep 2026 10:05:51 +0000 Subject: [PATCH 3/3] fix(lib): read the local-only drive set through normalizeSubject everywhere Normalizing only the writes broke the readers that did a raw `has`: `isLocalOnlyDrive('did:ad:drive:test')` missed a drive registered under that same spelling, because the set now held `atomic:drive:test`. That is how vaultAutoBackup's two failures came about: a restored drive was not recognised as local, and a drive switch dropped a pending backup. `isLocalOnlyDrive` and the `driveOf` fallback in `isLocalOnlySubject` now normalize like the rest. Covered by a test registering and reading back the legacy spelling. --- browser/lib/src/make-drive-local.test.ts | 16 ++++++++++++++++ browser/lib/src/store.ts | 9 +++++++-- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/browser/lib/src/make-drive-local.test.ts b/browser/lib/src/make-drive-local.test.ts index 6d462474e..5df1141fd 100644 --- a/browser/lib/src/make-drive-local.test.ts +++ b/browser/lib/src/make-drive-local.test.ts @@ -89,6 +89,22 @@ describe('local-only drive registration', () => { expect(store.isLocalOnlyDrive(DRIVE)).toBe(false); }); + it('matches a drive registered with the legacy scheme', async ({ + expect, + }) => { + // `vaultAutoBackup` registers and reads back `did:ad:drive:…` verbatim, + // which is the case a raw `has` on the set got wrong. + const { store } = await testStore(); + store.registerLocalOnlyDrive('did:ad:drive:test'); + + expect(store.isLocalOnlyDrive('did:ad:drive:test')).toBe(true); + expect(store.isLocalOnlyDrive('atomic:drive:test')).toBe(true); + + store.unregisterLocalOnlyDrive('did:ad:drive:test'); + + expect(store.isLocalOnlyDrive('did:ad:drive:test')).toBe(false); + }); + it('matches a resource whose drive propval uses the legacy scheme', async ({ expect, }) => { diff --git a/browser/lib/src/store.ts b/browser/lib/src/store.ts index 9df92cb08..e3aa10eb2 100644 --- a/browser/lib/src/store.ts +++ b/browser/lib/src/store.ts @@ -1019,7 +1019,9 @@ export class Store { } public isLocalOnlyDrive(drive: string): boolean { - return this.localOnlyDrives.has(drive); + // Normalized, like the write side: the set holds canonical keys, and a + // caller may hold the `did:ad:` spelling or a trailing slash. + return this.localOnlyDrives.has(this.normalizeSubject(drive)); } /** @@ -1117,7 +1119,10 @@ export class Store { return true; } - return this.localOnlyDrives.has(this.driveOf(normalized)); + // `driveOf` returns a raw `parent` propval, so normalize that too. + return this.localOnlyDrives.has( + this.normalizeSubject(this.driveOf(normalized)), + ); } /** Returns the ClientDbWorker if one has been set (may still be initializing). */