fix!: un scope nomme refuse se rouvre quand la cle arrive, et la barriere cesse d-etre serie
Revue adversariale : la barriere livree au tour precedent rendait le refus
TERMINAL. Le signal du trousseau s'ecrit `if (query !== null) publish(...)`, parce
qu'avant la barriere il n'y avait qu'une seule raison de n'avoir pas de requete -
rien n'avait encore ete tente - et qu'une cle qui arrive ne pouvait changer que ce
qu'une requete OUVERTE avait le droit de montrer. La barriere en a ajoute une
seconde : elle abandonne AVANT que la requete existe, donc le seul canal qui fait
converger une cle tardive passait a cote de l'etat qui en avait le plus besoin.
Une page qui s'abonne pendant que son trousseau se restaure, ou un document
partage sous les yeux de son destinataire, publiait `isError` et y restait. Ce
n'est pas une surface plus stricte, c'est une pire : echanger un vide mensonger
contre une erreur bloquee ne repare rien, et cela casse une garantie que ce paquet
publie.
Le signal rouvre donc un scope refuse, et seulement quand le refus est leve
(`namedDocumentsReachable`). Le garde-fou porte autant que la reouverture :
rouvrir a chaque changement de trousseau rejouerait la barriere pour chaque cle
qu'une connexion depose, redemanderait au reseau tout document dont le
telechargement avait ECHOUE - un echec n'est deliberement pas memoise - et
republierait la meme erreur a chaque abonne. La variante indiscriminee casse deux
tests, dont celui de demontage qui preexistait.
La barriere passe en parallele. Elle etait serie, donc N documents qui ne poussent
jamais de `State` coutaient N replis bornes, 8s chacun en production. Les refus
restent lus dans l'ordre du scope apres coup.
Et ce qui a laisse passer les deux : l'e2e ne parcourait pas ce chemin. Les deux
journeys `useShapeQuery` lisent `{graphs: "did:ng:i"}`, donc 43/43 n'a jamais rien
prouve sur la barriere. Quatre controles ajoutes sur un scope NOMME, contre le
vrai broker : la proprietaire lit son document, un porteur de rien recoit
`isError` et jamais un succes vide, un document PUBLIC s'ouvre pour lui depuis la
seule reference via la requete de la barriere, et une cle qui tombe pour un AUTRE
document laisse le scope refuse en place.
This commit is contained in:
@@ -79,7 +79,7 @@ Every row was read in this working copy. `fetchReadCap` reaches the network only
|
||||
- a named document, held and synced, holding nothing of the shape: the barrier passes, the ORM answers empty, `isSuccess` with `data: []`. **This is the line that must not move.**
|
||||
- a held document exposing no read cap: `fetchReadCap` answers `false` and MEMOISES it. An absence was read; it is an answer.
|
||||
- a held document with no inbox: `documentInboxAddress` answers `undefined`, `depositsForDocument` answers `[]`.
|
||||
- the INERT case throughout: before the first cap exists, `isEnforcing()` is false, `fetchReadCap` returns without a round-trip, `mayReach` answers true for everything, and nothing new refuses. Every row above is unchanged there.
|
||||
- the INERT case throughout: before the first cap exists, `isEnforcing()` is false, `fetchReadCap` returns without a round-trip, `mayReach` answers true for everything, and nothing new refuses. **CORRECTED 2026-09-21 after an adversarial review**: this row said "every row above is unchanged there", and that was FALSE. The REFUSAL is inert; the OPEN is not. `ensureRepoOpen` runs whatever the registry is enforcing, so a named scope now issues a real `doc_subscribe` and waits on the sync barrier where it used to open the query at once. That is deliberate - without it a named document this session has never synced answers `isSuccess` with `data: []`, the very lie the barrier removes - but it is a change, and calling it "unchanged" hid a cost. Pinned by `use-shape-query.test.ts` (u).
|
||||
|
||||
**Partial scopes.** `graphs: [docA, docB]` with only docB held refuses the WHOLE query rather than answering docB's objects. A short answer is the same lie as an empty one, and this repo has already ruled twice against skip-and-carry-on: `readUnion`'s per-document tolerance was removed as an invention in `1.0.0-dev.8`, and an ORM write refuses the whole batch when one document is not writable.
|
||||
|
||||
@@ -123,13 +123,27 @@ Four tests in `public-store.test.ts`, all of them `expect(await fetchReadCap(unr
|
||||
|
||||
Nothing else broke. The reason is worth recording: the three live `fetchReadCap` callers that read a STORE or an INBOX already hold that cap (`fileOwnStructure` / `holdOwnCap` file it at connection), so `fetchReadCap` answers from the registry and never reaches the network. The blast radius is much narrower than four call sites suggest - it is a third party naming a document it does not hold.
|
||||
|
||||
### CORRECTED after the adversarial review (round 1 was REFUTED)
|
||||
|
||||
Two findings, both reproduced here before anything was changed, both diagnoses verified independently.
|
||||
|
||||
**Finding 1, blocking - a refused named scope was TERMINAL.** Reproduced as `use-shape-query.test.ts` (r): a named document whose cap arrives AFTER the subscribe stayed `{isError: true, data: []}` for ever. Diagnosis confirmed by reading: the keyring handler is `if (query !== null) publish(query, generation)`, and the barrier aborts BEFORE any query is created, so the one channel that makes a late cap converge went straight past the state that needed it most. It broke a guarantee this package publishes and that `read-filter.ts`, `use-shape-query.ts` and `inbox.ts` all restate - a document shared with you while you watch becomes readable with no call from the application. **Trading a silent empty answer for a stuck error is worse than the defect**, and the ordinary sequence (a page subscribing while its keyring is still being restored) hits it.
|
||||
|
||||
Fix: the keyring signal now RE-OPENS a scope the barrier refused, gated on `namedDocumentsReachable()` - every named document reachable now. Blast radius traced before writing it, and the gate is the load-bearing half: re-opening on any keyring change would re-run the barrier for every cap a connection files, re-ask the network for any document whose fetch had FAILED (a failure is deliberately not memoised) and re-publish the same error to every subscriber each time. `barrierRefused` keeps a `configure` with no `ormSubscription` - which no cap will ever repair - out of the retry entirely. The indiscriminate variant was tried and breaks two tests, (s) and the pre-existing teardown test (k).
|
||||
|
||||
**Finding 2 - the INERT claim was false.** Reproduced by reading and pinned as (u): `ensureRepoOpen` runs whatever the registry is enforcing, so the inert case gained a real `doc_subscribe` and a barrier wait. The open is right and stays; what was wrong was calling it "unchanged". The measured COST was real though: the barrier was SERIAL, so N documents that never push a `State` cost N bounded fallbacks - 8s each in production. Now parallel, so the wait is the slowest rather than the sum, with the refusals still read in scope order afterwards. Pinned by (t), which fails on the serial form.
|
||||
|
||||
**And the thing that let both through: the e2e never walked this path.** Confirmed - both `useShapeQuery` journeys read `{graphs: "did:ng:i"}`, so `43 passed / 43` was never evidence for the barrier, and reporting it as such was the real failure of round 1. Four live checks added over a NAMED scope: the owner reads her own document (the barrier does not break a live named read); a holder of nothing is `isError` with the refusal's own message and `dataLen: 0`, never an empty success; a PUBLIC document opens for that same holder from a bare reference, through the barrier's own fetch; and a cap landing for ANOTHER document leaves the refused scope refused rather than churning.
|
||||
|
||||
**What the e2e still does NOT prove, stated plainly.** The POSITIVE recovery - the cap for the refused document itself arriving - is proven at unit level only (test r, non-vacuous). Driving it live needs a second actor delivering through `inbox.share`, and on one page that means switching the current identity while the observable is subscribed, which would re-open the refused scope under the OTHER identity. That is a pre-existing property of this observable (`publish` has always read the keyring of whoever is connected), it cannot arise in an application (one identity per page load, no call takes an identifier), and the repo has already ruled that such montages belong in the applicative suite - `capsShareCap` was moved there for exactly this reason. Left as a named follow-up rather than encoded here as if it were the library's behaviour.
|
||||
|
||||
### Gate
|
||||
|
||||
- `bun test` -> `431 pass / 0 fail`, `Ran 431 tests across 40 files` (baseline `421 pass / 0 fail`, measured before any change; +10 new tests, no test deleted or skipped)
|
||||
- `bun test` -> `435 pass / 0 fail`, `Ran 435 tests across 40 files` (baseline `421 pass / 0 fail`, measured before any change; +14 new tests, no test deleted or skipped)
|
||||
- `bunx tsc --noEmit -p packages/polyfill/tsconfig.json` -> exit 0
|
||||
- `bunx tsc --noEmit -p packages/polyfill/e2e/tsconfig.json` -> exit 0
|
||||
- `python3 ~/.claude/skills/concept/lint.py` -> 0 ERROR (warnings pre-existing; the new leaf carries none)
|
||||
- polyfill e2e, run at HEAD: `Polyfill e2e summary: 43 passed, 0 failed, 43 total - batch 3.3 min, slowest cold sync 27s`. Same count as T01.h's baseline. NOTE: the broker login is flaky - two of five runs died with `[e2e broker login] gave up after 45.0s - the wallet-list screen never changed`, at a different step each time, before any library code runs. A clean 43/43 was obtained twice, once at `75313f7` and once at HEAD.
|
||||
- polyfill e2e, run at HEAD: `Polyfill e2e summary: 47 passed, 0 failed, 47 total - batch 3.5 min, slowest cold sync 30s`. 43 was the count before this task; the four added are the named-scope checks described above. NOTE: the broker login is flaky - two of eight runs died with `[e2e broker login] gave up after 45.0s - the wallet-list screen never changed`, at a different step each time, before any library code runs.
|
||||
|
||||
### The contract
|
||||
|
||||
@@ -152,13 +166,16 @@ Nothing else broke. The reason is worth recording: the three live `fetchReadCap`
|
||||
- **VERIFIED: `useShapeQuery`'s empty success does not come from `fetchReadCap` at all.** That call site does not exist on this path - T01.d removed it with the scope translation. It comes from the capability filter emptying a named document's objects, and from a named-but-unsynced document answering nothing. Tests (l) and (n) fail without the barrier; the task's three-layer chain describes the ANCHORED path, which is a different (and already loud) one.
|
||||
- **VERIFIED: the blast radius is narrow.** Six of the eight `ensureRepoOpen` callers hold the cap they are about to use, so they never ask the network. That is why only four tests broke.
|
||||
- **VERIFIED: `g === ""` in `namedDocuments` is unreachable** - `isNuri("")` is false. Kept as documentation of the SDK's own alias, said so in the comment and the commit. Test (p) is therefore a behaviour guard, not a discriminating test, and is labelled as one.
|
||||
- **INFERRED: the barrier's cost.** A named scope now spends one public-store fetch and one sync barrier per document per session, serially. Memoised, so a re-subscribe is a map hit; the worst case is a cold scope naming many documents, each bounded by the 8s fallback. Stated in the contract; no measurement was taken.
|
||||
- **VERIFIED, and it was under-reported in round 1: the barrier's cost.** A named scope spends one public-store fetch and one sync barrier per document per session. The opens now run TOGETHER, so the wait is the slowest rather than the sum; the serial form cost N bounded fallbacks, 8s each in production, which an adversarial review measured. Stated in the contract.
|
||||
- **The round-1 Result contained two claims this review falsified**, both corrected above: that the INERT case was unchanged (the open is new), and that the e2e was evidence for the barrier (it never walked a named scope). Both were overstatements of what had actually been checked, and the second is the one that mattered - a green suite reported as proof of a path it does not exercise.
|
||||
|
||||
### Follow-ups, reported and not fixed
|
||||
|
||||
- `packages/polyfill/src/shared-wallet/account-registry.ts:78` imports `documentInboxAddress` and never uses it - a dead import. `tsconfig.base.json` sets no `noUnusedLocals`, so nothing flags it.
|
||||
- Every e2e run logs, at connection: `connect failed: docs.sparqlQuery: refused - the connected user does not hold this document's cap ... at readStoreCaps <- restoreOwnCaps <- restoreAndDrain`. That is `assertMayReach`'s own pre-existing message (this work would have produced a different one), so `restoreOwnCaps` is being refused one of the connected user's OWN store documents at connect, and the failure is logged rather than raised.
|
||||
- The e2e broker login fails intermittently at 45s on the wallet-list screen, before any library code runs - 2 of 5 runs in this session.
|
||||
- The e2e broker login fails intermittently at 45s on the wallet-list screen, before any library code runs - 2 of 8 runs in this session.
|
||||
- A two-actor journey for "a document is shared with someone while they are watching, and their named `useShapeQuery` converges" belongs in the applicative suite (`e2e/notebook.ts`), beside the journey that already checks "after Alice shares it, the same reference opens it". It is the live form of unit test (r).
|
||||
- Pre-existing and out of scope, surfaced while tracing the fix: this observable re-publishes, and now re-opens, under whoever is connected at that instant rather than under the identity that subscribed. Harmless for an application (one identity per page load, no call takes an identifier); it only shows on a harness playing several identities on one page.
|
||||
- `docs/internal-contract.md:7` still names `surface/watch-shape.ts`, `surface/read-model.ts` and a `storeRegistry` namespace listing three removed symbols. Out of this task's boundary.
|
||||
- `knowledge_what-a-failed-read-answers-upstream` cites `213338f6`, the checkout T01.h found to be three ORM releases behind. The facts it cites are engine-side rather than ORM-side, but T01.k owns reconciling what a citation's version means.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user