diff --git a/.tasks/doing/T01-align-the-published-surface.md b/.tasks/doing/T01-align-the-published-surface.md index f535b25..2ae70f4 100644 --- a/.tasks/doing/T01-align-the-published-surface.md +++ b/.tasks/doing/T01-align-the-published-surface.md @@ -44,6 +44,7 @@ The polyfill's published surface has drifted from the SDK's: invented names, inv - T01.i Clear two mechanical leftovers from the removal (deps: T01.h) - T01.j A read that cannot reach its document must fail, not answer empty (deps: T01.i) - T01.l pnpm everywhere, and two dependency ranges that are looser than they look (deps: T01.j) +- T01.m There must be ONE answer to whether a document is open in this session (deps: T01.j) - T01.k A provenance citation must say which version it describes (deps: T01.l) ## Learnings digest diff --git a/.tasks/todo/T01.g-adversarial-review-of-the-published-surface.md b/.tasks/todo/T01.g-adversarial-review-of-the-published-surface.md index d8d23dc..cd1f6b0 100644 --- a/.tasks/todo/T01.g-adversarial-review-of-the-published-surface.md +++ b/.tasks/todo/T01.g-adversarial-review-of-the-published-surface.md @@ -2,7 +2,7 @@ id: T01.g title: Adversarial review of the whole published surface parent: T01 -depends_on: [T01.a, T01.b, T01.c, T01.d, T01.e, T01.f, T01.h, T01.i, T01.j, T01.k, T01.l] +depends_on: [T01.a, T01.b, T01.c, T01.d, T01.e, T01.f, T01.h, T01.i, T01.j, T01.k, T01.l, T01.m] created: 2026-09-16 claimed_by: null claimed_at: null diff --git a/.tasks/todo/T01.m-one-answer-to-whether-a-document-is-open.md b/.tasks/todo/T01.m-one-answer-to-whether-a-document-is-open.md new file mode 100644 index 0000000..1e89078 --- /dev/null +++ b/.tasks/todo/T01.m-one-answer-to-whether-a-document-is-open.md @@ -0,0 +1,59 @@ +--- +id: T01.m +title: There must be ONE answer to whether a document is open in this session +parent: T01 +depends_on: [T01.j] +created: 2026-09-24 +claimed_by: null +claimed_at: null +blocked: false +--- + +## Description + +This task exists because T01.j was split. Six rounds of work there, each with an independent adversarial review, kept finding the same defect in a narrower corner, and the sixth review established why: **"is this document open in this session" has more than one answer, and the answers disagree.** That is not a bug in a caller. It is a missing invariant in the emulated verifier, and it needs its own scope and its own reviews. + +### The three records, and how they disagree - VERIFIED across T01.j's reviews + +- `held` in `packages/polyfill/src/emulated-verifier/open-repo.ts` - intended to mean "this session holds a live bootstrap subscription to this document". It replaced an `opened` set that meant the looser "we once called `doc_subscribe` and stopped waiting", precisely because two records of one fact had been found to disagree. +- `syncState` in the same module - `"synced"` / `"syncing"` / `"timed-out"` / `"unknown"`, the barrier signal. +- `fanOuts` in `packages/polyfill/src/emulated-verifier/subscribe.ts` - the actual listeners. This is the one nobody counted. + +**Proved failure A.** `configure()` is PUBLISHED (`src/index.ts:173`) and calls `resetDocSubscriptions()`, which clears every fan-out listener including the bootstrap one, and never tells `open-repo`. So `held` keeps a dead entry and `syncState` keeps `"synced"`. `openRepoUnguarded`'s short-circuit `if (held.has(nuri)) return;` (`open-repo.ts:302`) then skips the re-open for ever. Measured: after a second `configure(ngB)`, a named `useShapeQuery` over a previously synced document gives `ngB doc_subscribe calls = 0`, `syncState = synced`, snapshot `{isPending:false, isSuccess:true, data:[]}` - an empty success asserting authority for a document this session never subscribed, unrecoverable even through `refetch()`. + +**Proved failure B.** `held.set(nuri, unsub)` (`open-repo.ts:396`) runs SYNCHRONOUSLY, before `establish` has reached `doc_subscribe` at all (it awaits `sessionId()` first), and is undone only if `onSetupFailed` fires. A broker that rejects AFTER `OPEN_TIMEOUT_MS` therefore leaves the repo recorded as held, and the retry does not re-subscribe. Measured with a `doc_subscribe` rejecting at 200ms against a 20ms fallback: `subscribe calls after retry-in-window = 1` where the claimed behaviour requires 2. So `held` still means "we asked" - the exact looser meaning its predecessor was deleted for. + +### What to establish, and it is a design question before it is a change + +**Name the invariant first, in one sentence, and write it where the state lives.** Then make it true by construction rather than by agreement between modules. The shape of the answer is open, but weigh these, because they are the ones the evidence points at: + +- ONE record, derived rather than mirrored. If `fanOuts` is the physical truth - a listener exists or it does not - then `held` may have no reason to exist as a separate set, and openness becomes a question asked of the subscription layer. Check whether that is actually true of `fanOuts` before building on it. +- A record set only when the fact is true. `held` is currently set BEFORE the subscribe is attempted, which is what makes failure B possible. Whatever records openness must be written when openness is established, not when it is requested. +- A single teardown path. `configure()`, `resetDocSubscriptions()`, `resetOpenedRepos()` and `syncSession()` all tear down parts of this state. Enumerate every one of them and what each currently leaves behind, BEFORE choosing where the invariant lives. + +**Enumerate the readers before changing the writers.** T01.j's rounds went wrong repeatedly by fixing a writer without knowing who read it. Known readers of sync state outside `open-repo`: `index-deposit.noRowsMeansAbsent` and `useShapeQuery`. Verify that list rather than trusting it, and include `inbox-observer`, which the sixth review read but did not drive. + +**The line that must never break, held through six rounds:** a document that IS synced and holds nothing answers a SUCCESS with zero objects. Every repair in this family has been tempted to turn a genuine emptiness into an error, and that would only move the lie. + +### Two smaller items in the same territory, found and left by T01.j + +- `open-repo.ts:111` is an ORPHAN JSDoc block stacked on `:112`'s, so one declaration carries two doc comments. `:48-51` still describes the deleted `opened` set as the live mechanism while `:97` explains its removal - the module header contradicts itself. +- `cold-read-for-document.test.ts:123` carries a FALSE justification: it says "neither value lets a zero-row read be called a definitive absence", but `index-deposit.ts:146` returns true for `"unknown"` and `open-repo.ts:176` says so. The assertion is right; the reason written beside it is wrong. Fix the reason. + +### Verification + +**Failures A and B above are the acceptance tests.** Each must have a test that fails without the change. Do not close this task on an argument. + +**Hard gate:** `bun test` -> 0 fail, run SIX times (this territory has produced a one-in-four flake, and a single green run would have missed it); both typechecks exit 0; `python3 ~/.claude/skills/concept/lint.py` -> 0 ERROR; and the polyfill e2e run and reported verbatim - broker login flakes roughly 3 runs in 10 before library code runs, so re-run a flaked login rather than reporting it as a failure. + +Anti-cheat: no `any`, no `@ts-ignore`, no `as unknown as`, no deleted or skipped test, no weakened assertion. + +**A GUARANTEE AND THE TEST THAT PINS IT LAND IN THE SAME COMMIT.** T01.j published contract text ahead of the behaviour twice, and both times it was false when written. That rule is not optional here. + +**Every structural claim about this change must be MEASURED before it is written.** T01.j's Result asserted three times that a change had simplified or shrunk something, and all three were contradicted by `wc -l` or `git show --numstat`. If you claim a condition was removed, paste the numstat. + +**An independent adversarial review is part of this task.** + +## Progress + +## Result