DRAFT triage: builder-2's abandoned a12484 lane -- 11 commits, never PR'd, 6 open items - #531
Draft
wshallwshall wants to merge 11 commits into
Draft
DRAFT triage: builder-2's abandoned a12484 lane -- 11 commits, never PR'd, 6 open items#531wshallwshall wants to merge 11 commits into
wshallwshall wants to merge 11 commits into
Conversation
…y bare name (BACKLOG #1255) `testpaths` names two roots and both ship a `conftest.py`. Neither root is a package and no `importmode` is set, so pytest's default `prepend` gives both files the same importable name, `conftest`, and a run collecting both trees lets only one win `sys.modules`. The trap is silent rather than loud, which is what earns a guard instead of a comment: the two conftests duplicate the logging-quiesce machinery, so a mis-bound `import conftest` need not raise -- it can SUCCEED and hand back the wrong tree's implementation. Measured by AST over both files, they share 10 top-level names counting module-level constants, 8 counting only defs and classes. The item records 8; the two figures agree on the same population and differ only over whether `_ABOVE_CRITICAL` and `_QUIESCE_TARGETS` count, so the narrower rule is the item's own. THE ITEM'S RECOMMENDED FIX WAS MEASURED AND DOES NOT WORK, which is why this lands as a guard. `__init__.py` in BOTH roots is what #1255 proposes: both directories are named `tests`, so both conftests become `tests.conftest`, the collision moves up one level and turns fatal -- `_pytest.pathlib.ImportPathMismatchError`, and the whole suite fails to collect. A marker in the root tree only leaves the mis-bind permanently pointed at the web tree. `importmode = "importlib"` DOES remove the collision, and is deliberately NOT taken here rather than rejected. #1255 states it would break the 44 files under these roots that import the `tests` package; that did not reproduce on pytest 9.1.1 in a sandbox. One helper in a sandbox is not 44 real files, so the honest status is that the item's stated risk needs re-measuring on this tree before anyone adopts or dismisses it. Switching the import semantics of 691 files on an unreproduced premise is the change this guard exists to avoid needing. No present-tense defect is claimed: today's behaviour is correct because nothing does the bare import. This makes the house idiom -- shared helpers in named modules, imported package-qualified -- enforceable instead of customary. The guard ships its own controls, because an empty findings list is what both a clean tree and a dead walker return: a positive control on a planted import, a negative control pinning the three shapes that must NOT be findings, a scope control asserting every testpath root was reached and a floor on import statements seen, and a premise pin that reds if the two conftests ever stop claiming one name, telling the reader to re-price rather than leaving decoration behind. Roots are read from `pyproject.toml`, so a third testpath is covered without widening a literal. Verified: guard reds on a planted bare import naming the exact offender, reverts byte-identical and greens again; 13 passed across the guard and the partition test; ruff 0.15.22 clean; mypy strict clean; zero cp1252-unsafe characters.
…-not-measure from broken (BACKLOG #1290)
`test_subtree_re_resolution_picks_up_a_late_spawned_child` red-ed a REQUIRED context on `main`
(`test (windows-2022, py3.14)`, and the windows-2025 leg since) on the `walked_ok` assertion: the
process-table walk returned None on every one of 6 attempts in 30 s, the Windows path allowing 5 s
per walk. It is nondeterministic -- the identical leg passed on another branch in the same hour, and
re-running the failed job alone on the same head returned success.
The failure fires on the FIRST of the test's two assertions, so the re-resolution regression
assertion below it never runs. While this is red the test is not merely failing, it is SILENT on the
defect it was written to detect -- which is the worse half and the reason this is not just noise.
WHY MORE ATTEMPTS WERE NOT THE FIX. The old loop retried at the same 5 s bound, so six attempts
bought six identical cut-offs and no measurement. The extension grants a LONGER budget
(3 x `_PROBE_TIMEOUT_S`), which is what actually attacks a starved runner. It is entered ONLY when
no walk succeeded at all, so it cannot grind a genuine missing-child result into a pass; it is
bounded at two walks; it is trimmed to fit the per-test watchdog READ from `--timeout` rather than
hardcoded (ci.yml passes 60 s on ubuntu and 120 s on the Windows legs, and a copy here would drift
in the bad direction -- an extension outgrowing the watchdog turns a clean skip into a timeout kill
with no verdict); and it is undone at teardown.
THE SKIP IS NARROW ON PURPOSE, BECAUSE A SKIP-ON-LOAD IS HOW A PROBE REGRESSION HIDES FOREVER.
A walk that spends its whole budget measures the RUNNER, and now skips. A walk that returns None
WITHOUT spending its budget is a broken enumerator -- it errored or returned zero rows -- and still
FAILS. Those get opposite verdicts off `_BUDGET_CONSUMED_FRACTION`, stated once. `subprocess.run`
cannot raise `TimeoutExpired` before its timeout, so a genuine timeout always lands above that line
and an immediate error far below it.
ALSO FIXES THE TEST'S OWN INSTRUMENT, found while doing the above. The loop scored a walk by
`sampler._pids is not None`, but `_pids` RETAINS the last good resolution across a subsequent failed
walk -- that is the cache's purpose -- so once any walk succeeded, every later walk scored a success
and reported the stale cache as that walk's result. `_resolve_errored` is set per walk and answers
the question actually being asked.
No production code changed: `harness/load/connscale/probe.py` is untouched.
VERIFIED, both directions of the discriminator, by planting into the probe and reverting it
byte-identical each time:
broken enumerator, returns None instantly -> FAILED, "121 of them returned None WITHOUT spending
the timeout they were given", not downgraded
starved runner, each walk spends its budget -> SKIPPED, "6 walk(s) at a 5s budget, spending
5.0-5.0s", extension granted 0 walks against the
60 s watchdog, which is the correct answer there
healthy path -> 21 passed
Attribution of the 25 pre-existing mypy errors in this file done by controlled revert plus a second
instrument: the error MESSAGE SETS at HEAD and with this change are identical (not merely equal in
count), and zero of them mention any symbol this change adds. The 16 cp1252-unsafe characters in
this file are likewise unchanged by it -- census identical at HEAD and here. Neither is introduced
by this commit and neither is fixed by it. ruff 0.15.22 clean.
…ession verdict (BACKLOG #1290) Fixes a BLOCKING defect in b3bc8026, found by an adversarial review of that commit and reproduced here before changing anything. b3bc8026 is unpushed and in no PR, so this never reached anyone. THE DEFECT. `_BURN` is a BOUNDED loop -- measured on this box at 12.3 s, 12.2 s and 11.6 s, exiting on its own with rc=0. The bounded extension is entered only after the poll has spent `_RESOLUTION_DEADLINE_S` (30 s) with no successful walk, which is already about 2.5x the child's lifetime. So every extension walk necessarily runs after the child is dead, and when one SUCCEEDS -- which is precisely what it was built to do -- `walked_ok` flips, the skip arm is bypassed, and control falls through to the final assertion, which reports RE-RESOLUTION REGRESSION. That is a confident false accusation of the A3 product defect, on a run that never tested for it. It is strictly WORSE than the behaviour it replaced: before b3bc8026 the same conditions said "the process-table walk never succeeded", which was true. WHY MY OWN RED-FIRST MISSED IT, and this is the transferable part. I planted two violations and reported the fix proven "in both directions". Both were the same direction. A broken enumerator and a starved runner BOTH make every walk fail, so `walked_ok` never flips and only the skip and fail arms are reachable. The extension's only path is FAIL-THEN-SUCCEED, and neither plant produced it. Two plants that agree are not two directions. THE FIX is a liveness latch, not a wider tolerance. A successful walk records whether the target was still running at that moment; if no successful walk happened during the child's lifetime and the child is absent, the verdict is COULD NOT MEASURE (failure one) rather than a regression (failure two). A LATCH, NOT A SAMPLE, AND THAT DISTINCTION IS THE SECOND DEFECT. The first version of this fix sampled liveness at the LAST successful walk, and it suppressed the genuine regression: when the child is never found the poll runs the full 30 s, so the last walk is post-exit even in runs where EARLY walks succeeded while the child was alive and legitimately showed it missing. Latching on ANY in-lifetime success keeps the real finding and discards only the unmeasurable one. A NARROW FORM WAS REACHABLE BEFORE b3bc8026 TOO -- a walk failing past about 14 s then succeeding inside the 30 s deadline hits it identically. The extension widened the window and made post-exit success the DESIGNED path. This guard closes both, so the file ends up better than it started rather than merely repaired. VERIFIED ACROSS ALL FIVE PATHS, each plant reverted byte-identical afterwards: healthy -> 21 passed broken enumerator, fails fast without spending budget -> FAILED, enumeration failure starved runner, every walk spends its whole budget -> SKIPPED, enumeration timeout walk needs 8 s: fails at the 5 s bound, succeeds at 15 s -> SKIPPED, could not measure walk succeeds instantly, child ALIVE, child omitted -> FAILED, RE-RESOLUTION REGRESSION The last one is the control that matters: it proves the guard did not make the regression assertion unreachable. The first version of the fix skipped it, which is how the latch defect was caught. ruff 0.15.22 clean; no cp1252-unsafe character introduced; the 25 pre-existing mypy errors in this file are unchanged and none mentions a symbol this change adds.
…tead of a bare assert 0 > 0
tests/test_multishard_smoke.py has been red on main with `assert e.reads > 0` firing as `assert 0 > 0`,
and the failure could not be attributed. This does not change what the test ASSERTS. It changes what
the failure SAYS, which is the part that was unusable.
I HANDED THIS INVESTIGATION A HYPOTHESIS AND IT WAS WRONG. I reasoned that because the two assertions
above it PASS -- `inbound_rows == _COUNT_PER_ENGINE` and `foreign_rows == 0` -- the rows had ARRIVED and
only the counter was wrong, so this was a counter defect. That is refuted. Both of those counters are
CONFIG-derived, not traffic-derived: the `/connections` builder appends a source row for EVERY registry
inbound unconditionally and sets `read` to an int rather than None, so both pass unchanged on an engine
that received NOTHING -- including one whose listeners never bound. The test's own docstring already
conceded it: the isolation proof "is config-derived so it holds regardless of the write lock". `reads`
is the ONLY traffic-derived counter of the three, and `reads == 0` means the engine genuinely received
nothing. The pair I called discriminating carries zero traffic information.
THE ENGINE ALREADY KNOWS WHY, AND THE HARNESS WAS THROWING IT AWAY. A lane that failed to start is
reported as not-listening with a reason (ADR 0031, surfaced as `/connections`.error). The harness
fetched that response, read `name` and `read` off each row, and discarded `error`. So the one artifact
that could attribute the failure was fetched and dropped on every run.
`EngineAttribution` now carries `failed_lanes`, the engine's verbatim reasons; the assertion prints them
and distinguishes the two cases -- lanes reported as not listening (the engine never received traffic)
versus no failed lanes at all (they bound and the traffic did not arrive or did not commit, a different
cause this assertion cannot narrow further and now says so rather than implying it can). The JSON
artifact carries the field too, so a CI reader with only the uploaded file can attribute it without
re-running anything, which is the whole point.
Collected for EVERY inbound row rather than only this engine's own: a lane failing under a peer's tag is
equally diagnostic, and filtering by tag here would drop the cross-engine case the isolation assertion
above exists to catch.
READ DIRECTLY AS `row.error`, NOT `getattr(row, "error", None)`. `EngineClient.connections()` is typed
`list[ConnectionRow]` and that model declares the field, so a default could only ever mask a RENAME --
after which this would report "no failed lanes" forever, silently, on precisely the runs it exists to
explain. A diagnostic field that fails closed to "nothing to report" is worse than no field.
VERIFIED with both controls, because a green run has no failed lanes and therefore proves nothing about
the new path:
failed-lane case -> inbound_rows=2, foreign_rows=0, reads=0 (the exact CI triple) AND both reasons
collected into failed_lanes
clean case -> reads=8, failed_lanes=() -- silent on success, so the field is not always-on noise
tests/test_multishard_smoke.py: 2 passed. ruff 0.15.22 clean. No cp1252-unsafe character introduced.
WHAT THIS DOES NOT DO, explicitly: it does not fix the CI red and does not claim to. Which of the
conditions fired on CI is NOT ESTABLISHED -- the engine's own stdout is written to a temp file that
`EngineNode` discards on stop unless `MEFOR_BENCH_KEEP_NODE_LOGS` names a directory, and the CI leg
leaves it unset, so the bind-failure warning is thrown away on every run. Capturing that is the next
step and is a ci.yml change I have not made. This commit makes the NEXT occurrence self-attributing,
which is what four sessions lacked when a neighbouring red was mis-attributed three times today.
…E URGENT STOP NOT A FINISHED CHANGE AND NOT REVIEWED. Committed mid-flight because the usage ladder reached URGENT STOP, where the instruction is to commit whatever exists rather than risk losing a tree. A partial commit beats a lost one; an unreviewed commit on an unpushed branch costs nothing and can be amended. WHAT THIS IS: the BACKLOG #1292 store-side discriminator, being built to separate a harness reconciliation race from a real count-and-log intake loss. Its design stage returned confidence NOT-ESTABLISHED, which was the honest answer and the one I told the agent was acceptable -- the deliverable is an instrument that ATTRIBUTES the next occurrence, not a verdict on which branch is real. STATE, stated plainly so nobody mistakes this for finished work: - the implement stage was STILL RUNNING when this was committed - the ADVERSARIAL REVIEW STAGE HAD NOT RUN AT ALL - I have NOT verified any of it: no red-first, no ruff, no mypy, no test run by me - the two new modules (intake_audit.py, test_connscale_intake_audit.py) may be incomplete DO NOT LAND THIS. It needs the review stage, then my own verification, then almost certainly a correction commit -- the reviews of my last two commits each found a real defect, one of which let a genuine FD collapse pass. Assume the same here until shown otherwise. Also carried, unrelated and equally unverified: whatever the agent touched in driver.py, profile.py, report.py, runner.py, sender.py, docs/LOAD-TESTING.md and harness/load/__init__.py. The ledger is deliberately untouched, as it has been in every commit on this branch.
…on, and blamed the gauge by guessing
The adversarial review the wip commit said this needed has now run: six dimensions, each finding
handed to a refuter. 17 findings raised, 8 survived, 9 refuted as overstated or wrong. Then a
quality pass found a further defect in my own fix for one of them. Every fix below is verified
red-first -- the guard is broken, the test is watched to fail, the guard restored.
THE HIGH ONE. judge()'s vacuity guard tested `ledger.total` (confirmed + unconfirmed) while every
finding branch consumes only `ledger.confirmed`. A step where nothing was ever ACK-confirmed
therefore passed all six blindness guards and returned a CONCLUSIVE verdict -- "the shortfall is
not in intake" -- computed over a ZERO-ELEMENT comparison, with a green SLO row and all four smoke
assertions passing. The existing positive control guards the STORE side; nothing guarded the
LEDGER side. It is reachable on the harness's own headline fault, because the runner's excusal
clamps `excused` to 0 over budget. This is the blind-but-green direction the whole ordering exists
to prevent, and it cleared intake on precisely the step the reconcile calls a possible
accepted-and-dropped.
THE ONE I GOT WRONG FIRST, RECORDED BECAUSE THE SHAPE MATTERS. SAMPLING_LAG named the `engine_read`
gauge by pure elimination. My first correction inferred the missing state inside judge() by asking
`read_short <= len(ledger.unconfirmed)`. That inference holds only OVER budget. IN budget
`excused == unconfirmed`, so the unconfirmed sends are ALREADY subtracted out of `read_short` and
any residue is confirmed sends the gauge did not count -- a real finding my predicate silenced. And
over-budget needs `timeouts > 3/4 sent`, so the case I broke is the COMMON one. Measured against the
real producer arithmetic (sent=100, timeouts=5, read=93): old predicate `0 < 2 <= 5` fires and
silences; corrected predicate names the gauge. The fix is at the producer instead --
`_unexplained_shortfall` computes `sent - unconfirmed - read`, which means the same thing in both
worlds, and judge() consumes it rather than reconstructing budget arithmetic it does not own. An
absent value falls back to naming the gauge, so the unsafe direction is never the default.
ALSO FIXED, all confirmed by the review:
- engine_suspect now requires the POST-MORTEM moment. A live sweep pages over a store still being
written and can manufacture INVARIANT_SUSPECT; the prose already hedged that, but the machine
surface did not, and only a distant call site's choice of field kept it honest.
- the live moment reports an absent row WITHOUT the engine conclusion (`_conclusion`), so a
console line or JSON artifact cannot be quoted as an invariant violation the post-mortem
does not support.
- the live NOT_RUN reason said "no intake shortfall to attribute" when the audit was merely
DISABLED -- a false statement about the run, in the field whose only job is attribution, on
exactly the step someone opens the artifact to read.
- two docstrings promised full control ids "stay in the harness log line". Nothing logs one. False
in both directions: a reader finds none, and a maintainer reconciling prose against code would
make it true by logging exactly what the PHI rule exists to keep out.
TWO TESTS THAT PASSED WITH THE FEATURE BROKEN, which is the defect class this branch has now been
corrected for three times. The overflow test asserted only the verdict against an empty snapshot, so
a neighbouring guard produced the same PROBE_UNUSABLE and it passed with `if ledger.overflow:`
deleted outright. CORRELATION_SUSPECT had a positive test but no negative control, so dropping the
store-membership half of its predicate kept the suite green while turning every ordinary NAK into a
standing finding. Both now assert the discriminating detail and both go red when broken.
BEHAVIOUR CHANGE, STATED SO IT IS NOT A SURPRISE: a step where nothing was ever confirmed now
returns PROBE_UNUSABLE, which is not `conclusive`, so `_assert_intake_audit` FAILS there. It
previously passed with a vacuous clean verdict. That is the intended direction -- the item's own
text says a probe that cannot answer is a defect to classify deliberately, not a gap to excuse --
but it does mean this test can now red where it was falsely green.
Quality pass also collapsed three near-identical `_build` call sites into a `_matched` closure
beside the existing `_unusable` one, hoisted the duplicated counts, and gave the two audit moments
one spelling of the disabled reason so they cannot drift into disagreeing about why nothing ran.
VERIFIED: ruff check + format, mypy strict, 33 intake-audit tests, 241 connscale tests, and the
connscale end-to-end smoke (2 passed) -- which exercises the new verdict semantics for real rather
than at unit level. Red-first on all six new or repaired guards, including one that reproduces the
predicate bug I introduced. Glyph scan clean over 403 added lines against a 736-hit positive
control. The ledger stays untouched, as on every commit on this branch.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…tead of a monkeypatched one BACKLOG #321, BUILD HALF ONLY. Owner-directed. The floor raise in security.yml is NOT here -- see the bottom of this message. THE DEFECT. Every per-class test in tests/test_scan_forbidden.py runs behind the `sf` fixture, which monkeypatches synthetic values over FORBIDDEN / ESTATE_TOKENS / SITE_CODE_RE / _SITE_CODE_FILE. They prove the machinery matches a pattern someone handed it, and never touch the load path. With no prefix loaded both site detectors fall back to `_NEVER` (scan_forbidden.py:194), an empty negative lookahead that matches nothing anywhere -- so a blind scanner and a clean tree are the same green tick, and the suite that looks like per-class coverage CANNOT FAIL when the real token set is wrong. That is the shape of the original defect, reproduced inside its own test suite. TWO ARMS, split by what each is allowed to touch. BEHAVIOURAL: pins the source to the committed synthetic example and drives the REAL pipeline -- MEFOR_FORBIDDEN_TOKENS -> _resolve_token_text -> _parse_tokens -> compilation -> scan_file. Nothing is monkeypatched onto the globals, and probes are DERIVED from what loaded, so a set that loads blind never reaches an assertion: the derivation fails first. REAL SET: runs only where a real source is configured, and asserts STRUCTURE ONLY -- present, not the sentinel, every class counted. It never reads, builds with, or reports a real token. Proving the scanner catches a real token would require putting one in this file, which is exactly the disclosure the scanner exists to prevent (CLAUDE.md sec. 9); the test would become the leak. A HOLE I FOUND IN MY OWN FIRST CUT AND CLOSED. A configured-but-MANGLED source leaves TOKENS_PRESENT false, identically to having no source at all -- so skipping on that alone turned the documented cutover-mangling case (headers lost, comments only, a BOM before the first section) into a green tick. Only the ABSENCE of a source is now a skip; a source that exists and parsed to nothing FAILS. AND ONE THE RED-FIRST PASS FOUND IN MY OWN TEST, recorded because it is this item's exact subject. With FORBIDDEN forced empty, the [names] arm still PASSED -- green against the very class it names. The sets OVERLAP BY DESIGN (a customer name is typically in [names] AND [estate]), so the probe word drawn from [names] was also an estate token and the estate detector produced the hit. The probe is now filtered to a candidate no other detector can explain, and the arm goes red as it should. An over-determined assertion is not coverage, which is the whole reason this item exists. ASSERTED DELIBERATELY, AND NOT: - the estate arm asserts the scan_file PATH, not a count. [estate_body_only] tokens are held out of _ESTATE_FILE_RES and never enter scan_file, while raising the `estate` count identically -- so a count cannot tell a token the file scanner sees from one it does not. - never reason TEXT. The scanner substitutes a generic reason when a reason would itself match a detector, so asserting wording reads the substituted value rather than the finding. - a negative control per class, so an arm cannot be satisfied by a detector that flags everything, and a negative control on the blind state itself, so the guard cannot be asserting something vacuously true. RED-FIRST, ALL FIVE, each broken in scan_forbidden.py and watched to fail, then restored: site prefixes forced to the sentinel; estate held out of the file scan; names loaded empty; the site detector widened to any six-digit run; the blind fallback made unreachable. NOT DONE, AND NOT MINE TO DO: MEFOR_MIN_DETECTORS in .github/workflows/security.yml stays at names=7,estate=13,site_prefixes=1. Raising it to 8/14/2 hard-fails a required check until the owner updates BOTH the Actions and the Dependabot secrets -- both, or every Dependabot PR fails. I will ask rather than infer that from any message. VERIFIED: ruff check + format clean; 102 tests across the three scan_forbidden suites; glyph scan 0 over the new file against a 736-hit positive control. No token value appears in this file, in any assertion message, or in this commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…1303)
Three test files each spawned `cmd /c exit`, waited for it to EXIT, slept, and returned its pid as
"free". The pid is free at the moment it returns and NOTHING KEEPS IT FREE: between that return and
the moment the tool under test reads the record, the OS may hand it to a new process.
NOT A FLAKE RULING, AND THE DISTINCTION IS DELIBERATE. Intermittency was never the evidence. The race
is visible in the CONSTRUCTION -- a pid is acquired, released, and then relied upon across a gap
nothing holds. I refused to call the ubuntu SIGSEGV timing-dependent from intermittency alone earlier
tonight; this one meets the stricter standard, which is why it gets the label and that one does not.
THE COMMENT IS THE BEST EVIDENCE IN THE ITEM. One copy read:
time.sleep(0.3) # let the OS reap it before we claim the pid is gone
That states the intent exactly and the mechanism does the opposite. Reaping does not RESERVE a pid, it
RELEASES it for reuse -- so the sleep WIDENS the window it appears to guard. The hazard was reasoned
about and the direction inverted.
THE PATH TO THE OBSERVED FAILURE, traced through the real fence rather than guessed. Test-Record-
Liveness (scripts/coord/session-registry.ps1:181) reports DEAD when `Get-Process -Id` finds nothing,
and DEAD vetoes nothing. A REUSED pid IS running, and a test record carries no `startedAt` for the
reuse fence to consult, so the verdict becomes UNVERIFIED -- which DOES veto (occupancy.ps1:75). The
occupant list comes back non-empty and `assert d["Occupants"] == []` fires. Seen on windows-2025 in
run 32268545492, beside (not caused by) an unrelated crash in the same run; `cmd /c exit` is
Windows-only, which matches where it appeared.
THE FIX: one `tests/_dead_pid.py` returning 2147483647. Int32.MaxValue -- inside the `[int]` cast the
fence performs, NON-ZERO so it takes the liveness path rather than the UNREADABLE shortcut a falsy pid
triggers, and structurally unassignable (Linux caps pids at pid_max, ceiling ~2^22; Windows pids are
multiples of 4 far below 2^31). Dead BY CONSTRUCTION rather than by timing.
THE CONSTRAINT THAT SHAPED IT, AND IT IS PROVEN RATHER THAN ASSERTED. The test asserts BOTH
`Occupants == []` AND `Decision == "SKIP"` -- a dead record is neither a veto nor a permission. Any
remedy that stubbed, mocked or forced the liveness verdict would make it pass while exercising
NOTHING, converting a loud false-failure into a quiet always-pass. The real `Get-Process` call still
runs and still returns "not running" on its own. VERIFIED: adding DEAD to occupancy.ps1's veto states
turns the test RED, and restoring it turns it green -- so the veto path is still covered. That is the
direction this fix moves, stated because a fix here could so easily have moved the other way.
DUPLICATED SHAPE, NOT ONE SITE: three definitions, four call sites, across test_worktree_prune_merged,
test_coord_presence and test_session_registry -- now one shared helper. An earlier count of mine said
six call sites; that used `grep -c '_find_free_pid()'`, which also matches the DEFINITION line and
inflated every file by one. A peer had the right number and deferred to mine as "a different
question"; it was not, it was wrong.
VERIFIED: ruff check + format clean; 98 tests across the three suites (79 + 19); red-first on the veto
path as above. Ledger entry allocated from this worktree and landed in the same commit.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… inbox (BACKLOG #1302)
An id-addressed message could strand silently. `mail-drain.ps1:852` compares the recorded
`to.sessionId` against the reading session's harness id with `-ne`; an id from another namespace never
matches, so the message sat in the inbox until it was swept to expired/ -- with the send path printing
`Queued 1 message(s)` the whole time.
THE ASYMMETRY WAS THE DEFECT, AND IT IS WHY THE GUARD IS ON THE SEND SIDE. The drain already reported
its half ("N message(s) are addressed to a different session id and were left in the inbox"), so the
RECIPIENT was told. The SENDER was told nothing -- and the sender is the only party who can correct
the id. I did not touch the drain's filter: it is CORRECT. A worktree outlives its occupant, so an
id-addressed note must not reach a stranger, and loosening the match to "fix" delivery would trade a
silent non-delivery for a silent MIS-delivery, which is worse.
THE SHAPE. A harness session id is a bare UUID (the drain reads `$hook.session_id`). The MCP namespace
prefixes its own as `local_<uuid>`. Two id spaces for one session, compared literally.
MEASURED, AND I WAS THE ONE WHO CAUSED IT. Six of my own messages to the dispatcher stranded for a
whole session -- a level report, a CI mechanism diagnosis, two unprompted self-retractions and a
request to pull two never-started items. The recipient read my lane as silent and wrote "level
unreported" three times. They were found only by opening the box by hand, and were due to expire with
neither end told.
PARTIAL CONTROL, AND THE ITEM SAYS SO RATHER THAN LEAVING IT TO BE DISCOVERED. This catches a
wrong-NAMESPACE id. It does NOT catch a correctly-shaped but STALE one -- an id belonging to a session
that has ended fails identically and just as silently. A pass at send is not a promise of delivery.
THE MUST-NOT-TRIP ARM IS IN THE SAME TEST AS THE REFUSAL, deliberately: a guard that rejected
everything would otherwise pass by satisfying one half. No `-ToSessionId` at all is the ordinary
broadcast and still sends; a genuine bare-UUID id still sends. RED-FIRST IN BOTH DIRECTIONS, each
broken then restored -- disabling the guard reddens the refusal test, widening it to reject everything
reddens the must-not-trip test.
The refusal names the REMEDY, not just the rejection: the sender's next move is to drop the flag and
address by worktree path, and a message that only said "invalid" would leave them hunting an id.
VERIFIED: ruff clean; 103 tests across the two mail suites; red-first both ways as above. The #1302
banner was flipped by lifting the closed-alphabet character from an already-closed item and asserting
its membership before use -- never typed (sec. 11).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…orkflows (BACKLOG #321) The owner refreshed MEFOR_FORBIDDEN_TOKENS in both secret stores, so the floor can now assert the larger set. VERIFIED BY ME rather than taken from a relay -- names and dates only, no values, which is all `gh secret list` exposes: Actions MEFOR_FORBIDDEN_TOKENS 2026-08-21T12:45:15Z Dependabot MEFOR_FORBIDDEN_TOKENS 2026-08-21T12:45:23Z EIGHT SECONDS APART, so the half-done state this change was held for did not occur. If Actions had been updated and Dependabot had not, every Dependabot PR would hard-fail a required check. My stated constraint was "only after BOTH, and I will ask rather than infer" -- both are updated and the measurement is mine. RAISED IN TWO PLACES, NOT ONE. The release named `security.yml`. `branch-leak-scan.yml:88` carried the SAME literal and nobody named it. Raising only one would have left a second gate passing on the old floor -- a partial raise that reads as done. There are now zero occurrences of the old triple under .github/workflows/. PRE-FLIGHT BEFORE RAISING A FLOOR THAT HARD-FAILS A REQUIRED CHECK, counts only: names 8 estate 14 estate_file_scanned 13 site_prefixes 2 synthetic=False The real set satisfies 8/14/2 exactly, and `estate_file_scanned` at 13 matches the documented 12->13 move -- so the added token is FILE-SCANNED rather than body-only, which is the half that matters. NOT INERT, AND THAT IS CHECKED RATHER THAN ASSUMED. `token_floor_failure` passes at 8/14/2 and FAILS at 9/15/3 naming each short section ("names 8<9, estate 14<15, site_prefixes 2<3"). A floor that cannot fail is not a floor. EXPECT COLLATERAL HITS ON THE FIRST FULL SWEEP AND DO NOT READ THEM AS FINDINGS. The added site prefix is two digits, so it matches any delimited six-digit run in a 10000-wide band -- synthetic MRNs, sentinel ids, clamp ceilings. Triage noise, anticipated before the value was written. VERIFIED: 146 tests across the scanner + token-source + CI-pinning suites; 120 more across the workflow-lint and lockstep suites. No token value appears in this change, in any test, or in this message. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…inline pin (BACKLOG #332) `release.yml:255` ran `pip install "sigstore==4.4.0"` inside the job holding `id-token: write`. The very next commands sign the wheel, write the SLSA attestation and publish to PyPI, so anything that executes there runs with the credential that signs the artifacts -- a backdoored wheel produced at that point carries a VALID Sigstore bundle and VALID provenance, and every downstream verifier reports success. The compromise defeats the controls it is standing next to. TWO DEFECTS, AND A VERSION BUMP FIXES NEITHER: 1. The pin covered the TOP package only. ~30 transitive dependencies floated, unhashed, resolved at signing time. 2. NO Dependabot ecosystem parses an inline `pip install X==Y` in a workflow `run:` block -- dependabot.yml registers uv, github-actions and npm. So the pin had no updater, no trigger and no owner. Its own comment said "Re-evaluate to 4.5.0 once it has aged past the window" and nothing would ever have prompted that; it was 19 days overdue. test_ci_venv_pinning.py already worded the class: "a stale pin rots invisibly and a DELETED pin is invisible twice over." Routing it through uv.lock fixes both: the transitives are hash-pinned, and the group sits under the `uv` ecosystem Dependabot already watches. THE RECORDED DECISION WAS SPENT, NOT OVERRIDDEN. ADR 0034:350 deliberately kept sigstore OUT of the lock because routing it resolves 4.5.0, then <48h old against dependabot's 5-day cooldown -- pinning the SIGNING toolchain to a fresher artifact than the repo's own update policy allows would invert that policy at its highest-privilege point. That reasoning was CORRECT when written on 2026-07-29. MEASURED from PyPI's version-specific endpoint: 4.5.0 published 2026-07-28T07:34:00Z, so the window closed 2026-08-02, nineteen days before this. The ADR's own residual row still needs amending to say so -- that is the owner's, and it is NOT done here. GATE 2 PASSED, WHICH IS WHY THIS COULD LAND AT ALL. The known failure mode is a new group forcing a re-resolve across every other artifact -- semgrep was excluded by decision for exactly that. Measured: `uv lock` ADDED sigstore 4.5.0 plus 10 transitives and updated NOTHING, and re-running all six existing exports left them byte-identical (hashes checked against a pre-mutation snapshot). Verified with a detector control: perturbing one lock makes the same `git diff --exit-code` report 1. SEVEN-PLACE LOCKSTEP, NOT SIX. The structural test walked me through every site: the export, the DEP-1 `git diff --exit-code` set, the resync's export, its `git diff --quiet` short-circuit, and its `git add`. Missing any one leaves a Dependabot PR red with no bot-reachable path to green -- which is what happened to constraints.lock between #1193 and its fix. THE GUARD MOVED, IT WAS NOT DELETED. `sigstore` leaves RELEASE_PINNED_TOOLS because there is no longer an inline target to find. Deleting a row there is the exact regression that tuple exists to catch, so it is replaced by a stronger one: the lock exists, pins sigstore with `==`, EVERY requirement carries a hash, and release.yml installs from it with `--require-hashes`. All four in one test, and the hash check carries its own control because a detector that cannot see an unhashed requirement makes "0 unhashed" meaningless. RED-FIRST ON ALL THREE ARMS: reverting to the inline pin fails it, removing the lock fails it, and stripping the hashes fails it. The third arm initially PASSED -- because my mutation silently did nothing against a CRLF file, not because the guard was blind. A mutation that does not apply and a guard that does not catch produce the identical green, so the strip now asserts 193 -> 0 hashes before the test is allowed to mean anything. NOT DONE, deliberately: step 6's `build` and `cyclonedx-bom` are a separate change -- cyclonedx-bom is half of the byte-identity pair test_sbom_install_is_byte_identical_in_release_and_security enforces, so both halves must move together. And release.yml runs only on a tag push, so per ADR 0034 the first real exercise of this path is a `workflow_dispatch` dry-run before the next tag. VERIFIED: ruff clean; 56 tests across the pinning, lockstep and scanner suites; 255 more across the workflow/release/sbom suites. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Collaborator
Author
|
CORRECTION to this PR's own description: it does NOT merge clean. My instrument was wrong. The body above says "Merges clean into How I got a false clean. I used the old three-argument form, The correct instrument is And the exit code alone is not enough either. My first re-test passed a local ref that does not exist here, and What still holds: everything else in the description above. The branch is genuinely unlanded work, no PR ever pointed at it, the item states were measured with Re-tested the sibling claims with the corrected instrument: #530 is genuinely clean; #534 and |
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.
Draft on purpose. Do not arm. Opened by the lander to make an abandoned lane visible. Its session moved to a new worktree and left this behind, routing the judgment here.
Why a new branch name
claude/builder-2-a12484on origin and the local tip had diverged -- 8 remote-only, 16 local-only -- so pushing the tip was not a fast-forward.It is a rebase, not divergent work: all 8 remote-only commits have exact
patch-idmatches among the local commits, so the local branch is a rebased superset and nothing of substance is unique to the remote. A force-push would therefore have been safe in content, but force-pushing is the owner's call, so this went to a fresh ref instead. The original branch is untouched.What is on it
11 commits not on
main, none of them reachable frommain, citing seven items:uv.lock, not an inline pin-ToSessionIdat SEND, not at the inboxSix of the seven are still OPEN on
main(#1303 is absent from the ledger entirely). Measured withparse_items, max item 1328 as the positive control.State
main-- 0 conflict hunks -- though it is 37 behind.private/rescuetags/....gh pr list --json headRefNamereturns nothing for it.Why draft
One commit is explicitly
wip(1292): ... committed under USAGE URGENT STOP. The lane's session is gone, so nobody owns a rebase across 37 commits, and the leak-gate floor change touches both CI workflows. Withrequired_approving_review_count: 0, arming would merge all of that unread.Whoever picks this up: rebase onto
main, decide whether thewipcommit is finished, and consider splitting the CI leak-gate change out from the test fixes.