From e3cbea0096a029cad26b87030e0c202409a75546 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=98=93=E8=89=AF?= <1204183885@qq.com> Date: Sat, 7 Nov 2026 10:49:33 +0800 Subject: [PATCH] fix(cli): surface workflow agent approvals in the interactive TUI (#16425) (#16432) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(cli): surface workflow agent approvals in the interactive TUI (#16425) A backgrounded workflow agent that hit a tool-approval gate deadlocked in the interactive TUI: the registry parked the approval, the footer showed a "needs approval" badge, but there was no way to answer it, so the run never made progress. Two independent defects caused it. 1. `WorkflowRunRegistry.parkApproval` treated the display-only `approvalChangeCallback` as a sufficient host channel. A host that could only repaint the badge therefore accepted the approval and then had nothing to answer it with. It now fails closed unless a real `approvalRequestCallback` responder is registered, so a badge-only host rejects the park and the caller sees a cancelled request instead of a silent forever-wait. 2. `useBackgroundTaskView` never registered a responder at all, so even a host that wanted to answer had no channel. It now registers its snapshot refresh as the approval-request responder (and clears it on unmount), which makes the parked approval part of `entries` — the surface the background-tasks dialog renders and settles through `resolvePendingApproval`. `BackgroundTaskViewProvider` also promotes a parked workflow approval to the detail view once per `approvalId`, because the approval can only be answered from there; keying on the id keeps a user's "close" from bouncing straight back into the dialog on the next refresh. Scope: interactive TUI workflow approval path only. Headless and ACP hosts are untouched. Co-authored-by: Qwen-Coder Patrol-Run: qwen-issue-patrol/jmw4ig8e5z4 * fix(core): drop the redundant responder guard after the park gate TS narrows approvalRequestCallback to non-undefined once the park gate requires it, so the extra truthiness check is dead and fails tsc (TS2774). Co-authored-by: Qwen-Coder Patrol-Run: qwen-issue-patrol/jmw4ig8e5z4 * fix(cli): type the workflow approval fixture and format the new tests Co-authored-by: Qwen-Coder Patrol-Run: qwen-issue-patrol/jmw4ig8e5z4 * test(core): register a real approval responder in the orchestrator bridge test The narrowed park gate rejects an approval when the registry has no approvalRequestCallback, so a badge-only host (change callback only) leaves pendingApprovals empty and the bridge test times out. Register the responder the production host also provides, matching the gate's contract. Co-authored-by: Qwen-Coder Patrol-Run: qwen-pr-closeout/jmw4lb42xgf * fix(cli): surface each parked approval once, in list mode (#16432 R1-1, R1-2) The single-id latch keyed on the first pending entry's head approval, so a second approval parked behind a still-parked head never surfaced. Latch a set of approvalIds and select the entry holding an unsurfaced one. Auto-open also landed in detail mode, which arms the tool confirmation with focus while unmounting the composer: a stray Enter aimed at the prompt approved the background command. Auto-open now lands in list mode with the row selected; entering detail stays an explicit navigation. Tests: flip the surfacing contract to list mode, add the masked-approval regression (red on the single-id latch), drop a dead assignment (R1-8). * fix(cli): disarm auto-opened task dialog and latch only the displayed approval head An auto-opened background-task dialog unmounts the composer and keypress dispatch is a broadcast, so a letter still aimed at the composer could cancel (x) or pause (p) the selected workflow without confirmation, and a stray Enter could arm the approval. Tag auto-opens and disarm x/p/Enter in the dialog's own list branch until the first explicit navigation key acknowledges the open; user-initiated opens keep immediate behavior. The surfacing latch recorded the first unsurfaced approval, but the detail view renders only pendingApprovals[0]; an approval parked behind a still- parked head was marked surfaced without ever displaying and never prompted once it became head. Latch only the displayable queue head so the next approval prompts after the head drains. Addresses the two Criticals from the #16432 human review. * test(cli): pin the closed-dialog guard on the approval auto-surface effect The surfacing effect's dialogMode guard had no test that failed without it: every existing case fired refreshes only while the dialog was closed. Add a case that opens the detail view, parks a workflow approval mid-read, and asserts the view and selection stay put. Deleting the guard turns exactly this case red (#16432 R2-3). * fix(cli): hold the auto-open disarm until a real navigation key - Acknowledge the auto-open only on arrow keys. The selection matchers also carry bare k/j and Ctrl+P/Ctrl+N, and ordinary typed letters satisfy them, so prose typed at the composer the dialog just unmounted released the disarm and the next x/p/Enter reached the run (#16432 R2-1). - Swallow closeDialog while the auto-open is unacknowledged: Escape, Left and Ctrl+C burned the add-only latch before the prompt was ever seen and the workflow fell back to the badge-only hang this PR removes. Ctrl+C reaches the same path through closeAnyOpenDialog (#16432 R3-1). - Unfocus the pill in the auto-open writer, matching openDialog, so the pill's Enter cannot run openDialog() and clear the disarm (#16432 R3-2). - Advertise only "arrows activate" while disarmed instead of Enter view / x stop, which were swallowed (#16432 R3-3). - Record the parked head ids at close so an acknowledged close stays closed while another workflow is already parked, and a genuinely new approval still surfaces (#16432 R3-5). Co-authored-by: Qwen-Coder Patrol-Run: qwen-pr-closeout/jmw5b11gghf * style(cli): wrap the over-width hint condition to the prettier limit The R3 hint-row re-indent left the paused-agent condition one column over the 80-char limit, failing the Lint & Static gate at ac4c4d7a97. Formatting only; no behavior change. Refs #16432 * fix(cli): swallow bare r while the auto-open disarm is armed An auto-open lands mid-keystroke while the user is still typing into the composer the dialog just unmounted. Bare k/j still move the selection by design (R2-1), so prose can park the selection on a paused agent; the following `r` of the same sentence then reached resumeSelected() and restarted a run the user deliberately paused. Add `r` to the same deny-list that already swallows `return`/`x`/`p` until a real arrow key acknowledges the auto-open (#16432 R4-2). Co-authored-by: Qwen-Coder Patrol-Run: qwen-pr-closeout/jmw5jlokths --------- Co-authored-by: yiliang114 Co-authored-by: Qwen-Coder --- .../cli/src/ui/components/Footer.test.tsx | 1 + .../BackgroundTasksDialog.test.tsx | 187 +++++++++ .../background-view/BackgroundTasksDialog.tsx | 56 ++- .../background-view/LiveAgentPanel.test.tsx | 1 + .../BackgroundTaskViewContext.test.tsx | 393 ++++++++++++++++++ .../ui/contexts/BackgroundTaskViewContext.tsx | 84 ++++ .../ui/hooks/useBackgroundTaskView.test.ts | 83 +++- .../cli/src/ui/hooks/useBackgroundTaskView.ts | 9 + .../runtime/workflow-orchestrator.test.ts | 1 + .../src/agents/workflow-run-registry.test.ts | 23 +- .../core/src/agents/workflow-run-registry.ts | 17 +- 11 files changed, 833 insertions(+), 22 deletions(-) create mode 100644 packages/cli/src/ui/contexts/BackgroundTaskViewContext.test.tsx diff --git a/packages/cli/src/ui/components/Footer.test.tsx b/packages/cli/src/ui/components/Footer.test.tsx index 34928b4a8d5..e79cf3f20c7 100644 --- a/packages/cli/src/ui/components/Footer.test.tsx +++ b/packages/cli/src/ui/components/Footer.test.tsx @@ -189,6 +189,7 @@ const createBackgroundTaskState = ( selectedIndex: 0, dialogMode: 'closed', dialogOpen: false, + autoOpened: false, pillFocused: false, livePanelFocused: false, livePanelSelectedIndex: 0, diff --git a/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.test.tsx b/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.test.tsx index a8f89e665ae..153ad625e3f 100644 --- a/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.test.tsx +++ b/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.test.tsx @@ -1773,6 +1773,193 @@ describe('BackgroundTasksDialog', () => { ); }); + describe('auto-opened dialog disarm (#16432 Critical 1)', () => { + // The auto-open lands mid-keystroke and keypress dispatch is a + // broadcast: a letter still aimed at the unmounted composer must not + // cancel/pause the run, and a stray Enter must not arm the approval. + // The first explicit navigation key acknowledges the auto-open. + function parkedApproval(approvalId: string): WorkflowApproval { + return { + approvalId, + subagentId: 'sub-1', + callId: `call-${approvalId}`, + name: 'Shell', + description: 'run', + confirmationDetails: { + type: 'exec', + } as WorkflowApproval['confirmationDetails'], + at: Date.now(), + }; + } + + async function autoSurfacedWorkflow() { + const wf = workflowEntry({ + status: 'running', + pendingApprovals: [parkedApproval('wfap-1')], + }); + const h = setup([wf]); + // ink flushes mount effects on its own scheduler, outside act — + // wait for the provider's auto-open instead of asserting on the + // pre-effect frame. + await vi.waitFor(() => { + expect(h.probe.current!.state.dialogMode).toBe('list'); + }); + // The provider auto-opens in list mode — no explicit openDialog. + expect(h.probe.current!.state.autoOpened).toBe(true); + return h; + } + + it('ignores bare x until a navigation key, then cancels', async () => { + const h = await autoSurfacedWorkflow(); + + h.pressKey({ sequence: 'x' }); + expect(h.workflowCancel).not.toHaveBeenCalled(); + + h.pressKey({ name: 'up' }); + // The acknowledge re-registers the keypress closure in a passive + // effect; wait for it to flush so the next key sees autoOpened=false. + await vi.waitFor(() => { + expect(h.probe.current!.state.autoOpened).toBe(false); + }); + h.pressKey({ sequence: 'x' }); + expect(h.workflowCancel).toHaveBeenCalledWith( + 'wf_test1234', + expect.any(Number), + ); + }); + + it('ignores bare p until a navigation key, then pauses', async () => { + const h = await autoSurfacedWorkflow(); + + h.pressKey({ sequence: 'p' }); + expect(h.workflowPause).not.toHaveBeenCalled(); + + h.pressKey({ name: 'up' }); + await vi.waitFor(() => { + expect(h.probe.current!.state.autoOpened).toBe(false); + }); + h.pressKey({ sequence: 'p' }); + expect(h.workflowPause).toHaveBeenCalledWith('wf_test1234'); + }); + + it('ignores bare r until a navigation key, then resumes the selection', async () => { + // R4-2: bare k/j still move the selection (the R2-1 rule above), so + // prose typed at the unmounted composer can park the selection on a + // paused run. `r` was outside the deny-list, so the next `r` of the + // same sentence resumed a run the user deliberately paused. + const paused = entry({ id: 'a', agentId: 'a', status: 'paused' }); + const wf = workflowEntry({ + status: 'running', + pendingApprovals: [parkedApproval('wfap-1')], + }); + const h = setup([paused, wf]); + await vi.waitFor(() => { + expect(h.probe.current!.state.dialogMode).toBe('list'); + }); + expect(h.probe.current!.state.autoOpened).toBe(true); + + h.pressKey({ name: 'k' }); + expect(h.probe.current!.state.selectedIndex).toBe(0); + h.pressKey({ sequence: 'r' }); + expect(h.resume).not.toHaveBeenCalled(); + expect(h.probe.current!.state.autoOpened).toBe(true); + + h.pressKey({ name: 'up' }); + await vi.waitFor(() => { + expect(h.probe.current!.state.autoOpened).toBe(false); + }); + h.pressKey({ sequence: 'r' }); + expect(h.resume).toHaveBeenCalledWith('a'); + }); + + it('ignores Enter until a navigation key, then enters detail', async () => { + const h = await autoSurfacedWorkflow(); + + h.pressKey({ name: 'return' }); + expect(h.probe.current!.state.dialogMode).toBe('list'); + + h.pressKey({ name: 'up' }); + await vi.waitFor(() => { + expect(h.probe.current!.state.autoOpened).toBe(false); + }); + h.pressKey({ name: 'return' }); + expect(h.probe.current!.state.dialogMode).toBe('detail'); + }); + + it('keeps immediate x behavior for a user-initiated open', async () => { + const h = await autoSurfacedWorkflow(); + // Close the auto-open, then reopen deliberately: the disarm tag + // must not survive into a user-initiated open. + h.call(() => h.probe.current!.actions.acknowledgeAutoOpen()); + h.call(() => h.probe.current!.actions.closeDialog()); + h.call(() => h.probe.current!.actions.openDialog()); + expect(h.probe.current!.state.autoOpened).toBe(false); + + h.pressKey({ sequence: 'x' }); + expect(h.workflowCancel).toHaveBeenCalledWith( + 'wf_test1234', + expect.any(Number), + ); + }); + + it('does not treat a typed k or j as the acknowledgment', async () => { + // The selection matchers also carry bare k/j, and prose typed at the + // composer the dialog just unmounted satisfies them. Acknowledging on + // those let the next `x`/`p` of the same sentence reach the run + // (#16432 R2-1). + const h = await autoSurfacedWorkflow(); + + h.pressKey({ sequence: 'k' }); + h.pressKey({ sequence: 'j' }); + expect(h.probe.current!.state.autoOpened).toBe(true); + + h.pressKey({ sequence: 'x' }); + expect(h.workflowCancel).not.toHaveBeenCalled(); + + h.pressKey({ name: 'up' }); + await vi.waitFor(() => { + expect(h.probe.current!.state.autoOpened).toBe(false); + }); + h.pressKey({ sequence: 'x' }); + expect(h.workflowCancel).toHaveBeenCalledWith( + 'wf_test1234', + expect.any(Number), + ); + }); + + it('swallows Escape until the auto-open is acknowledged', async () => { + // A dismissal before the acknowledgment would burn the approval latch + // with the prompt never seen — the badge-only hang this PR removes + // (#16432 R3-1). + const h = await autoSurfacedWorkflow(); + + h.pressKey({ name: 'escape' }); + expect(h.probe.current!.state.dialogMode).toBe('list'); + expect(h.probe.current!.state.autoOpened).toBe(true); + + h.pressKey({ name: 'down' }); + await vi.waitFor(() => { + expect(h.probe.current!.state.autoOpened).toBe(false); + }); + h.pressKey({ name: 'escape' }); + expect(h.probe.current!.state.dialogMode).toBe('closed'); + }); + + it('advertises only the activation hint while auto-opened', async () => { + const h = await autoSurfacedWorkflow(); + + expect(h.lastFrame()).not.toContain('Enter view'); + expect(h.lastFrame()).not.toContain('x stop'); + expect(h.lastFrame()).toContain('activate'); + + h.pressKey({ name: 'up' }); + await vi.waitFor(() => { + expect(h.probe.current!.state.autoOpened).toBe(false); + }); + expect(h.lastFrame()).toContain('Enter view'); + }); + }); + describe('nested sub-agent display', () => { // Entries are passed pre-grouped (parent before child) — in production // useBackgroundTaskView applies reorderChildrenUnderParents before the diff --git a/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.tsx b/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.tsx index 1e0fc7d9227..e764acbccfd 100644 --- a/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.tsx +++ b/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.tsx @@ -1405,7 +1405,7 @@ export const BackgroundTasksDialog: React.FC = ({ availableTerminalHeight, terminalWidth, }) => { - const { entries, selectedIndex, dialogOpen, dialogMode } = + const { entries, selectedIndex, dialogOpen, dialogMode, autoOpened } = useBackgroundTaskViewState(); const isDetailMode = dialogMode === 'detail' || dialogMode === 'detail-from-panel'; @@ -1415,6 +1415,7 @@ export const BackgroundTasksDialog: React.FC = ({ closeDialog, enterDetail, exitDetail, + acknowledgeAutoOpen, cancelSelected, resumeSelected, toggleSelectedWorkflowPause, @@ -1777,6 +1778,15 @@ export const BackgroundTasksDialog: React.FC = ({ } if (dialogMode === 'list') { + // Only keys prose cannot produce acknowledge the auto-open. The + // selection matchers also carry bare `k`/`j` and Ctrl+P/Ctrl+N, and + // ordinary typed letters satisfy them — acknowledging on those let + // the next letter of the same sentence cancel or pause the run + // (#16432 R2-1). Bare k/j still move the selection; the disarm + // simply stays armed until a real arrow key arrives. + if (key.name === 'up' || key.name === 'down') { + acknowledgeAutoOpen(); + } if (keyMatchers[Command.SELECTION_UP](key)) { moveSelectionUp(); setPendingCancelEntryId(null); @@ -1787,6 +1797,22 @@ export const BackgroundTasksDialog: React.FC = ({ setPendingCancelEntryId(null); return; } + // An auto-open lands mid-keystroke and keypress dispatch is a + // broadcast, so a letter still aimed at the unmounted composer + // would cancel or pause the run, and Enter would arm the approval. + // Disarm those keys until a navigation key (above) acknowledges + // the open; user-initiated opens never carry the tag (#16432). + if ( + autoOpened && + (key.name === 'return' || + ((key.sequence === 'x' || + key.sequence === 'p' || + key.sequence === 'r') && + !key.ctrl && + !key.meta)) + ) { + return; + } if (key.name === 'return') { if (selectedEntry) enterDetail(); return; @@ -1914,16 +1940,26 @@ export const BackgroundTasksDialog: React.FC = ({ 'Esc cancel', ); } else if (dialogMode === 'list') { - hints.push('\u2191/\u2193 select', 'Enter view'); - if (selectedEntry && isStoppableEntry(selectedEntry)) { - hints.push('x stop'); + if (autoOpened) { + // Every destructive list key is swallowed until a navigation key + // acknowledges the auto-open, so advertising `Enter view` / `x stop` + // here would document keys that do nothing (#16432 R3-3). + hints.push('\u2191/\u2193 activate'); + } else { + hints.push('\u2191/\u2193 select', 'Enter view'); + if (selectedEntry && isStoppableEntry(selectedEntry)) { + hints.push('x stop'); + } + if (selectedWorkflowPauseHint) hints.push(selectedWorkflowPauseHint); + if (selectedEntryAllowsResume) hints.push('r resume'); + if ( + selectedEntry?.kind === 'agent' && + selectedEntry.status === 'paused' + ) { + hints.push('x abandon'); + } + hints.push('\u2190/Esc close'); } - if (selectedWorkflowPauseHint) hints.push(selectedWorkflowPauseHint); - if (selectedEntryAllowsResume) hints.push('r resume'); - if (selectedEntry?.kind === 'agent' && selectedEntry.status === 'paused') { - hints.push('x abandon'); - } - hints.push('\u2190/Esc close'); } else { hints.push('\u2190 back', 'Esc close'); if (selectedEntry && isStoppableEntry(selectedEntry)) { diff --git a/packages/cli/src/ui/components/background-view/LiveAgentPanel.test.tsx b/packages/cli/src/ui/components/background-view/LiveAgentPanel.test.tsx index 017d826c64b..a1c3b58032f 100644 --- a/packages/cli/src/ui/components/background-view/LiveAgentPanel.test.tsx +++ b/packages/cli/src/ui/components/background-view/LiveAgentPanel.test.tsx @@ -69,6 +69,7 @@ function renderPanel( selectedIndex: 0, dialogMode: options.dialogOpen ? ('list' as const) : ('closed' as const), dialogOpen: Boolean(options.dialogOpen), + autoOpened: false, pillFocused: false, livePanelFocused: Boolean(options.livePanelFocused), livePanelSelectedIndex: 0, diff --git a/packages/cli/src/ui/contexts/BackgroundTaskViewContext.test.tsx b/packages/cli/src/ui/contexts/BackgroundTaskViewContext.test.tsx new file mode 100644 index 00000000000..c4c8c26cb02 --- /dev/null +++ b/packages/cli/src/ui/contexts/BackgroundTaskViewContext.test.tsx @@ -0,0 +1,393 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ +// @vitest-environment jsdom + +import { describe, it, expect, vi } from 'vitest'; +import { renderHook, act } from '@testing-library/react'; +import type { Config, WorkflowTask } from '@qwen-code/qwen-code-core'; +import { + BackgroundTaskViewActionsContext, + BackgroundTaskViewProvider, + BackgroundTaskViewStateContext, + useBackgroundTaskViewActions, + useBackgroundTaskViewState, +} from './BackgroundTaskViewContext.js'; + +const workflowEntry = (pendingApprovalIds: string[] = []): WorkflowTask => + ({ + id: 'wf-1', + kind: 'workflow', + runId: 'wf-1', + description: 'demo', + meta: null, + status: 'running', + startTime: 100, + pendingApprovals: pendingApprovalIds.map((approvalId) => ({ + approvalId, + subagentId: 'sub-1', + callId: `call-${approvalId}`, + name: 'Shell', + description: 'run', + confirmationDetails: { type: 'exec' }, + at: 1_700_000_000_000, + })), + }) as unknown as WorkflowTask; + +const agentTask = () => + ({ + id: 'a1', + kind: 'agent', + agentId: 'a1', + description: 'desc', + isBackgrounded: true, + status: 'running', + startTime: 200, + abortController: new AbortController(), + outputFile: '/tmp/a.jsonl', + outputOffset: 0, + notified: false, + }) as unknown as Record; + +function makeConfig(workflows: () => WorkflowTask[]) { + let approvalRequestCb: (() => void) | undefined; + let approvalChangeCb: (() => void) | undefined; + const workflowReg = { + setStatusChangeCallback: vi.fn(), + setApprovalChangeCallback: vi.fn((next: (() => void) | undefined) => { + approvalChangeCb = next; + }), + setApprovalRequestCallback: vi.fn((next: (() => void) | undefined) => { + approvalRequestCb = next; + }), + list: workflows, + }; + const emptyReg = () => ({ + setStatusChangeCallback: vi.fn(), + setApprovalChangeCallback: vi.fn(), + getAll: () => [], + }); + const config = { + getBackgroundTaskRegistry: () => ({ + ...emptyReg(), + getAll: () => [agentTask()], + }), + getBackgroundShellRegistry: emptyReg, + getMonitorRegistry: emptyReg, + getWorkflowRunRegistry: () => workflowReg, + getMemoryManager: () => ({ + subscribe: vi.fn(() => vi.fn()), + listTasksByType: () => [], + }), + getProjectRoot: () => '/test/project', + } as unknown as Config; + return { + config, + fireApprovalRequest: () => approvalRequestCb?.(), + fireApprovalChange: () => approvalChangeCb?.(), + }; +} + +describe('BackgroundTaskViewProvider approval surfacing', () => { + it('surfaces a parked workflow approval in list mode exactly once', () => { + // A workflow approval can only be answered from the detail view; a + // footer badge the user has to hunt for leaves the run blocked + // forever. The provider must promote the parked approval to a + // selected row — and must not re-open the dialog every time the + // entry refreshes after the user closed it. Auto-open stays in list + // mode: detail would arm the tool confirmation, and a stray Enter + // aimed at the unmounted composer would approve the command + // (#16432 R1-2). + let workflows: WorkflowTask[] = [workflowEntry()]; + const { config, fireApprovalRequest, fireApprovalChange } = makeConfig( + () => workflows, + ); + const wrapper = ({ children }: { children: React.ReactNode }) => ( + + {children} + + ); + const { result } = renderHook( + () => ({ + state: useBackgroundTaskViewState(), + actions: useBackgroundTaskViewActions(), + }), + { wrapper }, + ); + + expect(result.current.state.dialogMode).toBe('closed'); + // Agent (startTime 200) sorts above the workflow (100). + expect(result.current.state.entries.map((e) => e.kind)).toEqual([ + 'agent', + 'workflow', + ]); + + workflows = [workflowEntry(['wfap-1'])]; + act(() => fireApprovalRequest()); + + expect(result.current.state.dialogMode).toBe('list'); + expect(result.current.state.selectedIndex).toBe(1); + expect(result.current.state.dialogOpen).toBe(true); + + // Closing must stick: a later refresh that still carries the same + // parked approval cannot bounce the user back into the dialog. + act(() => result.current.actions.acknowledgeAutoOpen()); + act(() => result.current.actions.closeDialog()); + expect(result.current.state.dialogMode).toBe('closed'); + act(() => fireApprovalChange()); + expect(result.current.state.dialogMode).toBe('closed'); + + // A genuinely new approval is a new prompt and surfaces again. + act(() => { + workflows = [ + { + ...workflowEntry(['wfap-2']), + } as unknown as WorkflowTask, + ]; + fireApprovalRequest(); + }); + expect(result.current.state.dialogMode).toBe('list'); + }); + + it('surfaces a later approval parked behind an already-surfaced one', () => { + // R1-1: a single-id latch keyed on the first pending entry's head + // masked every later approval while that head stayed parked. Each + // approvalId must get its own prompt, on its own entry's row. + const newer = { + ...workflowEntry(['wfap-new']), + id: 'wf-new', + runId: 'wf-new', + startTime: 300, + } as unknown as WorkflowTask; + const older = { + ...workflowEntry(), + id: 'wf-old', + runId: 'wf-old', + startTime: 50, + } as unknown as WorkflowTask; + let workflows: WorkflowTask[] = [newer, older]; + const { config, fireApprovalRequest } = makeConfig(() => workflows); + const wrapper = ({ children }: { children: React.ReactNode }) => ( + + {children} + + ); + const { result } = renderHook( + () => ({ + state: useBackgroundTaskViewState(), + actions: useBackgroundTaskViewActions(), + }), + { wrapper }, + ); + + // Newest-first: newer workflow (300), agent (200), older workflow (50). + expect(result.current.state.dialogMode).toBe('list'); + expect(result.current.state.selectedIndex).toBe(0); + + // Back out without answering; wfap-new stays parked. + act(() => result.current.actions.acknowledgeAutoOpen()); + act(() => result.current.actions.closeDialog()); + expect(result.current.state.dialogMode).toBe('closed'); + + // The older run parks its own approval: it must surface on its row, + // not stay hidden behind the latched head of the newer entry. + act(() => { + workflows = [ + newer, + { + ...workflowEntry(['wfap-old']), + id: 'wf-old', + runId: 'wf-old', + startTime: 50, + } as unknown as WorkflowTask, + ]; + fireApprovalRequest(); + }); + expect(result.current.state.dialogMode).toBe('list'); + expect(result.current.state.selectedIndex).toBe(2); + }); + + it('latches only the displayable queue head, not approvals parked behind it', () => { + // The detail view renders pendingApprovals[0] only. Latching a later + // approval marks it surfaced without ever displaying it: once the + // head drains unanswered, the "already surfaced" tail never + // auto-prompts — the exact hang this surfacing exists to remove. + let workflows: WorkflowTask[] = [workflowEntry(['wfap-a'])]; + const { config, fireApprovalRequest, fireApprovalChange } = makeConfig( + () => workflows, + ); + const wrapper = ({ children }: { children: React.ReactNode }) => ( + + {children} + + ); + const { result } = renderHook( + () => ({ + state: useBackgroundTaskViewState(), + actions: useBackgroundTaskViewActions(), + }), + { wrapper }, + ); + + // a parks → auto-open; the user backs out without answering. + expect(result.current.state.dialogMode).toBe('list'); + act(() => result.current.actions.acknowledgeAutoOpen()); + act(() => result.current.actions.closeDialog()); + expect(result.current.state.dialogMode).toBe('closed'); + + // b parks behind the still-parked head a: nothing new is displayable, + // so the dialog must stay closed and b must not be latched. + act(() => { + workflows = [workflowEntry(['wfap-a', 'wfap-b'])]; + fireApprovalRequest(); + }); + expect(result.current.state.dialogMode).toBe('closed'); + + // a leaves the queue unanswered; b becomes the displayable head and + // must get its own auto-prompt. + act(() => { + workflows = [workflowEntry(['wfap-b'])]; + fireApprovalChange(); + }); + expect(result.current.state.dialogMode).toBe('list'); + expect(result.current.state.selectedIndex).toBe(1); + }); + + it('unfocuses the pill when an approval auto-opens the dialog', () => { + // The pill subscribes on `pillFocused` alone: leaving it focused let its + // Enter run openDialog() and clear the disarm the auto-open installed + // (#16432 R3-2). + let workflows: WorkflowTask[] = []; + const { config, fireApprovalRequest } = makeConfig(() => workflows); + const wrapper = ({ children }: { children: React.ReactNode }) => ( + + {children} + + ); + const { result } = renderHook( + () => ({ + state: useBackgroundTaskViewState(), + actions: useBackgroundTaskViewActions(), + }), + { wrapper }, + ); + + act(() => result.current.actions.setPillFocused(true)); + act(() => { + workflows = [workflowEntry(['wfap-pill'])]; + fireApprovalRequest(); + }); + + expect(result.current.state.dialogMode).toBe('list'); + expect(result.current.state.autoOpened).toBe(true); + expect(result.current.state.pillFocused).toBe(false); + }); + + it('stays closed after an acknowledged close while another workflow is parked', () => { + // `dialogMode` is an effect dependency, so the close itself re-ran the + // surfacing effect and re-opened on the next parked head in the same + // frame — with two parked workflows Escape looked broken (#16432 R3-5). + let workflows: WorkflowTask[] = []; + const { config, fireApprovalRequest, fireApprovalChange } = makeConfig( + () => workflows, + ); + const wrapper = ({ children }: { children: React.ReactNode }) => ( + + {children} + + ); + const { result } = renderHook( + () => ({ + state: useBackgroundTaskViewState(), + actions: useBackgroundTaskViewActions(), + }), + { wrapper }, + ); + + act(() => { + workflows = [ + { + ...workflowEntry(['wfap-a']), + id: 'wf-a', + runId: 'wf-a', + startTime: 300, + } as unknown as WorkflowTask, + { + ...workflowEntry(['wfap-b']), + id: 'wf-b', + runId: 'wf-b', + startTime: 200, + } as unknown as WorkflowTask, + ]; + fireApprovalRequest(); + }); + expect(result.current.state.dialogMode).toBe('list'); + + act(() => result.current.actions.acknowledgeAutoOpen()); + act(() => result.current.actions.closeDialog()); + expect(result.current.state.dialogMode).toBe('closed'); + + // A refresh that still carries the same two parked heads must not + // bounce the user back into the dialog. + act(() => fireApprovalChange()); + expect(result.current.state.dialogMode).toBe('closed'); + }); + + it('leaves an open detail view alone when a new approval parks', () => { + // R2-3: pin the `dialogMode !== 'closed'` guard. Without it, any + // registry refresh while the user is reading the dialog yanks them + // back to the list and moves the selection out from under their next + // keypress. Deleting the guard must turn this case red. + let workflows: WorkflowTask[] = []; + const { config, fireApprovalRequest } = makeConfig(() => workflows); + const wrapper = ({ children }: { children: React.ReactNode }) => ( + + {children} + + ); + const { result } = renderHook( + () => ({ + state: useBackgroundTaskViewState(), + actions: useBackgroundTaskViewActions(), + }), + { wrapper }, + ); + + // The user opens the dialog themselves and reads the agent's detail. + act(() => result.current.actions.openDialog()); + act(() => result.current.actions.enterDetail()); + expect(result.current.state.dialogMode).toBe('detail'); + expect(result.current.state.selectedIndex).toBe(0); + + // A workflow parks an approval mid-read: the refresh must not steal + // the view or the selection. + act(() => { + workflows = [workflowEntry(['wfap-open'])]; + fireApprovalRequest(); + }); + expect(result.current.state.dialogMode).toBe('detail'); + expect(result.current.state.selectedIndex).toBe(0); + }); + + it('exposes the state and actions contexts to consumers', () => { + const { config } = makeConfig(() => []); + const wrapper = ({ children }: { children: React.ReactNode }) => ( + + {children} + + ); + const { result } = renderHook( + () => ({ + state: useBackgroundTaskViewState(), + actions: useBackgroundTaskViewActions(), + }), + { wrapper }, + ); + expect(result.current.state.entries.map((e) => e.kind)).toEqual(['agent']); + expect(result.current.actions.openDialog).toBeTypeOf('function'); + expect(BackgroundTaskViewStateContext).toBeDefined(); + expect(BackgroundTaskViewActionsContext).toBeDefined(); + }); +}); diff --git a/packages/cli/src/ui/contexts/BackgroundTaskViewContext.tsx b/packages/cli/src/ui/contexts/BackgroundTaskViewContext.tsx index 355da31c6e5..af694374c89 100644 --- a/packages/cli/src/ui/contexts/BackgroundTaskViewContext.tsx +++ b/packages/cli/src/ui/contexts/BackgroundTaskViewContext.tsx @@ -17,6 +17,7 @@ import { useCallback, useEffect, useMemo, + useRef, useState, } from 'react'; import type { Config } from '@qwen-code/qwen-code-core/config/config.js'; @@ -49,6 +50,13 @@ export interface BackgroundTaskViewState { dialogMode: BackgroundDialogMode; /** Convenience boolean: `dialogMode !== 'closed'`. */ dialogOpen: boolean; + /** + * True when the approval auto-surface effect opened the dialog and the + * user has not navigated explicitly yet. Keypress dispatch is a + * broadcast, so the dialog disarms its destructive list keys until a + * navigation key acknowledges the auto-open (#16432). + */ + autoOpened: boolean; /** * True when the footer pill owns keyboard focus (highlighted, awaiting * Enter to open the dialog). Mirrors the Arena tab-bar focus pattern. @@ -80,6 +88,8 @@ export interface BackgroundTaskViewActions { */ toggleSelectedWorkflowPause(): boolean | null; enterDetailFromPanel(): void; + /** Clear the auto-open disarm after the first explicit navigation. */ + acknowledgeAutoOpen(): void; setPillFocused(focused: boolean): void; setLivePanelFocused(focused: boolean): void; setLivePanelSelectedIndex(index: number): void; @@ -101,6 +111,7 @@ const DEFAULT_STATE: BackgroundTaskViewState = { selectedIndex: 0, dialogMode: 'closed', dialogOpen: false, + autoOpened: false, pillFocused: false, livePanelFocused: false, livePanelSelectedIndex: 0, @@ -117,6 +128,7 @@ const DEFAULT_ACTIONS: BackgroundTaskViewActions = { enterDetail: noop, exitDetail: noop, enterDetailFromPanel: noop, + acknowledgeAutoOpen: noop, cancelSelected: noop, resumeSelected: async () => {}, toggleSelectedWorkflowPause: () => null, @@ -151,6 +163,7 @@ export function BackgroundTaskViewProvider({ const [rawSelectedIndex, setRawSelectedIndex] = useState(0); const [dialogMode, setDialogMode] = useState('closed'); + const [autoOpened, setAutoOpened] = useState(false); const [pillFocused, setPillFocused] = useState(false); const [livePanelFocused, setLivePanelFocusedRaw] = useState(false); const [livePanelSelectedIndex, setLivePanelSelectedIndex] = useState(0); @@ -174,6 +187,60 @@ export function BackgroundTaskViewProvider({ if (livePanelFocused && !hasAgentEntries) setLivePanelFocusedRaw(false); }, [livePanelFocused, hasAgentEntries]); + // A workflow approval can only be answered from the detail view, and the + // registry rejects the call outright when no host answers it. Surface a + // parked approval once per approvalId so a blocked workflow shows up as a + // selected row instead of only a footer badge. Only the queue head is + // latchable: it is the one approval the detail view renders, so latching + // an approval parked behind it would mark it surfaced without ever + // displaying it, and it would never prompt once it becomes head (#16432). + // The latch stays a set because each entry has its own head. Auto-open + // lands in list mode, never detail: detail arms the tool confirmation with + // focus, and a stray Enter aimed at the composer the dialog just unmounted + // would approve the background command (#16432 R1-2). Entering detail + // stays an explicit user navigation. + const surfacedApprovalIdsRef = useRef>(new Set()); + // Parked queue-head ids at the moment the user closed the dialog. Without + // this, the close itself re-runs the effect, which finds the next entry + // that still holds a parked approval and re-opens in the same frame — with + // two parked workflows Escape looks broken and re-arms the disarm. A + // genuinely new approval changes the snapshot and surfaces normally. + const parkedHeadIds = entries + .map((entry) => + entry.kind === 'workflow' && entry.pendingApprovals[0] + ? entry.pendingApprovals[0].approvalId + : '', + ) + .join('|'); + const closedParkedHeadIdsRef = useRef(null); + useEffect(() => { + if (dialogMode !== 'closed') return; + if (closedParkedHeadIdsRef.current !== null) { + if (closedParkedHeadIdsRef.current === parkedHeadIds) return; + closedParkedHeadIdsRef.current = null; + } + const index = entries.findIndex( + (entry) => + entry.kind === 'workflow' && + !!entry.pendingApprovals[0] && + !surfacedApprovalIdsRef.current.has( + entry.pendingApprovals[0].approvalId, + ), + ); + const entry = entries[index]; + if (entry?.kind !== 'workflow') return; + const approval = entry.pendingApprovals[0]; + if (!approval) return; + surfacedApprovalIdsRef.current.add(approval.approvalId); + setRawSelectedIndex(index); + // Same invariant `openDialog` establishes: dialog open ⇒ pill unfocused. + // The pill subscribes on `pillFocused` alone, so leaving it focused lets + // its Enter run `openDialog()` and clear the disarm (#16432 R3-2). + setPillFocused(false); + setAutoOpened(true); + setDialogMode('list'); + }, [entries, dialogMode, parkedHeadIds]); + // rawSelectedIndex can fall out of range when entries shrink; clamp on read. const selectedIndex = entries.length === 0 @@ -194,12 +261,25 @@ export function BackgroundTaskViewProvider({ }, [entries.length, selectedIndex]); const openDialog = useCallback(() => { + setAutoOpened(false); setDialogMode('list'); setPillFocused(false); }, []); const closeDialog = useCallback(() => { + // A dismissal before the acknowledgment would consume the latch with the + // approval never seen: the dialog closes, the effect skips the latched + // head forever and the run falls back to the badge-only hang this PR + // removes. Swallow it like the other composer-aimed keys; Ctrl+C reaches + // here through `closeAnyOpenDialog` too (#16432 R3-1). + if (autoOpened) return; + closedParkedHeadIdsRef.current = parkedHeadIds; + setAutoOpened(false); setDialogMode('closed'); + }, [autoOpened, parkedHeadIds]); + + const acknowledgeAutoOpen = useCallback(() => { + setAutoOpened(false); }, []); const enterDetail = useCallback(() => { @@ -324,6 +404,7 @@ export function BackgroundTaskViewProvider({ selectedIndex, dialogMode, dialogOpen, + autoOpened, pillFocused, livePanelFocused, livePanelSelectedIndex, @@ -333,6 +414,7 @@ export function BackgroundTaskViewProvider({ selectedIndex, dialogMode, dialogOpen, + autoOpened, pillFocused, livePanelFocused, livePanelSelectedIndex, @@ -348,6 +430,7 @@ export function BackgroundTaskViewProvider({ enterDetail, enterDetailFromPanel, exitDetail, + acknowledgeAutoOpen, cancelSelected, resumeSelected, toggleSelectedWorkflowPause, @@ -364,6 +447,7 @@ export function BackgroundTaskViewProvider({ enterDetail, enterDetailFromPanel, exitDetail, + acknowledgeAutoOpen, cancelSelected, resumeSelected, toggleSelectedWorkflowPause, diff --git a/packages/cli/src/ui/hooks/useBackgroundTaskView.test.ts b/packages/cli/src/ui/hooks/useBackgroundTaskView.test.ts index 98f9ec98522..304d66cd60c 100644 --- a/packages/cli/src/ui/hooks/useBackgroundTaskView.test.ts +++ b/packages/cli/src/ui/hooks/useBackgroundTaskView.test.ts @@ -7,21 +7,29 @@ import { describe, it, expect, vi } from 'vitest'; import { renderHook, act } from '@testing-library/react'; -import type { Config, WorkflowTask } from '@qwen-code/qwen-code-core'; +import type { + Config, + WorkflowApproval, + WorkflowTask, +} from '@qwen-code/qwen-code-core'; import { useBackgroundTaskView, entryId } from './useBackgroundTaskView.js'; interface FakeRegistry { setStatusChangeCallback: ReturnType; setApprovalChangeCallback: ReturnType; + setApprovalRequestCallback: ReturnType; /** Test helper — invokes the currently-set status callback. */ fire: () => void; /** Test helper — invokes the currently-set approval callback. */ fireApproval: () => void; + /** Test helper — invokes the currently-set approval-request callback. */ + fireApprovalRequest: () => void; } function makeFakeRegistry(): FakeRegistry { let cb: (() => void) | undefined; let approvalCb: (() => void) | undefined; + let approvalRequestCb: (() => void) | undefined; return { setStatusChangeCallback: vi.fn((next: (() => void) | undefined) => { cb = next; @@ -29,8 +37,14 @@ function makeFakeRegistry(): FakeRegistry { setApprovalChangeCallback: vi.fn((next: (() => void) | undefined) => { approvalCb = next; }), + setApprovalRequestCallback: vi.fn( + (next: ((...args: unknown[]) => void) | undefined) => { + approvalRequestCb = next as (() => void) | undefined; + }, + ), fire: () => cb?.(), fireApproval: () => approvalCb?.(), + fireApprovalRequest: () => approvalRequestCb?.(), }; } @@ -507,6 +521,73 @@ describe('useBackgroundTaskView', () => { }); }); + it('registers a workflow approval-request responder that re-reads the snapshot', () => { + // `setApprovalChangeCallback` only repaints the "needs approval" badge; + // the registry fails closed unless a host also registers a responder for + // the request itself. Without this registration a parked workflow + // approval never becomes an entry the dialog can render or settle. + const pending: WorkflowApproval = { + approvalId: 'wfap-req', + subagentId: 'sub-1', + callId: 'call-req', + name: 'Shell', + description: 'run', + confirmationDetails: { type: 'exec' }, + at: 1_700_000_000_000, + } as WorkflowApproval; + let workflows = [ + { + id: 'wf-req', + kind: 'workflow' as const, + runId: 'wf-req', + description: 'demo', + meta: null, + status: 'running' as const, + startTime: 100, + pendingApprovals: [], + }, + ] as unknown as WorkflowTask[]; + let reads = 0; + const { config, workflowReg } = makeConfig({ + agents: () => [], + shells: () => [], + monitors: () => [], + workflows: () => { + reads += 1; + return workflows; + }, + }); + const { result, unmount } = renderHook(() => useBackgroundTaskView(config)); + + // 1. mount registers a responder (not just the badge callback). + expect(workflowReg.setApprovalRequestCallback).toHaveBeenCalledWith( + expect.any(Function), + ); + + // 3. behaviour, not just call count: the registry parks an approval and + // fires the request; the hook must re-read the registry so the parked + // approval shows up in `entries`. + const readsBefore = reads; + workflows = [ + { + ...workflows[0], + pendingApprovals: [pending], + } as unknown as WorkflowTask, + ]; + act(() => workflowReg.fireApprovalRequest()); + expect(reads).toBeGreaterThan(readsBefore); + expect(result.current.entries[0]).toMatchObject({ + kind: 'workflow', + pendingApprovals: [expect.objectContaining({ approvalId: 'wfap-req' })], + }); + + // 2. cleanup releases the single-slot responder. + unmount(); + expect(workflowReg.setApprovalRequestCallback).toHaveBeenLastCalledWith( + undefined, + ); + }); + it('clears all registry subscriptions on unmount', () => { const { config, agentReg, shellReg, monitorReg, workflowReg, memoryMgr } = makeConfig({ diff --git a/packages/cli/src/ui/hooks/useBackgroundTaskView.ts b/packages/cli/src/ui/hooks/useBackgroundTaskView.ts index abdb0aa4b4a..5da9837b710 100644 --- a/packages/cli/src/ui/hooks/useBackgroundTaskView.ts +++ b/packages/cli/src/ui/hooks/useBackgroundTaskView.ts @@ -319,6 +319,14 @@ export function useBackgroundTaskView( agentRegistry.setApprovalChangeCallback(refreshFromRegistry); workflowRegistry.setApprovalChangeCallback(refreshFromRegistry); + // The change callback above only repaints the "needs approval" badge; it + // cannot answer a request, and the registry now fails closed without a + // real responder. Register the TUI's snapshot refresh as the host + // channel: it makes the parked approval part of `entries`, which the + // background-tasks dialog renders and settles through + // `resolvePendingApproval`. + workflowRegistry.setApprovalRequestCallback(refreshFromRegistry); + // Memory listener fires only on dream-task transitions — // `subscribe({ taskType: 'dream' })` skips the per-extract notify // entirely so we don't pay the per-UserQuery O(n) signature cost @@ -345,6 +353,7 @@ export function useBackgroundTaskView( workflowRegistry.setStatusChangeCallback(undefined); agentRegistry.setApprovalChangeCallback(undefined); workflowRegistry.setApprovalChangeCallback(undefined); + workflowRegistry.setApprovalRequestCallback(undefined); unsubscribeMemory(); }; }, [config]); diff --git a/packages/core/src/agents/runtime/workflow-orchestrator.test.ts b/packages/core/src/agents/runtime/workflow-orchestrator.test.ts index 622b0cb25cc..d255f911a66 100644 --- a/packages/core/src/agents/runtime/workflow-orchestrator.test.ts +++ b/packages/core/src/agents/runtime/workflow-orchestrator.test.ts @@ -1963,6 +1963,7 @@ describe('createProductionDispatch', () => { it('bubbles a production subagent approval through the run registry and resumes after ProceedOnce', async () => { const registry = new WorkflowRunRegistry(); registry.setApprovalChangeCallback(() => {}); + registry.setApprovalRequestCallback(() => {}); const config = { getWorkflowRunRegistry: () => registry, } as unknown as Config; diff --git a/packages/core/src/agents/workflow-run-registry.test.ts b/packages/core/src/agents/workflow-run-registry.test.ts index c9213ba8a39..44a97a251a0 100644 --- a/packages/core/src/agents/workflow-run-registry.test.ts +++ b/packages/core/src/agents/workflow-run-registry.test.ts @@ -125,7 +125,12 @@ function bridged( registry?: WorkflowRunRegistry, ) { const { r, entry } = registered(runId, overrides, registry); - if (channel) r.setApprovalChangeCallback(() => {}); + if (channel) { + // A host that can answer: the badge callback alone is display-only and + // no longer qualifies as a parking channel. + r.setApprovalChangeCallback(() => {}); + r.setApprovalRequestCallback(() => {}); + } const emitter = new AgentEventEmitter(); const cleanup = r.bridgeApprovalEvents(runId, emitter); /** Emits approvalEvent(event) on `on` and returns it. */ @@ -428,6 +433,21 @@ describe('WorkflowRunRegistry', () => { expect(pending()).toEqual([]); }); + it('fails closed when only a badge (approval-change) callback is registered', async () => { + // The shape the interactive TUI had: an approval-change consumer that + // repaints "needs approval" but cannot answer the request. + const { r, entry } = registered('wf_badge_only_host'); + r.setApprovalChangeCallback(() => {}); + const emitter = new AgentEventEmitter(); + r.bridgeApprovalEvents('wf_badge_only_host', emitter); + + const request = approvalEvent(); + emitter.emit(AgentEventType.TOOL_WAITING_APPROVAL, request); + + await expectCancelled(request.respond); + expect(entry.pendingApprovals).toEqual([]); + }); + it('isolates approvals from two agents that share a provider callId', async () => { const { emit, resolve, pending } = bridged('wf_shared_call_id'); const { respond: firstRespond } = emit({ subagentId: 'agent-a' }); @@ -469,6 +489,7 @@ describe('WorkflowRunRegistry', () => { await expectCancelled(rejectedRespond); r.setApprovalChangeCallback(() => {}); + r.setApprovalRequestCallback(() => {}); const { respond: retryRespond } = emit({ timestamp: 1_700_000_000_200 }); expect(pending()).toMatchObject([parkedAt(1_700_000_000_200)]); diff --git a/packages/core/src/agents/workflow-run-registry.ts b/packages/core/src/agents/workflow-run-registry.ts index 953ea3f1cf8..456d3f10d91 100644 --- a/packages/core/src/agents/workflow-run-registry.ts +++ b/packages/core/src/agents/workflow-run-registry.ts @@ -1152,13 +1152,16 @@ export class WorkflowRunRegistry { releaseSource: () => void, ): string | 'duplicate' | 'rejected' { const entry = this.entries.get(runId); + // A change callback only repaints the "needs approval" badge; it cannot + // answer the request. Requiring a real responder is what keeps a parked + // approval from waiting forever on a badge-only host. if ( !entry || !isActiveWorkflowStatus(entry.status) || - (!this.approvalChangeCallback && !this.approvalRequestCallback) + !this.approvalRequestCallback ) { debugLogger.warn( - `Workflow approval rejected for ${runId}/${event.callId}: entry missing, not active, or no host channel`, + `Workflow approval rejected for ${runId}/${event.callId}: entry missing, not active, or no host responder`, ); return 'rejected'; } @@ -1203,9 +1206,7 @@ export class WorkflowRunRegistry { at: event.timestamp, }; const approvalRequestCallback = this.approvalRequestCallback; - const requestController = approvalRequestCallback - ? new AbortController() - : undefined; + const requestController = new AbortController(); this.approvalRuntimes.set(approvalId, { respond: event.respond, requestController, @@ -1219,11 +1220,7 @@ export class WorkflowRunRegistry { ...(dispatchId ? { dispatchId } : {}), }); this.emitApprovalChange(entry); - if ( - approvalRequestCallback && - requestController && - !requestController.signal.aborted - ) { + if (requestController && !requestController.signal.aborted) { try { const request = approvalRequestCallback( entry,