ci(guard) + engine: fix a CI gate that is green and testing nothing, plus PR 433's engine payload (BACKLOG #343/#328/#321/#1247/#1215) - #613
Open
wshallwshall wants to merge 20 commits into
Open
Conversation
…tent below INFO (BACKLOG #343) Records the design decision for #343 before the engine change lands, because the diff shows what and never why, and the why here is a rejection that is easy to re-litigate. THE REJECTED ALTERNATIVE IS THE LOAD-BEARING PART. Relaying child stderr at INFO with a per-line byte cap is the obvious compromise and it fails on the shape of this payload specifically: truncating an HL7 v2 message to its first N bytes keeps MSH and PID -- the header and the patient identifying segment -- and discards the clinically bulky remainder. A byte cap therefore preserves precisely the most identifying part of the record. It is the worst available redaction for this format, not merely a weak one, and it would place a CLAUDE.md section 9 violation inside the fix for the defect that violation is about. So content relays at DEBUG only, and INFO and above get an attributed rate-limited notice carrying identity and a count and no content. That satisfies section 9 BY CONSTRUCTION rather than by operator discipline: no call site above DEBUG carries content. States the hazard the decision CREATES rather than softening it: stderr=PIPE with no drainer blocks a flooding child, and the window that matters is bootstrap, where load_config() runs untrusted admin code before the boot reply is read. The stderr reader must start in the same window the stdout reader does. Findings are written conditionally per CLAUDE.md section 0 -- zero deployments, so a deploying site WOULD inherit this on first deployment; nothing is exposed today. Index row added in the same commit, as the ledger gate requires. ADR number allocated by this worktree via alloc.ps1 -- ownership is keyed on the committing worktree and is non-transferable.
…use a silent overwrite (BACKLOG #1247) The installed gate's content changed on this box while three sessions ran against it, and afterwards nobody could say who wrote it. The change was benign -- it moved the gate forward -- which is not the point: an unattributable write to a shared safety control is the same class of event whichever direction it moves the file. FOUR MECHANISMS COULD HAVE RECORDED IT AND NONE WAS PRESENT: no backup, no receipt, no log line, and no usable timestamp. The install site was a bare Copy-Item. THE MTIME WAS THE TRAP, NOT THE GAP, and this does not repair it. Copy-Item carries the SOURCE's LastWriteTime, so the installed copy inherits an mtime from whichever checkout installed it. A correct stale-gate report was RETRACTED on the strength of one, and the retraction reached three sessions and the owner before a baseline hash reproved the original finding. An absent record makes a reader say "unknown"; a WRONG record makes them say something false with confidence. A corrected timestamp would still be one mutable field asserting a fact nothing corroborates, so the record is the receipt. PROVENANCE IS THREE-WAY AND COLLAPSING IT TO A BOOLEAN LOSES THE POINT. UNRECORDED (a gate installed before this change) is the normal first-run state and must NOT be fatal -- refusing there would block the very re-install that adopts the mechanism. Only MODIFIED stops the install, because an overwrite destroys the only evidence that anything happened. -OverwriteUnverifiedGate proceeds deliberately. THE HELPER IS A SEPARATE FILE FOR A TESTABILITY REASON, not a stylistic one: install-gate.ps1 cannot be dot-sourced to reach its functions, because loading it performs a machine-global install into ~/.claude. A suite that had to install the gate to test the receipt would be a suite that installs the gate, and that is the owner's action by design. IT REUSES Get-GateHash RATHER THAN HASHING AFRESH. That digest folds CRLF, because git stores LF and a Windows checkout carries CRLF; a second basis would let the receipt and tests/test_gate_installed_parity.py disagree about one file. A DEFECT FOUND WHILE BUILDING THIS, RECORDED IN THE CODE BECAUSE IT IS THE SAME CLASS THE ITEM IS ABOUT: try is not an expression in PowerShell. Written as a parenthesised try/catch in a hashtable value, the parser reads try as a command name, ParseFile reports the file CLEAN, and at run time it discards the whole hashtable, writes a receipt containing the literal null, and still returns a path and exits 0. The round-trip guard did not catch it either, because null is valid JSON -- a check its own failure mode satisfies is not a check. Both are now fixed and commented. Tests split deliberately: behaviour against a temp directory (never the real gate), plus static assertions that the installer is WIRED to call it. Each half is blind to the other's failure -- the behavioural tests pass if nothing calls the functions, the static ones pass if every function is wrong.
…te (BACKLOG #321, Proposed 3) A required merge context exited 0 on content carrying a real site code. scan_forbidden.py:10-12 is explicit that gitleaks finds SECRETS, not this class, and the file WAS scanned -- the detectors simply did not match. Every existing site-code detector is PREFIX-DRIVEN, so with no token file loaded it degrades to the always-failing _NEVER sentinel: blind until the owner supplies data. This adds a STRUCTURAL detector that works with nothing loaded, which is what makes it a backstop rather than a second thing waiting on the owner. THE SHAPE HAD TO EARN ITS PRECISION, because a leak gate that cries wolf gets disabled and that is strictly worse than the hole it closes. Measured over the tracked tree (1,953 files): a bare delimited six-digit run matches 637 lines in 146 files. Requiring the run to JOIN a letter-bearing identifier segment across an underscore takes that to 5 -- every one of them this gate's own illustrations of the shape, now rewritten with the house placeholder. Post-fix: 0 new hits on content, 0 on paths, 0 on a full --path . walk including untracked files. Whole-tree scan cost 3.50s to 4.13s. IT CATCHES THE THING THAT GOT THROUGH. Replayed against the pre-removal revision the item cites, the detector fires on exactly one line of that file and on zero lines after the removal. WIDTH IS PINNED AT EXACTLY SIX WITH NO LEADING-ZERO CARVE-OUT. A 5-to-7 band only works with a no-leading-zero rule, and that is a silent under-detection hole in a gate whose filed defect IS a silent hole -- the numeric prefix accepts a leading zero, so it is reachable. A HAZARD ALL THREE CANDIDATE DESIGNS MISSED: each excluded a dot after the digit run, so a feed module FILENAME written in prose did not match -- and a filename was half the evidenced leak. Permitting the dot costs zero additional hits on either corpus. Both arms now cover it. THE SELF-SCAN CANARY NEEDED EIGHT DIGITS, NOT SIX. A probe literal in the scanner's own source must survive its own new detector AND a real prefix list no author can see. An eight-digit run has no six-digit window with a boundary on both sides, so it cannot be a site code under any prefix list. WHAT THIS DOES NOT CLOSE, and the module docstring now says so in the file itself: the item names TWO token classes and only the SITE-CODE half is closable by shape. A partner-product name is a proper noun in prose with no width, character class, separator or position regularity. That half closes only under Proposed 1, which is owner-run token data and is not mine. Shipping this as "the estate-identifier backstop" would let a green run read as "no estate identifiers present" -- the same defect the item diagnoses, recreated one level up. Tests assert TOKENS_PRESENT is False first: a prefix-free detector has no globals to monkeypatch, and that precondition is what makes the backstop claim non-vacuous. Includes a liveness guard that the detector is not the _NEVER sentinel, because no floor mechanism counts a structural detector. Verified independently of the build: scanner exits 0 over the tracked tree, detector fires on a synthetic positive and stays quiet on a git sha and a bare number; ruff check and format clean, mypy strict clean over 266 files, 106 passed on the directly-affected modules. NOT DONE, deliberately: docs/BACKLOG.md is untouched. The item's own proposed regex at :2703 matches NEITHER example the item itself gives, and correcting a ledger body is not a builder's.
…ntent below INFO (BACKLOG #343) The worker was spawned with stderr=None, so the child's stderr WAS the engine's own stderr, inherited raw. fd 1 is strictly framed; fd 2 had no discipline at all. Admin-authored Handler code could write arbitrary bytes into the operator's log of record -- forged engine lines, control sequences, or a printed message body. Both halves are conditional, not live: zero deployments, so a deploying site WOULD inherit this on first deployment. CONTENT RELAYS AT DEBUG AND ONLY DEBUG. At INFO and above the engine emits an attributed, rate-limited notice carrying identity and a COUNT and no content, so CLAUDE.md section 9 holds BY CONSTRUCTION rather than by operator discipline: there is no call site at which child stderr content becomes a record above DEBUG. The per-line byte cap was rejected in ADR 0166 for a reason specific to this payload -- truncating an HL7 v2 message to its first N bytes keeps MSH and PID, so it preserves precisely the most identifying part of the record. THE DEADLOCK THIS CREATES IS CLOSED IN THE SAME COMMIT. stderr=PIPE with no drainer blocks a flooding child, and the window that matters is bootstrap, where load_config() runs untrusted admin code before the boot reply is read. The relay thread starts in the same window the frame reader does. IDENTITY IS (inbound, pid, generation), NOT pid. An OS recycles pids, and the design turns on a stale generation's relay still draining a killed child while the live one runs. THE LINE CAP IS A MEMORY BOUND, NOT A REDACTION. A Handler can write megabytes with no newline, and an unbounded carry lets the child size the parent's heap. Reaching it splits one write across several DEBUG records and DISCARDS NOTHING, which is what distinguishes it from the rejected byte cap. PHI redaction is NOT reimplemented here -- it is a property of the log handlers this relay rides like any other record, and a second call site would be the drift SDS-3.5 warns about. Control-character scrubbing IS applied at this seam, because "one child write is one log record" is the relay's own framing contract and must not depend on how a host configured logging. ADR 0166 IS CORRECTED BY THIS BUILD, and the correction is the useful part: the draft claimed the stdout rebind leaves fd 1 "exclusively for frames". IT DOES NOT. Rebinding the NAME sys.stdout does not seal the descriptor -- sys.__stdout__.buffer, os.write(1, ...) and open(1, "wb") all still reach it. What keeps a raw writer harmless is unchanged: the closed-tag codec and the parent's unsolicited-frame check. Claiming the rebind sealed fd 1 would have been a compensating control resting on a false premise (SDS-3.7); what it actually buys is that the ACCIDENTAL case, print() in a Handler, can no longer sit one buffering change away from corrupting a frame. The ADR's attribution site is also corrected -- the identity is plumbed from RegistryRunner._sandbox_for in pipeline/wiring_runner.py, NOT engine.py, which builds only the policy and the config source -- and its line-number citations are replaced with symbol references, because a line anchor goes stale the moment the code it points at moves. Three consequences the ADR did not originally carry, each recorded when the build measured it: the drain is EOF-driven rather than cooperatively stopped (closing a pipe under a mid-read thread raises ValueError past an except OSError); close() takes a BOUNDED join, because this drain calls into logging and can be inside a handler's emit when logging.shutdown runs at exit; and the relay being the sole drainer makes a stalled off-box log collector back-pressure on a DEBUG-level child. Verified: mypy strict clean over 266 source files, ruff check and format clean over messagefoundry and tests, 87 passed across test_sandbox, test_sandbox_worker_logging, test_accepts_seam and test_phi_logging_inventory, in the lane venv built against constraints.lock.
… (BACKLOG #343) Both were found by the review round AFTER the implementation was committed, and both are claims rather than behaviour -- which is exactly the class that survives a green suite. F1. _reject_unsolicited's docstring asserted that the ADR 0166 stdout rebind means "the text layer cannot reach fd 1 at all", so "there is no benign case to preserve". BOTH FALSE, and this docstring is where a DESTRUCTIVE action is reasoned from -- drop the worker and dead-letter the message in hand. Rebinding the NAME sys.stdout leaves the descriptor open: sys.__stdout__ is still a text layer on fd 1, its .buffer IS the BufferedWriter the worker captured as the frame writer, and os.write(1, ...) and open(1, "wb") reach it too. The reviewer measured a benign one-liner that destroys the worker. THE SAME COMMIT ALREADY REFUSED THIS CLAIM TWICE -- in _sandbox_worker.py's bootstrap comment and in ADR 0166, both citing SDS-3.7 -- so the change contradicted itself and the false half was the one driving the destructive path. That is the compensating-control-on-a-false-premise shape, reintroduced one file away from where it had just been rejected. THE ACTION IS UNCHANGED AND NEVER NEEDED THE STRONGER CLAIM. A frame no outstanding request asked for violates the one-request-one-frame protocol whether it was written deliberately or by accident, and the queue cannot tell the difference: benign-but-unsolicited is still a frame the next dispatch would misread as its answer. The justification is now the protocol violation, not an impossibility. F2. The drain-ordering test's FALSIFICATION instruction was measurably wrong. It said to move the drain start below the boot-frame WRITE; ADR 0166 records the measurement that this does NOT wedge, because the parent then blocks on the reply while the drain is already running. It wedges when the drain starts below the point the boot REPLY is read. Corrected, with the reason kept: a falsification that does not falsify is worse than none, because it reads as a checked escape hatch and the one person who follows it concludes the guard is untestable rather than that the instruction was wrong. The PHI hunt found no section 9 violation, and found it by measurement rather than reading: the relay was driven through the real configure_logging() filter chain -- not caplog, which carries no filters -- at INFO, WARNING and DEBUG with a full synthetic HL7 body plus an over-cap terminator-free run. At INFO/WARNING: two records, both the counts-only notice, and none of the synthetic identifiers in either the records or the rendered stream. Verified: ruff check and format clean, mypy strict clean over 266 source files, tests/test_sandbox.py 34 passed.
… SQLite (BACKLOG #328) audit-verify cannot detect a truncated tail. The startup auto-verify is a BARE WALK -- it confirms the chain it can see is internally consistent and has nothing to compare that chain's LENGTH against, so removing the tail leaves a shorter, perfectly valid chain and nothing objects. THE STORED ANCHOR COULD NOT BE WIRED THERE, and that is why this was a design fork rather than a fill-in. expected_anchor is an EXACT point-in-time seal: it requires the CURRENT head to equal the recorded one, so any appended row diverges. A running instance writes audit rows, so consuming a stored anchor at startup would alarm on essentially every restart. That property is DESIGNED and pinned by test_an_anchor_goes_stale_on_the_next_appended_row; this adds a SECOND comparator beside it and does not weaken that test, which still passes unmodified. expected_prefix asks the weaker, useful question: was the recorded state ever true, and has the chain only GROWN since? It holds the head captured AT the recorded position against the recorded one. CHOSEN OVER SEAL-ON-STOP / CHECK-ON-START BECAUSE THAT IS BLIND EXACTLY WHERE THE THREAT LIVES. Sealing during a clean shutdown detects truncation across a clean stop, and a tamperer does not shut down cleanly. A control that needs the adversary's cooperation to arm itself is a ceremony, not a control. Prefix-assertion is also strictly stronger than the item asked for: it catches a mid-chain rewrite too. WRITTEN ONCE AND EXPORTED rather than stated per backend -- audit_prefix_verdict lives beside audit_row_hash / audit_mac_bytes, which postgres.py and sqlserver.py already import. A predicate restated per backend is the copy-versus-single-source defect BACKLOG #1253 catalogues, where a later hardening reaches one copy and silently misses the rest. THE CAPTURE IS A POSITION TEST, NOT A DATA-DEPENDENT BRANCH, so it does not reintroduce the early return the walk deliberately avoids (ASVS 11.2.4, stated in the walk's own comments). A prefix_head of None means the walk never reached the recorded position -- the truncation case -- and must FAIL rather than pass vacuously. THE NEGATIVE CONTROL CAUGHT A VACUOUS TEST OF MY OWN AND THE FIRST VERSION IS RECORDED IN THE TEST. Deleting the head compare left the suite GREEN. The rewrite-a-row-in-place construction breaks the hash chain, so the WALK reports "chain broken" and returns before the comparator is consulted -- the test passed for a reason unrelated to what it claimed. Rebuilt as a SAME-COUNT TAIL REPLACEMENT (truncate behind the engine's back, then let the engine append replacements through its own API, so the chain is internally valid and the count is restored); only the head at the anchored position distinguishes it. With that shape, deleting the head compare reds exactly that test and no other. SQLite only in this commit. Postgres and SQL Server share the primitive but NOT the loop: both SQLite and Postgres carry a running `count`, while SQL SERVER USES len(rows) AND TRACKS NO POSITION, so its capture needs a counter the other two already have. Wiring those two plus the Store protocol is the next layer, deliberately not folded in here. Verified: mypy strict clean over 266 source files, ruff check and format clean, tests/test_audit_integrity.py 48 passed, mutation-proved in both directions with the plant asserted before the run.
…Server and the Store protocol (BACKLOG #328) Completes 839575e, which added audit_prefix_verdict and wired it into SQLite only. The predicate is still written ONCE, beside audit_row_hash / audit_mac_bytes, and imported by both server backends -- restating it per backend is the copy-versus-single-source defect BACKLOG #1253 catalogues. SQL SERVER IS THE ODD ONE OUT AND GENERALISING FROM EITHER TWIN WOULD HAVE SHIPPED A NO-OP THERE. The SQLite and Postgres walks carry a running `count`; SQL Server reports len(rows) and tracks NO POSITION at all. A prefix capture needs a position, so this introduces the counter the other two already had. My own written recipe for this item said "all three backends share one loop shape" -- that was WRONG, and reading the third backend rather than generalising from the first two is what caught it. A fix derived from the twins would have passed on two backends and silently done nothing on the third, which is the shape this item exists to prevent one level up. The Store protocol now declares expected_prefix, so a caller can reach it polymorphically rather than only through a concrete SQLite store. That is what makes the startup auto-verify able to consume it later; nothing calls it yet, deliberately -- wiring the caller is a separate decision with its own alerting consequences. Verified: mypy strict clean over 266 source files with the protocol and all three implementations agreeing, ruff check and format clean over messagefoundry/store, tests/test_audit_integrity.py 48 passed. The SQLite behaviour -- including the mutation-proved negative control from 839575e -- is unchanged by this commit; Postgres and SQL Server carry no local suite here and their legs are CI's.
…ne file The leak gate's `_ESTATE_ID_SHAPE` content check was deliberately left ungated on `_SITE_SKIP_*`, and the stated reason was "the anchor already removes that storm (measured: zero matches across those files)". That was true when it was written and is not true now. `messagefoundry/auth/data/common_passwords.txt` grew to 15,256 lines on main after this detector was written. The anchored pattern fires NINE times on it, every hit a wordlist entry that happens to join a letter-bearing segment to a six-digit run. `forbidden-content` is a blocking required context run `--path .` over the whole tree, and branch protection is `strict = true`, so this is not optional. The blanket `_SITE_SKIP_*` gate is still NOT used, because that set includes `.svg` and a flame-graph SVG's frame labels are function names -- a transform function name is one of the two forms this detector exists for. Instead a new `_ESTATE_SKIP_NAMES` names the ONE file whose content is a generated wordlist rather than authored text. By NAME only, never by suffix, so `.lock` and `.svg` keep their estate-shape content scanning. The file-NAME check is untouched: a file whose own name carries the shape is still a hit. Verified with the control, not just the absence: common_passwords.txt 9 hits -> 0 a planted PT_123456_ADT elsewhere still 1 hit whole tree, --path . 2047 files, 0 hits, exit 0 The expired claim is rewritten rather than deleted, and now reads as the history it is. A measured claim is only as current as its measurement, and a security control resting on a stale one is the SDS-3.7 shape. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ays total `tests/test_gate_install_receipt.py` arrives on this branch; the manifest that classifies every non-engine test arrives from main. Neither side is wrong on its own and the merge puts the file in neither list. Main's `test_tooling_partition.py::test_every_non_engine_test_is_classified` globs `tests/test_*.py` and fails any file classified by neither list. It runs on all three required `test` legs, so this reds every one of them. Verified by running main's own predicate either side of the change: 1 failed before, 9 passed after. One line, placed in sorted position beside its sibling `test_gate_installed_parity.py`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ping the file Supersedes my own earlier fix on this branch, which gated the `_ESTATE_ID_SHAPE` content check on a new `_ESTATE_SKIP_NAMES` set. That was wrong and this branch already said so in a test I had not read. `tests/test_scan_tokens_source.py::test_estate_identifier_shape_is_not_gated_by_the_site_skip_suffixes` writes `def xform_<code>_to_erp_mfn` into `requirements.lock`, `art.svg` AND `common_passwords.txt` and asserts the shape is caught in every one, then writes a bare digit run and asserts it is not. It pins a real property: a transform function name inside a password file is exactly as much of a leak as anywhere else. My skip defeated that, and the test failed on all three engine legs -- correctly. The nine hits are genuine coincidences in a GENERATED wordlist, verified line by line on main at 6e758a8. Seven are the list's own `ABUSE_<digits>_ABUSE` abuse-report markers; two are ordinary passwords of the form word_word_<6 digits>. None is a site code. So they are allowlisted, which is the mechanism the scanner's own failure message names, with three anchored patterns rather than a file-level skip: ^ABUSE_\d{6}_ABUSE$ ^lky_vipnyc_\d{6}$ ^dungklose_\d{6}$ Verified with the control that distinguishes a fix from a blinding: common_passwords.txt 9 hits -> 0 xform_123456_to_erp_mfn IN THAT FILE still 1 hit whole tree, --path . 2047 files, 0 hits tests/test_scan_tokens_source.py 77 passed The original design comment is restored verbatim. Its "measured: zero matches across those files" reasoning did expire -- that observation stands -- but the remedy is to excuse the shapes actually present, not to stop looking. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…st failure (BACKLOG #1260) THREE LAYERS OF NAMING SAID "TESTS FAILED" AND NOT ONE WAS TRUE. Confirmed live on PR #398 tonight: the check is named `test (windows-2025, py3.14)`, the step `Tests (pytest)`, and the process exited 139 -- 128 + SIGSEGV(11) -- printing "Segmentation fault". ZERO tests failed. There is no pytest summary line and no FAILED id anywhere in the log, because the process died before it could write one. A reader at any of those three layers reaches for a test regression that does not exist. GitHub reports `steps.tests.outcome` as "failure" for ANY non-zero exit, so a segfault and a failing assertion are indistinguishable to everything downstream. The distinction has to be made at the step, which is what this does. PASS/FAIL IS UNCHANGED. The exit code is captured and re-raised, so a crash still reds the leg; only the log line and the annotation become true. Exercised for real rather than reasoned about: rc=139 emits the annotation and exits 139, rc=134 likewise with signal 6, rc=1 emits NOTHING and exits 1 -- so a genuine test failure can never be relabelled as a crash -- and rc=0 stays clean. IT DELIBERATELY DOES NOT RETRY, and that is the item's other half left open on purpose. scripts/ci/retry-native-crash.sh exists and is correct, but its documented scope is the pyodbc/py3.14 SQL Server binding crash (upstream #1459), down to a "REMOVE THIS WRAPPER once #1459 ships a fix" instruction. The engine suite does not use pyodbc, so this leg's crash has a DIFFERENT and currently unknown cause. Wrapping it here would couple an unrelated leg's crash handling to that removal note, and retrying an unknown-cause crash is closer to laundering than the pyodbc case, where the cause is documented upstream. That is a decision, not an oversight, and it is recorded at the call site. THE GUARD THAT PINS THIS STEP HAD TO BE RE-AIMED, AND THE FIRST RE-AIM WAS WRONG. tests/test_ci_engine_step_excludes_webconsole.py located the step by scanning for a line starting `run: pytest -q`, which pinned it to being a ONE-LINE `run:`. Wrapping the invocation broke the LOOKUP rather than any assertion, and the failure read "no engine step found" instead of "the step moved" -- a locator coupled to a step's spelling blocks every change to how that step is invoked. Broadening the scan to any `pytest -q` line was measured WORSE: it matches ci.yml's EARLIER doc-guards step (`pytest -q -rs $DOC_GUARDS`) and asserts against the wrong invocation entirely. That was caught only because the test went red on it. Disambiguating by `--ignore-glob` would have been circular -- that is the thing under assertion, so the locator would be satisfied by its own subject and could never fail. It now locates the step STRUCTURALLY by name, via the parsed workflow. Verified: ci.yml still parses as YAML (10 jobs, the step's run is a block); the three pinning tests pass; and the guard is mutation-proved in two directions with each plant asserted before the run -- swapping --ignore-glob for plain --ignore reds it, and renaming the step reds it. Both restored byte-identical. yaml was already a test dependency (three sibling tests import it).
This branch rewrote the locator to find the engine step STRUCTURALLY, by step
name, precisely so the guard would stop being coupled to how the step is
spelled. It then required the `pytest` token to be FIRST on the line, which is a
spelling coupling by another name.
PR 566 landed `bash scripts/ci/retry-native-crash.sh` in front of the
invocation. The locator stopped finding it and failed as "the 'Tests (pytest)'
step runs no `pytest` command" -- reporting an absent command rather than a
wrapped one, which is the same misleading-shape failure the rewrite was for.
It now slices FROM the token, requiring a word boundary before it. Every
assertion below is unchanged. Matching mid-line is safe ONLY because the step is
already located by name -- a mid-line search over the whole file is what would
hit the doc-guards step, as this test's own comment records.
WHAT THIS ALSO SHOWS, and it is a live defect on main rather than a note about
this branch: main's version of this locator scans for a line beginning
`run: pytest -q`. On main the engine step is wrapped and no longer matches, and
the ONLY line that does is `ci.yml:1102` -- the TOOLING step. So main's guard
for "the engine step subtracts the console package" is asserting against the
tooling step and passing because that step happens to carry the same
`--ignore-glob`. This branch's rewrite is the fix for that, and its comment
predicted the failure mode before it occurred.
Verified with a control that distinguishes the two steps, which is the whole
point:
engine step's --ignore-glob removed, tooling step untouched 1 failed
restored 3 passed
On main that same control would pass wrongly, because the locator there is
reading the wrong step.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…il channel (BACKLOG #1215) Three records described a channel that is wired and delivering as an unwired prototype. No behaviour changes; every edit is a comment or a status line. The reason it is worth a commit is that all three fail in the same direction -- they invite a reader to reason about a RUNNING mechanism as hypothetical. (1) ADR 0161's Status line and its "Status and what gates wiring" section both called the code a prototype "deliberately not wired", with "nothing live in any session". FALSE AT HEAD: scripts/coord/install-coordination.ps1:238-239 carries a SessionStart row AND a Stop row, both pointing at scripts/hooks/mail-drain.ps1. The ADR's decision, measurements and trade-offs are untouched -- only its claim about its own subject's deployment state was wrong. (2) mail-drain.ps1's header said "THIS DOES NOT WIRE ANYTHING ... install-coordination.ps1's rows are untouched". THAT SENTENCE WAS FALSE IN THE COMMIT THAT ADDED IT: fdec72c (#210) introduced those rows itself. A "this changes nothing" claim is worth exactly as much as the diff it ships beside, and it is a shape to distrust -- the claim is about the commit's own blast radius, so the only thing that can refute it is the commit, and a reader who trusts the comment never opens it. (3) THE MARKER PARAGRAPH HAS NOW BEEN WRONG IN BOTH DIRECTIONS, which is why it is written out rather than quietly corrected. It first said "marker state can only ever suppress a re-display"; that was false against the code of the day and was corrected to "A MARKER THEREFORE DOES GATE A CONSUME". Then the consuming path changed underneath the correction and re-inverted it. The shipped guard is `$markerPath -and -not $consuming`, so the marker check is SKIPPED ENTIRELY when consuming, and the code's own comment says so at that site: "A CONSUMING DRAIN THEREFORE IGNORES MARKERS ENTIRELY". The ORIGINAL claim is now the true one. THE THIRD IS THE DANGEROUS ONE AND THE ITEM NAMES WHY: the wrong sentence sat inside a paragraph whose whole subject was correcting a previous falsehood, so it read as the CHECKED statement -- the most convincing form a wrong sentence can take. A reader who doubted it and re-read the header got the inversion CONFIRMED. Only the running code disagreed, and only at a different site. Verified: mail-drain.ps1 parses clean (every session runs it at SessionStart and Stop); 169 passed / 2 skipped across the mail, session-mail, ADR-index and feature-map suites; and the no-glyph rule enforced with the cp1252 encodability test rather than by eye -- my added lines introduce NO non-cp1252 character, measured against the diff rather than the file, because the ADR carries pre-existing ones elsewhere that are not mine to sweep.
wshallwshall
enabled auto-merge (squash)
August 26, 2026 15:49
wshallwshall
disabled auto-merge
August 26, 2026 16:02
wshallwshall
changed the title
… before writing BACKLOG #343 SHIPPED, #328/#321/#1215 PARTIAL, #1247 CONTESTED (not a closure -- see the note explaining why). Each verified against the actual code in this PR before the banner was written, not carried forward from any earlier claim: - #343: matches the item's own "Fix direction" exactly (stderr=PIPE, reader thread, stdout rebind). Old open-status banner removed, replaced rather than left contradicting the new SHIPPED one. - #328: the comparator this item names as its prerequisite ships on all three backends; the three-file startup wiring it also needs does not. Verified by checking which files the two comparator commits actually touch. - #321: Proposed 3 (the structural shape backstop) ships; Proposed 1 (owner-run token data) is owner-only and stays open. - #1215: two of the item's false claims are corrected here; a third (ADR 0161 + one inline comment) is Builder 1's separate, disjoint work landing on PR 604 -- confirmed disjoint before writing this note. - #1247: two competing implementations exist (this PR and PR #607) with opposite default security postures on a receipt mismatch. Not a closure -- explicitly flags the dispute so a reader does not mistake this PR's landing for a settled question. Verified: backlog_status_check.py clean (602 items), ADDED/LOST set-difference against origin/main is empty in both directions, and each item's open/closed state matches intent (#343 closes, the other four stay open). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…d the limb it named as missing Dispatcher caught this: PR #604 merged to main (871f146) between when this PR's #1215 note was written and now, landing the exact limb the note named as "not done" (ADR 0161's status line, its wiring section, the line-258 comment). Left as written, the note would have read as still-accurate the moment this PR merged, when it was actually already wrong. Corrected to name PR 604 explicitly as a third, separately-landed limb, and to flag a fourth defect the item names that NEITHER PR touches: one warning glyph still in docs/adr/0161-*.md, verified present on main directly before writing this note. The item stays open on that account -- not fully closed by this PR plus 604 combined, which the note now says explicitly rather than leaving ambiguous. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ADR is 0176
The renumbering was mine and so is this. Earlier today I tried to hand-edit the ADR
allocation registry, a PreToolUse hook correctly refused it, and re-allocating through
alloc.ps1 issued 0176 instead of the 0166 this branch had already been written against.
The file was renamed; 31 citations to the old number were not.
WHY THE FULL POPULATION AND NOT THE ONE CI NAMES. Exactly one of the 31 is a markdown
link, so exactly one turns test_every_relative_link_in_the_repo_resolves red. Repairing
that single line and re-arming would have landed the other 30 green and silent, and the
link test is structurally incapable of catching them because only links resolve. The red
check sees three percent of the defect.
lines citing ADR 0166 or its slug on this branch 31
replaced 30
deliberately NOT replaced 1
THE ONE LEFT ALONE IS NOT A CITATION. tests/test_scan_tokens_source.py:1330 carries
"docs/adr/0166-sandbox-child-stderr-capture.md" as a SYNTHETIC NEAR-MISS PATH in a
negative-control table. Its own docstring says half the entries carry a six-digit run on
purpose because no tracked path does, and that a table drawn only from real paths would
be a control that cannot fail. Rewriting it to 0176 would edit test data toward the very
shape it exists to exclude. A grep for a citation also matches a string that merely looks
like one.
Verified: the link at docs/adr/0087-sandbox-subprocess-isolation.md:349 now targets
0176-sandbox-child-stderr-is-captured-and-relayed-content-below-info.md, which exists on
this branch. The diff is 30 insertions and 30 deletions across 11 files -- a 1:1 line
replacement, which is also the control proving no line-ending rewrite occurred, since a
re-encode would have touched every line rather than thirty.
No engine behaviour changes. Every replaced line is prose, a comment, or a docstring.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part B of stranded PR #433 (
claude/hopeful-chaplygin-3158c6), per Cleaner's grading(
.git/mefor-coord/handoffs/CLEANER-2026-08-26-FOR-LANDER-stranded-pr-grading.md) and the owner's2026-08-26 ruling to disarm that PR's auto-merge and split it. This is the "engine payload" half --
13 commits, cherry-picked onto a fresh branch off current main rather than resolving conflicts on
the original ref, so its now-disarmed auto-merge is never at risk of firing on this content.
HOLD -- owner-ruled needs a human read before merging, unarmed for that reason now, not #1247.
#1247's posture question is resolved: the owner took this PR's fail-closed default unmodified over
PR #607's fail-open one. Separately, the owner also ruled that this PR needs an actual human review
pass before merging -- 2,065 lines of engine and test code with
required_approving_review_count = 0otherwise means landing unread, the same reasoning that disarmed PR 433 itself. That rulingdoesn't lapse just because #1247 resolved. Not re-arming until someone reads it.
Leads with the highest-priority item.
538f541e6fixes a required CI guard(
tests/test_ci_engine_step_excludes_webconsole.py) that is currently GREEN ON MAIN AND ASSERTINGNOTHING: PR #566 wrapped the engine test step behind
scripts/ci/retry-native-crash.sh, so theguard's string-match locator (
stripped.startswith("run: pytest -q")) no longer finds the enginestep -- the only surviving match is the unrelated tooling step, which already carries the exclusion
the guard is checking for, so every assertion passes against the wrong invocation. Verified with a
positive AND negative control before pushing: ran the fixed test as-is (3 passed), then temporarily
stripped the engine step's
--ignore-globfromci.ymland re-ran (1 failed, correctly), thenrestored the file (confirmed empty diff). The fix needed one extra commit beyond Cleaner's KEEP list
to work at all --
83e32b7d8'stests/half (itsci.ymlhalf is the already-superseded partCleaner named; the
tests/half does the structural by-step-name rewrite that538f541e6thenpatches for a wrapped invocation). Landing
538f541e6alone hits a merge conflict that resolvesinto nonsense without this prerequisite; both are included. This fix is downstream of the already-
SHIPPED #1260 (the crash-retry wrapper) rather than an implementation of it -- #1260's own banner
already reads SHIPPED on main, so it is not re-cited here.
Everything else, in landing order:
ddb7b0731+ its ADR file -- ADR 0166 filing (BACKLOG fix(gate): recognise the interpreter flag by RULE and read rule 3d's victim quote-aware (BACKLOG #1097, #1064) #343). Re-allocated to ADR 0176: theledger gate correctly caught that 0166 is no longer free on main (decay since Cleaner's analysis),
and allocated 0176 in its place. File renamed, self-references and the README index entry updated
accordingly. (The commit message for this one still says "ADR 0166" -- a cosmetic mismatch from
rewriting history I chose not to force through after a
git reset --hardwas declined; the fileitself and the index are correct.)
5774bf9dd-- worktree gate install receipts (BACKLOG #1247). Owner-ruled the winner over PRfix(worktree): a write to the machine-global gate now leaves a record (BACKLOG #1247) #607's fail-open alternative; that half of #1247 is settled. Still not landing yet -- see the HOLD
note above, which is now about review, not posture.
b3ecfc7be-- estate-identifier leak-gate backstop (BACKLOG backlog: amend #1020 and #1022 -- partial, not closed; both stay OPEN #321, Proposed 3 of 3 -- Proposed 1,owner-run token data, stays open). Claim on backlog: amend #1020 and #1022 -- partial, not closed; both stay OPEN #321 was held by a worktree confirmed dead via
fleet.ps1(freshest record: INTERRUPTED, 119 hours old) and absent fromgit worktree list;force-released per the claim gate's own documented path, not overridden.
a113717b1+5d9133114-- sandbox child stderr capture + its review-fixup (BACKLOG fix(gate): recognise the interpreter flag by RULE and read rule 3d's victim quote-aware (BACKLOG #1097, #1064) #343 --SHIPPED, verified against the item's own "Fix direction" before writing the banner).
a956293e8+bb3208969-- monotonic-prefix audit comparator, SQLite then Postgres/SQLServer/Store protocol (BACKLOG docs(asvs 11.2.2): the crypto agility seam contract, and a false comment corrected by execution #328 -- PARTIAL: the comparator this item names as its prerequisite
ships on all three backends; the three-file startup wiring the item also needs does not, and stays
open).
d143e4d66,459fb9536,020149178-- estate-shape skip reasoning, tooling-manifest entry,wordlist-collision allowlist. Auto-merged clean, no BACKLOG-gated claim needed.
a85f7acc1-- native-crash-as-crash CI naming, thetests/prerequisite described above.919da04a2-- the CI guard fix itself.54eb29316-- two of four false claims inscripts/hooks/mail-drain.ps1corrected (BACKLOG #1215-- PARTIAL, one limb of at least four named by the item). A third limb landed separately and
earlier on
main, as PR docs(adr)+fix(hooks): the session-mail channel is wired, and four places still called it a prototype (BACKLOG #1215) #604 (871f146ae) -- coordinated live with Builder 1, who held #1215'sclaim for that disjoint half (ADR 0161's status line + a line-258 comment), confirmed non-
overlapping before either landed. A fourth limb -- a warning glyph in
docs/adr/0161-*.md--is untouched by both PRs, verified still present on
mainbefore writing the ledger note. Theitem is not closed by this PR plus docs(adr)+fix(hooks): the session-mail channel is wired, and four places still called it a prototype (BACKLOG #1215) #604 together; the note says so explicitly.
Dropped from
65ef2801c: its ADR 0161 hunks. Cleaner's grading called this DROP on therefuter's grounds -- the branch's text asserts an approval step happened; main's landed correction
deliberately does not, on the owner's ruling that ADR 0161 stays
Proposed. Confirmed with Builder 1that their own in-flight #1215 work covers ADR 0161 properly, so nothing is lost by dropping it here.
Skipped as fully redundant:
4c0b65283(ADR index rebuild) -- its intended end state was alreadyreached by resolving the two 0166-then-0176 README conflicts above against current main; the
commit's own diff came back empty once resolved, so it was skipped rather than committed as a no-op.
docs/BACKLOG.mdupdated in this PR for the five items above, each verified against the actuallanded diff before writing (SHIPPED for #343; PARTIAL for #328, #321, #1215; a settled-posture note
for #1247, still not a closure). This is why the
backlog-hygienegate now passes -- it did not onthe first push, correctly, since the original body cited six BACKLOG numbers with no ledger update
at all.
Verified before pushing:
ruff checkandruff format --checkclean across the branch,mypy messagefoundryclean (267 files), the full test suites for every touched test file pass (165tests), and
backlog_status_check.py+ an ADDED/LOST set-difference check both clean (0 items addedor lost; #343 correctly flips closed, #328/#321/#1215/#1247 correctly stay open). SQLite-backed
monotonic-prefix tests pass; Postgres/SQL Server legs need their CI containers to confirm.
Does not touch
claude/hopeful-chaplygin-3158c6itself. Part A (the BACKLOG #1100 doc audit) is aseparate PR.
Co-Authored-By: Claude Opus 5 noreply@anthropic.com