From ac441471f940dd42601a112a05757d5b755daf3e Mon Sep 17 00:00:00 2001
From: Patrick Buckley <eous@users.noreply.github.com>
Date: Thu, 23 Jul 2026 07:24:31 -0700
Subject: [PATCH] fix(#894): teardown-gate the retry ARM; pin both teardown
 sentinels
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.
---
 tests/test_coordinator_page.py                | 23 ++++++++++++++++---
 .../console/static/coordinator/coordinator.js | 11 ++++++++-
 2 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/tests/test_coordinator_page.py b/tests/test_coordinator_page.py
index 8ba4e951..6cc5385f 100644
--- a/tests/test_coordinator_page.py
+++ b/tests/test_coordinator_page.py
@@ -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()") :]
diff --git a/turnstone/console/static/coordinator/coordinator.js b/turnstone/console/static/coordinator/coordinator.js
index b08bc27a..596e3a7e 100644
--- a/turnstone/console/static/coordinator/coordinator.js
+++ b/turnstone/console/static/coordinator/coordinator.js
@@ -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;