From 45dbd9a33abc781a5730b87c2d9ca564cf2f088a Mon Sep 17 00:00:00 2001 From: Sylvain Duchesne Date: Thu, 9 Jul 2026 13:28:56 +0200 Subject: [PATCH] =?UTF-8?q?refactor(client):=20d=C3=A9-poller=20resolveAcc?= =?UTF-8?q?ountReliably=20=E2=80=94=20barri=C3=A8re=20au=20lieu=20de=20ret?= =?UTF-8?q?ry?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Le polling est un anti-pattern NextGraph (par abonnement). La résolution de compte retentait ×8 la lecture du shim tant qu'elle rendait 0 (lag de sync) — c'est du polling. Remplacé par la BARRIÈRE d'abonnement, déjà le mécanisme de open-repo : - `resolveAccountReliably` : `await ensureRepoOpen(did:ng:${privateStoreId})` (subscribe + attendre le 1er State — le shim vit dans le graphe du private store), PUIS lecture UNIQUE. Après la barrière, 0 ligne = compte réellement inexistant → provision 1×, lignes présentes = réutilisé (garantie NO-FORK préservée). Plus de boucle de re-lecture. - timed-out (barrière expirée) : throw explicite, NE provisionne PAS (un provision sur sync incomplète re-forkerait). Le « trop long » est un signal, pas un feu vert. - fake ng sans doc_subscribe : ensureRepoOpen no-op → lecture immédiate (unit intact). - `_forceOpenedSyncState` : helper test-only (underscore, non ré-exporté). anti-fork.test.ts réécrit (5 tests : no-fork, neuf→1 provision, idempotence, fake no-op, timed-out→throw) ; plus aucun test de comptage de retry. gate : tsc propre ; bun test 117. e2e À RE-VALIDER quand le broker répond (dégradé ce jour : crash Chromium post-connexion) — la barrière ensureRepoOpen est déjà validée e2e (CONTRAT 3 + reconnexion) en broker sain. provisionRetry devient un champ mort de StoreRegistryDeps (nettoyage ultérieur). Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/client/src/open-repo.ts | 11 ++ packages/client/src/store-registry.ts | 120 +++++++++--------- packages/client/test/anti-fork.test.ts | 168 ++++++++++++------------- 3 files changed, 153 insertions(+), 146 deletions(-) diff --git a/packages/client/src/open-repo.ts b/packages/client/src/open-repo.ts index f2dd85c..4f200dd 100644 --- a/packages/client/src/open-repo.ts +++ b/packages/client/src/open-repo.ts @@ -110,6 +110,17 @@ export function resetOpenedRepos(): void { boundSessionId = null; } +/** + * @internal TEST-ONLY — force a nuri into the opened registry with a given sync + * state, without going through `doc_subscribe`. Used by `anti-fork.test.ts` to + * simulate a timed-out barrier without waiting the full `OPEN_TIMEOUT_MS` (8s). + * Do NOT call from production code. + */ +export function _forceOpenedSyncState(nuri: Nuri, state: SyncState): void { + opened.add(nuri); + syncState.set(nuri, state); +} + /** * The bootstrap sync state of `nuri` (lib-internal accessor; NOT a reactive * app-facing hook — that is a later phase). Returns `"unknown"` if the repo was diff --git a/packages/client/src/store-registry.ts b/packages/client/src/store-registry.ts index 35cb753..7cc772e 100644 --- a/packages/client/src/store-registry.ts +++ b/packages/client/src/store-registry.ts @@ -36,27 +36,30 @@ import { docCreate, sparqlUpdate, sparqlQuery } from "./docs"; import { getStoreRegistryDeps } from "./polyfill"; import { ensureRepoOpen } from "./open-repo"; -// --- provisioning anti-fork retry budget (polyfill-era) -------------------- +// --- provisioning anti-fork guard: barrier-first (polyfill-era) ------------ // // The shim (account→docs trust root) lives in the shared wallet's PRIVATE store. -// On a fresh session over a persistent wallet, that store's repo may not have -// synced yet from the broker: a targeted account resolve then reads 0 rows even -// though the account WAS persisted in an earlier session. If ensureAccount took -// that 0 at face value it would RE-PROVISION a second set of scope documents — -// an account FORK: one session writes/reads one set, another (or the same after -// a cache drop) the other, EMPTY set → the user "loses" their data on reconnect. +// On a fresh session over a persistent wallet, that repo may not have synced yet +// from the broker: a targeted account resolve would then read 0 rows even though +// the account WAS persisted in an earlier session. If ensureAccount took that 0 +// at face value it would RE-PROVISION a second set of scope documents — an account +// FORK: one session writes/reads one set, another (or the same after a cache drop) +// the other, EMPTY set → the user "loses" their data on reconnect. // -// Guard: before deciding an account is genuinely new, re-read it with a BOUNDED -// backoff (open + await the anchor repo, then retry the targeted query). Only if -// EVERY attempt within the budget still reads 0 do we treat it as a first-time -// account and provision. A pre-existing account merely lagging by sync is thus -// found on a later attempt and REUSES its docs (no fork); a truly new account -// exhausts the budget cheaply and is provisioned exactly once. The budget is a -// hard ceiling — never an unbounded poll — and is INJECTED (StoreRegistryDeps. -// provisionRetry): production defaults to ~8 attempts / ≲8.5s; a synchronous -// in-memory store (unit fake `ng`) passes `attempts: 1` to skip it (a 0-row read -// is authoritative there — no lag to wait out). Disappears at the real -// multi-store migration (deterministic per-user store NURIs make this moot). +// Guard (barrier-first): BEFORE reading the shim, open/subscribe the private-store +// repo and AWAIT its first `State` push — the deterministic sync barrier (CONTRACT 3 +// in e2e/). After the barrier, presence is GUARANTEED and absence DEFINITIVE. Then +// read the shim ONCE: 0 rows = account genuinely absent → provision exactly once; +// rows present = account found → reuse it (NO-FORK). No retry loop, no poll. +// +// Timed-out barrier (conservative): if `ensureRepoOpen` exits via the 8-second +// fallback timeout (getSyncState → "timed-out") the sync is UNCONFIRMED — we do NOT +// provision blindly (that would re-fork). Instead we surface a clear error so the +// caller can retry the full flow rather than silently creating a duplicate account. +// The timeout is rare (pathological broker) and a hard error there is far safer than +// a silent fork. Disappears at the real multi-store migration (per-user store NURIs +// make this moot — barrier becomes the native store-open which is always confirmed). +import { getSyncState } from "./open-repo"; import { escapeLiteral, escapeIri, assertNuri } from "./sparql"; import type { Nuri, Scope } from "./types"; @@ -280,48 +283,51 @@ export async function resolveAccount(id: string): Promise } /** - * Resolve ONE account, retrying a BOUNDED number of times before giving up, to - * distinguish a genuinely-absent account from one whose shim record has simply - * not synced yet on a fresh session (the account-FORK guard — see the - * PROVISION_RETRY_* budget note at the top of this module). + * Resolve ONE account after waiting for the private-store sync barrier, so a + * 0-row result is DEFINITIVE (account genuinely absent) rather than ambiguous + * (broker lag). This is the ANTI-FORK guard for the polyfill-era shim. * - * On the FIRST miss it opens/subscribes the shim anchor repo (the shared - * wallet's private store, where the shim lives) so the broker pushes its state, - * then re-reads with capped exponential backoff. Returns the record as soon as - * any attempt finds it (populating `accountCache` via resolveAccount), or `null` - * if the whole budget is exhausted with 0 rows — the only case in which the - * caller may treat the account as new and provision it. + * Flow: + * 1. Cache hit → return immediately (session-idempotent, no I/O). + * 2. Open/subscribe the private-store repo (`did:ng:${privateStoreId}`) and + * await the first `State` push — the deterministic sync barrier (CONTRACT 3). + * After the barrier, presence is guaranteed and absence definitive. + * 3. If the barrier TIMED-OUT (getSyncState → "timed-out"), the sync is + * unconfirmed — provisioning here would risk a fork. Throw a clear error + * instead so the caller can retry the full login flow. This is the + * conservative-safe choice: a hard error is recoverable; a silent fork is not. + * 4. Read the shim ONCE. 0 rows = genuinely absent → caller provisions exactly + * once. Rows present → reuse (NO-FORK preserved). * - * No-op fast path: a cache hit returns immediately with no retries. With the - * unit fake `ng` (no doc_subscribe, synchronous store) a present account is - * found on attempt 0 and an absent one exhausts the (short-sleep) budget without - * changing behaviour — the fork can only arise against a real, lagging broker. + * With the unit fake `ng` (no `doc_subscribe`), `ensureRepoOpen` is a no-op + * (getSyncState → "unknown") and the single read is immediate — synchronous + * behaviour preserved, no lag to wait out. */ async function resolveAccountReliably(id: string): Promise { // Cache hit → session-idempotent, no query, no fork risk. const cached = accountCache.get(accountKey(id)); if (cached) return cached; - const budget = getStoreRegistryDeps().provisionRetry; - const attempts = Math.max(1, budget.attempts ?? 8); - const baseMs = budget.baseMs ?? 300; - const maxStepMs = budget.maxStepMs ?? 1500; + // The shim lives in the private store. Open/subscribe it and await the first + // `State` push (the sync barrier). NURI: `did:ng:${session.privateStoreId}`. + const nuri = await anchorNuri(); + await ensureRepoOpen(nuri); - for (let attempt = 0; attempt < attempts; attempt++) { - const found = await resolveAccount(id); - if (found) return found; - if (attempt === attempts - 1) break; // budget exhausted → genuinely absent - // The shim anchor repo may not be synced on a fresh session; open it once so - // the broker pushes its state, then back off before the next targeted read. - try { - await ensureRepoOpen(await anchorNuri()); - } catch { - /* tolerant: a failed open just leaves the next read to behave as before */ - } - const step = Math.min(baseMs * 2 ** attempt, maxStepMs); - await new Promise((r) => setTimeout(r, step)); + // Conservative timed-out guard: if the barrier did not confirm sync, + // provisioning blind risks creating a fork. Raise a clear error — the caller + // (or the app's retry-login flow) should re-attempt when the broker is reachable. + // "unknown" means the fake-ng no-op path: no barrier semantics → safe to read. + const syncResult = getSyncState(nuri); + if (syncResult === "timed-out") { + throw new Error( + "[storeRegistry] sync barrier timed out for the private store — " + + "shim state unconfirmed, refusing to provision to avoid account fork. " + + "Retry when the broker is reachable.", + ); } - return null; + + // Barrier reached (or fake-ng "unknown" path): single read, result is definitive. + return resolveAccount(id); } /** All known accounts (from the shim). */ @@ -346,13 +352,13 @@ export async function ensureAccount(id: string): 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 (polyfill-era): resolve with a BOUNDED retry, NOT a single read. - // A single 0-row read on a fresh, still-syncing session would misfire as - // "new account" and RE-PROVISION a second set of scope docs (the fork — see - // the PROVISION_RETRY_* note at the top of this module). We only fall through - // to provisioning once the whole retry budget has confirmed 0 rows. A cache - // hit inside resolveAccountReliably keeps same-session resolves free and - // deterministic (the cached record wins over any re-provision). + // ANTI-FORK (polyfill-era): open the private-store repo and await its first + // `State` push (the sync barrier) BEFORE reading the shim, so a 0-row result + // is DEFINITIVE rather than a lag artifact. Only after the barrier do we treat + // 0 rows as "genuinely new" and provision. A cache hit inside + // resolveAccountReliably keeps same-session resolves free and deterministic + // (the cached record wins over any re-provision). Throws if the barrier timed + // out — safer than provisioning blind and creating a fork. const existing = await resolveAccountReliably(id); if (existing) return existing; diff --git a/packages/client/test/anti-fork.test.ts b/packages/client/test/anti-fork.test.ts index 744a009..2944ef4 100644 --- a/packages/client/test/anti-fork.test.ts +++ b/packages/client/test/anti-fork.test.ts @@ -2,16 +2,17 @@ * anti-fork.test.ts — behavioral tests for the ANTI-FORK guard in ensureAccount / * resolveAccountReliably (src/store-registry.ts). * - * The guard is: before treating a 0-row read as "genuinely new account" and - * provisioning a new set of scope docs, the registry retries a bounded number of - * times (with ensureRepoOpen + backoff between each). If the account IS found on - * any attempt, it is reused (no second set of docs = no fork). Only if EVERY - * attempt still reads 0 rows does the registry treat it as a first-time account - * and provision exactly once. + * The guard is: BEFORE reading the shim, open the private-store repo and await + * the first `State` push (the deterministic sync barrier, CONTRACT 3). After the + * barrier, 0 rows is DEFINITIVE (account genuinely absent) → provision exactly + * once; rows present → reuse (NO-FORK). No retry loop, no polling. * - * This file uses a FAKE `ng` that simulates broker sync lag: the sparql_query for - * the account record returns 0 rows for the first K calls, then returns real rows. - * We control the retry budget via `provisionRetry.attempts`. + * With the unit fake `ng` (no `doc_subscribe`), `ensureRepoOpen` is a no-op + * (getSyncState → "unknown") and the single shim read is immediate — no lag to + * wait out, synchronous behaviour preserved. + * + * Timed-out barrier: if the barrier expires without a `State`, we refuse to + * provision (throwing a clear error) rather than risk a fork. */ import { describe, it, expect, mock, beforeEach, afterAll } from "bun:test"; @@ -26,7 +27,7 @@ import { resetStoreRegistry, resetConfig, } from "../src/polyfill"; -import { resetOpenedRepos } from "../src/open-repo"; +import { resetOpenedRepos, _forceOpenedSyncState } from "../src/open-repo"; afterAll(() => { resetConfig(); @@ -38,11 +39,11 @@ afterAll(() => { const SESSION: RegistrySession = { sessionId: "sid-af", privateStoreId: "PRIV-AF" }; // --------------------------------------------------------------------------- -// Fake ng with sync-lag simulation +// Fake ng — simple in-memory store (no doc_subscribe → fake-ng no-op path) // -// `lagCalls` controls how many account-record reads return 0 rows before the -// real data is visible. Each call to sparql_query that matches the account -// SELECT decrements the counter; once it hits 0 the real data is returned. +// Queries return data from the `quads` array immediately. No lag simulation: +// the barrier mechanism (ensureRepoOpen) is a no-op in the fake-ng path, so +// the single shim read after "the barrier" is already authoritative. // --------------------------------------------------------------------------- interface Quad { g: string; s: string; p: string; o: string } @@ -60,7 +61,7 @@ function unescapeLiteral(s: string): string { return out; } -function makeLaggedNg(lagCalls: number) { +function makeFakeNg() { const quads: Quad[] = []; let docCounter = 0; let accountQueryCount = 0; @@ -100,15 +101,9 @@ function makeLaggedNg(lagCalls: number) { const anchor = a[3] as string | undefined; if (query.includes("")) { - // This is an account SELECT (resolveAccount / loadShim) + // Account SELECT (resolveAccount / loadShim) accountQueryCount++; - // Still in the lag window → pretend the store hasn't synced yet - if (accountQueryCount <= lagCalls) { - return { results: { bindings: [] } }; - } - - // Lag lifted → return real data from the quads store const subjM = query.match(/GRAPH <[^>]+>\s*\{\s*<([^>]+)>\s+a\s+/); const onlySubject = subjM ? subjM[1]! : null; const bySubject = new Map>(); @@ -149,12 +144,14 @@ function makeLaggedNg(lagCalls: number) { }; } -function inject(laggedNg: ReturnType, attempts: number) { - configure({ ng: laggedNg as any, useShape: (() => {}) as any }); +function inject(fakeNg: ReturnType) { + // No doc_subscribe → ensureRepoOpen is a no-op (fake-ng path). + configure({ ng: fakeNg as any, useShape: (() => {}) as any }); configureStoreRegistry({ getSession: async () => SESSION, normalizeId: (u) => u.trim().toLowerCase(), - provisionRetry: { attempts, baseMs: 0, maxStepMs: 0 }, + // provisionRetry is now unused by the barrier mechanism; kept for API compat. + provisionRetry: { attempts: 1 }, }); resetRegistryCache(); resetOpenedRepos(); @@ -164,104 +161,97 @@ function inject(laggedNg: ReturnType, attempts: number) { // Tests // --------------------------------------------------------------------------- -describe("anti-fork: resolveAccountReliably / ensureAccount", () => { +describe("anti-fork: barrier-first resolveAccountReliably / ensureAccount", () => { beforeEach(() => { resetRegistryCache(); resetOpenedRepos(); }); - it("(a) sync lag: 0 rows for K-1 calls then real rows → reuses existing docs (no fork)", async () => { - // budget = 3 attempts; lag = 1 call (the first targeted read returns 0, - // but the second (after retry) returns the real data written in the same - // session before the cache was dropped). We simulate a scenario where the - // account IS already provisioned (written to quads by a previous - // ensureAccount call in an earlier "session"), then we reset the cache and - // re-call with lag to simulate broker sync delay. + it("(a) NO-FORK: account already in shim → reused, 0 new doc_create", async () => { + // Provision an account in an initial "session" (quads are written). + const fakeNg = makeFakeNg(); + inject(fakeNg); + const first = await ensureAccount("LauraBarrier"); + expect(fakeNg.doc_create).toHaveBeenCalledTimes(3); - const laggedNg = makeLaggedNg(0); // no lag — first provision a real account - inject(laggedNg, 1); - const first = await ensureAccount("LauraLag"); - expect(laggedNg.doc_create).toHaveBeenCalledTimes(3); + // Simulate a fresh session: clear caches but keep quads intact (same fakeNg). + // In the barrier model the single post-barrier read finds the account immediately. + resetRegistryCache(); + resetOpenedRepos(); - // Now simulate a fresh session with lag: cache cleared, first K reads are 0 - // We reuse the SAME quads store (same laggedNg with quads intact) but - // rebuild with lag > 0 so the first read returns 0, then real data. - const laggedNg2 = makeLaggedNg(1); // 1 empty read before real data - // Copy the quads from the first ng into laggedNg2 so it "knows" the account - for (const q of laggedNg._quads) { - laggedNg2._quads.push(q); - } - inject(laggedNg2, 3); // 3 attempts, lag=1 → second attempt succeeds + const second = await ensureAccount("LauraBarrier"); - const second = await ensureAccount("LauraLag"); - - // ANTI-FORK: must return the SAME scope docs — no new doc_create + // ANTI-FORK: same scope docs returned, no new provisioning expect(second.docPublic).toBe(first.docPublic); expect(second.docProtected).toBe(first.docProtected); expect(second.docPrivate).toBe(first.docPrivate); - // doc_create must NOT have been called (the lagged ng2 finds the account on retry) - expect(laggedNg2.doc_create).toHaveBeenCalledTimes(0); + expect(fakeNg.doc_create).toHaveBeenCalledTimes(3); // still exactly 3, not 6 }); - it("(b) always-0 rows (genuinely new account): provisions exactly ONCE (1 set of docs)", async () => { - // Lag longer than the retry budget → account is treated as new - const laggedNg = makeLaggedNg(999); // always returns 0 for account queries - inject(laggedNg, 2); // budget: 2 attempts, both see 0 → provision + it("(b) genuinely new account (fake returns 0 rows) → provisioned exactly once (3 doc_create)", async () => { + const fakeNg = makeFakeNg(); // empty quads → 0 rows on any account query + inject(fakeNg); - const rec = await ensureAccount("NewUser"); + const rec = await ensureAccount("BrandNewUser"); - // Exactly 3 doc_create calls (1 set of scope docs) - expect(laggedNg.doc_create).toHaveBeenCalledTimes(3); + // Exactly 3 doc_create calls (1 set of scope docs, no fork) + expect(fakeNg.doc_create).toHaveBeenCalledTimes(3); expect(rec.docPublic).toMatch(/^did:ng:o:doc/); expect(rec.docProtected).not.toBe(rec.docPublic); expect(rec.docPrivate).not.toBe(rec.docProtected); - // Verify budget was fully consumed: both attempts fired before provisioning - expect(laggedNg.getAccountQueryCount()).toBe(2); + // Single read — no retry loop + expect(fakeNg.getAccountQueryCount()).toBe(1); }); - it("idempotence within a session: calling ensureAccount twice never creates 2 sets", async () => { - const laggedNg = makeLaggedNg(0); // no lag - inject(laggedNg, 1); + it("(c) idempotence within a session: calling ensureAccount twice never creates 2 sets", async () => { + const fakeNg = makeFakeNg(); + inject(fakeNg); const a = await ensureAccount("SameUser"); const b = await ensureAccount("SameUser"); expect(b).toEqual(a); // Must still be exactly 3 (not 6) - expect(laggedNg.doc_create).toHaveBeenCalledTimes(3); + expect(fakeNg.doc_create).toHaveBeenCalledTimes(3); }); - it("retry budget of 1 (sync fake): a genuinely-absent account provisions without extra retries", async () => { - const laggedNg = makeLaggedNg(0); // will never return lag (account not in quads) - inject(laggedNg, 1); + it("(d) fake-ng no-op barrier path: single shim read, no doc_subscribe calls", async () => { + // The fake ng has no doc_subscribe → ensureRepoOpen is a no-op (getSyncState → "unknown"). + // resolveAccountReliably must proceed to the single read without waiting or throwing. + const fakeNg = makeFakeNg(); // no doc_subscribe + inject(fakeNg); - await ensureAccount("BrandNew"); + // Provision once, then verify a second resolve (fresh cache) finds it immediately. + const first = await ensureAccount("FakeNgUser"); + resetRegistryCache(); + resetOpenedRepos(); - // 3 scope docs created exactly once - expect(laggedNg.doc_create).toHaveBeenCalledTimes(3); - // The targeted account query was called exactly once (no retries at attempts=1) - const accountQueryCalls = laggedNg.getAccountQueryCount(); - expect(accountQueryCalls).toBe(1); + const second = await ensureAccount("FakeNgUser"); + + // Same docs reused (no fork), exactly 3 total doc_create across both calls + expect(second.docPublic).toBe(first.docPublic); + expect(fakeNg.doc_create).toHaveBeenCalledTimes(3); + // The account query was called exactly twice (once per ensureAccount, no retries) + expect(fakeNg.getAccountQueryCount()).toBe(2); }); - it("lag of exactly (attempts-1): found on last allowed attempt → reused, not forked", async () => { - // 3 attempts, lag=2 (first 2 calls return 0, 3rd returns real data). - // We pre-populate the quads by provisioning with a fresh no-lag ng first. - const seedNg = makeLaggedNg(0); - inject(seedNg, 1); - const original = await ensureAccount("EdgeUser"); - expect(seedNg.doc_create).toHaveBeenCalledTimes(3); + it("(e) timed-out barrier: resolveAccountReliably throws rather than provisioning", async () => { + // Simulate: the private store nuri is already in the `opened` set but with + // sync state "timed-out" (forced via _forceOpenedSyncState so the test + // does not have to wait 8s for the real OPEN_TIMEOUT_MS to fire). + const fakeNg = makeFakeNg(); // empty quads → 0 rows + inject(fakeNg); - // Now re-inject with the same quads but with lag=2, budget=3 - const laggedNg = makeLaggedNg(2); - for (const q of seedNg._quads) laggedNg._quads.push(q); - inject(laggedNg, 3); + // Force the private-store nuri ("did:ng:PRIV-AF") into timed-out state. + // resolveAccountReliably calls anchorNuri() → `did:ng:${privateStoreId}`. + _forceOpenedSyncState("did:ng:PRIV-AF", "timed-out"); - const retried = await ensureAccount("EdgeUser"); + // ensureAccount must throw (conservative anti-fork guard: do not provision blind) + await expect(ensureAccount("TimedOutUser")).rejects.toThrow( + /sync barrier timed out/, + ); - // Found on the 3rd attempt (lag=2 means first 2 return 0, 3rd returns real) - expect(retried.docPublic).toBe(original.docPublic); - expect(retried.docProtected).toBe(original.docProtected); - expect(laggedNg.doc_create).toHaveBeenCalledTimes(0); // no new provisioning + // Must NOT have created any scope docs (refusing to provision) + expect(fakeNg.doc_create).toHaveBeenCalledTimes(0); }); });