Merge pull request #381 from corbitsdev/cl-5642-gate-blocked-turns-are-exempted-from-the-stall-watchdog-only

Fold gate blocked-ness into turn state so the stall watchdog sees it

Author: TheGreatAxios

src/tui-opentui/gate-wire.ts

@@ -242,13 +242,51 @@ function recordOperatorDecision(
   appendStreamRow(shell, { role: "system", text, meta: "operator" })
 }
 
+/**
+ * Blocked-ness is domain state, not a paint detail: the turn watchdog and the
+ * painter both need to know a gate is outstanding, whether or not it has
+ * reached the screen yet. This is the only place that sees a gate's full
+ * lifecycle (raised, possibly queued, eventually resolved), so it is the one
+ * that reports it — callers fold the pair into their own turn state.
+ */
+export type GateLifecycleHooks = {
+  /** A gate was raised — queued or opened, whichever comes first. */
+  readonly onGateOpened: () => void
+  /** A previously raised gate resolved. */
+  readonly onGateClosed: () => void
+}
+
+const NOOP_GATE_HOOKS: GateLifecycleHooks = {
+  onGateOpened: () => {},
+  onGateClosed: () => {},
+}
+
+/**
+ * Wrap a gate's `resolve` so `onGateClosed` fires exactly once no matter
+ * which of accept / cancel / auto-deny settles it first.
+ */
+function onceClosed<T>(
+  onGateClosed: () => void,
+  resolve: (value: T) => void,
+): (value: T) => void {
+  let closed = false
+  return (value) => {
+    if (!closed) {
+      closed = true
+      onGateClosed()
+    }
+    resolve(value)
+  }
+}
+
 /**
  * Subscribe the permission/operator gate events to the shell's overlays.
  * Returns a dispose function that removes exactly the listeners this call added.
  */
 export function wireGates(
   emitter: EventEmitter,
   shell: AppShell,
+  hooks: GateLifecycleHooks = NOOP_GATE_HOOKS,
 ): () => void {
   // The shell has one overlay host, and opening onto a busy one is a no-op.
   // Gates cannot be dropped that way — a lost ask_operator blocks the run with
@@ -270,6 +308,8 @@ export function wireGates(
   })
 
   function onPermission(ev: PermissionGateEvent): void {
+    hooks.onGateOpened()
+    const resolve = onceClosed(hooks.onGateClosed, ev.resolve)
     const choices = permissionChoicesFromRequest(ev.request)
     const collapsedBody = permissionBodyFromRequest(ev.request, { hint: true })
     // Nothing was collapsed → no expand affordance, so the overlay leaves the
@@ -318,7 +358,7 @@ export function wireGates(
             ...(sel.id !== undefined ? { id: sel.id } : {}),
           }
           recordDecision(shell, ev.request, choices, gateSelection)
-          ev.resolve(approvalOutcomeFromSelection(choices, gateSelection))
+          resolve(approvalOutcomeFromSelection(choices, gateSelection))
         },
         // Esc must settle the awaited promise (as a deny), not abandon it —
         // an unresolved gate hangs the run until the process is killed.
@@ -328,7 +368,7 @@ export function wireGates(
           clearTimers()
           const gateSelection = { index: 0, id: PERMISSION_DENY_ID }
           recordDecision(shell, ev.request, choices, gateSelection)
-          ev.resolve(approvalOutcomeFromSelection(choices, gateSelection))
+          resolve(approvalOutcomeFromSelection(choices, gateSelection))
         },
       })
     }
@@ -358,7 +398,7 @@ export function wireGates(
         const idx = pending.indexOf(open)
         if (idx >= 0) pending.splice(idx, 1)
       }
-      ev.resolve({ allow: false, message })
+      resolve({ allow: false, message })
     }
     function onAbort(): void {
       autoDeny("tool no longer running; permission request denied")
@@ -378,6 +418,8 @@ export function wireGates(
   }
 
   function onOperator(ev: OperatorGateEvent): void {
+    hooks.onGateOpened()
+    const resolve = onceClosed(hooks.onGateClosed, ev.resolve)
     const choices = operatorChoicesFromOptions(ev.options)
     // Guarded the same way as the permission gate: correctness must not rest
     // on callers of closeInsetOverlay remembering to null the cancel hook
@@ -396,7 +438,7 @@ export function wireGates(
         if (settled) return
         settled = true
         recordOperatorDecision(shell, ev.question, sel.label)
-        ev.resolve(
+        resolve(
           operatorResultFromSelection(ev.options, {
             index: sel.index,
             ...(sel.id !== undefined ? { id: sel.id } : {}),
@@ -409,15 +451,15 @@ export function wireGates(
         if (settled) return
         settled = true
         recordOperatorDecision(shell, ev.question, text)
-        ev.resolve(operatorCustomResult(text))
+        resolve(operatorCustomResult(text))
       },
       // Esc must settle the awaited promise (as a cancel), not abandon it —
       // an unresolved gate hangs the run until the process is killed.
       onCancel: () => {
         if (settled) return
         settled = true
         recordOperatorDecision(shell, ev.question, "Cancelled")
-        ev.resolve(operatorCancelResult())
+        resolve(operatorCancelResult())
       },
     }))
   }

src/tui-opentui/product-host.ts

@@ -486,7 +486,10 @@ export async function mountProductHost(
   // leave the terminal wedged with nobody able to restore it.
   let disposeGates: () => void
   try {
-    disposeGates = wireGates(config.eventEmitter, shell)
+    disposeGates = wireGates(config.eventEmitter, shell, {
+      onGateOpened: () => bridge.gateOpened(),
+      onGateClosed: () => bridge.gateClosed(),
+    })
   } catch (err: unknown) {
     try {
       renderer.destroy()

src/tui-opentui/runtime-bridge.ts

@@ -54,7 +54,8 @@ import {
   clearQuotaWait,
   initialTurnState,
   turnStateFromEvent,
-  turnStateBlocked,
+  turnStateGateClosed,
+  turnStateGateOpened,
   turnStateOnInterrupt,
   turnStateOnSubmit,
   type TurnState,
@@ -161,6 +162,14 @@ export type SessionBridge = {
     attachments?: readonly PendingImageAttachment[],
   ) => void
   interrupt: () => void
+  /**
+   * A permission or operator gate was raised — queued or already displayed.
+   * Blocks the turn (and exempts it from the stall watchdog) until a matching
+   * `gateClosed` call. Multiple outstanding gates nest correctly.
+   */
+  gateOpened: () => void
+  /** A previously raised gate resolved. */
+  gateClosed: () => void
   dispose: () => void
   /** Current derived turn phase (progress label, stall clock, quota window). */
   readonly turn: TurnState
@@ -748,11 +757,7 @@ export function attachSessionBridge(
   }
 
   const paintPhase = (): void => {
-    // The gate overlay is the only "blocked" signal the shell sees; the gate
-    // wiring resolves approvals itself and emits no bridge event.
-    const gated =
-      shell.overlayKind === "permissions" || shell.overlayKind === "operator"
-    const turn = gated ? turnStateBlocked(bag.turn) : bag.turn
+    const turn = bag.turn
     // The landing mark rides this same re-entry: it animates through the
     // draw/fill loop while a turn is live and holds its filled frame otherwise.
     paintLanding(shell, now(), turn.isProcessing)
@@ -893,6 +898,24 @@ export function attachSessionBridge(
     paintPhase()
   }
 
+  /**
+   * A permission or operator gate was raised — queued or already on screen,
+   * the turn does not distinguish. Called from the gate wiring itself, not
+   * derived from `shell.overlayKind`, so a gate still waiting behind another
+   * overlay exempts the turn from the stall watchdog just as an open one does.
+   */
+  const gateOpened = (): void => {
+    if (bag.disposed) return
+    bag.turn = turnStateGateOpened(bag.turn)
+    paintPhase()
+  }
+
+  const gateClosed = (): void => {
+    if (bag.disposed) return
+    bag.turn = turnStateGateClosed(bag.turn, now())
+    paintPhase()
+  }
+
   const tick = (): void => {
     if (bag.disposed) return
     const nowMs = now()
@@ -995,6 +1018,8 @@ export function attachSessionBridge(
     },
     submit,
     interrupt: doInterrupt,
+    gateOpened,
+    gateClosed,
     get turn() {
       return bag.turn
     },

src/tui-opentui/stall-watchdog.test.ts

@@ -50,6 +50,19 @@ describe("shouldAbortForStall", () => {
     expect(shouldAbortForStall({ ...base, status: "stopping" })).toBe(false)
   })
 
+  // Two independent exemptions (a gate open on the operator, a sibling tool
+  // call still outstanding) must both keep exempting when combined — neither
+  // one's guard may accidentally require the other's condition to also hold.
+  test("a gate open and a sibling tool call each exempt alone, and together", () => {
+    const gateOnly = { ...base, status: "blocked" as const }
+    const toolCallOnly = { ...base, activeToolCalls: ["call-2"] }
+    const both = { ...base, status: "blocked" as const, activeToolCalls: ["call-2"] }
+
+    expect(shouldAbortForStall(gateOnly)).toBe(false)
+    expect(shouldAbortForStall(toolCallOnly)).toBe(false)
+    expect(shouldAbortForStall(both)).toBe(false)
+  })
+
   test("a settled turn with nothing in flight is not a stall", () => {
     expect(
       shouldAbortForStall({

src/tui-opentui/turn-monitor.test.ts

@@ -227,6 +227,7 @@ describe("turn progress label", () => {
       try {
         t.bridge.handle({ type: "inference.start", data: {} })
         t.shell.overlayKind = "permissions"
+        t.bridge.gateOpened()
         t.tick()
         expect(t.shell.turnPhase).toEndWith("blocked")
 
@@ -347,6 +348,48 @@ describe("stall watchdog", () => {
     })
   })
 
+  test("an open gate is exempt no matter how long the operator takes", async () => {
+    await withTestRenderer(async (h) => {
+      const t: Harness = await setup(h)
+      try {
+        t.bridge.submit("build it", "immediate")
+        t.port.clear()
+        t.bridge.gateOpened()
+
+        // Far past the stall timeout — an operator reading an approval must
+        // never have the run torn down underneath them.
+        t.advance(20 * 60_000)
+        t.tick()
+        expect(t.port.calls).toEqual([])
+        expect(t.shell.statusFlash).not.toBe(STALL_NOTICE_MESSAGE)
+      } finally {
+        t.bridge.dispose()
+      }
+    })
+  })
+
+  test("a gate queued but not yet displayed gets the same exemption", async () => {
+    await withTestRenderer(async (h) => {
+      const t: Harness = await setup(h)
+      try {
+        t.bridge.submit("build it", "immediate")
+        t.port.clear()
+        // The gate is raised but nothing else has changed `shell.overlayKind`
+        // — this is the "queued behind another overlay" shape from
+        // gate-wire.ts, where the gate is not nominally displayed yet.
+        t.bridge.gateOpened()
+        expect(t.shell.overlayKind).toBeNull()
+
+        t.advance(20 * 60_000)
+        t.tick()
+        expect(t.port.calls).toEqual([])
+        expect(t.shell.statusFlash).not.toBe(STALL_NOTICE_MESSAGE)
+      } finally {
+        t.bridge.dispose()
+      }
+    })
+  })
+
   test("a live tool run is not treated as a stall", async () => {
     await withTestRenderer(async (h) => {
       const t: Harness = await setup(h)
@@ -367,6 +410,36 @@ describe("stall watchdog", () => {
       }
     })
   })
+
+  // The gate exemption (this fix) and the parallel-tool-call exemption
+  // (CL-5641) are independent guards feeding the same stall check — a run
+  // with both outstanding must stay exempt, and closing the gate while the
+  // tool call is still out must not re-expose it to the clock.
+  test("a gate open alongside a live sibling tool call stays exempt", async () => {
+    await withTestRenderer(async (h) => {
+      const t: Harness = await setup(h)
+      try {
+        t.bridge.submit("build it", "immediate")
+        t.bridge.handle({
+          type: "inference.tool_call.end",
+          data: { name: "task", callId: "c1" },
+        })
+        t.bridge.gateOpened()
+        t.port.clear()
+
+        t.advance(20 * 60_000)
+        t.tick()
+        expect(t.port.calls).toEqual([])
+
+        t.bridge.gateClosed()
+        t.advance(20 * 60_000)
+        t.tick()
+        expect(t.port.calls).toEqual([])
+      } finally {
+        t.bridge.dispose()
+      }
+    })
+  })
 })
 
 describe("repetition guard", () => {

src/tui-opentui/turn-state.test.ts

@@ -2,8 +2,9 @@ import { describe, expect, test } from "bun:test"
 
 import {
   initialTurnState,
-  turnStateBlocked,
   turnStateFromEvent,
+  turnStateGateClosed,
+  turnStateGateOpened,
   turnStateOnInterrupt,
   turnStateOnSubmit,
 } from "./turn-state.js"
@@ -184,9 +185,64 @@ describe("turn transitions", () => {
   })
 
   test("gate blocks without ending the turn", () => {
-    const s = turnStateBlocked(turnStateOnSubmit(initialTurnState(0), 1))
+    const s = turnStateGateOpened(turnStateOnSubmit(initialTurnState(0), 1))
     expect(s.status).toBe("blocked")
     expect(s.isProcessing).toBe(true)
+    expect(s.blockedGateCount).toBe(1)
+  })
+
+  test("a second queued gate keeps the turn blocked until both clear", () => {
+    const running = turnStateOnSubmit(initialTurnState(0), 1)
+    const bothOpen = turnStateGateOpened(turnStateGateOpened(running))
+    expect(bothOpen.status).toBe("blocked")
+    expect(bothOpen.blockedGateCount).toBe(2)
+
+    const oneClosed = turnStateGateClosed(bothOpen, 5)
+    expect(oneClosed.status).toBe("blocked")
+    expect(oneClosed.blockedGateCount).toBe(1)
+
+    const allClosed = turnStateGateClosed(oneClosed, 9)
+    expect(allClosed.status).toBe("running")
+    expect(allClosed.blockedGateCount).toBe(0)
+    expect(allClosed.lastActivityAt).toBe(9)
+  })
+
+  test("a gate still open at interrupt keeps its count into the next turn", () => {
+    // The overlay is not closed by an interrupt — nothing else resolves it —
+    // so a turn that ends while a gate is still outstanding must not lose
+    // count of it: the eventual close belongs to this gate, not to whatever
+    // turn happens to be live when the operator finally answers.
+    const interrupted = turnStateOnInterrupt(
+      turnStateGateOpened(turnStateOnSubmit(initialTurnState(0), 1)),
+      2,
+    )
+    expect(interrupted.status).toBe("stopped")
+    expect(interrupted.blockedGateCount).toBe(1)
+
+    const nextTurn = turnStateOnSubmit(interrupted, 3)
+    expect(nextTurn.status).toBe("blocked")
+    expect(nextTurn.blockedGateCount).toBe(1)
+
+    // The stale gate from before the interrupt finally resolves — it must
+    // settle the count the new turn inherited, not resurrect a status the
+    // new turn never asked for.
+    const resolved = turnStateGateClosed(nextTurn, 9)
+    expect(resolved.status).toBe("running")
+    expect(resolved.blockedGateCount).toBe(0)
+  })
+
+  test("closing a stale gate after the turn settled does not resurrect it", () => {
+    const done = turnStateFromEvent(
+      turnStateGateOpened(turnStateOnSubmit(initialTurnState(0), 1)),
+      { type: "inference.done", data: {} },
+      2,
+    )
+    expect(done.status).toBe("done")
+    expect(done.blockedGateCount).toBe(1)
+
+    const resolved = turnStateGateClosed(done, 9)
+    expect(resolved.status).toBe("done")
+    expect(resolved.blockedGateCount).toBe(0)
   })
 })