From 3c663e0db981dc8047a7c6c44750f7b2e30d2d5b Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Thu, 6 Aug 2026 08:08:46 -0500 Subject: [PATCH] docs: close the ADR 0160 Phase 1 tail -- the 104th file, and what was left on purpose PR #235 removed 103 files. A 104th, docs/releases/HANDOFF-232-router-steps.md, entered main via PR #225 (ADR 0076 Amendment D, BACKLOG #232) WHILE #235 was open, so #235 -- which removed 101 paths named INDIVIDUALLY -- could not have covered a file created after its commit was built. A .gitignore rule does not untrack, so /docs/releases/ left it behind and the directory came back holding exactly one file. Untracked here, with custody taken by the vault FIRST (09e3cd4e, pushed) as with the other 103. No new ignore rule was needed: /docs/releases/ already covers the path once nothing under it is tracked, which is now true. Verified against a tracked-files-only `git archive` export -- the directory is empty in a fresh checkout. THE SHAPE IS THE LESSON, not the file. A removal that ENUMERATES paths has a window, open for exactly as long as the PR is, during which the directory it is clearing can be refilled. Nothing detected this: it surfaced only because a merge conflict forced a re-read of the tree. Recorded in ADR 0160 so a future phase re-checks the directory at MERGE time rather than trusting the file list computed when the branch was cut. ADR 0076's link to that file is unlinked to its plain name -- the same treatment the other 37 got, and necessary here because untracking it would otherwise leave a 404 in a document that stays. NEW SECTION D6 -- CONSIDERED AND LEFT. Three sets were found during Phase 1 and deliberately not acted on. They are recorded because a sweep run against D1 surfaces all three again, and an unrecorded "we looked and left it" is indistinguishable from an oversight: * 11 files citing docs/releases/ in PROSE. Not links; nothing 404s. Provenance reads true after the file moves. ADR 0160's OWN citations must never be "cleaned" -- it is the ADR that removed the directory and the paths are its evidence. * 22 handoff DOCUMENTS outside docs/releases/. A name-based sweep reports 155; 133 of those are benchmark DATA files that merely sit inside HANDBACK_* directories, so the naive number overstates it six-fold. The bench handoffs carry the measurement narrative for the data beside them, and removing them would strip rationale from records that stay. * Unanchoring /HANDOFF-*.md. REJECTED, and the reasoning is kept in .gitignore beside the rule so it is refuted where it will next be proposed: the two locations that matter (repo root, docs/releases/) are both already covered, and an unanchored pattern would fail closed on docs/benchmarks/, where 21 handoffs are tracked ON PURPOSE and a new benchmark run legitimately produces another. Verified: ruff check + format clean (1044 files), 53 doc guards pass, link guards re-run against a tracked-files-only export (17 passed), backlog_status_check 337 items, all 155 ADR table rows uniform at 5 cells. --- .gitignore | 12 ++ ...yped-action-vocabulary-action-list-lens.md | 2 +- ...rator-and-security-review-material-only.md | 37 ++++ docs/adr/README.md | 2 +- docs/releases/HANDOFF-232-router-steps.md | 200 ------------------ 5 files changed, 51 insertions(+), 202 deletions(-) delete mode 100644 docs/releases/HANDOFF-232-router-steps.md diff --git a/.gitignore b/.gitignore index dd787053..aa2c5f7f 100644 --- a/.gitignore +++ b/.gitignore @@ -176,5 +176,17 @@ scripts/security/scan-tokens.local.txt # currently matches: session handoffs kept landing in the repo's front door, and a pattern fails # closed on the next one instead of waiting for somebody to notice. A handoff that genuinely must # ship can still be force-added (`git add -f`). +# +# ⛔ DO NOT "FIX" THE ANCHOR BY DROPPING THE LEADING SLASH. It looks like a gap -- the root-anchored +# form matches nothing under docs/ -- and it was proposed as a tightening on exactly that reasoning. +# It is the wrong fix twice over: +# * The two places a handoff must not land are the repo ROOT and docs/releases/, and BOTH are +# already covered -- the root by the rule below, the directory by the rule above it. The case +# that motivated the change (PR #225 landing a handoff in docs/releases/ while the Phase 1 PR was +# open) is closed by that directory rule the moment nothing is tracked there, which is now true. +# * An unanchored `HANDOFF-*.md` would fail closed on `docs/benchmarks/`, where 21 handoff +# documents are TRACKED ON PURPOSE: they carry the measurement narrative for the ~133 data files +# beside them, and a new benchmark run legitimately produces another. Silently ignoring those +# would break working practice to close a gap that is already shut. /docs/releases/ /HANDOFF-*.md diff --git a/docs/adr/0076-typed-action-vocabulary-action-list-lens.md b/docs/adr/0076-typed-action-vocabulary-action-list-lens.md index 235f3752..3be89b30 100644 --- a/docs/adr/0076-typed-action-vocabulary-action-list-lens.md +++ b/docs/adr/0076-typed-action-vocabulary-action-list-lens.md @@ -664,7 +664,7 @@ accepts. The criteria are nonetheless **built and tested** — the amendment lan > > **In force means the grammar changed, not that the build is done.** The shipped `lens parse` emits no > `route` row yet — the BACKLOG #232 build is handed off in -> [`../releases/HANDOFF-232-router-steps.md`](../releases/HANDOFF-232-router-steps.md), which carries the +> `HANDOFF-232-router-steps.md`, a maintainer-internal document, which carries the > files, the verified anchors, and the falsifications. The Acceptance Criteria in §D.6 are **build gates, > not caveats**. diff --git a/docs/adr/0160-public-repo-content-policy-operator-and-security-review-material-only.md b/docs/adr/0160-public-repo-content-policy-operator-and-security-review-material-only.md index 0fe6a5be..8f2b81c0 100644 --- a/docs/adr/0160-public-repo-content-policy-operator-and-security-review-material-only.md +++ b/docs/adr/0160-public-repo-content-policy-operator-and-security-review-material-only.md @@ -86,6 +86,17 @@ were tracked in the **repository root** — session handoffs in the project's fr covered by a `/HANDOFF-*.md` rule rather than by their two filenames, so the next one fails closed instead of waiting to be noticed. **103 files** in total. +⚠️ **A 104th followed, and the shape is the lesson.** `docs/releases/HANDOFF-232-router-steps.md` +entered `main` via **PR #225** *while the Phase 1 PR was open*. Phase 1 removed 101 paths **named +individually**, so a file created after that commit was built was never in the list — and a +`.gitignore` rule **does not untrack**, so `/docs/releases/` left it behind and the directory came +back holding one file. Removed in the follow-up that also records this paragraph. + +**A removal that enumerates paths has a window, open for exactly as long as the PR is, during which +the directory it is clearing can be refilled.** Nothing detected this: it surfaced only because the +merge conflict forced a re-read of the tree. If a future phase clears a directory, re-check it at +merge time rather than trusting the file list computed when the branch was cut. + **Order of operations, and it is the load-bearing part.** Custody moved to the vault (`wshallwshall/MessageFoundry`) and was committed there **before** anything left the public tree. Gitignoring alone would have left 103 files as single **unversioned** copies — no history, no @@ -156,6 +167,32 @@ already names. **Phase 3 — in-file references.** Strip Claude-Code process prose from documents that otherwise stay (`docs/AI.md`, `docs/ARCHITECTURE.md`, `docs/Code_Quality_Standards.md`). Surgical edits, not removals. +### D6 — considered and LEFT, so the next sweep does not re-derive them + +Three sets were found while executing Phase 1 and deliberately not acted on. They are recorded +because a sweep run against D1 will surface all three again, and an unrecorded "we looked and left +it" is indistinguishable from an oversight. + +**1. Eleven files citing `docs/releases/` paths in PROSE.** Not links — nothing 404s. They read as +provenance: *"the v0.1 execution plan §Q3 set the two-tier gate"* stays true after the file moves. +The two citations that *were* rewritten during Phase 1 are a different case: a `.toml` comment and a +test docstring naming a path a reader would try to open. ⛔ **ADR 0160's own citations must never be +"cleaned"** — this is the ADR that removed the directory, and the paths are the evidence. + +**2. Twenty-two handoff DOCUMENTS outside `docs/releases/`** — 21 under `docs/benchmarks/`, one at +`docs/quality-gates/HANDOFF-mutation-coverage.md`. ⚠️ A name-based sweep reports **155** matches +here; **133 of those are benchmark DATA files** (json/txt) that merely sit inside directories named +`HANDBACK_*`. Overstating the finding six-fold is the first trap. The second is that the bench +handoffs carry the **measurement narrative** for the data beside them — removing them strips the +rationale from records that stay, which is the defect the Consequences section below names. And the +Phase 1 justification does not transfer: `docs/releases/` **misrepresented the project** to a first +reader, whereas a benchmark handoff reads as exactly what it is. + +**3. Unanchoring `/HANDOFF-*.md`.** Rejected, with the reasoning kept in `.gitignore` beside the rule +so it is refuted where it will next be proposed: the two locations that matter are already covered, +and an unanchored pattern would fail closed on `docs/benchmarks/`, where handoffs are tracked on +purpose. + ### D5 — the process TOOLING stays tracked: DECLINED on measured cost, not blocked on someone The obvious next step after Phase 1 is `scripts/coord/` (9), `scripts/worktree/` (11) and diff --git a/docs/adr/README.md b/docs/adr/README.md index 24899643..6f0b4b5d 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -186,5 +186,5 @@ what is withheld and what you can request. | [0157](0157-demotion-safety-fence-scope-on-post-claim-writes-and-a-bounded-graph-stop.md) | **Demotion safety — fence scope on post-claim writes, and a bounded graph stop** — an HA re-check found the leadership lease itself **sound** (DB-clock expiry on both backends, atomic acquire/renew, a real `leader_epoch` token checked inside the claim transaction; scopes B and C were probed and cleared, not assumed) and two things wrong around it. **F1:** the epoch fence guards *some* claims and **nothing after them** — `claim_ready` (the UNORDERED path) carries no epoch predicate on either backend, and every post-claim disposition write (`mark_done`, `mark_failed`, `dead_letter_now`, `complete_with_response`, plus batch twins) resolves by bare `id` with no epoch, owner or status precondition, while `release_claimed` two methods away *does* carry `AND status=$4`. The sharp write is `dead_letter_now`: a demoted node assigning a **terminal** disposition and finalizing the message, breaching the store finalizer's single authority — and a DEAD row is never re-claimed, so H2 skip-and-complete cannot heal it. **F2:** demotion budgets **detection only, never the stop** — `_check_fence` flips a boolean and cancels no listener, worker or in-flight send; `engine.py`'s graph-poll interval is the *only* arithmetic consumer of `(ttl − fence)` in the package and it sizes a poll. Measured budget on stock defaults is **≈8.0 s** (fence 20 + a 1.0 s fence tick + a 1.0 s poll against a 30 s DB-clock expiry) *minus* the renew round trip, which is bounded only by `[store].command_timeout = 30` — **exactly equal** to `leader_lease_ttl_seconds`, so the margin can reach zero and `_fence_ordering` (ordering-only) never notices. Against that, teardown stops inbounds **sequentially** at up to 10.0 s per socket listener (5.0 client grace + 5.0 `wait_closed`, off a module constant unrelated to lease timing) and **unbounded** for file/DB/DICOM inbounds, at a 1,500-connection target. **Decision (6 clauses):** guard writes that make a row **TERMINAL**, never one that returns it to PENDING (fencing the L1 hand-over would convert a permitted duplicate into a forbidden **strand**); two predicates of **opposite polarity** (claim fail-closed, resolve fail-**open**, because a rejected resolve leaves the row INFLIGHT); no `status` conjunct; a demoted node **retains** its stale epoch (`None` means *no fence*, so clearing it disarms the guard); fence every claim path incl. `claim_ready`; and a `TeardownReason{SHUTDOWN,DEMOTE}` bounded, concurrent, edge-triggered demotion stop. Rejected: an `owner=` predicate (dead on SQL Server, which claims `owner=NULL`), a per-claim token (correct, but changes the `Store` protocol — filed, not folded in), fencing writes that ADMIT a message (converts a duplicate into a **loss**), a lease-anchored absolute deadline (makes the monotonic clock load-bearing on Windows, and degrades to an unconditional cancel at the 30/30 collision), and wrapping teardown in `wait_for` **from outside** — `self._running = False` is the last statement of `_teardown_unsafe`, so a cancelled teardown leaves the node **permanently un-re-promotable, silently**. Also records the sequencing asymmetry: Postgres bounds a stranded INFLIGHT row at ~90 s via its periodic sweep, while **SQL Server has no periodic in-flight recovery at all** (`reclaim_expired_leases` is Postgres-only; the runner's `hasattr` gate is the sole exclusion), so the same row is an **unbounded strand today**, with no HA scenario involved. Corrects a code comment attributing a teardown-ordering constraint to "ADR 0066 D3" — that decision does not exist (`grep -c D3` → 0). Single-node SQLite byte-identical (structurally: `set_leader_epoch` is a hard `return None`). #26-clean | **Accepted (2026-08-01)** — C1 (terminal writes only, fail-open) and C6 (bounded, abandon-don't-await) owner-decided; **increments 1, 4 and 5 built 2026-08-02**. Inc 1 fences `claim_ready` plus the eight terminal resolves on Postgres, a rejected resolve rolling the whole disposition back — queue flip, ledger row, event row and finalize together — and then **re-pending** the row. **Three drafted clauses were corrected during the build, each because the draft was strand-direction or false.** C3 said leave the fenced row INFLIGHT for recovery: on SQL Server there is no periodic in-flight recovery at all, so that is an unbounded strand *manufactured by the fence itself* — the one outcome the ADR forbids. C4 alone was a silent total halt: `_reconcile_graph` had only two branches, so `is_leader() and running` matched neither and a live leader held a stale epoch, claiming **nothing**, with no exception and no alert — closed by re-stamping the epoch every reconcile pass. And the cross-backend claim that a demoted SQL Server node "claims nothing" was **wrong**: `claim_ready` is unguarded there, so retaining the epoch covers only the three FIFO claim paths until Inc 3. Evidence is mutation-verified in both directions, which is how the *first* structural gate was caught being blind to its own subject — it keyed on whether a method **mentioned** the guard constant, so deleting the guard from `claim_fifo_heads`' emitted SQL left the mention intact and the gate stayed **green**. **Inc 0/2/3 not built, and Inc 2 is MIS-SPECIFIED in this ADR** — its owner-blind, age-based sweep has no populated `owner` column on SQL Server to discriminate with and would re-pend rows a live leader is working; the real defect is the absence of recovery at graph re-start. Do not build it as written | | [0158](0158-silent-controls-green-signals-that-mean-nothing-and-shape-over-detection.md) | **Silent controls -- green signals that mean nothing, and shape over detection** -- a defect class that recurred at least a dozen times across independent surfaces in one working day (2026-08-01), in at least two sub-classes: **(1) a bound or claim stated INDEPENDENTLY of the thing it bounds** (test: *what measurement backs this?*) and **(2) a control that cannot OBSERVE or ACT ON its own failure** (test: *if this control were broken, what would tell me?* -- if the answer is the control, that is the defect). Spine: **a signal that does not carry enough information to act on forces every reader to re-derive significance by hand, and eventually one of them derives it wrong**; a correct-but-useless RED costs what a silent green costs. Anchor instances: `ci.yml`'s unsourced "~2x headroom" against a real margin near 1.0x, after a PR was killed at 26:07 on the 26:00 step cap with no test failing and passed at 22:25 on a re-run of the same commit; a `UserPromptSubmit` hook that probed a script path this repo has never contained, printed a reassuring status message and exited 0; a gate shim with no `else` on the miss path; **a validator whose input is derived from its subject is satisfied by construction** (zizmor's paths filter excluded the lock its own pinned version arrives through); **an equality check satisfiable by coincidence is not one** (three copies of a hook at identical byte counts differing on one comment line by a 5-for-5-character substitution). Carries the worked inversion: **a measurement beats an estimate only when it measures THE SAME QUANTITY** -- a correct estimate was retracted for a stated "measurement" that was the JOB not the STEP, and a peer amplified it. Seven retractions are recorded **inside the ADR**, including of its own corrections (the pool size, the maximum's filter, the sizing criterion, a BACKLOG number, and line numbers copied from a commit message). Every instance and retraction carries a **found by:** tag, because **no retraction in this document's own production was made by the author of the claim it retracts** -- the bound on that finding is stated. **Shape over detection is reported as a RATIO, not flattered:** three fixes are covered by tests in required CI legs, two by tests that always skip in CI, one by a workflow change with a live residual, the rest corrected prose or still open. Decision splits ENFORCED rules (each naming its gate) from CONVENTION (unenforced, knowingly re-breakable); links rather than restates [CLAUDE.md](../../CLAUDE.md) section 11 and [Secure_Development_Standards](../Secure_Development_Standards.md) section 3; no engine behaviour changes | Proposed (2026-08-01) -- records a class; the coordination-layer fixes it cites are already built | | [0159](0159-cancellation-safe-pooled-connection-release-mid-txn-discard-at-the-acquire-chokepoint.md) | **Cancellation-safe pooled-connection release — quarantine at the `_acquire` chokepoint** (BACKLOG #348) — `SqlServerStore`'s write idiom is `except Exception: await conn.rollback(); raise`, used at **90 of the 91** `self._acquire()` sites. `CancelledError` derives from `BaseException`, so on a cancellation **no rollback runs**, and aioodbc does not compensate: `Pool.release()` appends a non-closed connection straight back onto the free deque with no rollback, reset or transaction check (0.5.0 `pool.py:196-205`), and `_ContextManager.__aexit__` uses the *same* `release` on the exception path (`utils.py:60-62`). The next borrower inherits an open transaction still holding X locks. **Measured on a live SQL Server**: cancelling `release_claimed` left **7** X locks on `queue`, `reschedule_claimed` 7, `mark_done` 9, `enqueue_ingress` 11 (the pre-ACK ingress commit), against **0** for the `claim_fifo_heads` control; the connection returned to the free list, a raw writer got **1222**, and a real second claim yielded **EMPTY-all** — which ADR 0066 §9 sanctions, so the symptom is *silence*, not an error. Fix: at `_acquire`, on a non-`Exception` `BaseException` only, **synchronously** drop the driver handle (`conn._conn = None` — aioodbc derives `closed` from it and re-adds only `if not conn.closed`) **with no await in front of it**, then close the raw handle off-loop under a 5s bound. The ordering *is* the guarantee: a cleanup that awaits first is defeated by the **second** cancellation that shutdown's cancel-then-gather delivers. A plain `await conn.rollback()` was built and **rejected on measurement** — it runs in the default executor bounded only by `command_timeout` (30s) with no upstream timeout, and a cancel measured **1.005s** against a 1.0s rollback, stalling exactly the **demotion** path (`_stop_graph` cancels but does **not** close the store, so the poisoned connection is re-borrowed there). Corrects the lead that found it on two points: it is **not** a two-method asymmetry, and `claim_fifo_heads` does **not** shield against it — its guard is a `SET LOCK_TIMEOUT` *reset* guard and ADR 0114 §2 plus a frozen test record that **no rollback** runs on its cancellation path; it ends clean because the guard **commits**. Ordinary errors keep today's rollback-and-recycle behaviour, pinned by controls that pass before and after. SQL Server only — Postgres is safe twice over (asyncpg rolls back on any `BaseException` and its pool resets under `shield`), SQLite has no pool | Accepted (2026-08-02) — built and verified the same day; gate failed 6/12 pre-fix, passes 12/12 post-fix, live repro 7→0 locks | -| [0160](0160-public-repo-content-policy-operator-and-security-review-material-only.md) | **Public-repo content policy -- operator and security-review material only** -- the owner stated twice on 2026-08-04 that the public repo should carry only what someone RUNNING MessageFoundry needs plus what a SECURITY REVIEW needs (ADRs named explicitly as the second), and that material about how the project is built with Claude Code comes out. **It was recorded NOWHERE** -- a session grepped every local and remote ref and found nothing -- while being cited as a constraint on four other sessions' work, which is exactly the standing the coordinating session had REFUSED from a peer the same day over a relayed owner ruling. That asymmetry is why this exists: an unrecorded policy enforced across isolated sessions is indistinguishable, to the session receiving it, from an invented one. The question is new because the 2026-07-27 cutover retired the publish-time deny-list that used to answer it, so it is now answered per file, at authoring time, with no gate and no written rule. Measured at `c90dcb5f`: `docs/` 580 tracked, `docs/adr/` **156 (stays)**, `docs/releases/` **101** (mostly `*-MULTISESSION-PLAN.md`, the largest concentration of process material), `scripts/` 61. Decision: a two-part test (operator need OR reviewer need), applied in three reversible phases -- `docs/releases/` first, then the individually-tracked process docs, then in-file prose -- with **no enforcement gate proposed and the reason stated**, because a path-based gate is evaded or wrong and a keyword gate fires on every legitimate use of "session". ⭐ **`CLAUDE.md` is a genuine policy-versus-tooling conflict and stays tracked as a named exception**: by the test it should go, but `git worktree add` CANNOT deliver an untracked file, so while it was untracked every new worktree came up with **zero project conventions loaded** (verified on two live worktrees; it only looked right in the primary because an untracked copy happened to sit on disk). Rejected: keep everything (a public `docs/` that is majority process material misrepresents the project to its intended reader), reinstate a publish deny-list (retired deliberately; re-adds a sync problem and the slug-rewrite damage class), and move the process DOCS to `claude-multisession` (the tooling move is owner-authorised but recorded in no ruling -- context, not constraint). ⚠️ Explicitly **NOT a confidentiality control** -- `git log` retains everything removed, nothing here was ever treated as secret, and anything genuinely sensitive belongs in the vault and always did | **Accepted (2026-08-06). Phase 1 EXECUTED; Phase 2 DECLINED; Phase 3 Proposed.** Phase 1 removed **103 files** -- `docs/releases/` (101) plus two session handoffs found tracked in the repository ROOT, now covered by a `/HANDOFF-*.md` pattern that fails closed on the next one. Custody moved to the vault and was PUSHED BEFORE removal, because gitignoring alone leaves unversioned single copies that `git clean -xdf` erases -- the two mechanisms are complements, not alternatives. Owner rulings recorded: the vault is the destination (open question 3), `claude-multisession` is REJECTED for this material, and the governing rule is **do not break anything**, applied per item as *prove the mechanism or leave the item alone*. ⛔ **D5 DECLINES the process tooling move** (`scripts/coord\|worktree\|hooks`, ~28 files + 26 tests, and the four docs describing them) on measured cost rather than blocking it on anyone: the tempting cheap gate does not work, because the 26 test files split **exactly 13/13** between `which("pwsh")`-only skips (which run on hosted ubuntu) and `or os.name != "nt"` skips (which **silently skip** there) -- so a Linux-only vault leg covers half the suite and reports GREEN, the same silent-control defect relocated. A correct gate needs ubuntu AND windows, the vault is private so Windows bills at 2x, and the vault's CI is `disabled_manually` on every workflow but one (`ci.yml` last ran 2026-07-27, failed). Benefit is cosmetic -- unlike Phase 1's plans, nobody mistakes dev tooling for the product -- so it is DECLINED, not deferred, because a permanently-blocked item reads as work someone still owes. ⚠️ Records two findings that outlive it: `tests/test_feature_map_claims.py` resolves links against the FILESYSTEM, so ignored-but-present files make it **blind in any working tree** (measured: a re-introduced dangling link passed locally, failed against a `git archive` export) -- verify link changes against a tracked-files-only export; and `git rm --cached` spares only the tree it runs in, CONFIRMED when the Phase 1 rebase deleted the files from the working tree, which is what will happen to the primary and all active worktrees on merge. Corrects the draft's "cannot be docs-only" reasoning: two of its three cited non-doc files are comments and the third's entry is an EXCLUSION list, so the real cause is that the mechanism IS a `.gitignore` edit, which ci.yml classifies as code by construction. Open: **does `docs/BACKLOG.md` stay?** | +| [0160](0160-public-repo-content-policy-operator-and-security-review-material-only.md) | **Public-repo content policy -- operator and security-review material only** -- the owner stated twice on 2026-08-04 that the public repo should carry only what someone RUNNING MessageFoundry needs plus what a SECURITY REVIEW needs (ADRs named explicitly as the second), and that material about how the project is built with Claude Code comes out. **It was recorded NOWHERE** -- a session grepped every local and remote ref and found nothing -- while being cited as a constraint on four other sessions' work, which is exactly the standing the coordinating session had REFUSED from a peer the same day over a relayed owner ruling. That asymmetry is why this exists: an unrecorded policy enforced across isolated sessions is indistinguishable, to the session receiving it, from an invented one. The question is new because the 2026-07-27 cutover retired the publish-time deny-list that used to answer it, so it is now answered per file, at authoring time, with no gate and no written rule. Measured at `c90dcb5f`: `docs/` 580 tracked, `docs/adr/` **156 (stays)**, `docs/releases/` **101** (mostly `*-MULTISESSION-PLAN.md`, the largest concentration of process material), `scripts/` 61. Decision: a two-part test (operator need OR reviewer need), applied in three reversible phases -- `docs/releases/` first, then the individually-tracked process docs, then in-file prose -- with **no enforcement gate proposed and the reason stated**, because a path-based gate is evaded or wrong and a keyword gate fires on every legitimate use of "session". ⭐ **`CLAUDE.md` is a genuine policy-versus-tooling conflict and stays tracked as a named exception**: by the test it should go, but `git worktree add` CANNOT deliver an untracked file, so while it was untracked every new worktree came up with **zero project conventions loaded** (verified on two live worktrees; it only looked right in the primary because an untracked copy happened to sit on disk). Rejected: keep everything (a public `docs/` that is majority process material misrepresents the project to its intended reader), reinstate a publish deny-list (retired deliberately; re-adds a sync problem and the slug-rewrite damage class), and move the process DOCS to `claude-multisession` (the tooling move is owner-authorised but recorded in no ruling -- context, not constraint). ⚠️ Explicitly **NOT a confidentiality control** -- `git log` retains everything removed, nothing here was ever treated as secret, and anything genuinely sensitive belongs in the vault and always did | **Accepted (2026-08-06). Phase 1 EXECUTED; Phase 2 DECLINED; Phase 3 Proposed.** Phase 1 removed **103 files** -- `docs/releases/` (101) plus two session handoffs found tracked in the repository ROOT, now covered by a `/HANDOFF-*.md` pattern that fails closed on the next one. Custody moved to the vault and was PUSHED BEFORE removal, because gitignoring alone leaves unversioned single copies that `git clean -xdf` erases -- the two mechanisms are complements, not alternatives. Owner rulings recorded: the vault is the destination (open question 3), `claude-multisession` is REJECTED for this material, and the governing rule is **do not break anything**, applied per item as *prove the mechanism or leave the item alone*. ⛔ **D5 DECLINES the process tooling move** (`scripts/coord\|worktree\|hooks`, ~28 files + 26 tests, and the four docs describing them) on measured cost rather than blocking it on anyone: the tempting cheap gate does not work, because the 26 test files split **exactly 13/13** between `which("pwsh")`-only skips (which run on hosted ubuntu) and `or os.name != "nt"` skips (which **silently skip** there) -- so a Linux-only vault leg covers half the suite and reports GREEN, the same silent-control defect relocated. A correct gate needs ubuntu AND windows, the vault is private so Windows bills at 2x, and the vault's CI is `disabled_manually` on every workflow but one (`ci.yml` last ran 2026-07-27, failed). Benefit is cosmetic -- unlike Phase 1's plans, nobody mistakes dev tooling for the product -- so it is DECLINED, not deferred, because a permanently-blocked item reads as work someone still owes. ⚠️ Records two findings that outlive it: `tests/test_feature_map_claims.py` resolves links against the FILESYSTEM, so ignored-but-present files make it **blind in any working tree** (measured: a re-introduced dangling link passed locally, failed against a `git archive` export) -- verify link changes against a tracked-files-only export; and `git rm --cached` spares only the tree it runs in, CONFIRMED when the Phase 1 rebase deleted the files from the working tree, which is what will happen to the primary and all active worktrees on merge. Corrects the draft's "cannot be docs-only" reasoning: two of its three cited non-doc files are comments and the third's entry is an EXCLUSION list, so the real cause is that the mechanism IS a `.gitignore` edit, which ci.yml classifies as code by construction. Followed by a 104th file: `docs/releases/HANDOFF-232-router-steps.md` entered main via PR #225 WHILE the Phase 1 PR was open, and since a removal that enumerates paths cannot cover a file created after its commit was built -- and gitignore does not untrack -- the directory came back holding one file. **D6 records three sets considered and LEFT** so a later sweep does not re-derive them: 11 prose citations (provenance, not links; 0160's own must never be "cleaned"), 22 handoff documents outside `docs/releases/` (a name sweep reports 155, but 133 are benchmark DATA under `HANDBACK_*` dirs, and the rest carry the measurement narrative for the data beside them), and unanchoring `/HANDOFF-*.md` (rejected -- both locations that matter are already covered, and it would fail closed on `docs/benchmarks/` where handoffs are tracked on purpose). Open: **does `docs/BACKLOG.md` stay?** | | [0161](0161-async-session-mail-for-unreachable-peers.md) | **Async session mail for unreachable peers** (BACKLOG #1028) — the realtime session channel cannot address two peer classes, **structurally rather than by a filter**: a session launched by the VS Code extension is never entered into the Desktop app's in-memory map of sessions it spawned, and a session under a different login sits behind an independent config root (measured 2026-08-05: one repo, one moment, Desktop sessions on one root and VS Code sessions on a second, neither side visible to the other). The client's compiled peer-to-peer protocol is inert — the registry field carrying a peer's socket address is written by no code path and the lookup **fails silently green**, an empty peer list rather than an error. Decision: an **async file-drop lane** under `/mefor-coord/mail/`, keyed by the recipient's normalised **worktree path** (not session id, which `/clear` re-mints; not worktree name, which is a creation-time label — one worktree was observed on four branches under four sessions in a day), with the key function held in **one definition** dot-sourced by both ends because a drifted second copy fails silently on both sides. Location is the leak control: nothing under `.git` can enter a commit, and `mefor-coord` is not a ref namespace, so `push --mirror` cannot carry it — which is why worktree paths are plain text in the queue deliberately. Explicitly **does not replace** the realtime channel for desktop-to-desktop. **The claim primitive is the load-bearing part, and it was forced by measurement:** `[System.IO.File]::Move` **returns success without moving** for losers under contention — 16 racers x 500 rounds on .NET 10.0.9 / Windows 10.0.26200, instrumented in C# because PowerShell scriptblock closures do not capture loop variables reliably; **every** round had more than one racer return with no exception, in **375 of 500** rounds **all sixteen** did, and the obvious fix — verify `File.Exists(dst) && !File.Exists(src)` — was **true for all 16 racers in all 500 rounds**, because the winner's move makes it true for everybody. A destination unique per claimer yielded exactly one winner in **500 of 500**. Controls confirm the no-op is contention-only (a `Move` of a nonexistent or already-moved source raises). `scripts/coord/claim.ps1` is **not** affected — exclusive `CreateNew` plus a per-PID-unique target — do not "fix" it. Also decided: the **on-disk filename is authoritative and the JSON `id` is discarded** (sanitising it would be a weaker control that looks identical), the drain **never emits a runnable command**, one body sanitiser at the single injection point so a body cannot forge the frame, receiver-side caps (a send-time cap is bypassed by the write that *is* the transport), and a content rule with the force of the secrets rule. Bad consequences are stated rather than softened: the **write-side trust boundary cannot be enforced** by this design (any process running as the user can drop into any inbox, so every `from.*` field is a self-assertion), **delivery duplicates the body into a transcript no prune reaches**, and the urgent `asyncRewake` tier is **one-shot** because the rewake belongs to the process Claude Code spawned and a self-respawned grandchild's exit code is heard by nobody | **Proposed (2026-08-05)** — the code is a **PROTOTYPE and is deliberately NOT WIRED**; the installer rows exist but no config root has been installed from them. Wiring is a separate owner-approved step gated on BACKLOG #1028, and the ten EARS criteria link to functions that exist in `tests/test_session_mail.py`. **Arm B of the claim measurement was later corrected:** it was taken with 16 threads in ONE process, and re-measuring with 16 separate processes showed `File.Exists(own destination)` reporting a win to more than one racer in 46 of 800 rounds, so the shipped verdict is an exclusive open, not an existence check | diff --git a/docs/releases/HANDOFF-232-router-steps.md b/docs/releases/HANDOFF-232-router-steps.md deleted file mode 100644 index aa25ea90..00000000 --- a/docs/releases/HANDOFF-232-router-steps.md +++ /dev/null @@ -1,200 +0,0 @@ -# Handoff — BACKLOG #232: Steps view for routers (2026-08-05) - -> **Status going in.** The ADR gate is **discharged**: [ADR 0076](../adr/0076-typed-action-vocabulary-action-list-lens.md) -> **Amendment D** (owner-ratified 2026-08-05) widens the grammar with a `route` row kind, and -> [`CLAUDE.md`](../../CLAUDE.md) §12's carve-out now names Routers. **No feature code was written** — -> this lane was design-only. What remains is a straight build against a settled contract, not a -> decision. Everything below lets a session build it without re-deriving the design work. -> -> **This is a NOT-DEPLOYED beta with zero production instances.** #232 is a **low-severity capability -> gap** — no correctness or security risk, and no prior router Steps contract to preserve, so there is -> no migration or compatibility shim to build. Where this doc says a router "would" render or an older -> IDE "would" break, that conditional is deliberate. - -## What #232 wants, and why it is not "point the lens at routers" - -`lens parse` emits rows per `@handler` only — ADR 0076 §3 put routers "out of v1 scope" — so a -`@router` gets **no Steps view at all**: no "View as Steps" CodeLens on the def, no rows. An analyst who -can read a Handler as steps drops back to raw Python exactly where **destination selection and fan-out** -are decided. - -A router does not mutate `msg`; it **selects destinations**. The shipped shape is a guard-and-return of -handler names ([`samples/config/IB_DEMO_ORU_router.py:22-27`](../../samples/config/IB_DEMO_ORU_router.py)): - -```python -@router("demo_oru_router") -def route_demo_oru(msg): - if msg["MSH-9.1"] != "ORU": - return [] - return ["demo_oru_relay"] -``` - -The reason the v1 grammar excluded this is a **grammar gap, not a veto**: the v1 vocabulary is a -field-mutation roster (`copy_field`/`set_field`/…) over the mutable `Message` API, and it had **no row -kind** for a routing return. Amendment D §D.2 records the full finding (the #26 carve-out's "Handlers" -wording is INCIDENTAL; nothing that put routers out of v1 is a live blocker). Read Amendment D -(§D.1–§D.8) before building — it is the settled contract this handoff implements. - -## The four build components - -1. **A `route` row kind** (DECIDED, over overloading `send`). `send` rows carry outbound-connection - names and belong to the outbound delivery stage; `route` rows carry **handler** names and belong to - the routed stage. Different namespace, different pipeline stage — overloading `send` would fuse the - two. Contract (Amendment D §D.3): - `{ kind: "route", handlers: [..], unrouted?: true, line_start, line_end, nesting }`. `handlers` - follows `send`'s literal-or-empty rule; `unrouted: true` is the additive discriminator for a - routed-nowhere return, mapping to the store disposition **UNROUTED** (logged, never dropped). -2. **`return []` disambiguation by the enclosing decorator** (DECIDED — see "The `return []` resolution" - below). -3. **A router-specific Add-palette group** — routing-relevant items only (route-to-handler, a guard, a - comment). No transform verbs, no lookups (Amendment D §D.6: a router stays pure destination-selection; - `db_lookup`/`fhir_lookup` raise outside a live Handler). -4. **Coverage-partition + byte-stable-splice parity** — a `@router` body must tile into - `route`/`control`/`note`/`code` rows that exactly partition the def body, and a `route` edit must be a - byte-stable row-scoped splice, exactly as the handler path already guarantees. - -## Files the build will touch - -Line numbers drift — anchors below were verified on this branch; locate exactly at build time. The build -touches **at least** the following: - -- **[`messagefoundry/lens.py`](../../messagefoundry/lens.py)** — the parser. - - Add a `_router_name(node)` keyed on `_callee_name(dec.func) == "router"`, mirroring `_handler_name` - (`:343-360`). - - In `parse_source`, stop the router `continue` at **`:306`** (`continue # not a @handler (router or - plain def) — out of v1 scope`) and emit a router entry discriminated by role (e.g. a `role` field, or - a `router`/`handler` discriminator) so a consumer can tell a router projection from a handler one. - - Thread the def's **role** from `parse_source` through `_partition_suite` (`:470`) → `_emit_stmt` - (`:537`) → `_classify_simple` (`:667`) so a router `return []` classifies as a `route`/`unrouted` - row while a handler `return []` stays **byte-identical** to today's `send`/`filtered` row - (`:678-686`). - - Keep the coverage-partition invariant (`_partition_suite` already tiles gaps/blanks/comments); a - router body just recognizes `route` instead of `send` on its returns. -- **[`ide/src/editorToolbar.ts`](../../ide/src/editorToolbar.ts)** — the CodeLens/toolbar gate. - - `hasHandler` at **`:64`** gates the "View as Steps" affordance on a `@handler`; extend it to routers - (add a `hasRouter`, or generalize to "has a Steps-renderable element"). - - The per-element CodeLens gate `if (el.kind === "handler")` at **`:88`** adds "View as Steps" for - handlers only; add the same lens for `el.kind === "router"`. - - The `messagefoundry.activeFileHasHandler` context key at **`:131-132`** drives the editor-title - button; add/relax it so a router-only file also offers the button. -- **[`ide/src/stepsView.ts`](../../ide/src/stepsView.ts)**, **[`ide/src/stepsModel.ts`](../../ide/src/stepsModel.ts)**, - and **[`ide/media/stepsWebview.js`](../../ide/media/stepsWebview.js)** — render the `route` kind and the - router Add-palette group. The kind MUST be threaded through **both** the TS model and the CSP-isolated - webview (which cannot import from `src/`) — this is the same dual-implementation discipline Amendment A - §A.7 records for `note`; a kind added to one alone renders a blank, titleless row in the other. -- **[`scripts/quality/lens_coverage.py`](../../scripts/quality/lens_coverage.py)** — router coverage - stats. Out of scope for the first build increment, but the coverage scan may be extended to count - router rows so the router recognition rate is measurable the way the handler rate is. - -## The `return []` resolution (DECIDED) - -The enclosing decorator disambiguates. Verified against the engine's own return normalizer -`_handler_names` ([`messagefoundry/pipeline/dryrun.py:98-101`](../../messagefoundry/pipeline/dryrun.py), -`list[str] | str | None`; `[]` == routed nowhere): - -| in a `@router` | `route` row | -|---|---| -| `return []` / `return ()` / `return None` / a bare `return` | `handlers: []`, `unrouted: true` (message logged **UNROUTED**, never dropped) | -| `return ["a", "b"]` / `return ("a",)` (string-literal names) | `handlers: ["a", "b"]` | -| `return "a"` (bare string literal) | `handlers: ["a"]` | -| a non-literal element (`return [pick(msg)]`, `return names`) | `handlers: []`, **no** `unrouted` (dynamic; mirrors `send`'s empty-on-non-literal) | - -In a `@handler`, `return []` is **unchanged**: it stays the `send` row with `filtered: true` -(`lens.py:678-686`). The role branch is the only thing that makes a router return classify as `route`, so -AC-R3 (assert the handler leg first) is the guard that the branch never regresses the handler path. - -## ADRs to amend at build time - -- **ADR 0076 — done here** (Amendment D). No further 0076 amendment is expected for the row kind. -- **ADR 0089 (recognition-first) and ADR 0108 (send fan-out)** — on current analysis these are - **leveraged, not widened**: neither names routers, and the router path reuses their recognition and - partition machinery rather than changing their contracts. The builder should **re-confirm** this the - moment the router body needs a construct those ADRs own (e.g. an accumulator-style routing idiom), and - amend if so — but nothing found in this lane requires it. - -## Red-by-design tripwires — do not "fix" them early - -[`ide/src/test/suite/editor-toolbar.test.ts:62`](../../ide/src/test/suite/editor-toolbar.test.ts) asserts: - -```ts -assert.strictEqual(hasHandler('@router("IB")\ndef route(msg): ...'), false); -``` - -This is **correct today** (a router-only file cannot open as Steps) and **inverts when the build lands** -(a router file will open as Steps, so `hasHandler` — or its router-aware successor — must report the file -as Steps-renderable). It is a deliberate tripwire, **not a flake and not a regression**. When you build: -update this assertion as part of the change, and do not touch it before then. A builder who "fixes" it -early, or who reads its future inversion as a regression, has misread it. - -**At least two further published assertions invert the same way.** They are recorded here so the builder -updates them deliberately and does not read their post-build failure as a regression. Each is a **correct -statement about the shipped code today** — the build is not done, so none is a false present-tense claim; -the build is what inverts them: - -- **`tests/test_lens_parse.py:435` — `test_routers_are_out_of_scope`.** Its body ends - `assert [c["handler"] for c in contracts] == ["h"] # router excluded`. Once a `@router` gets a - projection, that list would gain the router and both the assertion and the test's intent must be - rewritten — update it in the same change that adds the `route` row. -- **`docs/testing/master-test-plan/13-steps-editor.md` — STEPS-52, STEPS-46, and the `:39` "already - covered" row.** STEPS-52 (`:149`, **P1**) is literally "Routers have no Steps view, at the provider - level too"; STEPS-46 (`:143`) says the "View as Steps" CodeLens "appears only on a `@handler`"; the - `:39` evidence row records "routers out of scope (`:435`)". Amendment D §D.1 supersedes that scope, so - all three describe the **pre-build** state and would need reconciling when the build lands. **Editing - the master test plan is out of this lane's scope** — it is flagged here (and surfaced to the owner) so - the builder reconciles it deliberately rather than being surprised when STEPS-52 has to become "routers - **do** have a Steps view". - -These are forward-reconciliation notes, not defects: every one is true of the code as shipped, and the -build is what inverts them — the same relationship the `editor-toolbar.test.ts:62` tripwire has. - -## Contract-version skew (must be handled, not discovered) - -Mirror Amendment A §A.7. `parse_source` emits no schema version and the extension shells whatever -`messagefoundry` is on `PATH`. An older IDE receiving `kind: "route"` hits the default-less title switch -and renders a **blank, titleless row**. **Gate `route` emission behind a flag or a contract version**, and -thread the kind through both `ide/src/stepsModel.ts` and `ide/media/stepsWebview.js`. `route` is additive -for handlers (a `route` kind appears only for a `@router`), so the flag/contract gate is the whole -compatibility story. - -## Corrected anchors (the item's own citations drifted) - -- The lens router skip is at **`lens.py:306`** — the #232 item body says `:305` (off-by-one). `:344-347` - in the item points inside `_handler_name`, whose full span is `:343-360`. -- The sample router is [`samples/config/IB_DEMO_ORU_router.py:22-27`](../../samples/config/IB_DEMO_ORU_router.py). -- **Citation trap (§3 vs §4).** The #232 item, its BACKLOG banner, and its ranked-table row all say the - `route` kind "widens the ADR 0076 §3 grammar". Per ADR 0076 §2 the *grammar* rule points at **§4** - (recognition grammar + degradation ladder); **§3** is the row **enum**. The `route` kind touches §3's - enum **and** §4's grammar — cite it that way (Amendment D does). The BACKLOG banner is corrected in this - lane; the **ranked-table row (`docs/BACKLOG.md:234`) is off-limits to this lane and is flagged to the - owner** to correct separately. - -## Gates - -All docs edits in this lane; the build's gates are the project quartet plus the doc guards: - -``` -python -m ruff check . -python -m ruff format --check . -python -m mypy messagefoundry -python -m pytest -q # name BOTH testpaths if you touch messagefoundry_webconsole/ -python scripts/docs/backlog_status_check.py -``` - -When the build lands, follow the project's falsify-every-new-test rule: for each new router test (the -coverage-partition property, the route-return classification, the handler-vs-router byte-stability guard, -the router-palette scope, the contract-version-skew guard), break the thing on purpose, watch the new -test go red, then restore — and report the falsification you actually ran. - -## Open questions (settle with the owner before or during the build) - -1. **Amendment D acceptance status.** Recorded as ACCEPTED (owner-ratified, build handed off), parallel to - Amendment A, with a **counted** Acceptance-Criteria block. Confirm it should be counted (not PROPOSED - under a distinct heading like Amendments B/C). -2. **Router-body recognition scope.** Recommendation: recognize routing constructs only — `route` returns, - control rows, `note`, `code` — and offer only routing-relevant Add-palette items. A router must stay - pure destination-selection. Confirm. -3. **`unrouted` label.** Recommendation: an additive `unrouted: true` on the route row (distinct from the - handler `send`'s `filtered: true`), matching the store disposition UNROUTED. Confirm the flag/wording. -4. **The stale ranked-table row.** `docs/BACKLOG.md:234` says the `route` kind "widens the ADR 0076 §3 - grammar" and reads better as "§3 enum + §4 grammar". It is off-limits to this lane (never touch the - ranked table). Owner to decide whether to correct it outside this lane.