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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 72 additions & 0 deletions packages/cli/src/acp-integration/session/Session.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2744,6 +2744,78 @@ describe('Session', () => {
);
});

it('emits terminalSequence returned by permission notification hooks over ACP', async () => {
const notificationHookSpy = vi
.spyOn(core, 'fireNotificationHook')
.mockResolvedValue({ terminalSequence: '\x07' });
const executeSpy = vi.fn().mockResolvedValue({
llmContent: 'ok',
returnDisplay: 'ok',
});
const onConfirmSpy = vi.fn().mockResolvedValue(undefined);
const invocation = {
params: { path: '/tmp/file.txt' },
getDefaultPermission: vi.fn().mockResolvedValue('ask'),
getConfirmationDetails: vi.fn().mockResolvedValue({
type: 'info',
title: 'Need permission',
prompt: 'Allow?',
onConfirm: onConfirmSpy,
}),
getDescription: vi.fn().mockReturnValue('Inspect file'),
toolLocations: vi.fn().mockReturnValue([]),
execute: executeSpy,
};
const tool = {
name: 'read_file',
kind: core.Kind.Read,
build: vi.fn().mockReturnValue(invocation),
};

mockToolRegistry.getTool.mockReturnValue(tool);
mockConfig.getApprovalMode = vi
.fn()
.mockReturnValue(ApprovalMode.DEFAULT);
mockConfig.getPermissionManager = vi.fn().mockReturnValue(null);
mockConfig.getDisableAllHooks = vi.fn().mockReturnValue(false);
mockConfig.getMessageBus = vi.fn().mockReturnValue({});
mockChat.sendMessageStream = vi.fn().mockResolvedValue(
createStreamWithChunks([
{
type: core.StreamEventType.CHUNK,
value: {
functionCalls: [
{
id: 'call-terminal-sequence',
name: 'read_file',
args: { path: '/tmp/file.txt' },
},
],
},
},
]),
);

try {
await session.prompt({
sessionId: 'test-session-id',
prompt: [{ type: 'text', text: 'run tool' }],
});
await new Promise<void>((resolve) => setImmediate(resolve));
} finally {
notificationHookSpy.mockRestore();
}

expect(mockClient.extNotification).toHaveBeenCalledWith(
'qwen/notify/session/terminal-sequence',
{
v: 1,
sessionId: 'test-session-id',
terminalSequence: '\x07',
},
);
});

it('allows info confirmation tools in plan mode', async () => {
const executeSpy = vi.fn().mockResolvedValue({
llmContent: 'ok',
Expand Down
34 changes: 33 additions & 1 deletion packages/cli/src/acp-integration/session/Session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2587,7 +2587,7 @@ export class Session implements SessionContext {
);

if (hooksEnabled && messageBus) {
void fireNotificationHook(
this.fireNotificationHookWithTerminalSequence(
messageBus,
`Qwen Code needs your permission to use ${fc.name}`,
NotificationType.PermissionPrompt,
Expand Down Expand Up @@ -3167,4 +3167,36 @@ export class Session implements SessionContext {
debugLogger.warn(msg);
}
}

/**
* Fire a notification hook and forward any terminalSequence to the ACP
* client as an extNotification. Fire-and-forget — errors are logged at
* debug level.
*/
private fireNotificationHookWithTerminalSequence(
messageBus: MessageBus,
message: string,
notificationType: NotificationType,
title?: string,
): void {
void fireNotificationHook(messageBus, message, notificationType, title)
.then((hookResult) => {
if (!hookResult.terminalSequence) return;
return this.client.extNotification(
'qwen/notify/session/terminal-sequence',
{
v: 1,
sessionId: this.sessionId,
terminalSequence: hookResult.terminalSequence,
},
);
})
.catch((err: unknown) => {
debugLogger.debug(
`ACP terminalSequence notification dropped ` +
`(session=${this.sessionId}): ` +
`${err instanceof Error ? err.message : String(err)}`,
);
});
}
}
45 changes: 45 additions & 0 deletions packages/cli/src/serve/httpAcpBridge.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5077,6 +5077,51 @@ describe('createHttpAcpBridge', () => {
await bridge.shutdown();
});

it('publishes terminal_sequence when the child fires terminalSequence notification', async () => {
let capturedConn: AgentSideConnection | undefined;
const factory: ChannelFactory = async () => {
const { clientStream, agentStream } = createInMemoryChannel();
const fakeAgent = new FakeAgent();
capturedConn = new AgentSideConnection(() => fakeAgent, agentStream);
return {
stream: clientStream,
exited: new Promise<
| { exitCode: number | null; signalCode: NodeJS.Signals | null }
| undefined
>(() => {}),
kill: async () => {},
killSync: () => {},
};
};
const bridge = makeBridge({ channelFactory: factory });
const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A });

const abort = new AbortController();
const iter = bridge.subscribeEvents(session.sessionId, {
signal: abort.signal,
});

void capturedConn!.extNotification(
'qwen/notify/session/terminal-sequence',
{
v: 1,
sessionId: session.sessionId,
terminalSequence: '\x07',
},
);

const collected: Array<{ type: string; data: unknown }> = [];
for await (const e of iter) {
collected.push({ type: e.type, data: e.data });
if (collected.length === 1) break;
}
expect(collected[0]?.type).toBe('terminal_sequence');
expect(collected[0]?.data).toEqual({ terminalSequence: '\x07' });

abort.abort();
await bridge.shutdown();
});

it('drops unknown extNotification methods, kinds, and missing sessionIds silently', async () => {
let capturedConn: AgentSideConnection | undefined;
const factory: ChannelFactory = async () => {
Expand Down
43 changes: 28 additions & 15 deletions packages/cli/src/serve/httpAcpBridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -749,27 +749,32 @@ class BridgeClient implements Client {
private readonly inFlightRestoreIds = new Set<string>();

/**
* PR 14b: handle child→bridge ACP `extNotification` calls. Only one
* method is recognized today — `qwen/notify/session/mcp-budget-event`
* — translating the McpClientManager's budget-event payload into a
* session-scoped SSE frame. Unknown methods, unknown event kinds,
* and missing sessionIds are dropped silently for forward-compat
* (a future child can add new notification methods without breaking
* this handler; an older daemon can ignore them cleanly).
*
* Codex review fix #1: when the sessionId IS present but the
* `byId`-resolvable entry is not yet registered (the child fired
* the event during its own `newSession` handler, before
* `connection.newSession` returned to `doSpawn`), buffer the frame
* and replay it on `drainEarlyEvents`.
* Handle child→bridge ACP `extNotification` calls and translate them into
* session-scoped SSE frames.
*/
async extNotification(
method: string,
params: Record<string, unknown>,
): Promise<void> {
if (method !== 'qwen/notify/session/mcp-budget-event') return;
const sessionId = params['sessionId'];
if (typeof sessionId !== 'string') return;

if (method === 'qwen/notify/session/terminal-sequence') {
const { v: _v2, sessionId: _sid2, ...tsRest } = params;
void _v2;
void _sid2;
const terminalSequence = tsRest['terminalSequence'];
if (
typeof terminalSequence !== 'string' ||
terminalSequence.length === 0
) {
return;
}
this.publishExtNotification(sessionId, 'terminal_sequence', tsRest);
return;
}

if (method !== 'qwen/notify/session/mcp-budget-event') return;
const kind = params['kind'];
const type =
kind === 'budget_warning'
Expand All @@ -787,10 +792,18 @@ class BridgeClient implements Client {
void _v;
void _sid;
void _kind;
this.publishExtNotification(sessionId, type, rest);
}

private publishExtNotification(
sessionId: string,
type: string,
data: Record<string, unknown>,
): void {
const entry = this.resolveEntry(sessionId);
const frame: Omit<BridgeEvent, 'id' | 'v'> = {
type,
data: rest,
data,
...(entry?.activePromptOriginatorClientId
? { originatorClientId: entry.activePromptOriginatorClientId }
: {}),
Expand Down
1 change: 1 addition & 0 deletions packages/cli/src/services/notificationService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ function createMockTerminal(): TerminalNotification {
notifyKitty: vi.fn(),
notifyGhostty: vi.fn(),
notifyBell: vi.fn(),
writeTerminalSequence: vi.fn(() => true),
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const mockTerminal: TerminalNotification = {
notifyKitty: vi.fn(),
notifyGhostty: vi.fn(),
notifyBell: vi.fn(),
writeTerminalSequence: vi.fn(() => true),
};

const mockSettings: LoadedSettings = {
Expand Down
12 changes: 9 additions & 3 deletions packages/cli/src/ui/hooks/useAttentionNotifications.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,15 @@ export const useAttentionNotifications = ({
'Qwen Code is waiting for your input',
NotificationType.IdlePrompt,
'Waiting for input',
).catch(() => {
// Silently ignore errors - fireNotificationHook has internal error handling
});
)
.then((hookResult) => {
if (hookResult.terminalSequence) {
terminal.writeTerminalSequence(hookResult.terminalSequence);
}
})
.catch(() => {
// Silently ignore errors - fireNotificationHook has internal error handling
});
}
idleNotificationSentRef.current = true;
}
Expand Down
17 changes: 17 additions & 0 deletions packages/cli/src/ui/hooks/useTerminalNotification.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,21 @@ describe('buildTerminalNotification', () => {
// BEL should NOT be wrapped in DCS passthrough
expect(writeRaw.mock.calls[0]![0]).not.toContain('\x1bPtmux');
});

it('writeTerminalSequence emits valid OSC sequence', () => {
delete process.env['TMUX'];
delete process.env['STY'];
const terminal = buildTerminalNotification(writeRaw);
const result = terminal.writeTerminalSequence('\x1b]9;hello\x07');
expect(result).toBe(true);
expect(writeRaw).toHaveBeenCalledTimes(1);
expect(writeRaw.mock.calls[0]![0]).toContain('\x1b]9;hello\x07');
});

it('writeTerminalSequence rejects invalid sequence', () => {
const terminal = buildTerminalNotification(writeRaw);
const result = terminal.writeTerminalSequence('plain text');
expect(result).toBe(false);
expect(writeRaw).not.toHaveBeenCalled();
});
});
6 changes: 6 additions & 0 deletions packages/cli/src/ui/hooks/useTerminalNotification.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
oscKittyNotify,
oscGhosttyNotify,
} from '../../utils/osc.js';
import { emitTerminalSequence } from '../../utils/terminalSequence.js';

// ── Types ──────────────────────────────────────────────────────────

Expand All @@ -29,6 +30,8 @@ export interface TerminalNotification {
notifyKitty: (opts: { message: string; title: string; id: number }) => void;
notifyGhostty: (opts: { message: string; title: string }) => void;
notifyBell: () => void;
/** Validate and emit a hook-provided terminal escape sequence. */
writeTerminalSequence: (sequence: string) => boolean;
}

// ── Factory (no React context needed) ──────────────────────────────
Expand Down Expand Up @@ -59,5 +62,8 @@ export function buildTerminalNotification(
// Wrapping would make it opaque DCS payload and lose that fallback.
writeRaw(BEL);
},
writeTerminalSequence(sequence: string) {
return emitTerminalSequence(sequence, writeRaw);
},
};
}
Loading
Loading