diff --git a/packages/client/src/discovery.ts b/packages/client/src/discovery.ts index 7def0ed..e6837b6 100644 --- a/packages/client/src/discovery.ts +++ b/packages/client/src/discovery.ts @@ -39,6 +39,7 @@ import * as inbox from "./inbox"; import { subscribeDoc } from "./subscribe"; +import { ensureRepoOpen } from "./open-repo"; import { ensureAccount, reservedAccount } from "./store-registry"; import { getCaps } from "./polyfill"; import type { Nuri, PrincipalId } from "./types"; @@ -154,6 +155,19 @@ export async function submitToIndex(ref: unknown, opts?: SubmitOptions): Promise */ export async function readIndex(): Promise { const target = await indexInboxNuri(); + // COLD-START heal (polyfill-era): on a FRESH session over a persistent wallet the + // discovery-index inbox repo is not yet in the verifier's `self.repos`, so the + // anchored `inbox.read` below would resolve an unopened repo and silently return 0 + // deposits — the same self-inflicted cold-read gap `readScopeIndex`/`readUnion` + // heal. This is what made the PUBLIC read's discovery fold come back empty on a + // reconnect, so a fresh page's home stayed empty for tens of seconds while the doc + // slowly synced by other means. Open/subscribe the index repo ONCE and await its + // first `State` (the sync barrier) before the anchored read. Idempotent per session; + // no-op with the unit fake ng (no `doc_subscribe`). This is done HERE (a cold direct + // reader) rather than inside `inbox.read`, because `inbox.watch` already holds the + // repo open via its own subscription and must not spawn a second bootstrap open. See + // open-repo.ts. + await ensureRepoOpen(target); const deposits = await inbox.read(target); const seen = new Set(); const entries: IndexEntry[] = []; diff --git a/packages/client/src/inbox.ts b/packages/client/src/inbox.ts index 863fb8f..9f0ba93 100644 --- a/packages/client/src/inbox.ts +++ b/packages/client/src/inbox.ts @@ -156,6 +156,13 @@ export async function post(targetInbox: Nuri, opts: PostOptions): Promise */ export async function read(targetInbox: Nuri): Promise { const sid = await sessionId(); + // NOTE: cold-start repo opening is done by the COLD DIRECT readers that need it + // (e.g. `discovery.readIndex` → `ensureInboxRepoOpen`), NOT here — `inbox.watch` + // already holds the repo open via its own `subscribeDoc`, so opening a second + // bootstrap subscription from inside a watch's re-read would be redundant and can + // race the watch's own initial-`State` delivery. Keeping `read` a pure anchored + // read leaves both callers correct: the watch path stays event-driven, and the + // cold direct-read path opens the repo explicitly before calling `read`. // NO explicit `GRAPH <…>` clause — read the anchored DEFAULT graph (see the // note in `post`). The anchor (`targetInbox`) scopes the query to that repo's // default graph, exactly where `post` writes. diff --git a/packages/client/src/store-registry.ts b/packages/client/src/store-registry.ts index 2a1f7e1..6791261 100644 --- a/packages/client/src/store-registry.ts +++ b/packages/client/src/store-registry.ts @@ -381,56 +381,101 @@ async function createDoc(): Promise { return docCreate(s.sessionId, "Graph", "data:graph", "store", undefined); } +/** + * In-flight `ensureAccount` promises, keyed by account key — so CONCURRENT + * `ensureAccount(id)` calls for the SAME account share ONE resolve-or-provision. + * + * ── Why this is load-bearing (the reconnection FORK root cause) ───────────── + * On a fresh page over the persistent wallet, SEVERAL independent callers hit + * `ensureAccount(A)` near-simultaneously: `watchShape('public')` and + * `watchShape('protected')` each resolve their doc set (`listMyEntityDocs` → + * `ensureAccount`) and subscribe their container (`scopeIndexDoc` → + * `ensureAccount`), plus the app's owned-events effect. Each runs BEFORE the shim + * has synced, so each `resolveAccountReliably` reads 0 rows and, independently, + * PROVISIONS a brand-new set of scope docs — an account FORK PER caller. Measured: + * a single new account accrued EIGHT distinct `docPublic`/`docProtected` values in + * one session. `canonicalDoc` then makes a fresh reader pick the lexicographic-min + * doc, which differs from the (cached, freshly-created) doc the WRITER wrote into → + * the reader lists an empty scope index → the home/participation reads come back + * empty (the reconnection bug's last layer). + * + * De-duping concurrent provisions collapses those N racing provisions into ONE: + * the first caller resolves-or-provisions; every concurrent caller awaits the SAME + * promise and gets the SAME record, so no duplicate docs are ever minted and writer + * and reader converge on one canonical scope doc. Not polling: a bounded in-memory + * promise map (mirrors open-repo.ts `inFlight`), cleared the instant it settles. + */ +const ensureInFlight = new Map>(); + /** * Ensure an account exists in the shim, creating its 3 scope documents on * first sight. Idempotent — returns the existing record if already present. + * Concurrency-safe: concurrent calls for the same account share one provision + * (see {@link ensureInFlight}) so a fresh page never FORKS the account. */ export async function ensureAccount(id: string): Promise { const key = accountKey(id); - // HOT PATH: targeted O(1) lookup — does THIS account already exist? — instead - // of a full-shim scan (loadShim). Off the read/write hot path entirely. - // - // ANTI-FORK: resolve via the BOUNDED retry, so a transient 0 rows (the shim not - // yet synced on a fresh page over the persistent wallet) is not mistaken for - // "account genuinely absent" → a fork. Only after the whole retry budget still - // reads 0 do we treat the account as new and provision. See resolveAccountReliably. - const existing = await resolveAccountReliably(id); - if (existing) return existing; + // A completed provision/resolve is cached → no query, no fork risk. + const cached = accountCache.get(key); + if (cached) return cached; + // A concurrent provision for the SAME account is already running → await it, + // instead of racing a second (forking) provision. This is the anti-fork guard. + const pending = ensureInFlight.get(key); + if (pending) return pending; - const [docPublic, docProtected, docPrivate] = await Promise.all([ - createDoc(), - createDoc(), - createDoc(), - ]); - const record: AccountRecord = { id, docPublic, docProtected, docPrivate }; + const p = (async (): Promise => { + // HOT PATH: targeted O(1) lookup — does THIS account already exist? — instead + // of a full-shim scan (loadShim). Off the read/write hot path entirely. + // + // ANTI-FORK: resolve via the BOUNDED retry, so a transient 0 rows (the shim not + // yet synced on a fresh page over the persistent wallet) is not mistaken for + // "account genuinely absent" → a fork. Only after the whole retry budget still + // reads 0 do we treat the account as new and provision. See resolveAccountReliably. + const existing = await resolveAccountReliably(id); + if (existing) return existing; - const s = await session(); - const anchor = await anchorNuri(); - const subj = accountSubject(id); - // `subj` is already IRI-safe (accountSubject → escapeIri). `anchor` is a - // trusted-shaped NURI → assertNuri. `id` is UNTRUSTED text in a LITERAL - // position → escapeLiteral. The doc NURIs come from `ng` but are stored as - // literals here, so they are escaped as literals too (defence in depth). - const update = ` - INSERT DATA { - GRAPH <${assertNuri(anchor)}> { - <${subj}> a <${P.type}> ; - <${P.id}> "${escapeLiteral(id)}" ; - <${P.docPublic}> "${escapeLiteral(docPublic)}" ; - <${P.docProtected}> "${escapeLiteral(docProtected)}" ; - <${P.docPrivate}> "${escapeLiteral(docPrivate)}" . - } - }`; + const [docPublic, docProtected, docPrivate] = await Promise.all([ + createDoc(), + createDoc(), + createDoc(), + ]); + const record: AccountRecord = { id, docPublic, docProtected, docPrivate }; + + const s = await session(); + const anchor = await anchorNuri(); + const subj = accountSubject(id); + // `subj` is already IRI-safe (accountSubject → escapeIri). `anchor` is a + // trusted-shaped NURI → assertNuri. `id` is UNTRUSTED text in a LITERAL + // position → escapeLiteral. The doc NURIs come from `ng` but are stored as + // literals here, so they are escaped as literals too (defence in depth). + const update = ` + INSERT DATA { + GRAPH <${assertNuri(anchor)}> { + <${subj}> a <${P.type}> ; + <${P.id}> "${escapeLiteral(id)}" ; + <${P.docPublic}> "${escapeLiteral(docPublic)}" ; + <${P.docProtected}> "${escapeLiteral(docProtected)}" ; + <${P.docPrivate}> "${escapeLiteral(docPrivate)}" . + } + }`; + try { + await sparqlUpdate(s.sessionId, update, anchor, "ensureAccount"); + } catch (error) { + console.error("[storeRegistry] ensureAccount persist failed:", error); + } + // Feed the per-account cache, and the full-shim cache if it is already loaded + // (so allAccounts / the fan-out see the freshly-created account too). + accountCache.set(key, record); + cache?.set(key, record); + return record; + })(); + + ensureInFlight.set(key, p); try { - await sparqlUpdate(s.sessionId, update, anchor, "ensureAccount"); - } catch (error) { - console.error("[storeRegistry] ensureAccount persist failed:", error); + return await p; + } finally { + ensureInFlight.delete(key); } - // Feed the per-account cache, and the full-shim cache if it is already loaded - // (so allAccounts / the fan-out see the freshly-created account too). - accountCache.set(key, record); - cache?.set(key, record); - return record; } // --- resolvers ------------------------------------------------------------ diff --git a/packages/client/test/store-registry.test.ts b/packages/client/test/store-registry.test.ts index d2af8f0..0065333 100644 --- a/packages/client/test/store-registry.test.ts +++ b/packages/client/test/store-registry.test.ts @@ -188,6 +188,27 @@ test("ensureAccount is idempotent (case/@-insensitive key), no extra docs", asyn expect(fake.doc_create).toHaveBeenCalledTimes(3); // not 6 }); +test("ensureAccount de-dupes CONCURRENT provisions (anti-fork): one account, 3 docs", async () => { + // The reconnection FORK: several callers (watchShape public+protected, the + // container subs, the owned-events effect) hit ensureAccount(SAME id) BEFORE + // the shim has synced, so each reads 0 rows and independently provisions a new + // set of scope docs — N forks, N×3 docs, duplicate docPublic/docProtected in the + // shim → a fresh reader picks a different canonical doc than the writer wrote to. + // The in-flight de-dup collapses N concurrent provisions into ONE. + const results = await Promise.all([ + ensureAccount("Bob"), + ensureAccount("Bob"), + ensureAccount("@bob"), + ensureAccount("BOB"), + ensureAccount("bob"), + ]); + // Exactly ONE set of 3 docs was created — not 5×3. + expect(fake.doc_create).toHaveBeenCalledTimes(3); + // Every caller got the SAME record (same docs), so writer/reader can never + // disagree on the canonical scope doc. + for (const r of results) expect(r).toEqual(results[0]!); +}); + test("loadShim round-trips a persisted account across a cache reset", async () => { await ensureAccount("Bob"); resetRegistryCache(); // force a re-read from the fake store