test(uploads) + fix(ci): the retention runner had no audit guard, and a native crash reported as a test failure (BACKLOG #1224, BACKLOG #1260)

.github/workflows/ci.yml

@@ -753,6 +753,9 @@ jobs:
           FAULT_TIMEOUT: ${{ matrix.fault_timeout }}
           PYTEST_TIMEOUT: ${{ matrix.pytest_timeout }}
           PYTEST_WORKERS: ${{ matrix.pytest_workers }}
+          # BACKLOG #1260: empty on purpose -- see the comment above the run line. The wrapper's
+          # default clause names the pyodbc class, which is not established for this leg.
+          RETRY_NATIVE_CRASH_CAUSE: ""
         # `--ignore-glob` subtracts the web console package, which runs as its OWN step below. BACKLOG
         # #1027 added that package to the root `testpaths` so a bare local `pytest` stops silently
         # excluding it; without this subtraction the SAME 356 tests would then run TWICE on every leg.
@@ -790,7 +793,18 @@ jobs:
         # what happens when the selection lives in the run body -- the obvious spelling (`--ignore`) was
         # the one that silently did nothing. One list, read in one place, asserted by
         # tests/test_tooling_partition.py, which also pins the two halves of this wiring.
-        run: pytest -q -n "$PYTEST_WORKERS" --dist loadfile -m 'not tooling' --ignore-glob='*messagefoundry-webconsole*' -o faulthandler_timeout="$FAULT_TIMEOUT" --timeout="$PYTEST_TIMEOUT"
+        # BACKLOG #1260: WRAPPED SO A NATIVE CRASH IS NOT REPORTED AS A TEST FAILURE. A segfault kills
+        # the interpreter, so pytest returns 139/134 with no verdict and THREE layers of naming then
+        # say "tests failed" -- the check name, the step name, and `steps.tests.outcome`, which is what
+        # `step_margin.py` below consumes. None of them is true: the engine was fine and a process died.
+        # The wrapper re-runs ONLY on 139/134 and re-raises exit 1 immediately, so it cannot mask a
+        # regression.
+        #
+        # RETRY_NATIVE_CRASH_CAUSE IS DELIBERATELY EMPTY HERE. The wrapper's default clause names the
+        # pyodbc py3.14 class, which is ESTABLISHED for the database legs and is NOT established for
+        # this one -- the item filing this refuses to conclude it. An empty value makes the annotation
+        # say CAUSE NOT ESTABLISHED rather than assert a mechanism nobody has measured on this leg.
+        run: bash scripts/ci/retry-native-crash.sh pytest -q -n "$PYTEST_WORKERS" --dist loadfile -m 'not tooling' --ignore-glob='*messagefoundry-webconsole*' -o faulthandler_timeout="$FAULT_TIMEOUT" --timeout="$PYTEST_TIMEOUT"
 
       # THE MARGIN CHECK (BACKLOG #344 proposal 1). Last in the job so a LOW margin cannot skip a suite
       # that has not run yet -- a step `if:` with no status function carries an implicit `success()`,

docs/BACKLOG.md

@@ -10999,7 +10999,7 @@ gate is the wrong shape, validation of the walk is the right one.
 
 ## 1224. The `upload.prune` audit row names the file's owner as the actor of an automated sweep
 
-> 🔢 **Re-scored 2026-08-20 -> P3.** Value **2/10** · Difficulty **2/10** · _fill-in_. Attacked the shipped claim at both cited sites and it survived on the code limb. Site 1 (request-path opportunistic sweep) is now messagefoundry/api/app.py:3918-3936: `actor="system"`, `client` argument removed entirely, and detail carries file_id + uploader + uploader_id, with a comment naming BACKLOG #1224 and citing ADR 0150 decision 4 for dropping the client. Site 2 (background UploadRetentionRunner) is messagefoundry/api/app.py:5891-5907: the lifespan-owned `_audit_upload_prune` closure also writes `actor="system"` with uploader/uploader_id in detail. Enumerated prune call sites repo-wide: `prune_expired(` is called at app.py:3918 and uploads.py:648 (inside `run_once`) only, so at least those two are the whole surface and both are attributed to the system. Where the claim fails is the item's explicit third limb -- "assert the emitted row's actor is the system principal and its uploader detail is unchanged, for BOTH the request-path prune and the background runner". Only the request-path assertion exists (tests/test_upload_api.py:794-814, which also asserts the null client). The runner-path test (tests/test_uploads.py:389) injects a list-appending stub for `audit=` and asserts `[m.file_id for m in audited]`; it never constructs or inspects an audit row, so it would pass unchanged if `_audit_upload_prune` were reverted to `actor=meta.uploader`. That closure is reachable only from the API lifespan, and no test drives a lifespan, so the fixed line has no execution coverage at all. Under the not-deployed rule this is worded conditionally: the false attribution would no longer be written on a first deployment from either path, but the path the item singled out as the one a partial fix abandons is guarded by nothing. _(was 6/10 · 2/10.)_
+> ✅ **SHIPPED 2026-08-23 -- verified by content at `8b80ef30`, tests only, no engine code changed.** Both call sites were already correct; the open limb was coverage. **THIS ROW'S OWN ACCOUNT OF THE GAP IS FALSE AT HEAD.** It says no test 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, so the loop body carrying `actor="system"` never executes. **A PATH THAT RUNS BUT NEVER ENTERS ITS BRANCH IS INVISIBLE TO COVERAGE-BY-EXECUTION**, which is a different defect from an unreached path and the reason the old suite could not see it. **MUTATION-VERIFIED, ANCHOR CHECKED UNIQUE FIRST so it could not land on the wrong site:** the lifespan closure re-pointed to `actor=meta.uploader` reds the new guard while the sibling request-path test stays green; restoring makes both pass. Reverting the fix previously left 13,578 tests green. 🔢 **Re-scored 2026-08-20 -> P3.** Value **2/10** · Difficulty **2/10** · _fill-in_. Attacked the shipped claim at both cited sites and it survived on the code limb. Site 1 (request-path opportunistic sweep) is now messagefoundry/api/app.py:3918-3936: `actor="system"`, `client` argument removed entirely, and detail carries file_id + uploader + uploader_id, with a comment naming BACKLOG #1224 and citing ADR 0150 decision 4 for dropping the client. Site 2 (background UploadRetentionRunner) is messagefoundry/api/app.py:5891-5907: the lifespan-owned `_audit_upload_prune` closure also writes `actor="system"` with uploader/uploader_id in detail. Enumerated prune call sites repo-wide: `prune_expired(` is called at app.py:3918 and uploads.py:648 (inside `run_once`) only, so at least those two are the whole surface and both are attributed to the system. Where the claim fails is the item's explicit third limb -- "assert the emitted row's actor is the system principal and its uploader detail is unchanged, for BOTH the request-path prune and the background runner". Only the request-path assertion exists (tests/test_upload_api.py:794-814, which also asserts the null client). The runner-path test (tests/test_uploads.py:389) injects a list-appending stub for `audit=` and asserts `[m.file_id for m in audited]`; it never constructs or inspects an audit row, so it would pass unchanged if `_audit_upload_prune` were reverted to `actor=meta.uploader`. That closure is reachable only from the API lifespan, and no test drives a lifespan, so the fixed line has no execution coverage at all. Under the not-deployed rule this is worded conditionally: the false attribution would no longer be written on a first deployment from either path, but the path the item singled out as the one a partial fix abandons is guarded by nothing. _(was 6/10 · 2/10.)_
 >
 > **Filed 2026-08-11 -- read off `origin/main`, TWO sites not one.** The retention sweep is **automated and owner-blind**, but its audit row attributes the deletion to the pruned file's uploader:
 > ```
@@ -12406,7 +12406,7 @@ BUILDS it.*
 
 ## 1260. a native crash in the main suite reports as a test failure, and the crash-retry wrapper does not cover that leg
 
-> 🔢 **Re-scored 2026-08-20 -> P1.** Value **6/10** · Difficulty **2/10** · _quick win_. Gap stands on both halves: the main suite's run line at ci.yml:780 is a bare pytest with no wrapper while every retry-native-crash invocation sits in the database legs, and the only downstream signal is steps.tests.outcome fed to step_margin.py (:801-814), which cannot distinguish exit 139 from a test failure (value 6). The remainder is two small workflow edits, wrapping the step and surfacing the crash exit distinctly, plus the must-trip and must-not-retry arms the item specifies (difficulty 2). _(was 6/10 · 2/10.)_
+> ✅ **SHIPPED 2026-08-23 -- verified by content at `16ef31ea`.** **THE ROW NAMED TWO LIMBS AND THERE WERE FOUR.** The third changed the shape of the fix: `scripts/ci/retry-native-crash.sh` HARD-CODED the pyodbc attribution into every message it emits. That class is established for the nine database call sites (`ci.yml:1644`-`:1860`) and is NOT established for the main suite -- as this row itself refuses to conclude. **Wrapping the step as written would have fixed a legibility defect by printing an unmeasured mechanism onto the busiest leg in the repo:** a true observation carrying an invented cause. The clause is now per-caller and DEFAULTS to the pyodbc text, so all nine existing sites read exactly as before and only the main suite opts out; opt-IN would have silently stripped nine correct attributions to fix one wrong one. **AND THE OPT-OUT MESSAGE NAMES NO CLASS AT ALL** -- an earlier draft said "do not assume the pyodbc class", which puts the token in the annotation so a log grep for `pyodbc` matches the ONE leg where the class is explicitly not established. Its own test caught that. **CITATION DRIFT, re-derived from the symbol: the BANNER is right and the BODY has drifted** -- `ci.yml:780` is the run line as cited, but the step is at `:728` not `:648`/`:673`, the invocations are at `1644`-`1860` not the `1156`-`1374` band, and there are NINE, not ten. **FOUR MUTATIONS, each anchor verified unique**, each reddening one arm alone. **STATED LIMIT:** a process killed by `SIGSEGV` exits `139` and `139` is what the wrapper branches on, so the tests drive the contract it implements -- they do NOT show that a real crash produces `139`, which stays the wrapper's own documented premise. 🔢 **Re-scored 2026-08-20 -> P1.** Value **6/10** · Difficulty **2/10** · _quick win_. Gap stands on both halves: the main suite's run line at ci.yml:780 is a bare pytest with no wrapper while every retry-native-crash invocation sits in the database legs, and the only downstream signal is steps.tests.outcome fed to step_margin.py (:801-814), which cannot distinguish exit 139 from a test failure (value 6). The remainder is two small workflow edits, wrapping the step and surfacing the crash exit distinctly, plus the must-trip and must-not-retry arms the item specifies (difficulty 2). _(was 6/10 · 2/10.)_ **AMENDED: THERE WERE FOUR, and the fourth is a FUTURE-READER defect rather than a runtime one** (`01c03cee`). The wrapper's own removal note read *"remove this wrapper once mkleehammer/pyodbc#1459 ships"* and named ONE call site -- the throughput-invariant step -- when `ci.yml` had TEN. **`mkleehammer/pyodbc#1459` covers the DATABASE legs and says nothing about any other caller**, so a good-faith deletion on the strength of that upstream fix would have SILENTLY STRIPPED the main suite's crash handling: the one leg whose cause is explicitly NOT established. The note now states the discriminator IN THE WORKFLOW rather than in a memory -- any caller setting `RETRY_NATIVE_CRASH_CAUSE=""` has declared its cause unproven, so `mkleehammer/pyodbc#1459` does not license removing the wrapper from it -- and `test_the_removal_note_cannot_silently_strip_an_opted_out_caller` FAILS if an opted-out caller exists while the note stops saying so.
 >
 > **Filed 2026-08-14 - not started. THE COMPENSATING CONTROL EXISTS, IS CORRECTLY WRITTEN, AND DOES NOT COVER THE PATH THAT FAILED.** Two independent halves: a **reporting** defect that misnames a crash as a test failure, and a **coverage** gap in the retry wrapper. Either can be fixed without the other, and fixing only the second leaves every future crash still misreported.
 > **THREE LAYERS OF NAMING SAID "TESTS FAILED" AND NOT ONE OF THEM WAS TRUE.** Observed on a pull request whose Windows leg went red: the check is named `test (windows-2025, py3.14)`, the step is named `Tests (pytest)`, and the process exited **139** -- `128 + SIGSEGV(11)` -- printing *"Segmentation fault"*. **Zero tests failed.** The reporting seat proved the log was searchable before concluding absence: `pytest` appears **28 times** in a 100 KB log while there is **no pytest summary line and no FAILED test id anywhere in it**, because the process died before it could produce one. A reader at any of those three layers reaches for a test regression that does not exist.

scripts/ci/retry-native-crash.sh

@@ -22,17 +22,49 @@
 # codes below. A genuine test failure exits 1 and is re-raised immediately, never retried.
 # Our own Python cannot cause a native segfault — a real logic regression surfaces as a
 # pytest assertion (exit 1), so this wrapper can never hide one. Each retry emits a visible
-# ::warning:: (grep CI logs for "Native crash" to track the flake frequency against #1459).
+# ::warning:: (grep CI logs for "NATIVE CRASH" to track the flake frequency against #1459).
 #
-# REMOVE THIS WRAPPER once #1459 ships a fix and pyproject's pyodbc floor moves to the fixed
-# release (the throughput-invariant step in .github/workflows/ci.yml calls this).
+# REMOVING THIS WRAPPER IS NOT A BLANKET DELETION -- READ THE CALLER SET FIRST (BACKLOG #1260).
+# #1459 covers the DATABASE legs. It says nothing about any other caller, and this note previously
+# named one call site ("the throughput-invariant step") when ci.yml had TEN.
+#
+# THE DISCRIMINATOR IS IN THE WORKFLOW, NOT IN A MEMORY: any caller setting
+# RETRY_NATIVE_CRASH_CAUSE="" has declared that ITS crash cause is NOT established as the pyodbc
+# class. A fix to #1459 therefore does not license removing the wrapper from that leg -- doing so
+# would silently strip its crash handling on the strength of an upstream fix that does not address
+# it. Today the engine test suite is such a caller.
+#
+# SO: when #1459 ships and pyproject's pyodbc floor moves, remove the wrapper from the pyodbc
+# callers, and decide each opted-out caller SEPARATELY on its own evidence.
+# `tests/test_ci_retry_native_crash.py` fails if an opted-out caller exists and this note stops
+# saying so, because the whole defect is a future reader deleting one line in good faith.
+#
+# THE ATTRIBUTION IS PER-CALLER, AND THAT IS THE POINT (BACKLOG #1260). The pyodbc class above is
+# ESTABLISHED for the database legs and is NOT established anywhere else. A wrapper that names it
+# unconditionally would print a cause it has not measured onto every leg it is ever added to -- a
+# true observation (a native crash happened) carrying an invented mechanism, which is the harder
+# error to catch because the part a reader checks is true. So callers where the class is NOT known
+# set RETRY_NATIVE_CRASH_CAUSE="" and the message says so in words.
 #
 # Usage: scripts/ci/retry-native-crash.sh <cmd> [args...]
 # Env:   RETRY_NATIVE_CRASH_ATTEMPTS (default 3)
+#        RETRY_NATIVE_CRASH_CAUSE    attribution clause; default names the pyodbc class (correct for
+#                                    the database legs). Set to "" on any leg where it is unproven.
 set -uo pipefail
 
 attempts="${RETRY_NATIVE_CRASH_ATTEMPTS:-3}"
 
+# Defaulting to the pyodbc clause keeps the nine database-leg call sites saying exactly what they
+# say today; only a caller that has NOT established the class has to opt out.
+default_cause=" -- likely the pyodbc py3.14 parameter-binding segfault (mkleehammer/pyodbc#1459)"
+cause="${RETRY_NATIVE_CRASH_CAUSE-$default_cause}"
+if [ -z "$cause" ]; then
+  # DELIBERATELY NAMES NO CLASS, NOT EVEN TO WARN AGAINST ONE. An earlier draft said "do not
+  # assume the pyodbc class" and its own test caught it: that puts the token in the annotation,
+  # so a log grep for pyodbc matches the ONE leg where the class is explicitly not established.
+  cause=" -- CAUSE NOT ESTABLISHED for this leg; do not infer the database-leg crash class"
+fi
+
 # A process killed by signal N exits with 128+N. 139 = 128+SIGSEGV(11) (the observed
 # segfault); 134 = 128+SIGABRT(6) (the param/TVP path can abort() with a core dump instead).
 is_native_crash() {
@@ -51,9 +83,9 @@ for n in $(seq 1 "$attempts"); do
     exit "$rc"
   fi
   if [ "$n" -lt "$attempts" ]; then
-    echo "::warning::Native crash (exit ${rc}) on attempt ${n}/${attempts} — likely the pyodbc py3.14 parameter-binding segfault (mkleehammer/pyodbc#1459); retrying."
+      echo "::warning::NATIVE CRASH (exit ${rc}) on attempt ${n}/${attempts}${cause}; retrying."
   fi
 done
 
-echo "::error::Command still crashing after ${attempts} attempts (exit ${rc}); see mkleehammer/pyodbc#1459."
+echo "::error::NATIVE CRASH persisted: still crashing after ${attempts} attempts (exit ${rc})${cause}."
 exit "$rc"