fix!: la garde circulaire disparait, et une nouvelle tentative se fait au nom de qui a ete refuse
Troisieme tour de revue adversariale, trois constats, tous reproduits avant correction. 1. La garde etait CIRCULAIRE, et c'etait structurel. `namedDocumentsReachable` demandait si la capacite est DETENUE. Pour un document en store public, le seul canal qui puisse jamais la deposer est `fetchReadCap` - l'appel meme que la garde retenait. Elle attendait une reponse que seule l'action qu'elle bloquait pouvait produire, et toute cette classe restait irrecuperable. La bonne question n'est pas une meilleure garde, c'est de RETENTER. Les deux facons d'etre refuse se reparent par des choses opposees, et le memo les distingue deja : un echec n'est deliberement pas memoise, donc une nouvelle tentative redemande vraiment au reseau - c'est la reparation de cette classe, pas un cout a eviter ; une absence est memoise, donc une nouvelle tentative ne demande a personne et coute un instantane, jusqu'au jour ou la capacite arrive d'ailleurs, seule chose qui repare celle-la. La garde est supprimee : le module est plus petit qu'au tour precedent. 2. La reouverture faisait un ACCES sous la mauvaise identite. Republier REFILTRE une requete existante ; rouvrir EMET un abonnement et une requete au store public pour un document que cet abonne a nomme. C'est un acte neuf, pas un ancien qui ressort, et l'etat final etait `isSuccess` avec `data: []` pour un document illisible - le mensonge que cette tache supprime, par la porte du correctif. `refusedFor` retient QUI a ete refuse ; on ne retente que tant que c'est encore lui le connecte. Etabli plutot qu'affirme : une application ne peut pas changer d'identite en cours de session. `ensureIdentity` est le seul appel publie qui en etablit une, et `resolveIdentity` commence par `getCurrentUser()`. Corrige quand meme. 3. `Promise.all` court-circuite a la premiere rejection ARRIVEE, et un fetch qui rejette n'attend pas la barriere de synchronisation : il double un refus portant sur un document nomme avant lui. Le comportement est corrige plutot que le commentaire - `allSettled` puis parcours dans l'ordre du scope. Et le quatrieme controle e2e est retire, pas repare : il passait avec toute la machinerie de reprise supprimee. Un controle qui ne peut echouer pour aucune regression de ce qu'il garde se lit comme de la couverture et vaut moins que rien.
This commit is contained in:
@@ -129,7 +129,7 @@ Two findings, both reproduced here before anything was changed, both diagnoses v
|
||||
|
||||
**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).
|
||||
Fix, round 2: the keyring signal RE-OPENS a scope the barrier refused, gated on `namedDocumentsReachable()`. **That gate was itself wrong and round 3 refuted it - see below.**
|
||||
|
||||
**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.
|
||||
|
||||
@@ -137,13 +137,33 @@ Fix: the keyring signal now RE-OPENS a scope the barrier refused, gated on `name
|
||||
|
||||
**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.
|
||||
|
||||
### CORRECTED AGAIN - round 2's repair was REFUTED, three findings, all reproduced here first
|
||||
|
||||
**Finding 1, blocking and STRUCTURAL - the gate was circular.** `namedDocumentsReachable()` tested `mayReach`, i.e. the cap is HELD. For a named document in a PUBLIC store the only channel that can ever file that cap is `fetchReadCap` - the very call that had failed and set the flag. The gate waited on an answer that only the action it gated could produce, so that whole class was unrecoverable however many caps arrived. Reproduced as `use-shape-query.test.ts` (v): the broker hiccups on the `exposedReadCap` read, the fetch rejects, the broker heals, an ordinary cap arrival follows, and the view never re-asks.
|
||||
|
||||
**The readiness question, answered rather than patched.** There are exactly two ways this barrier can refuse, and the memo already tells them apart. The store could not be ASKED: a failure is deliberately not memoised (an earlier fix in this repo), so a re-attempt genuinely re-asks the network - which IS the repair for this class, not a cost to avoid. Or the store ANSWERED and there is no cap to be had: an absence is memoised, so a re-attempt asks nobody, re-runs the barrier against what is already known, and republishes the same refusal for the price of one snapshot - and it stops the day the cap arrives from elsewhere, which is the only thing that repairs THAT class. So the right signal is not a cleverer predicate, it is **attempting again**: cheap for the class it cannot help, correct for the class it can, self-limiting because the moment an attempt succeeds there is a query again. `namedDocumentsReachable()` is DELETED - the repair makes the module smaller than round 2 left it. Pinned by (v) and, for the cost, by (s), which now asserts both halves (the re-attempt ran, and it spent no round-trip) so it cannot pass vacuously.
|
||||
|
||||
**Finding 2 - the re-open performed an access under the wrong identity, and my "pre-existing" defence was wrong in kind.** The reviewer is right: re-publishing RE-FILTERS an existing query, while a re-open NEWLY ISSUES a subscribe and a public-store fetch for a document that subscriber named. That is a new action taken on behalf of whoever happens to be connected, not an old one resurfacing, and the end state was `{isSuccess: true, data: []}` for a document the subscriber cannot read - this task's own defect, through the repair's door. Fixed by binding the refusal to a holder: `refusedFor` records WHOSE barrier failed, and a re-attempt is made only while that holder is still the connected one. Pinned by (w), which fails without the binding.
|
||||
|
||||
**ESTABLISHED rather than asserted, as asked: an application cannot change identity mid-session.** `ensureIdentity()` is the only published call that settles one, and it delegates to `settleIdentity` -> `resolveIdentity`, whose first act is `const already = getCurrentUser()` - so a second call returns the identity already set. `setCurrentUser` is not exported from `src/index.ts`. So Finding 2's sequence is reachable only from a harness playing several identities on one page. It is fixed anyway, because a new access issued in someone else's name is a defect in kind whatever can reach it, and because the fix costs one comparison.
|
||||
|
||||
**Finding 3 - the determinism claim was false.** `Promise.all` short-circuits on the first rejection to ARRIVE, and a fetch that rejects does not wait for the sync barrier, so it overtakes a refusal on a document named before it. I chose to fix the BEHAVIOUR rather than the claim: `Promise.allSettled` plus a walk in scope order, so the document reported is the first one the caller named that failed, whatever it failed of, and the parallelism is kept. Pinned by (x), which fails on `Promise.all`.
|
||||
|
||||
**The fourth e2e check was removed, not repaired.** "A cap landing for ANOTHER document leaves a refused scope refused" passed with the whole re-attempt machinery deleted - the outcome is a standing refusal either way. A check that cannot fail for any regression of the thing it guards reads as coverage and is worse than none. Its properties are measured in the unit suite by (v), (s) and (w).
|
||||
|
||||
### The decision to KEEP the barrier rather than take the escape hatch
|
||||
|
||||
Taken deliberately, at the second offer. Removing it would restore, on a published read, a silent `isSuccess` with `data: []` for a document the caller cannot reach - the defect this task exists for, now proven live rather than argued. The escape hatch's own premise is "a stuck error is worse than a silent empty"; after these three repairs there is no stuck error left to weigh against it. Every class that can refuse is re-attempted on the only readiness signal this library has, and the one class that is not - the identity changed under the observable - cannot arise in an application and is answered honestly rather than by acting in someone else's name. The repairs are also subtractive: a circular predicate deleted, a short-circuit replaced by an ordered walk, a flag given the one extra fact it was missing. What remains is a scope refused in a session whose keyring never moves again, which stays `isError` until `refetch()` - the published meaning of `isError` on this surface, and what it already meant for every other failure it carries.
|
||||
|
||||
### Gate
|
||||
|
||||
- `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)
|
||||
- `bun test` -> `438 pass / 0 fail`, `Ran 438 tests across 40 files` (baseline `421 pass / 0 fail`, measured before any change; +17 new tests, no test deleted or skipped - (s) was REWRITTEN, not weakened: it asserted the targeting predicate that round 3 proved circular, and now asserts the property that is actually true and worth guarding, on both halves so it cannot pass vacuously)
|
||||
- `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: 47 passed, 0 failed, 47 total - batch 3.4 min, slowest cold sync 29s`. 43 was the count before this task; the four added are the named-scope checks described above. NOTE: the broker login is flaky and independent of this work - three of ten runs in this session died with `[e2e broker login] gave up after 45.0s - the wallet-list screen never changed`, at a different step each time, on the login UI before any library code runs. Every such run was re-run to a clean result.
|
||||
- polyfill e2e, run at HEAD: `Polyfill e2e summary: 46 passed, 0 failed, 46 total - batch 3.5 min, slowest cold sync 29s`. 43 was the count before this task; three named-scope checks were added and a fourth removed as non-discriminating. NOTE: the broker login is flaky and independent of this work - three of eleven runs in this session died with `[e2e broker login] gave up after 45.0s - the wallet-list screen never changed`, at a different step each time, on the login UI before any library code runs. Every such run was re-run to a clean result.
|
||||
|
||||
**What the e2e PROVES**, and it is the whole of it: against the real broker, a named scope reads for the holder who owns the document; a holder of nothing is refused out loud with `dataLen: 0` rather than told the document is empty; and a public document opens for that same holder from nothing but its reference, through the barrier's own fetch. **What it does NOT prove**: anything about the re-attempt. Every recovery path - the public class repaired by re-asking, the cost of a doomed re-attempt, and the holder a re-attempt is made as - is measured only in the unit suite, against the `CapRegistry`, which is where that mechanism lives and which the e2e's own header says the broker does not implement. Driving the positive recovery live needs a second actor delivering through `inbox.share`, and belongs in the applicative suite.
|
||||
|
||||
### The contract
|
||||
|
||||
@@ -175,7 +195,7 @@ Fix: the keyring signal now RE-OPENS a scope the barrier refused, gated on `name
|
||||
- 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 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.
|
||||
- This observable re-PUBLISHES under whoever is connected at that instant rather than under the identity that subscribed. That part is genuinely pre-existing and out of scope - `publish` has always read the keyring of the current holder. The RE-ATTEMPT no longer does: it is bound to the holder that was refused (`refusedFor`), because issuing a subscribe and a fetch is a new access rather than a re-filtering, and the two must not be lumped together - round 2's Result did lump them, and that was wrong.
|
||||
- `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