fix(#894): teardown-gate the retry ARM; pin both teardown sentinels

Review round 2 (1 minor bug + 1 minor quality, security/perf zero):

- The retry's arm site was gated on historyStale alone, so a clear_ui
  refetch in flight at destroy() that then FAILS re-arms the timer
  AFTER destroy's clearTimeout — a no-op fire (the visHandler fire
  guard holds) but the orphan pins the dead closure for its 2s delay,
  contradicting destroy's dead-not-inert invariant.  The arm gate is
  now historyStale && visHandler, matching the fire guard; the seam
  matrix (destroy/closeSession/live x arm-and-fire windows) closes
  with that one term.  The edit-resend in the same .then stays
  deliberately ungated on teardown: the rewind committed server-side
  and the workstream outlives the pane UI, so the committed edit
  still delivers (comment at site).
- Pins: the arm gate (mutation-verified — the contract test fails
  against a gate-stripped mutant), the fire guard's visHandler term
  (sole coordCloseSession protection), and the re-arm clearTimeout.

G1/G2/G3 re-run READY; 136 static-pin tests green.
This commit is contained in:
Patrick Buckley
2026-07-23 07:24:31 -07:00
parent 85214f433f
commit ac441471f9
2 changed files with 30 additions and 4 deletions
+20 -3
View File
@@ -670,17 +670,34 @@ def test_coordinator_history_stale_latch_contract():
"(mirrors interactive's !_replayQueue pin)."
)
# 5. Bounded retry: exactly one arm site, and its fire guard yields to
# an in-flight fetch.
# 5. Bounded retry: exactly one arm site; the ARM is teardown-gated;
# the fire guard yields to an in-flight fetch and carries the
# teardown sentinel.
assert body.count("staleRetryTimer = setTimeout") == 1, (
"the stale retry must be armed in exactly one place (clear_ui "
".then) — bounded by construction."
)
assert body.count("if (historyStale && visHandler) {") == 1, (
"the arm must be gated on the teardown sentinel too — the "
"clear_ui .then can settle after destroy()/coordCloseSession, "
"and an ungated arm recreates the orphan timer destroy's cancel "
"exists to kill."
)
retry_arm = body.index("staleRetryTimer = setTimeout")
assert "!refetchesInFlight" in body[retry_arm : retry_arm + 700], (
retry_fire = body[retry_arm : retry_arm + 700]
assert "!refetchesInFlight" in retry_fire, (
"the retry's fire guard must yield to an in-flight refetch "
"(mirrors interactive's !_replayQueue pin)."
)
assert "visHandler" in retry_fire, (
"the retry's fire guard must carry the teardown sentinel — it is "
"the SOLE protection for the coordCloseSession path, which nulls "
"visHandler but does not cancel the timer."
)
assert "if (staleRetryTimer) clearTimeout(staleRetryTimer);" in body, (
"re-arming on a newer clear_ui must cancel the pending timer "
"first, or a double clear_ui leaks a timer."
)
# 6. Teardown: terminal cancel in destroy(); NOT in closeStreamTransport.
destroy_slice = body[body.index("function destroy()") :]
@@ -3683,7 +3683,16 @@ function createCoordinatorPane(root, wsId, opts) {
// so a wipe would strand a dangling ref), or torn down
// (visHandler — destroy() also cancels this timer outright,
// but coordCloseSession only nulls visHandler).
if (historyStale) {
//
// The ARM is visHandler-gated too: this .then can settle AFTER
// a teardown (destroy/close-session during the in-flight
// refetch), and destroy's clearTimeout already ran — an arm
// here would recreate the orphan timer destroy exists to kill
// (a no-op fire, but it pins the dead closure for 2s). The
// edit-resend below stays UNgated on teardown by design: the
// rewind committed server-side and the workstream outlives the
// pane UI, so the user's committed edit still delivers.
if (historyStale && visHandler) {
if (staleRetryTimer) clearTimeout(staleRetryTimer);
staleRetryTimer = setTimeout(() => {
staleRetryTimer = null;