Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 48 additions & 6 deletions src/tui-opentui/gate-wire.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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))
},
})
}
Expand Down Expand Up @@ -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")
Expand All @@ -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
Expand All @@ -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 } : {}),
Expand All @@ -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())
},
}))
}
Expand Down
5 changes: 4 additions & 1 deletion src/tui-opentui/product-host.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
37 changes: 31 additions & 6 deletions src/tui-opentui/runtime-bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ import {
clearQuotaWait,
initialTurnState,
turnStateFromEvent,
turnStateBlocked,
turnStateGateClosed,
turnStateGateOpened,
turnStateOnInterrupt,
turnStateOnSubmit,
type TurnState,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -995,6 +1018,8 @@ export function attachSessionBridge(
},
submit,
interrupt: doInterrupt,
gateOpened,
gateClosed,
get turn() {
return bag.turn
},
Expand Down
13 changes: 13 additions & 0 deletions src/tui-opentui/stall-watchdog.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
73 changes: 73 additions & 0 deletions src/tui-opentui/turn-monitor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down Expand Up @@ -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)
Expand All @@ -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", () => {
Expand Down
60 changes: 58 additions & 2 deletions src/tui-opentui/turn-state.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ import { describe, expect, test } from "bun:test"

import {
initialTurnState,
turnStateBlocked,
turnStateFromEvent,
turnStateGateClosed,
turnStateGateOpened,
turnStateOnInterrupt,
turnStateOnSubmit,
} from "./turn-state.js"
Expand Down Expand Up @@ -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)
})
})

Expand Down
Loading
Loading