fix(gateway): fit the drain to the ARMED watchdog deadline, one source (#838 P1 close-out) - #861
Conversation
Verification on f96a4c0 (interpreter:
|
…FleetReview r1) FleetReview round 1 on f96a4c0: 5/6 arms independently found the same P1 in the finding-4 re-arm. All P0/P1 closed here. P1 (5 arms) — absolute-vs-relative unit mismatch in _rearm_shutdown_watchdog. _armed_shutdown_deadline_s is ABSOLUTE (from the start of stop()); every consumer treats it that way. arm_shutdown_watchdog is RELATIVE (deadline = time.monotonic() + delay). The top-of-stop() arming is at t=0 where the two coincide, so handing the absolute value into a LATER re-arm charged the pre-drain elapsed twice and scheduled os._exit at elapsed + deadline. At clamp 300 / drain 180 / teardown 70 / elapsed 40 that fires at 330 against a launchd SIGKILL at 300 — the backstop never runs, no forensic dump, no ordered lock/PID release, SIGKILL mid-teardown. Exactly the class this change exists to close. Fix: publish the absolute deadline (drain + cron consume it unchanged), arm max(deadline - elapsed_now, 0). Skip the re-arm entirely if that is <= 0 rather than replacing a live backstop with a zero-delay no-op. P1 (B-state) — chained pytest.approx. `a == approx(x) == approx(y)` also evaluates approx == approx, which is invalid; split into two asserts. P1 (L6) — the re-arm was UNREACHABLE under pytest (PYTEST_CURRENT_TEST early-return), so deleting the whole block left the suite green. New test_stop_rearms_the_watchdog_with_REMAINING_time_not_the_absolute_deadline drives the real stop() with the marker cleared and a 0.9s pre-drain cost, reads both values handed to arm_shutdown_watchdog. P2 (L6) — the call-site wiring was untested: armed_deadline_s / deadline_s are structurally None under pytest, so the spies could not see them. New test_stop_threads_the_one_deadline_into_both_the_drain_and_the_cron_leash clears the marker and asserts both consumers got the same armed-derived deadline (hand-computed 50 / 28 at the production clamp). P3 (B-assert-ctx, C) — the forensic snapshot re-derived watchdog_delay_s with elapsed_s=0, so a dump written after a re-arm reported the superseded deadline. Now reads the published value. Verified (.venv interpreter): focused file 75 passed, rc=0 (was 73) 50-file per-file slice 567 passed / 0 failed, every file rc=0 + summary mutation gate on the new tests, each reverted after: re-arm passes the absolute deadline again -> CAUGHT (armed 250.901 vs 250) re-arm never invoked -> CAUGHT (got [250.0], expected 2) drain armed_deadline_s=None -> CAUGHT cron deadline_s=None -> CAUGHT
Round 2 — all FleetReview P0/P1 from
|
FleetReviewReviewed with 2 of 3 model families — openai unavailable. Confidence: 3/5 Findings
FleetReview provenance · models: C=claude-code-opus-5, D=grok-4.6, G=grok-4.6 · cost: $11.23 · duration: 34m 59s · rounds: 2 · files examined: 3 |
…vation DIVERGES The round-2 call-site test (test_stop_threads_the_one_deadline_into_both_*) is pinned at the production clamp 60, where the outer min() of the arming expression binds and a consumed deadline equals a re-derived one (both 28). An independent 4-mutant gate run showed it therefore SURVIVES hard-wiring armed_deadline_s=None at gateway/run.py:20042 — i.e. the gate for finding 1 could not detect finding 1. That is the #838 root pattern (measuring the one geometry where two different deadlines coincide) reproduced in the suite. Adds test_drain_consumes_the_REARMED_deadline_at_the_inner_leash_geometry: clamp 300 / configured 180 / measured teardown 70 / real 0.9s pre-drain cost, where consumed = 250.9 - 70 = 180.9 and re-derived = 250 - 70 = 180. All expected values hand-computed, independent of every resolver. Verified: 76 passed rc=0 on the focused file. Mutant armed_deadline_s=None at the call site: SURVIVED before, CAUGHT after.
Round 3 — a gate hole I found in my OWN round-2 test, on
|
FleetReviewReviewed with 2 of 3 model families — openai unavailable. Confidence: 3/5 Findings
FleetReview provenance · models: C=claude-code-opus-5, D=grok-4.6, G=grok-4.6 · cost: $27.40 · duration: 35m 38s · rounds: 1 · files examined: 3 |
…ain the re-arm (FleetReview r2) F1 (P1) resolve_stop_drain_deadline_s trusted armed_deadline_s unclamped. The published _armed_shutdown_deadline_s is only wall-clamped when the ARMING ran with signal_driven=True; an in-band stop(restart=True) arms with it False, so the published value is the raw inner leash (240 at clamp 60). A supervisor SIGTERM landing mid-stop flips the flag, and the drain/cron reads then see signal_driven=True + that stale 240 -> deadline 225 against an uncatchable SIGKILL at 60. The extend-only re-arm cannot rescue it: the correct value (50) is EARLIER and fails the extend-only guard. Now clamped to the same wall the recomputation branch is bounded by. Measured: 225 -> 35; inert at both normal geometries (50->28, 250.9->180.9). F4 (P1) the re-arm was guarded only by PYTEST_CURRENT_TEST, but its 'bounded by the SIGKILL wall' invariant only exists on launchd: resolve_launchd_shutdown_watchdog_delay short-circuits without a live ExitTimeOut, so on systemd/docker-s6/foreground the new deadline is an uncapped elapsed+drain+max(grace,reserve) (measured 240/245/270/540 at elapsed 0/5/30/300). No benefit there either -- resolve_stop_drain_deadline_s returns None without a budget, so the drain is never elapsed-charged. Gated to the launchd signal path. F2 (P1) the re-arm committed shutdown state before the replacement backstop was live. If arm_shutdown_watchdog raised, _stop_impl_body unwound at the drain fit -- before drain/persist/teardown -- and the outer finally then set every event, disarming the ORIGINAL watchdog too. Now: arm first, contain the exception, commit + retire the old one only after arming returns. F3 rejected as a false finding: deadline_s IS passed, at run.py:20092 (deadline_s=_stop_deadline_s). The reviewer read restart.py only. Proven live by mutant M4 -- dropping it goes RED. Verified (.venv): focused 80 passed rc=0 (was 76). Each new test proven RED on 384be7f in a clean-room worktree: F1 'deadline 225.0 ... expected 35', F4 'got [240.0, 239.99998]', F2 RuntimeError propagated out of stop().
…t stopwatch)
A mutation-gate baseline run took 143s instead of 7s under concurrent load and
produced 1 failure. Cause: pytest.approx(250.0, abs=0.35) / approx(250.9, abs=0.5)
against a real asyncio.sleep(0.9) — a stopwatch reading, which load stretches.
Converted to the relational invariants the values actually encode, each of which
jitter can only strengthen:
published > 250 and <= 290 (re-arm extended, still inside the wall)
second <= 250 and < published (armed the REMAINING time, not the absolute)
armed > 250, deadline > 180 (consumed the re-armed deadline; re-derivation
yields EXACTLY 250/180, so > is discriminating)
deadline == approx(armed - 70, abs=0.01) (one deadline, exact, duration-free)
Mutation gate re-verified after the conversion — see the next commit's evidence.
FleetReviewReviewed with 2 of 3 model families — openai unavailable. Confidence: 3/5 Findings
FleetReview provenance · models: C=claude-code-opus-5, D=grok-4.6, G=grok-4.6 · cost: $25.27 · duration: 27m 32s · rounds: 1 · files examined: 3 |
Round 4 — all 4 FleetReview P1 from
|
FleetReviewReviewed with 2 of 3 model families — openai unavailable. Confidence: 3/5 Findings
FleetReview provenance · models: C=claude-code-opus-5, D=grok-4.6, G=grok-4.6 · cost: $27.38 · duration: 32m 31s · rounds: 1 · files examined: 3 |
… stop Verify 123 passed, 2 skipped across six gateway test files. Four isolated mutants failed: cron deadline handoff, thread-start signal, failed-arm caller, wall-bound shortening; the arming wall mutant failed after adding a direct independent wall assertion. Accept and pin zero cron floor when teardown consumes the window.
|
🤖 merged-by: apollo · lane: fr-pause-0922 · gate: BYPASS: FR PAUSED by Ace ruling 2026-09-22 (state/fleetreview-pause-20260922.md); t_74bb9994 approved · why: argus run 7579 PASS-WITH-CAVEATS/SHIP, blocked only on #911 which Aegis merged; FR paused; CI green; Apollo merge pass 2026-09-22 |
Kyzcreig
added
the
fleetreview:post-merge
1 participant
Closes the 4 unresolved P1 from FleetReview's trusted record on #838
(#838 (comment)),
which merged 7 minutes after that record was posted. New branch off main; #838 is not reopened.
ROOT PATTERN: there were TWO deadlines and the drain was fitted to the wrong one.
resolve_elapsed_adjusted_drainfitted againstexit_timeout - hard_exit_reserve_s - reserve,but the watchdog that actually calls
os._exitis armed fromresolve_armed_shutdown_watchdog_delay=min(effective_drain + max(grace, reserve), hard_exit).Those agree only when the OUTER
min()binds (the gui-clamped 60). On a non-gui-clampedsystem-domain job where the INNER leash binds (clamp 300 / configured 180 / measured teardown 70:
armed 250 vs hard-exit-derived 220) the drain overran into the teardown window.
F1 — one function returns the armed deadline; every site consumes it
New
resolve_stop_drain_deadline_s()ingateway/restart.pyis THE ONE DEADLINE(armed watchdog minus teardown reserve).
resolve_elapsed_adjusted_drainnow derivesno arithmetic of its own — it calls the resolver.
gateway/run.pycomputes it once in_stop_impl_bodyfrom_armed_shutdown_deadline_s(the value actually handed toarm_shutdown_watchdog, captured at the arming site — not a second derivation) andthreads it to both the drain fit and the cron leash.
Also extracted
resolve_stop_teardown_reserve_s()as THE ONE RESERVE, so themax(cleanup_reserve, measured)+ actionable-ceiling filter exists once instead ofthree copies that could drift.
F2 — cron branch consumes the deadline instead of re-deriving from the SIGKILL wall
resolve_cron_drain_budget(deadline_s=...): when supplied it REPLACES thewatchdog_delay/cleanup_reserve_sderivation entirely, so the ceiling is exactly thedeadline - elapsedthe non-cron drain is fitted to. The old path clamped to the rawExitTimeOutand held back only the 10sCRON_DRAIN_CLEANUP_RESERVE_S, which raised thebudget back to the hard-exit instant three statements after the elapsed adjustment.
The "cron floor only ever EXTENDS" intent is preserved (the
max(drain, ...)stands;drainis itself already deadline-fitted, so it is not a loophole).F3 — invariant test no longer re-derives the function under test
The old sweep recomputed its expected deadline with
resolve_launchd_shutdown_watchdog_delay(clamp, clamp, ...)— the same expression the codeused — so it held by construction. Replaced with hard-coded worked examples
(clamp 300 / drain 180 / teardown 70 / grace 60 -> concrete numbers) plus an oracle that
observes the armed wall rather than recomputing it.
F4 — watchdog re-armed after the elapsed is measured
The top-of-
stop()arming sizes a RELATIVE drain budget before the pre-drain cost is known,so the armed window silently absorbed that elapsed and the teardown reserve paid for it.
_rearm_shutdown_watchdog()extends the deadline by exactly the measured elapsed once known.EXTEND-ONLY and still bounded by
resolve_launchd_shutdown_watchdog_delay(so never pastexit_timeout - hard_exit_reserve_s, and a no-op when the wall already binds). Re-arm orderis: arm the replacement, THEN disarm the superseded thread — no instant without a backstop —
and the
finallysets every event armed on the path so a superseded thread cannot hard-exita completed shutdown.
Verification
Focused slice on this head,
.venvinterpreter: see PR comments for the measured run.Independent verification by argus (card t_f753b2b5) against a real
GatewayRunner.stop()with in-flight cron, observing the armed wall from the actual
arm_shutdown_watchdog()call:control fork/main dd7e9bc 10/10 UNSAFE -> this head 0/10 unsafe,
cron_past_armed_wall0/10,with all 4 control paths preserved (non_launchd_zero_drain 50.0, cron_optout_zero 28/28,
in_band_restart 50/100, inner_leash_clamp300 180/180).
Card: t_74bb9994. Gateway restart is gated on this landing.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.