diff --git a/src/tui-opentui/gate-wire.ts b/src/tui-opentui/gate-wire.ts index ef8945f92..99926f138 100644 --- a/src/tui-opentui/gate-wire.ts +++ b/src/tui-opentui/gate-wire.ts @@ -242,6 +242,43 @@ 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( + 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. @@ -249,6 +286,7 @@ function recordOperatorDecision( 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,7 +451,7 @@ 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. @@ -417,7 +459,7 @@ export function wireGates( if (settled) return settled = true recordOperatorDecision(shell, ev.question, "Cancelled") - ev.resolve(operatorCancelResult()) + resolve(operatorCancelResult()) }, })) } diff --git a/src/tui-opentui/product-host.ts b/src/tui-opentui/product-host.ts index f1e8de68d..e85ab55f8 100644 --- a/src/tui-opentui/product-host.ts +++ b/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() diff --git a/src/tui-opentui/runtime-bridge.ts b/src/tui-opentui/runtime-bridge.ts index 8af595b33..9d86b682a 100644 --- a/src/tui-opentui/runtime-bridge.ts +++ b/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 }, diff --git a/src/tui-opentui/stall-watchdog.test.ts b/src/tui-opentui/stall-watchdog.test.ts index 6eac1df12..91fe39e94 100644 --- a/src/tui-opentui/stall-watchdog.test.ts +++ b/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({ diff --git a/src/tui-opentui/turn-monitor.test.ts b/src/tui-opentui/turn-monitor.test.ts index dc4bca865..517faf03f 100644 --- a/src/tui-opentui/turn-monitor.test.ts +++ b/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", () => { diff --git a/src/tui-opentui/turn-state.test.ts b/src/tui-opentui/turn-state.test.ts index f987c20e5..f99bd1805 100644 --- a/src/tui-opentui/turn-state.test.ts +++ b/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) }) }) diff --git a/src/tui-opentui/turn-state.ts b/src/tui-opentui/turn-state.ts index d0cb54cc8..86a4ecd33 100644 --- a/src/tui-opentui/turn-state.ts +++ b/src/tui-opentui/turn-state.ts @@ -135,6 +135,16 @@ export type TurnState = { * gets long enough to trip `detectRepetition` on its own. */ readonly consecutiveMatchingCycles: number + /** + * Outstanding approval/operator gates, counted from the moment each is + * raised — queued behind another overlay or already on screen, both count. + * `status` reads "blocked" whenever this is above zero, which is what + * exempts the turn from the stall watchdog: an operator reading a prompt + * must be indistinguishable from a live tool call as far as the silence + * clock is concerned. Painter and watchdog both read this one field rather + * than each re-deriving blocked-ness from the shell's overlay. + */ + readonly blockedGateCount: number } export function initialTurnState(nowMs: number): TurnState { @@ -155,14 +165,35 @@ export function initialTurnState(nowMs: number): TurnState { repeatingSinceTokenCount: null, cycleFingerprint: null, consecutiveMatchingCycles: 0, + blockedGateCount: 0, } } +/** + * Reset to a fresh turn state, except an outstanding gate's count carries + * across the boundary rather than being dropped. Interrupting or settling a + * turn does not close the permission/operator overlay still on screen — + * nothing else resolves it — so losing count of it here would let its + * eventual `turnStateGateClosed` decrement an unrelated later turn's count + * instead, either dropping that turn's own exemption early or masking a + * stall it never earned. The status this resets to (`"stopped"`, `"done"`, + * ...) is left as given: every one of those is already `!== "running"`, + * which is all the watchdog requires, so there is no need to relabel it + * `"blocked"` and risk `turnStateGateClosed` later reviving it to `"running"`. + */ +function carryBlockedGateCount(prior: TurnState, fresh: TurnState): TurnState { + return prior.blockedGateCount === 0 + ? fresh + : { ...fresh, blockedGateCount: prior.blockedGateCount } +} + /** Operator submitted a prompt: the run is live and awaiting first tokens. */ export function turnStateOnSubmit(state: TurnState, nowMs: number): TurnState { return { ...state, - status: "running", + // A gate left over from a prior turn still blocks the operator from + // doing anything else, so the new turn inherits the exemption too. + status: state.blockedGateCount > 0 ? "blocked" : "running", isProcessing: true, awaitingResponse: true, streamingType: null, @@ -181,13 +212,43 @@ export function turnStateOnSubmit(state: TurnState, nowMs: number): TurnState { } /** Ctrl+C / watchdog abort: nothing is in flight and no prompt may be replayed. */ -export function turnStateOnInterrupt(_state: TurnState, nowMs: number): TurnState { - return { ...initialTurnState(nowMs), status: "stopped" } +export function turnStateOnInterrupt(state: TurnState, nowMs: number): TurnState { + return carryBlockedGateCount(state, { + ...initialTurnState(nowMs), + status: "stopped", + }) } -/** A pending approval gate blocks the turn without ending it. */ -export function turnStateBlocked(state: TurnState): TurnState { - return { ...state, status: "blocked", isProcessing: true } +/** + * A gate was raised — queued or opened, the turn does not distinguish. + * The first outstanding gate blocks the turn without ending it; further + * gates just add to the count so the turn stays blocked until all clear. + */ +export function turnStateGateOpened(state: TurnState): TurnState { + const blockedGateCount = state.blockedGateCount + 1 + return { + ...state, + status: "blocked", + isProcessing: true, + blockedGateCount, + } +} + +/** + * A gate resolved. Only the last outstanding gate clearing returns the turn + * to "running" — earlier ones just decrement the count. `lastActivityAt` + * moves to `nowMs` so the stall clock restarts from the moment the operator + * actually answered, rather than crediting silence spent reading the prompt. + */ +export function turnStateGateClosed(state: TurnState, nowMs: number): TurnState { + const blockedGateCount = Math.max(0, state.blockedGateCount - 1) + if (blockedGateCount > 0) return { ...state, blockedGateCount } + return { + ...state, + status: state.status === "blocked" ? "running" : state.status, + lastActivityAt: nowMs, + blockedGateCount, + } } export function clearQuotaWait(state: TurnState): TurnState { @@ -490,11 +551,11 @@ export function turnStateFromEvent( lastActivityAt: nowMs, } } - return { + return carryBlockedGateCount(state, { ...initialTurnState(nowMs), status: "done", quota: state.quota, - } + }) /** * The other turn terminator: `agent.send()` resolves on connector.reply, @@ -506,11 +567,11 @@ export function turnStateFromEvent( if (state.activeToolCalls.length > 0) { return { ...state, awaitingResponse: false, lastActivityAt: nowMs } } - return { + return carryBlockedGateCount(state, { ...initialTurnState(nowMs), status: "done", quota: state.quota, - } + }) case "inference.error": { const quota = quotaFromInferenceError(event.data, nowMs) @@ -522,15 +583,24 @@ export function turnStateFromEvent( } case "reactor.done": - return { ...initialTurnState(nowMs), quota: state.quota } + return carryBlockedGateCount(state, { + ...initialTurnState(nowMs), + quota: state.quota, + }) case "reactor.error": - return { ...initialTurnState(nowMs), status: "failed" } + return carryBlockedGateCount(state, { + ...initialTurnState(nowMs), + status: "failed", + }) case "run": return event.state === "busy" ? turnStateOnSubmit(state, nowMs) - : { ...initialTurnState(nowMs), quota: state.quota } + : carryBlockedGateCount(state, { + ...initialTurnState(nowMs), + quota: state.quota, + }) default: return state