review(conc): wrapper exit-code fix verified safe (red still propagates) + correct my set -e pre-review miss; inbox consumed
All checks were successful
continuous-integration/drone/push Build is passing
All checks were successful
continuous-integration/drone/push Build is passing
This commit is contained in:
@ -211,3 +211,50 @@ the push build, not the code again:
|
||||
double-!testme same PR → 2nd blocks on the app lock (visible in its drone log) then runs; (d) one
|
||||
full green end-to-end run. Evidence to come from Drone build logs + cc-ci state (abra app ls /
|
||||
lslocks / docker), cold from my own access path.
|
||||
|
||||
## 2026-06-10T05:00Z — wrapper exit-code fix verified + CORRECTION to my P1 pre-review (inbox consumed)
|
||||
|
||||
Consumed ADVERSARY-INBOX.md (deleted) — Builder reported an M2 live-verify finding + fix. Folded in:
|
||||
|
||||
**The defect (real, Builder-found, build 269 plausible#3):** the drone exec step shell is `set -e`.
|
||||
On a NORMAL (green) harness exit the P1 EXIT trap still fired and its `kill -TERM -- -$PID` of the
|
||||
already-exited process group returned ESRCH (exit 1), which under `set -e` poisoned the step's exit
|
||||
status to 1 — a fully GREEN run (all tiers pass, level=4) reported RED.
|
||||
|
||||
**CORRECTION — my P1 pre-review was wrong on this point.** In my 04:23Z pre-review I claimed to have
|
||||
"empirically tested" green-on-red exit-code masking and REFUTED it. That test was run with plain
|
||||
`bash -c` WITHOUT `set -e` — the wrong shell mode. The real drone step runs `set -e`, where the bug
|
||||
manifests. I re-ran the matrix correctly now (bash -e), reproducing the bug (old wrapper + green +
|
||||
set -e → exit 1) and confirming I had the shell mode wrong. Lesson: model the EXACT runtime
|
||||
(set -e) for shell-trap behavior. The Builder caught this live; I did not. Owning it.
|
||||
NB the failure direction was false-RED (green reported red) — fail-safe-ish, not a green-on-red
|
||||
(no failing run was ever reported green); still a real defect.
|
||||
|
||||
**The fix (e1c4198 on branch, merged to main b7a009c) — independently verified by me, cold under
|
||||
`set -e` (the correct mode this time):**
|
||||
```
|
||||
setsid cc-ci-run runner/run_recipe_ci.py & PID=$!
|
||||
trap 'kill -TERM -- "-$PID" 2>/dev/null || true' TERM EXIT
|
||||
rc=0; wait "$PID" || rc=$?
|
||||
trap - TERM EXIT
|
||||
exit "$rc"
|
||||
```
|
||||
My 4-path matrix (all under `bash -e`, exact-shape repros):
|
||||
- A green harness → step exit 0 ✓ (poisoning gone: `|| true` on the trap kill + `trap - EXIT` before exit)
|
||||
- B **red harness (exit 7) → step exit 7 ✓ — NOT masked to green.** Critical false-GREEN check
|
||||
PASSES: `wait || rc=$?` captures the real rc and `exit "$rc"` propagates it. The
|
||||
"failing PR must report RED" gate is preserved by the fix.
|
||||
- C old wrapper + green + set -e → exit 1 ✓ (bug reproduced — root-cause confirmed)
|
||||
- D cancel (TERM to wrapper mid-wait) → wrapper exits 143 AND the child received TERM
|
||||
(CHILD_GOT_TERM logged) ✓ — cancel-forwarding semantics unchanged; the `trap - TERM EXIT` runs
|
||||
only AFTER `wait` returns (post-forward), so it can't disarm the forward during a real cancel.
|
||||
|
||||
Verdict on the fix: CORRECT and SAFE — resolves the false-RED poisoning without introducing
|
||||
false-GREEN, and preserves cancel forwarding. Folds cleanly into the pending M2 review.
|
||||
|
||||
**M1 status unaffected:** M1 PASS was for the code/suites/lint/diff of d3fe9e2; this wrapper
|
||||
exit-code-under-set-e is a LIVE behavior M1's checks could not exercise (the trap only runs in the
|
||||
real drone exec shell). main now = d3fe9e2 + this .drone.yml wrapper fix; the fix is verified above.
|
||||
Open for the formal M2 verdict: re-confirm lint green on the new .drone.yml (yamllint), the push
|
||||
build green, and live (a) cancel-no-leak / (b) parallel both-green / (c) double-!testme blocks /
|
||||
(d) one full green run — cold, once the Builder posts the M2 claim with evidence.
|
||||
|
||||
@ -1,17 +0,0 @@
|
||||
# Builder → Adversary (2026-06-10, M2 in progress)
|
||||
|
||||
M2 live-verify surfaced ONE wrapper defect, fixed + re-merged — please fold into your M2 review:
|
||||
|
||||
- Build 269 (plausible#3) ran fully GREEN (all tiers pass, level=4, deploy-count 1/1) but the
|
||||
step exited 1: the drone step shell is `set -e`, and on a NORMAL harness exit the P1 EXIT trap
|
||||
still fired; its `kill -TERM -- "-$PID"` of the already-gone process group returned ESRCH,
|
||||
poisoning the green exit status. Reproduced minimally on-host with both `sh -e` and `bash -e`.
|
||||
- Fix e1c4198 (branch) merged to main at b7a009c: capture `rc` from `wait`, `trap - TERM EXIT`
|
||||
before exiting, `|| true` on the trap kill. Verified all three paths on-host: green rc=0,
|
||||
red rc=7 propagated, TERM-to-wrapper → child receives TERM, wrapper exits 143. Cancel
|
||||
forwarding unchanged (M2(a) evidence stands: build 267 cancel → harness teardown, zero leak).
|
||||
- Build 268 (immich#2, still running) predates the fix; even if its run is green the step will
|
||||
exit 1 the same way — I will re-trigger both recipes for the formal (b) "both green" after
|
||||
268 finishes, then (c) double-!testme.
|
||||
|
||||
M2(a) evidence is in JOURNAL-conc.md; formal claim comes after (b)-(d).
|
||||
Reference in New Issue
Block a user