diff --git a/packages/cli/src/serve/acp-http/transport.test.ts b/packages/cli/src/serve/acp-http/transport.test.ts index d960085af7c..5cf79c5d1b4 100644 --- a/packages/cli/src/serve/acp-http/transport.test.ts +++ b/packages/cli/src/serve/acp-http/transport.test.ts @@ -6296,6 +6296,56 @@ describe('ACP Streamable HTTP transport (over the wire)', () => { // (takeFrames already locked + aborted `sess`; afterEach force-closes.) }); + it('keeps availableSkillDetails verbatim on pumped session_update frames (#9234)', async () => { + // Mirror of the SSE redaction tests: the SDK/browser surface strips + // `_meta.availableSkillDetails`, but the /acp surface must keep + // delivering it untouched — desktop clients parse the skill bodies + // for display/editing. + const connId = await initialize(); + await newSession(connId); + const sess = await openStream(connId, 'sess-1'); + const got = takeFrames(sess, 1); + await new Promise((r) => setTimeout(r, 50)); + const skillDetails = [ + { name: 'bugfix', body: 'skill body', filePath: '/skills/bugfix' }, + ]; + bridge.queues.get('sess-1')?.push({ + type: 'session_update', + data: { + sessionId: 'sess-1', + update: { + sessionUpdate: 'available_commands_update', + availableCommands: [{ name: 'help', description: 'Help' }], + _meta: { + availableSkills: ['bugfix'], + availableSkillDetails: skillDetails, + }, + }, + }, + }); + const frames = (await got) as Array<{ + method: string; + params: { + sessionId: string; + update: { _meta?: { availableSkillDetails?: unknown } }; + }; + }>; + expect(frames[0]).toMatchObject({ + method: 'session/update', + params: { + sessionId: 'sess-1', + update: { + sessionUpdate: 'available_commands_update', + availableCommands: [{ name: 'help', description: 'Help' }], + _meta: { + availableSkills: ['bugfix'], + availableSkillDetails: skillDetails, + }, + }, + }, + }); + }); + it('session/load while a session/close is in-flight → rejected (TOCTOU guard)', async () => { let releaseClose: () => void = () => {}; bridge.closeGate = new Promise((r) => (releaseClose = r)); diff --git a/packages/cli/src/serve/routes/session.ts b/packages/cli/src/serve/routes/session.ts index f90de2ac94b..fe1226988e3 100644 --- a/packages/cli/src/serve/routes/session.ts +++ b/packages/cli/src/serve/routes/session.ts @@ -93,6 +93,10 @@ import { } from '../server/session-export.js'; import { setDaemonTelemetryWorkspace } from '../server/telemetry.js'; import { createSessionOrganizationService } from '../session-organization-helpers.js'; +import { + omitSkillDetailsForSdkSurface, + omitSkillDetailsFromReplayArrays, +} from '../skill-details-redaction.js'; import { replayTranscriptRecordPage } from '../../acp-integration/session/history-replay-page.js'; import { GENERATION_MAX_PROMPT_BYTES } from '../../acp-integration/generation.js'; import { @@ -2100,7 +2104,9 @@ export function registerSessionRoutes( }); return; } - res.status(200).json(session); + // Same replay-array shape as the load response; redact skill + // bodies for the browser surface (#9234). + res.status(200).json(omitSkillDetailsFromReplayArrays(session)); } catch (err) { sendBridgeError(res, err, { route, sessionId }); } @@ -2413,7 +2419,9 @@ export function registerSessionRoutes( } } } - res.status(200).json(session); + // The load response embeds the replay snapshot inline; redact the + // skill bodies there just like the SSE egress does (#9234). + res.status(200).json(omitSkillDetailsFromReplayArrays(session)); } catch (err) { sendBridgeError(res, err, { route, @@ -2596,7 +2604,16 @@ export function registerSessionRoutes( } } if (!res.writable) return; - res.status(201).json(result); + // Branch/side-task responses carry the same replay snapshot shape as + // load; apply the same redaction (#9234). The helper returns its + // input unchanged when no replay arrays are present (checkpoint + // branches), so apply it unconditionally rather than re-deriving the + // bridge's variant discrimination here. + res + .status(201) + .json( + omitSkillDetailsFromReplayArrays(result as BridgeBranchedSession), + ); }, ), ); @@ -2660,7 +2677,7 @@ export function registerSessionRoutes( } return; } - res.status(201).json(result); + res.status(201).json(omitSkillDetailsFromReplayArrays(result)); }, ), ); @@ -2828,7 +2845,13 @@ export function registerSessionRoutes( }, ); if (result === undefined) return; - res.status(200).set('Cache-Control', 'no-store').json(result); + res + .status(200) + .set('Cache-Control', 'no-store') + .json({ + ...result, + events: (result.events ?? []).map(omitSkillDetailsForSdkSurface), + }); } catch (err) { sendBridgeError(res, err, { route, @@ -2948,11 +2971,13 @@ export function registerSessionRoutes( return { v: 1 as const, sessionId, - events: replay.updates.map((update) => ({ - v: 1 as const, - type: 'session_update' as const, - data: update, - })), + events: replay.updates.map((update) => + omitSkillDetailsForSdkSurface({ + v: 1 as const, + type: 'session_update' as const, + data: update, + }), + ), ...(replay.nextCursor && !cursorTooLarge ? { nextCursor: replay.nextCursor } : {}), diff --git a/packages/cli/src/serve/routes/sse-events.ts b/packages/cli/src/serve/routes/sse-events.ts index 94695e3f183..3b4d2df77c2 100644 --- a/packages/cli/src/serve/routes/sse-events.ts +++ b/packages/cli/src/serve/routes/sse-events.ts @@ -33,6 +33,7 @@ import { parseMaxQueuedQuery, } from '../server/request-helpers.js'; import { parseEventEpochHeader } from '../sse-last-event-id.js'; +import { omitSkillDetailsForSdkSurface } from '../skill-details-redaction.js'; import type { WorkspaceRegistry } from '../workspace-registry.js'; import { requireSessionRuntime } from './session-runtime.js'; import { @@ -125,6 +126,7 @@ interface RegisterSseEventsRoutesDeps { type OmitId = Omit; function formatSseFrame(event: BridgeEvent | OmitId): string { + const shaped = omitSkillDetailsForSdkSurface(event); // SSE format: id (optional), event (optional), data, blank line. // The `id:` line is intentionally omitted when `event.id` is absent — // terminal/synthetic frames (e.g. daemon-side `stream_error`) must not @@ -142,7 +144,7 @@ function formatSseFrame(event: BridgeEvent | OmitId): string { // `_meta.serverTimestamp`: EventBus stamps normal session frames when they // are published so SSE and load/replay share the same event time. Keep this // fallback for synthetic frames that do not pass through EventBus. - const existingMeta = (event as { _meta?: Record })._meta; + const existingMeta = (shaped as { _meta?: Record })._meta; const existingServerTimestamp = existingMeta?.['serverTimestamp']; const serverTimestamp = typeof existingServerTimestamp === 'number' && @@ -150,13 +152,13 @@ function formatSseFrame(event: BridgeEvent | OmitId): string { ? existingServerTimestamp : Date.now(); const stamped = { - ...event, + ...shaped, _meta: { ...(existingMeta ?? {}), serverTimestamp }, }; const dataJson = JSON.stringify(stamped); const idLine = - 'id' in event && event.id !== undefined ? `id: ${event.id}\n` : ''; - return `${idLine}event: ${event.type}\ndata: ${dataJson}\n\n`; + 'id' in shaped && shaped.id !== undefined ? `id: ${shaped.id}\n` : ''; + return `${idLine}event: ${shaped.type}\ndata: ${dataJson}\n\n`; } export function registerSseEventsRoutes( diff --git a/packages/cli/src/serve/server.test.ts b/packages/cli/src/serve/server.test.ts index b4a76a9fc3e..4c669037443 100644 --- a/packages/cli/src/serve/server.test.ts +++ b/packages/cli/src/serve/server.test.ts @@ -11617,6 +11617,63 @@ describe('createServeApp', () => { expect(bridge.resumeCalls).toEqual([]); }); + it('redacts skill bodies from virtual subagent load replay (#9234)', async () => { + const bridge = fakeBridge(); + const app = createServeApp( + { ...baseOpts, workspace: WS_BOUND }, + undefined, + { bridge }, + ); + const sessionId = createVirtualSubagentSessionId('parent-1', 'agent-1'); + const commandsEvent = { + id: 1, + v: 1, + type: 'session_update', + data: { + sessionId: 'parent-1', + update: { + sessionUpdate: 'available_commands_update', + availableCommands: [{ name: 'help', description: 'Help' }], + _meta: { + availableSkills: ['bugfix'], + availableSkillDetails: [ + { name: 'bugfix', body: 'x'.repeat(600_000) }, + ], + }, + }, + }, + }; + const loadSpy = vi + .spyOn(VirtualSubagentSessions.prototype, 'load') + .mockResolvedValue({ + sessionId, + workspaceCwd: WS_BOUND, + attached: true, + clientId: 'client-v', + state: {}, + compactedReplay: [commandsEvent], + liveJournal: [], + }); + + try { + const res = await request(app) + .post(`/session/${sessionId}/load`) + .set('Host', `127.0.0.1:${baseOpts.port}`) + .send({}); + + expect(res.status).toBe(200); + const replay = res.body.compactedReplay as Array<{ + data: { update: Record }; + }>; + const meta = replay[0]!.data.update['_meta'] as Record; + expect(meta['availableSkills']).toEqual(['bugfix']); + expect(meta).not.toHaveProperty('availableSkillDetails'); + expect(JSON.stringify(res.body)).not.toContain('x'.repeat(64)); + } finally { + loadSpy.mockRestore(); + } + }); + it('passes the requested initial history page size to load', async () => { const bridge = fakeBridge(); const app = createServeApp( @@ -11881,6 +11938,164 @@ describe('createServeApp', () => { ]); }); + it('redacts skill bodies from the load response replay arrays (#9234)', async () => { + const commandsEvent = { + id: 1, + v: 1, + type: 'session_update', + data: { + sessionId: 'persisted-replay', + update: { + sessionUpdate: 'available_commands_update', + availableCommands: [{ name: 'help', description: 'Help' }], + _meta: { + availableSkills: ['bugfix'], + availableSkillDetails: [ + { name: 'bugfix', body: 'x'.repeat(600_000) }, + ], + }, + }, + }, + } satisfies BridgeEvent; + const textEvent = { + id: 2, + v: 1, + type: 'session_update', + data: { + sessionId: 'persisted-replay', + update: { + sessionUpdate: 'agent_message_chunk', + content: { type: 'text', text: 'hi' }, + }, + }, + } satisfies BridgeEvent; + // The in-flight journal can hold a fresher snapshot than the compacted + // turns (mid-turn load); it must be redacted too. + const journalCommandsEvent = { + id: 3, + v: 1, + type: 'session_update', + data: { + sessionId: 'persisted-replay', + update: { + sessionUpdate: 'available_commands_update', + availableCommands: [{ name: 'help', description: 'Help' }], + _meta: { + availableSkills: ['bugfix'], + availableSkillDetails: [ + { name: 'bugfix', body: 'y'.repeat(600_000) }, + ], + }, + }, + }, + } satisfies BridgeEvent; + const bridge = fakeBridge({ + loadImpl: async (req) => ({ + sessionId: req.sessionId, + workspaceCwd: req.workspaceCwd, + attached: false, + clientId: 'client-load', + state: {}, + compactedReplay: [commandsEvent], + liveJournal: [journalCommandsEvent, textEvent], + }), + }); + const app = createServeApp(baseOpts, undefined, { bridge }); + const res = await request(app) + .post('/session/persisted-replay/load') + .set('Host', `127.0.0.1:${baseOpts.port}`) + .send({}); + + expect(res.status).toBe(200); + expect(res.body).toMatchObject({ sessionId: 'persisted-replay' }); + const replay = res.body.compactedReplay as Array<{ + data: { update: Record }; + }>; + // Pin the full envelope (id/v/type/data.sessionId) so envelope-level + // regressions in the reshape cannot ship green (review R3-2). + expect(replay[0]).toEqual({ + ...commandsEvent, + data: { + ...commandsEvent.data, + update: { + ...commandsEvent.data.update, + _meta: { availableSkills: ['bugfix'] }, + }, + }, + }); + const journal = res.body.liveJournal as Array<{ + data: { update: Record }; + }>; + expect(journal[0]).toEqual({ + ...journalCommandsEvent, + data: { + ...journalCommandsEvent.data, + update: { + ...journalCommandsEvent.data.update, + _meta: { availableSkills: ['bugfix'] }, + }, + }, + }); + expect(journal[1]).toEqual(textEvent); + expect(JSON.stringify(res.body)).not.toContain('x'.repeat(64)); + expect(JSON.stringify(res.body)).not.toContain('y'.repeat(64)); + // Bus events are shared with other subscribers (e.g. the /acp pump); + // the redaction must reshape immutably, never mutate the source. + expect( + (commandsEvent.data.update._meta as Record)[ + 'availableSkillDetails' + ], + ).toBeDefined(); + }); + + it('redacts flat persisted-transcript frames in replay arrays (#9234)', async () => { + // Persisted-transcript frames carry the ACP update flat under `data` + // (no `update` wrapper); the redactor must handle both shapes. + const flatCommandsEvent = { + id: 7, + v: 1, + type: 'session_update', + data: { + sessionUpdate: 'available_commands_update', + availableCommands: [{ name: 'help', description: 'Help' }], + _meta: { + availableSkills: ['bugfix'], + availableSkillDetails: [ + { name: 'bugfix', body: 'LEAK-CANARY-SKILL-BODY'.repeat(100) }, + ], + }, + }, + } satisfies BridgeEvent; + const bridge = fakeBridge({ + loadImpl: async (req) => ({ + sessionId: req.sessionId, + workspaceCwd: req.workspaceCwd, + attached: false, + clientId: 'client-load', + state: {}, + compactedReplay: [flatCommandsEvent], + }), + }); + const app = createServeApp(baseOpts, undefined, { bridge }); + const res = await request(app) + .post('/session/persisted-flat/load') + .set('Host', `127.0.0.1:${baseOpts.port}`) + .send({}); + + expect(res.status).toBe(200); + const replay = res.body.compactedReplay as Array<{ + data: Record; + }>; + expect(replay[0]).toEqual({ + ...flatCommandsEvent, + data: { + ...flatCommandsEvent.data, + _meta: { availableSkills: ['bugfix'] }, + }, + }); + expect(JSON.stringify(res.body)).not.toContain('LEAK-CANARY-SKILL-BODY'); + }); + it('passes client identity headers through to load/resume bridge calls', async () => { for (const action of ['load', 'resume'] as const) { const bridge = fakeBridge(); @@ -17371,6 +17586,132 @@ describe('createServeApp', () => { await fsp.rm(runtimeDir, { recursive: true, force: true }); } }); + + it('redacts skill bodies from the branch response replay arrays (#9234)', async () => { + const commandsEvent = { + id: 1, + v: 1, + type: 'session_update', + data: { + sessionId: 'branched-session', + update: { + sessionUpdate: 'available_commands_update', + availableCommands: [{ name: 'help', description: 'Help' }], + _meta: { + availableSkills: ['bugfix'], + availableSkillDetails: [ + { name: 'bugfix', body: 'x'.repeat(600_000) }, + ], + }, + }, + }, + } satisfies BridgeEvent; + const bridge = fakeBridge(); + bridge.branchSession = vi.fn(async (sessionId) => ({ + sessionId: 'branched-session', + workspaceCwd: WS_BOUND, + attached: false, + clientId: 'client-branch', + state: {}, + displayName: 'Branched', + forkedFrom: { sessionId, displayName: 'Source' }, + compactedReplay: [commandsEvent], + })); + const runtime = makeWorkspaceRuntimeForTest({ + workspaceId: 'branch-redaction', + workspaceCwd: WS_BOUND, + primary: true, + bridge, + generationGuard: createWorkspaceGenerationGuard(), + }); + const app = createServeApp( + { ...baseOpts, workspace: WS_BOUND }, + undefined, + { workspaceRegistry: createWorkspaceRegistry([runtime]) }, + ); + + const res = await request(app) + .post('/session/source-session/branch') + .set('Host', `127.0.0.1:${baseOpts.port}`) + .send({}); + + expect(res.status).toBe(201); + const replay = res.body.compactedReplay as Array<{ + data: { update: Record }; + }>; + const update = replay[0]!.data.update; + expect(update['sessionUpdate']).toBe('available_commands_update'); + expect(update['availableCommands']).toEqual([ + { name: 'help', description: 'Help' }, + ]); + const meta = update['_meta'] as Record; + expect(meta['availableSkills']).toEqual(['bugfix']); + expect(meta).not.toHaveProperty('availableSkillDetails'); + expect(JSON.stringify(res.body)).not.toContain('x'.repeat(64)); + }); + }); + + describe('POST /session/:id/side-task (skill-detail redaction, #9234)', () => { + it('redacts skill bodies from the side-task response replay arrays', async () => { + const commandsEvent = { + id: 1, + v: 1, + type: 'session_update', + data: { + sessionId: 'side-task-session', + update: { + sessionUpdate: 'available_commands_update', + availableCommands: [{ name: 'help', description: 'Help' }], + _meta: { + availableSkills: ['bugfix'], + availableSkillDetails: [ + { name: 'bugfix', body: 'x'.repeat(600_000) }, + ], + }, + }, + }, + } satisfies BridgeEvent; + const bridge = fakeBridge(); + bridge.createSideTaskSession = vi.fn(async () => ({ + sessionId: 'side-task-session', + workspaceCwd: WS_BOUND, + attached: false, + clientId: 'client-side-task', + state: {}, + liveJournal: [commandsEvent], + })); + const runtime = makeWorkspaceRuntimeForTest({ + workspaceId: 'side-task-redaction', + workspaceCwd: WS_BOUND, + primary: true, + bridge, + generationGuard: createWorkspaceGenerationGuard(), + }); + const app = createServeApp( + { ...baseOpts, workspace: WS_BOUND }, + undefined, + { workspaceRegistry: createWorkspaceRegistry([runtime]) }, + ); + + const res = await request(app) + .post('/session/source-session/side-task') + .set('Host', `127.0.0.1:${baseOpts.port}`) + .send({ name: 'follow-up' }); + + expect(res.status).toBe(201); + const journal = res.body.liveJournal as Array<{ + data: { update: Record }; + }>; + const update = journal[0]!.data.update; + expect(update['sessionUpdate']).toBe('available_commands_update'); + expect(update['availableCommands']).toEqual([ + { name: 'help', description: 'Help' }, + ]); + const meta = update['_meta'] as Record; + expect(meta['availableSkills']).toEqual(['bugfix']); + expect(meta).not.toHaveProperty('availableSkillDetails'); + expect(JSON.stringify(res.body)).not.toContain('x'.repeat(64)); + }); }); describe('POST /session/:id/fork', () => { @@ -21008,6 +21349,50 @@ describe('createServeApp', () => { expect(bridge.resumeCalls).toHaveLength(0); }); + it('redacts skill bodies from flat transcript events (#9234)', async () => { + const sid = '55555555-bbbb-cccc-dddd-aaaaaaaaaaab'; + const bridge = fakeBridge({ + sessionTranscriptImpl: async (req) => ({ + v: 1, + sessionId: req.sessionId, + events: [ + { + v: 1, + type: 'session_update', + data: { + sessionUpdate: 'available_commands_update', + availableCommands: [{ name: 'help', description: 'Help' }], + _meta: { + availableSkills: ['bugfix'], + availableSkillDetails: [ + { name: 'bugfix', body: 'x'.repeat(600_000) }, + ], + }, + }, + }, + ], + hasMore: false, + }), + }); + await writeTranscriptSession(sid); + const app = createServeApp({ ...baseOpts, workspace: wsDir }, undefined, { + bridge, + boundWorkspace: wsDir, + }); + + const res = await request(app) + .get(`/session/${sid}/transcript`) + .set('Host', `127.0.0.1:${baseOpts.port}`); + + expect(res.status).toBe(200); + const event = res.body.events[0] as { data: Record }; + expect(event.data['sessionUpdate']).toBe('available_commands_update'); + const meta = event.data['_meta'] as Record; + expect(meta['availableSkills']).toEqual(['bugfix']); + expect(meta).not.toHaveProperty('availableSkillDetails'); + expect(JSON.stringify(res.body)).not.toContain('x'.repeat(64)); + }); + it('forwards an exclusive persisted-record boundary', async () => { const sid = '55555555-bbbb-cccc-dddd-bbbbbbbbbbbb'; const bridge = fakeBridge({ @@ -25549,6 +25934,141 @@ describe('GET /session/:id/events (SSE)', () => { expect(JSON.parse(frames[1]!.data!)).not.toHaveProperty('promptId'); }); + it('omits skill bodies from available_commands_update frames (#9234)', async () => { + // The daemon-side snapshot embeds every skill's full SKILL.md body for + // ACP clients; the SSE surface must strip it while keeping the command + // entries and the skill name list. + const sharedUpdate = { + sessionUpdate: 'available_commands_update', + availableCommands: [{ name: 'help', description: 'Help' }], + _meta: { + availableSkills: ['bugfix'], + availableSkillDetails: [ + { + name: 'bugfix', + description: 'Fix a bug', + body: 'x'.repeat(600_000), + filePath: '/skills/bugfix/SKILL.md', + level: 'project', + modelInvocable: true, + }, + ], + }, + }; + const bridge = fakeBridge({ + async *subscribeImpl() { + yield { + id: 1, + v: 1, + type: 'session_update', + data: { sessionId: 'sess-A', update: sharedUpdate }, + }; + yield { + id: 2, + v: 1, + type: 'session_update', + data: { + sessionId: 'sess-A', + update: { + sessionUpdate: 'agent_message_chunk', + content: { type: 'text', text: 'hi' }, + }, + }, + }; + }, + }); + const app = createServeApp(baseOpts, undefined, { bridge }); + + const res = await request(app) + .get('/session/sess-A/events') + .set('Host', `127.0.0.1:${baseOpts.port}`); + + expect(res.status).toBe(200); + const payloads = res.text + .split('\n\n') + .map((raw) => { + const dataLine = raw + .split('\n') + .find((line) => line.startsWith('data: ')); + // Skip non-frame prelude lines such as `retry: 3000`. + if (!dataLine) return undefined; + return JSON.parse(dataLine.slice('data: '.length)) as { + id?: number; + data?: { update?: Record }; + }; + }) + .filter( + ( + payload, + ): payload is { + id?: number; + data?: { update?: Record }; + } => payload !== undefined, + ); + expect(payloads).toHaveLength(2); + // Pin the envelope the reshape must preserve (SSE id line, schema + // version, session attribution) — review R3-2 mutant M1. + expect(payloads[0]).toMatchObject({ + id: 1, + v: 1, + type: 'session_update', + data: { sessionId: 'sess-A' }, + }); + const commandsUpdate = payloads[0]!.data!.update!; + expect(commandsUpdate['sessionUpdate']).toBe('available_commands_update'); + expect(commandsUpdate['availableCommands']).toEqual([ + { name: 'help', description: 'Help' }, + ]); + const meta = commandsUpdate['_meta'] as Record; + expect(meta['availableSkills']).toEqual(['bugfix']); + expect(meta).not.toHaveProperty('availableSkillDetails'); + expect(payloads[1]!.data!.update).toMatchObject({ + sessionUpdate: 'agent_message_chunk', + content: { type: 'text', text: 'hi' }, + }); + expect(res.text).not.toContain('x'.repeat(64)); + // Bus events are shared with other subscribers (e.g. the /acp pump); + // the strip must reshape immutably, never mutate the source event. + expect(sharedUpdate._meta).toHaveProperty('availableSkillDetails'); + }); + + it('drops an available_commands_update _meta left empty by skill-detail stripping (#9234)', async () => { + const bridge = fakeBridge({ + async *subscribeImpl() { + yield { + id: 1, + v: 1, + type: 'session_update', + data: { + sessionId: 'sess-A', + update: { + sessionUpdate: 'available_commands_update', + availableCommands: [{ name: 'help', description: 'Help' }], + _meta: { + availableSkillDetails: [{ name: 'bugfix', body: 'body' }], + }, + }, + }, + }; + }, + }); + const app = createServeApp(baseOpts, undefined, { bridge }); + + const res = await request(app) + .get('/session/sess-A/events') + .set('Host', `127.0.0.1:${baseOpts.port}`); + + expect(res.status).toBe(200); + const dataLine = res.text + .split('\n') + .find((line) => line.startsWith('data: ')); + expect(dataLine).toBeDefined(); + const payload = JSON.parse(dataLine!.slice('data: '.length)) as { + data?: { update?: Record }; + }; + expect(payload.data!.update).not.toHaveProperty('_meta'); + }); + it('correlates the SSE response, daemon lifecycle log, and request span', async () => { const predecessor = '019535d9-3df7-7a61-8f6d-6f37c39c5f19'; const setAttribute = vi.fn(); diff --git a/packages/cli/src/serve/skill-details-redaction.test.ts b/packages/cli/src/serve/skill-details-redaction.test.ts new file mode 100644 index 00000000000..210ceaf9266 --- /dev/null +++ b/packages/cli/src/serve/skill-details-redaction.test.ts @@ -0,0 +1,163 @@ +import { describe, expect, it } from 'vitest'; +import type { BridgeEvent } from '@qwen-code/acp-bridge/eventBus'; +import { + omitSkillDetailsForSdkSurface, + omitSkillDetailsFromReplayArrays, +} from './skill-details-redaction.js'; + +interface CommandsData { + sessionUpdate: string; + availableCommands: Array<{ name: string; description: string }>; + _meta?: Record; +} +interface WrappedEvent extends BridgeEvent { + data: { sessionId: string; update: CommandsData }; +} +interface FlatEvent extends BridgeEvent { + data: CommandsData; +} + +function wrappedCommandsEvent(): WrappedEvent { + return { + id: 1, + v: 1, + type: 'session_update', + data: { + sessionId: 'sess-1', + update: { + sessionUpdate: 'available_commands_update', + availableCommands: [{ name: 'help', description: 'Help' }], + _meta: { + availableSkills: ['bugfix'], + availableSkillDetails: [{ name: 'bugfix', body: 'skill body' }], + other: 'kept', + }, + }, + }, + }; +} + +function flatCommandsEvent(): FlatEvent { + return { + id: 2, + v: 1, + type: 'session_update', + data: { + sessionUpdate: 'available_commands_update', + availableCommands: [{ name: 'help', description: 'Help' }], + _meta: { + availableSkills: ['bugfix'], + availableSkillDetails: [{ name: 'bugfix', body: 'skill body' }], + }, + }, + }; +} + +describe('omitSkillDetailsForSdkSurface', () => { + it('strips availableSkillDetails from wrapped frames, keeping the rest', () => { + const shaped = omitSkillDetailsForSdkSurface(wrappedCommandsEvent()); + expect(shaped).toEqual({ + id: 1, + v: 1, + type: 'session_update', + data: { + sessionId: 'sess-1', + update: { + sessionUpdate: 'available_commands_update', + availableCommands: [{ name: 'help', description: 'Help' }], + _meta: { availableSkills: ['bugfix'], other: 'kept' }, + }, + }, + }); + }); + + it('strips availableSkillDetails from flat persisted-transcript frames', () => { + const shaped = omitSkillDetailsForSdkSurface(flatCommandsEvent()); + expect(shaped).toEqual({ + id: 2, + v: 1, + type: 'session_update', + data: { + sessionUpdate: 'available_commands_update', + availableCommands: [{ name: 'help', description: 'Help' }], + _meta: { availableSkills: ['bugfix'] }, + }, + }); + }); + + it('drops _meta entirely when stripping leaves it empty', () => { + const event = wrappedCommandsEvent(); + event.data.update._meta = { + availableSkillDetails: [{ name: 'bugfix', body: 'skill body' }], + }; + const shaped = omitSkillDetailsForSdkSurface(event); + expect(shaped.data.update).not.toHaveProperty('_meta'); + }); + + it('passes through non-available_commands_update events unchanged', () => { + const event: BridgeEvent = { + id: 3, + v: 1, + type: 'session_update', + data: { + sessionId: 'sess-1', + update: { + sessionUpdate: 'agent_message_chunk', + content: { type: 'text', text: 'hi' }, + }, + }, + }; + expect(omitSkillDetailsForSdkSurface(event)).toBe(event); + }); + + it('passes through commands frames without availableSkillDetails unchanged', () => { + const event = wrappedCommandsEvent(); + event.data.update._meta = { availableSkills: ['bugfix'] }; + expect(omitSkillDetailsForSdkSurface(event)).toBe(event); + }); + + it('never mutates the source event', () => { + const event = wrappedCommandsEvent(); + omitSkillDetailsForSdkSurface(event); + expect(event.data.update._meta?.['availableSkillDetails']).toEqual([ + { name: 'bugfix', body: 'skill body' }, + ]); + }); +}); + +describe('omitSkillDetailsFromReplayArrays', () => { + it('redacts both arrays when both are present', () => { + const shaped = omitSkillDetailsFromReplayArrays({ + sessionId: 'sess-1', + compactedReplay: [wrappedCommandsEvent()], + liveJournal: [flatCommandsEvent()], + }); + expect(shaped.sessionId).toBe('sess-1'); + expect(shaped.compactedReplay[0].data.update._meta).not.toHaveProperty( + 'availableSkillDetails', + ); + expect(shaped.liveJournal[0].data._meta).not.toHaveProperty( + 'availableSkillDetails', + ); + }); + + it('redacts a single present array', () => { + const shaped = omitSkillDetailsFromReplayArrays({ + sessionId: 'sess-1', + liveJournal: [wrappedCommandsEvent()], + }); + expect(shaped.liveJournal[0].data.update._meta).toEqual({ + availableSkills: ['bugfix'], + other: 'kept', + }); + }); + + it('returns its input unchanged when no replay arrays are present', () => { + const session = { + sessionId: 'sess-1', + displayName: 'Branch', + compactedReplay: undefined, + }; + expect(omitSkillDetailsFromReplayArrays(session)).toBe(session); + }); +}); diff --git a/packages/cli/src/serve/skill-details-redaction.ts b/packages/cli/src/serve/skill-details-redaction.ts new file mode 100644 index 00000000000..1786c3e8398 --- /dev/null +++ b/packages/cli/src/serve/skill-details-redaction.ts @@ -0,0 +1,88 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import type { BridgeEvent } from '@qwen-code/acp-bridge/eventBus'; + +/** + * `available_commands_update` snapshots embed every installed skill's full + * SKILL.md body under `update._meta.availableSkillDetails` for ACP clients + * that display or edit skill files (e.g. desktop). The SDK/browser surface + * (SSE streams and REST responses) only reads the command entries and the + * `availableSkills` name list, so with many skills installed the bodies are + * hundreds of kilobytes of dead weight that every browser tab parses and + * discards on each snapshot (#9234). The `/acp` surface keeps delivering the + * full snapshot; apply this at every SDK/browser egress point. Frames are + * shared with other bus subscribers, so reshape immutably instead of + * mutating. + */ +export function omitSkillDetailsForSdkSurface< + T extends { type: string; data: unknown }, +>(event: T): T { + if (event.type !== 'session_update') return event; + const data = asRecord(event.data); + if (!data) return event; + // Two documented frame shapes: eventBus-wrapped (`data.update.*`) and + // persisted-transcript flat (`data.*`); the bridge's + // `transcriptEventRecordId` accepts both, so the redactor must too. + const wrapped = asRecord(data['update']); + const flat = !wrapped && data['sessionUpdate'] !== undefined; + const candidate = flat ? data : wrapped; + if ( + !candidate || + candidate['sessionUpdate'] !== 'available_commands_update' + ) { + return event; + } + const meta = asRecord(candidate['_meta']); + if (!meta || !('availableSkillDetails' in meta)) return event; + const trimmedMeta: Record = {}; + for (const [key, value] of Object.entries(meta)) { + if (key !== 'availableSkillDetails') trimmedMeta[key] = value; + } + const nextCandidate: Record = { ...candidate }; + if (Object.keys(trimmedMeta).length > 0) { + nextCandidate['_meta'] = trimmedMeta; + } else { + delete nextCandidate['_meta']; + } + if (flat) return { ...event, data: nextCandidate }; + return { ...event, data: { ...data, update: nextCandidate } }; +} + +/** + * `POST /session/:id/load` embeds the replay snapshot (compacted turns plus + * the in-flight journal) directly in the response body; apply the same + * redaction to those frames as the SSE egress does. + */ +export function omitSkillDetailsFromReplayArrays< + T extends { + compactedReplay?: BridgeEvent[]; + liveJournal?: BridgeEvent[]; + }, +>(session: T): T { + if (!session.compactedReplay && !session.liveJournal) return session; + return { + ...session, + ...(session.compactedReplay + ? { + compactedReplay: session.compactedReplay.map( + omitSkillDetailsForSdkSurface, + ), + } + : {}), + ...(session.liveJournal + ? { + liveJournal: session.liveJournal.map(omitSkillDetailsForSdkSurface), + } + : {}), + }; +} + +function asRecord(value: unknown): Record | undefined { + return typeof value === 'object' && value !== null && !Array.isArray(value) + ? (value as Record) + : undefined; +}