Skip to content

Commit dbf21e4

Browse files
committed
Resolve gate overlays on Esc instead of hanging
Esc on a permission or operator overlay called closeInsetOverlay, which dropped the accept callback without ever invoking the caller's awaited resolve. The run deadlocked until the process was killed. Add an onCancel hook to the overlay opts and thread it through the open/close paths for the promise-backed overlay kinds only, so the gate wiring can resolve deny/cancel from it. Deliberate accept and answer-submit paths null the hook first so closing there does not also fire it. The operator gate also gains its own settled flag mirroring the permission gate, so double-resolve is prevented by construction rather than by callers remembering to null the hook. Update the operator "no way to answer" copy now that Esc actually settles the question.
1 parent 533f6da commit dbf21e4

4 files changed

Lines changed: 141 additions & 3 deletions

File tree

src/tui-opentui/gate-wire.test.ts

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { withTestRenderer, type Harness } from "./harness.js"
99
import { OVERLAY_MAX_FRACTION } from "./geometry/index.js"
1010
import {
1111
acceptOverlaySelection,
12+
closeInsetOverlay,
1213
createAppShell,
1314
exitOverlayAnswerMode,
1415
handleOverlayAnswerKey,
@@ -572,6 +573,71 @@ describe("permission.gate auto-deny", () => {
572573
})
573574
})
574575

576+
describe("Esc on a gate overlay settles the awaited promise", () => {
577+
test("permission.gate: Esc denies instead of abandoning the promise", async () => {
578+
await withTestRenderer(async (h) => {
579+
const shell = createAppShell(h.renderer, {
580+
terminal: { columns: 80, rows: 24 },
581+
run: "idle",
582+
})
583+
const emitter = new EventEmitter()
584+
let resolved: unknown
585+
let resolveCount = 0
586+
try {
587+
wireGates(emitter, shell)
588+
emitter.emit("permission.gate", {
589+
request: baseRequest(),
590+
resolve: (outcome: unknown) => {
591+
resolveCount += 1
592+
resolved = outcome
593+
},
594+
})
595+
expect(shell.overlayKind).toBe("permissions")
596+
597+
closeInsetOverlay(shell)
598+
599+
expect(shell.overlayList).toBeNull()
600+
expect(resolveCount).toBe(1)
601+
expect(resolved).toEqual({ allow: false })
602+
} finally {
603+
shell.dispose()
604+
}
605+
})
606+
})
607+
608+
test("operator.gate: Esc cancels instead of abandoning the promise", async () => {
609+
await withTestRenderer(async (h) => {
610+
const shell = createAppShell(h.renderer, {
611+
terminal: { columns: 80, rows: 24 },
612+
run: "idle",
613+
})
614+
const emitter = new EventEmitter()
615+
let resolved: unknown
616+
let resolveCount = 0
617+
try {
618+
wireGates(emitter, shell)
619+
emitter.emit("operator.gate", {
620+
question: "Proceed?",
621+
options: ["Cancel", "Continue"],
622+
resolve: (result: unknown) => {
623+
resolveCount += 1
624+
resolved = result
625+
},
626+
})
627+
expect(shell.overlayKind).toBe("operator")
628+
629+
closeInsetOverlay(shell)
630+
631+
expect(shell.overlayList).toBeNull()
632+
expect(resolveCount).toBe(1)
633+
expect(resolved).toEqual({ kind: "cancel" })
634+
} finally {
635+
shell.dispose()
636+
}
637+
})
638+
})
639+
})
640+
575641
describe("permission overlay height", () => {
576642
const openGate = (shell: AppShell, scopeCount: number): void => {
577643
const emitter = new EventEmitter()

src/tui-opentui/gate-wire.ts

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,19 @@ export function wireGates(
301301
recordDecision(shell, ev.request, choices, gateSelection)
302302
ev.resolve(approvalOutcomeFromSelection(choices, gateSelection))
303303
},
304+
// Esc must settle the awaited promise (as a deny), not abandon it —
305+
// an unresolved gate hangs the run until the process is killed.
306+
onCancel: () => {
307+
if (settled) return
308+
settled = true
309+
clearTimers()
310+
ev.resolve(
311+
approvalOutcomeFromSelection(choices, {
312+
index: 0,
313+
id: PERMISSION_DENY_ID,
314+
}),
315+
)
316+
},
304317
})
305318
}
306319

@@ -346,11 +359,18 @@ export function wireGates(
346359

347360
function onOperator(ev: OperatorGateEvent): void {
348361
const choices = operatorChoicesFromOptions(ev.options)
362+
// Guarded the same way as the permission gate: correctness must not rest
363+
// on callers of closeInsetOverlay remembering to null the cancel hook
364+
// before dispatching accept — a future accept-via-close path that forgets
365+
// would otherwise double-resolve this promise.
366+
let settled = false
349367
openOrQueue(() => openOperatorOverlay(shell, {
350368
body: ev.question,
351369
choices: choices.items,
352370
itemIds: choices.itemIds,
353371
onAccept: (sel: OverlaySelection) => {
372+
if (settled) return
373+
settled = true
354374
ev.resolve(
355375
operatorResultFromSelection(ev.options, {
356376
index: sel.index,
@@ -360,7 +380,18 @@ export function wireGates(
360380
},
361381
// The ask_operator contract offers a free-form answer, so the overlay
362382
// must be able to send one back rather than only an option index.
363-
onTextAnswer: (text: string) => ev.resolve(operatorCustomResult(text)),
383+
onTextAnswer: (text: string) => {
384+
if (settled) return
385+
settled = true
386+
ev.resolve(operatorCustomResult(text))
387+
},
388+
// Esc must settle the awaited promise (as a cancel), not abandon it —
389+
// an unresolved gate hangs the run until the process is killed.
390+
onCancel: () => {
391+
if (settled) return
392+
settled = true
393+
ev.resolve(operatorCancelResult())
394+
},
364395
}))
365396
}
366397

src/tui-opentui/overlays.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,8 @@ export type OpenPermissionsOpts = {
8989
readonly onAccept?: (selection: OverlaySelection) => void
9090
/** Per-open expand/collapse of collapsed command payloads. */
9191
readonly onToggleExpand?: () => void
92+
/** Per-open Esc/dismiss; host binds resolve(ApprovalOutcome) so Esc denies instead of hanging. */
93+
readonly onCancel?: () => void
9294
}
9395

9496
export function openPermissionsOverlay(
@@ -108,6 +110,7 @@ export function openPermissionsOverlay(
108110
? { onToggleExpand: opts.onToggleExpand }
109111
: {}),
110112
...(opts?.onAccept !== undefined ? { onAccept: opts.onAccept } : {}),
113+
...(opts?.onCancel !== undefined ? { onCancel: opts.onCancel } : {}),
111114
})
112115
}
113116

@@ -120,6 +123,8 @@ export type OpenOperatorOpts = {
120123
readonly onAccept?: (selection: OverlaySelection) => void
121124
/** Per-open free-text answer; host binds the custom OperatorResult. */
122125
readonly onTextAnswer?: (text: string) => void
126+
/** Per-open Esc/dismiss; host binds resolve(cancel) so Esc cancels instead of hanging. */
127+
readonly onCancel?: () => void
123128
}
124129

125130
/**
@@ -128,7 +133,7 @@ export type OpenOperatorOpts = {
128133
* offering "Enter choose" against an empty list.
129134
*/
130135
const NO_WAY_TO_ANSWER =
131-
"No options were offered and this question takes no typed answer. Press Esc to dismiss it."
136+
"No options were offered and this question takes no typed answer. Press Esc to cancel it."
132137

133138
export function openOperatorOverlay(
134139
shell: AppShell,
@@ -150,6 +155,7 @@ export function openOperatorOverlay(
150155
...(opts?.onTextAnswer !== undefined
151156
? { onTextAnswer: opts.onTextAnswer }
152157
: {}),
158+
...(opts?.onCancel !== undefined ? { onCancel: opts.onCancel } : {}),
153159
})
154160
}
155161

src/tui-opentui/shell.ts

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1685,6 +1685,7 @@ type PriorOverlaySnapshot = {
16851685
readonly onAction: ((itemId: string, key: KeyEvent) => boolean) | null
16861686
readonly answer: OverlayAnswerState | null
16871687
readonly titleText: string
1688+
readonly onCancel: (() => void) | null
16881689
}
16891690

16901691
type ShellInternals = {
@@ -1712,6 +1713,12 @@ type ShellInternals = {
17121713
overlayAnswer: OverlayAnswerState | null
17131714
/** Bare title of the open overlay, so its key hints can be re-composed. */
17141715
overlayTitleText: string
1716+
/**
1717+
* Per-open dismiss hook for promise-backed overlays (permissions, operator).
1718+
* Esc/closeInsetOverlay invokes this instead of silently dropping the
1719+
* pending promise the way palette/mentions/copy overlays correctly do.
1720+
*/
1721+
overlayOnCancel: (() => void) | null
17151722
/** Fired once the shell has no overlay open, so queued gates can re-open. */
17161723
overlayClosedListeners: Set<() => void>
17171724
/**
@@ -2715,6 +2722,12 @@ export type OpenListOverlayOpts = {
27152722
* Scoped to this open only, the way `onToggleExpand` and `typeToFilter` are.
27162723
*/
27172724
readonly onCycle?: (itemId: string, direction: -1 | 1) => void
2725+
/**
2726+
* Per-open Esc/dismiss hook for promise-backed overlays (permissions,
2727+
* operator). Invoked by closeInsetOverlay before the accept path is
2728+
* cleared, so the caller's awaited promise resolves instead of hanging.
2729+
*/
2730+
readonly onCancel?: () => void
27182731
/**
27192732
* Description-zone source. Called with the focused item's id on every move
27202733
* (falling back to its label when no `itemIds` were supplied). Returning
@@ -2788,6 +2801,7 @@ export function openListOverlay(
27882801
onAction: bag.overlayOnAction,
27892802
answer: bag.overlayAnswer,
27902803
titleText: bag.overlayTitleText,
2804+
onCancel: bag.overlayOnCancel,
27912805
}
27922806
}
27932807
// Leave prior overlay focus frame; palette will stack above it.
@@ -2816,6 +2830,7 @@ export function openListOverlay(
28162830
bag.overlayOnCycle = opts?.onCycle ?? null
28172831
bag.overlayDescribe = opts?.describe ?? null
28182832
bag.overlayOnAction = opts?.onAction ?? null
2833+
bag.overlayOnCancel = opts?.onCancel ?? null
28192834
} else if (!bag.priorOverlay) {
28202835
// Bare palette (no primary under it): no accept payload.
28212836
bag.overlayItemIds = opts?.itemIds ? [...opts.itemIds] : []
@@ -2825,6 +2840,7 @@ export function openListOverlay(
28252840
bag.overlayOnCycle = opts?.onCycle ?? null
28262841
bag.overlayDescribe = opts?.describe ?? null
28272842
bag.overlayOnAction = opts?.onAction ?? null
2843+
bag.overlayOnCancel = opts?.onCancel ?? null
28282844
}
28292845
if (!isPalette) {
28302846
bag.overlayAnswer =
@@ -3072,6 +3088,10 @@ export function handleOverlayAnswerKey(
30723088
text: `answered: ${text}`,
30733089
meta: "overlay",
30743090
})
3091+
// Deliberate submit, not a dismiss — closeInsetOverlay must not also fire
3092+
// the Esc/cancel path.
3093+
const bag = internals.get(shell)
3094+
if (bag) bag.overlayOnCancel = null
30753095
closeInsetOverlay(shell)
30763096
submit(text)
30773097
return true
@@ -3157,6 +3177,13 @@ export function closeInsetOverlay(shell: AppShell): void {
31573177
}
31583178
const bag = internals.get(shell)
31593179
const prior = wasPalette ? bag?.priorOverlay ?? null : null
3180+
// Permissions/operator overlays back a caller awaiting ev.resolve — Esc must
3181+
// still settle that promise (as a deny/cancel) or the caller hangs forever.
3182+
// Palette/mentions/copy have no such awaited caller, so they drop silently.
3183+
const cancelable =
3184+
!prior &&
3185+
(shell.overlayKind === "permissions" || shell.overlayKind === "operator")
3186+
const onCancel = cancelable ? bag?.overlayOnCancel ?? null : null
31603187

31613188
shell.overlayList = null
31623189
shell.overlayKind = null
@@ -3165,7 +3192,8 @@ export function closeInsetOverlay(shell: AppShell): void {
31653192
shell.paletteCommands = []
31663193
shell.copyTargets = null
31673194
clearOverlayBody(shell)
3168-
// Esc / dismiss: drop accept path without invoking callbacks.
3195+
// Esc / dismiss: drop accept path without invoking it (onCancel above is
3196+
// captured before this clears, and is invoked separately once state settles).
31693197
if (bag && !prior) {
31703198
bag.overlayItemIds = []
31713199
bag.overlayOnAccept = null
@@ -3174,6 +3202,7 @@ export function closeInsetOverlay(shell: AppShell): void {
31743202
bag.overlayDescribe = null
31753203
bag.overlayOnAction = null
31763204
bag.overlayAnswer = null
3205+
bag.overlayOnCancel = null
31773206
}
31783207

31793208
// Pop exactly one frame (palette or overlay).
@@ -3202,6 +3231,7 @@ export function closeInsetOverlay(shell: AppShell): void {
32023231
bag.overlayOnAction = prior.onAction
32033232
bag.overlayAnswer = prior.answer
32043233
bag.overlayTitleText = prior.titleText
3234+
bag.overlayOnCancel = prior.onCancel
32053235
// If focus was not stacked (edge case), re-open overlay frame.
32063236
if (focusOwner(shell.focus) !== "overlay") {
32073237
shell.focus = openOverlay(shell.focus, OVERLAY_FRAME_ID, {
@@ -3230,6 +3260,7 @@ export function closeInsetOverlay(shell: AppShell): void {
32303260
relayout(shell, { overlayMode: "closed" })
32313261
applyFocus(shell)
32323262
notifyOverlayClosed(shell)
3263+
onCancel?.()
32333264
}
32343265

32353266
/**
@@ -3436,6 +3467,9 @@ export function acceptOverlaySelection(shell: AppShell): void {
34363467
}
34373468
// Capture before close clears per-open state.
34383469
const perOpen = bag?.overlayOnAccept ?? null
3470+
// This is a deliberate accept, not a dismiss — closeInsetOverlay must not
3471+
// also fire the Esc/cancel path below.
3472+
if (bag) bag.overlayOnCancel = null
34393473

34403474
if (bag?.overlayEchoChoice !== false) {
34413475
appendStreamRow(shell, {
@@ -5023,6 +5057,7 @@ export function createAppShell(
50235057
overlayOnAction: null,
50245058
overlayAnswer: null,
50255059
overlayTitleText: "",
5060+
overlayOnCancel: null,
50265061
overlayClosedListeners: new Set(),
50275062
paletteCatalog: paletteCatalogOpt,
50285063
paletteFilter: null,

0 commit comments

Comments
 (0)