fix(cli): surface workflow agent approvals in the interactive TUI (#16425) (#16432)

* 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 <[email protected]>
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 <[email protected]>
Patrol-Run: qwen-issue-patrol/jmw4ig8e5z4

* fix(cli): type the workflow approval fixture and format the new tests

Co-authored-by: Qwen-Coder <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
Patrol-Run: qwen-pr-closeout/jmw5jlokths

---------

Co-authored-by: yiliang114 <[email protected]>
Co-authored-by: Qwen-Coder <[email protected]>
This commit is contained in:
authored and GitHub committed 2026-11-07 10:49:33 +08:00
1 parent 49ac6dae86
commit e3cbea0096
11 files changed
+833 -22

No files matched your search

@@ -189,6 +189,7 @@ const createBackgroundTaskState = (
selectedIndex: 0,
dialogMode: 'closed',
dialogOpen: false,
autoOpened: false,
pillFocused: false,
livePanelFocused: false,
livePanelSelectedIndex: 0,
@@ -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
@@ -1405,7 +1405,7 @@ export const BackgroundTasksDialog: React.FC<BackgroundTasksDialogProps> = ({
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<BackgroundTasksDialogProps> = ({
closeDialog,
enterDetail,
exitDetail,
acknowledgeAutoOpen,
cancelSelected,
resumeSelected,
toggleSelectedWorkflowPause,
@@ -1777,6 +1778,15 @@ export const BackgroundTasksDialog: React.FC<BackgroundTasksDialogProps> = ({
}
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<BackgroundTasksDialogProps> = ({
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<BackgroundTasksDialogProps> = ({
'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)) {
@@ -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,
@@ -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<string, unknown>;
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 }) => (
<BackgroundTaskViewProvider config={config}>
{children}
</BackgroundTaskViewProvider>
);
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 }) => (
<BackgroundTaskViewProvider config={config}>
{children}
</BackgroundTaskViewProvider>
);
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 }) => (
<BackgroundTaskViewProvider config={config}>
{children}
</BackgroundTaskViewProvider>
);
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 }) => (
<BackgroundTaskViewProvider config={config}>
{children}
</BackgroundTaskViewProvider>
);
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 }) => (
<BackgroundTaskViewProvider config={config}>
{children}
</BackgroundTaskViewProvider>
);
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 }) => (
<BackgroundTaskViewProvider config={config}>
{children}
</BackgroundTaskViewProvider>
);
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 }) => (
<BackgroundTaskViewProvider config={config}>
{children}
</BackgroundTaskViewProvider>
);
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();
});
});
@@ -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<BackgroundDialogMode>('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<Set<string>>(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<string | null>(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,
@@ -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<typeof vi.fn>;
setApprovalChangeCallback: ReturnType<typeof vi.fn>;
setApprovalRequestCallback: ReturnType<typeof vi.fn>;
/** 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({
@@ -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]);
@@ -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;
@@ -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)]);
@@ -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,