test(uploads) + fix(ci): the retention runner had no audit guard, and a native crash reported as a test failure (BACKLOG #1224, BACKLOG #1260) - #566
Merged
Conversation
…(BACKLOG #1224)
Two paths prune expired uploads and both audit as actor="system": the save-time sweep in
upload_file, and the background UploadRetentionRunner wired in create_managed_app's lifespan.
ONLY THE FIRST HAD A TEST.
MEASURED, WHICH IS WHY THE ROW WAS RIGHT TO ASK FOR THIS. Reverting the lifespan closure to
actor=meta.uploader left the ENTIRE suite green -- 13578 passed, byte-identical to baseline --
while the same mutation on the request-path site reddened exactly one test.
THE CODE IS CORRECT AT BOTH SITES AND I CHANGED NEITHER. The item's other limbs are already
shipped; this is the third, and it is a test.
WHY THE SUITE MISSED IT, AND IT IS NOT THAT NOTHING DRIVES A LIFESPAN. Tests DO construct the
real closure and DO reach run_once -- test_asvs_gcm_invocation_bound.py enters a managed app's
lifespan. It prunes ZERO files, because nothing in those fixtures is aged, so the loop body
carrying actor="system" never executes. A PATH THAT RUNS BUT NEVER ENTERS ITS BRANCH IS
INVISIBLE TO COVERAGE-BY-EXECUTION.
TWO LIFESPANS ARE LOAD-BEARING. _run calls run_once BEFORE its first sleep, so the sweep
happens at startup; the file has to be aged BETWEEN two startups. One lifespan cannot both
create and prune.
PROVED ABLE TO FAIL, with the anchor verified UNIQUE before mutating so it could not hit the
wrong site:
lifespan site -> actor=meta.uploader MY GUARD REDS, assert 'op' == 'system'
the SIBLING request-path test STAYS GREEN -- the control proving the two sites are
independently covered, and that the old suite genuinely
could not see this one
restored both pass
The sweep runs in a task the lifespan does not await, so the test POLLS for the row rather than
sleeping a fixed amount -- a fixed sleep is either flaky or slow.
25 passed in tests/test_upload_api.py, ruff check and format clean.
…(BACKLOG #1260) A segfault kills the interpreter, so pytest returns 139 with no verdict -- and THREE layers of naming then say "tests failed": the check name, the step name `Tests (pytest)`, and `steps.tests.outcome`, which is what step_margin.py consumes. NOT ONE OF THEM IS TRUE. The engine was fine; a process died. LEGIBILITY BEFORE COVERAGE, and the order is the item's, not mine. Wrapping the step makes the failure RETRY; it does not make it LEGIBLE, and a retry that then succeeds erases the crash from view entirely. So the annotations changed first: NATIVE CRASH is now the leading token in both the retry warning and the persisted-crash error. THE ATTRIBUTION WAS THE PART I DID NOT EXPECT. The wrapper hard-coded "likely the pyodbc py3.14 parameter-binding segfault" into every message. That class is ESTABLISHED for the nine database call sites and is NOT established for the main suite -- the item explicitly refuses to conclude it. Wrapping the step as-is would have fixed a legibility defect by printing a mechanism nobody has measured onto the busiest leg in the repo. So the clause is now per-caller. It DEFAULTS to the pyodbc text, which keeps all nine existing call sites saying exactly what they say today; only a caller that has not established the class opts out, and the main suite does. Making it opt-IN would have silently stripped nine correct attributions to fix one wrong one. MY OWN FIRST DRAFT PLANTED THE TOKEN ANYWAY and its own test caught it: the opt-out message read "do not assume the pyodbc class", which puts the word in the annotation, so a log grep for pyodbc would match the one leg where the class is explicitly not established. It now names no class. CITATION DRIFT, RE-DERIVED FROM THE SYMBOL RATHER THAN INHERITED. The row's banner cites ci.yml:780 and is CORRECT -- that is the run line. Its body cites :648 and :673 for the step, which is now :728; it puts the wrapper invocations in the 1156-1374 band, which is now 1644-1860; and it says ten invocations, which is now nine. PROVED ABLE TO FAIL, four mutations, each anchor verified unique first: unwrap the main-suite step -> the coverage arm reds, alone drop the empty-cause declaration -> the cause-unproven arm reds, alone retry on ANY non-zero exit -> the never-retry-a-failure arm reds, alone always use the pyodbc clause -> the not-established arm reds, alone TWO TRAPS THE WORK HIT ITSELF. Passing the bare name "bash" to subprocess let Windows resolve it against the child's PATH, which found WSL's bash rather than the Git Bash `shutil.which` reported -- so the skipif guarded one interpreter while the test ran another. And the control-character gate caught a real BACKSPACE byte in a comment, where `System32\bash` in a non-raw string became 0x08. Both are fixed and the first is written into the test as a comment. 145 passed across tests/test_ci_*.py plus the partition gate; manifest line included.
wshallwshall
changed the title
wshallwshall
enabled auto-merge (squash)
August 23, 2026 23:11
wshallwshall
disabled auto-merge
August 23, 2026 23:20
…was never about (BACKLOG #1260) THE DEFECT IS A FUTURE-READER ONE AND NOTHING AT RUNTIME CAN CATCH IT. The wrapper's header tells whoever fixes upstream pyodbc #1459 to remove it. That is right for the database legs and wrong for any caller whose crash cause is not that class -- and my own commit had just made the main engine suite exactly such a caller. So I fixed the ANNOTATION so it would not assert an unmeasured cause, and left the REMOVAL INSTRUCTION able to delete that leg's crash handling on the strength of an upstream fix that does not address it. THE NOTE WAS ALREADY UNDERSTATING ITSELF BEFORE I TOUCHED IT: it named one call site, "the throughput-invariant step", when ci.yml had TEN. THE DISCRIMINATOR IS NOW IN THE WORKFLOW RATHER THAN IN A MEMORY. Any caller setting RETRY_NATIVE_CRASH_CAUSE="" has declared its cause is not established as the pyodbc class, so a #1459 fix does not license removing the wrapper there. The note says to remove it from the pyodbc callers and decide each opted-out caller separately. PROVED ABLE TO FAIL, and the second arm is the one I care about: revert the note to its one-caller wording -> only the new guard reds remove the opt-out from ci.yml entirely -> the guard reds TOO, refusing to pass vacuously That second mutation is the point. A guard conditioned on "if any caller opts out" would quietly become a no-op the day nobody does, so it asserts the precondition it depends on rather than skipping when the precondition is absent. FOUND BY THE LANDER, from a 2026-08-14 commit on PR 433 that implements this item differently and argues in its own comment against wrapping this leg at all. Their design and mine disagree; the row's 2026-08-20 re-score still names the bare pytest as the gap, so the wrapper stays. This is the one objection of theirs that neither design answered. 22 passed across the CI retry, engine-step and partition suites.
wshallwshall
enabled auto-merge (squash)
August 23, 2026 23:26
…fect #1224 says no test drives a lifespan. False at HEAD: tests construct the real closure and reach run_once, but prune zero files, so the loop body never executes. A path that runs but never enters its branch is invisible to coverage-by-execution. #1260 named two limbs and there were three. retry-native-crash.sh hard-coded the pyodbc attribution into every message, so wrapping the main suite as written would have printed an unmeasured mechanism onto the busiest leg in the repo. The clause is per-caller now, defaulting to the pyodbc text so the nine database sites are unchanged. Both verified by content, not from the builders' reports. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…rips The wrapper's removal note said "remove once #1459 ships" and named one call site when ci.yml had ten. #1459 covers the database legs only, so a good-faith deletion would have silently stripped the main suite's crash handling -- the one leg whose cause is explicitly not established. My banner said three limbs. 01c03ce is the fourth, and it landed after the branch I wrote that banner on had already forked. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Line 12409 cited the upstream pyodbc segfault three times as a bare `#1459`. In `docs/BACKLOG.md` a bare `#N` reads as a ledger citation, and 1459 is above the allocator's high-water mark of 1352 -- so the reference resolves to nothing today and would silently begin resolving to unrelated work the day somebody allocates that number, with nothing anywhere reporting a problem. All three now read `mkleehammer/pyodbc#1459`. This is the form the same branch already uses in `scripts/ci/retry-native-crash.sh`, and the form `docs/testing/master-test-plan/` uses, so it is the house convention rather than a new one. `dangling_citation_check.py` exempts it via _FOREIGN_REPO, which keys on the character preceding the hash, so it works inside backticks. The sentences are correct as written; only the citation was unqualified. Nothing else on the line changed: every other line is byte-identical and the CRLF count is unchanged at 16112. Ledger prose authored by the Dispatcher seat; applied by the Lander seat. 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.
A guard for the retention runner's audit attribution.
tests/test_upload_api.pyonly, +88, no engine code.No production change, deliberately. Both prune sites already audit with
actor="system"; the open limb of BACKLOG #1224 was that nothing pinned the runner path, so a regression there would have been silent.Verified by mutation, not by the suite going green
The anchor was checked unique first, so the mutation could not land on the wrong site. Then:
actor=meta.uploaderThat control is the finding. It shows the two sites are independently covered, and that the old suite genuinely could not see the runner one — reverting that site previously left 13,578 tests green.
25 passed in the module,
ruffclean.The row's stated mechanism is wrong, and the author flagged it
BACKLOG #1224 says "no test drives a lifespan". That is false at this head — tests do construct the closure and do reach
run_once. They prune zero files, so the loop body never executes.A path that runs but never enters its branch is invisible to coverage-by-execution. The conclusion the row reached was right; the mechanism it named was not, and the author is amending the row's mechanism rather than only its status.
One state the tracking does not have
#1224sits on the owner's 27-item completed list while reading closed nowhere. It is built and unlanded — a third state between open and closed that the list cannot express.Landing notes
backlog-hygienedoes not fire:touches_codeglobs onlymessagefoundry/,ide/andmessagefoundry_webconsole/, and this touchestests/— established by running the gate's own logic against the pushed tip.Two commits off current main
b6178de5, zero behind.Second item on this branch: BACKLOG #1260
The branch name names half of what is on it, and so did this PR until now. The author said so
when handing it over rather than letting the name mislead the landing.
16ef31easits directly on8b80ef30; the two share zero files and are independent in substance -- one is a missing test,the other a CI wiring fix.
.github/workflows/ci.yml,scripts/ci/retry-native-crash.sh,tests/test_ci_retry_native_crash.py,tests/tooling_manifest.txt. +204 -4.Legibility first, then coverage, and the order is the point
Wrapping the step alone makes a native crash retry rather than become legible, and a
successful retry erases the crash from the record entirely. So the reporting change lands ahead of
the retry, not beside it.
The clause that was not in the brief
The wrapper hard-coded "likely the pyodbc py3.14 parameter-binding segfault" into every message.
That class is established for the nine database call sites and NOT for the main suite. Wrapping
the step as written would have fixed a legibility defect by printing an unmeasured mechanism onto
the busiest leg in the repo.
The clause is now per-caller. It defaults to the pyodbc text, so all nine sites keep exactly
what they say today, and the main suite opts out.
Verified
145 passed across
tests/test_ci_*.pyplus the partition gate; 25 passed intest_upload_api.py;four mutations each redding exactly one arm;
actionlintand the control-char gate green.Two gates caught the author and both were right: the control-char hook found a real BACKSPACE byte
where
System32�ashin a non-raw string became0x08, and the claim gate refused the firstcommit over an unclaimed number.
Co-Authored-By: Claude Opus 5 noreply@anthropic.com