diff --git a/packages/acp-bridge/src/bridge.test.ts b/packages/acp-bridge/src/bridge.test.ts index df7862c110d..8c1f24d42f6 100644 --- a/packages/acp-bridge/src/bridge.test.ts +++ b/packages/acp-bridge/src/bridge.test.ts @@ -3856,6 +3856,77 @@ describe('createAcpSessionBridge', () => { await bridge.shutdown(); }); + it('does NOT reconcile when applyModelServiceId roundtrip fails on attach', async () => { + // F4oaj: the attach-time model apply (`applyModelServiceId`) gates + // reconcile on the same `succeeded` flag as `setSessionModel`. When the + // agent rejects `unstable_setSessionModel`, `publishModelSwitched` never + // runs and the cache is unchanged, so reconciliation must be skipped (no + // status read) — otherwise a corrective `model_switched` would be paired + // with the `model_switch_failed`. The agent's status deliberately drifts + // so any (incorrect) reconcile would produce an observable corrective. + let statusReads = 0; + const factory: ChannelFactory = async () => { + const { clientStream, agentStream } = createInMemoryChannel(); + const fakeAgent = new FakeAgent({ + extMethodImpl: (method) => { + if (method === 'qwen/status/session/context') { + statusReads += 1; + return Promise.resolve({ + state: { models: { currentModelId: 'qwen-turbo' } }, + }); + } + return Promise.resolve({}); + }, + }); + const augmented = new Proxy(fakeAgent, { + get(target, prop) { + if (prop === 'unstable_setSessionModel') { + return async () => { + throw new Error('agent denied'); + }; + } + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return (target as any)[prop]; + }, + }); + new AgentSideConnection(() => augmented as Agent, agentStream); + return { + stream: clientStream, + exited: new Promise< + | { exitCode: number | null; signalCode: NodeJS.Signals | null } + | undefined + >(() => {}), + kill: async () => {}, + killSync: () => {}, + }; + }; + const bridge = makeBridge({ channelFactory: factory }); + // Spawn WITHOUT a model so the only model apply is the failing one on the + // second attach (a spawn-time apply would succeed and legitimately read + // status, muddying the assertion). + const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + const abort = new AbortController(); + const iter = bridge.subscribeEvents(session.sessionId, { + signal: abort.signal, + }); + + // Attach with a model — the agent rejects it. The attach swallows the + // failure (shared session stays alive) and surfaces it as a bus event. + await bridge.spawnOrAttach({ + workspaceCwd: WS_A, + modelServiceId: 'rejected', + }); + + const it = iter[Symbol.asyncIterator](); + const failed = await it.next(); + expect(failed.value?.type).toBe('model_switch_failed'); + // Give any (incorrectly) scheduled reconcile a tick to fire. + await new Promise((r) => setTimeout(r, 10)); + expect(statusReads).toBe(0); + abort.abort(); + await bridge.shutdown(); + }); + it('serializes concurrent model-change calls (FIFO)', async () => { const callOrder: string[] = []; const factory: ChannelFactory = async () => { @@ -7566,6 +7637,1324 @@ describe('extractErrorCode', () => { }); }); +// --------------------------------------------------------------------------- +// §2.3 side-channel state layer: publish helpers + reconciliation + snapshot +// --------------------------------------------------------------------------- + +describe('createHttpAcpBridge — side-channel state layer (#4511)', () => { + describe('publish helpers cache + generation', () => { + it('publishModelSwitched updates cache and publishes model_switched', async () => { + const factory: ChannelFactory = async () => { + const { clientStream, agentStream } = createInMemoryChannel(); + const fakeAgent = new FakeAgent(); + const augmented = new Proxy(fakeAgent, { + get(target, prop) { + if (prop === 'unstable_setSessionModel') { + return async () => ({}); + } + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return (target as any)[prop]; + }, + }); + new AgentSideConnection(() => augmented as Agent, 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, + }); + + await bridge.setSessionModel( + session.sessionId, + { sessionId: session.sessionId, modelId: 'qwen-max' }, + undefined, + ); + + const it2 = iter[Symbol.asyncIterator](); + const next = await it2.next(); + expect(next.value?.type).toBe('model_switched'); + expect((next.value?.data as { modelId: string }).modelId).toBe( + 'qwen-max', + ); + abort.abort(); + await bridge.shutdown(); + }); + + it('publishApprovalModeChanged publishes approval_mode_changed on setSessionApprovalMode', async () => { + const factory: ChannelFactory = async () => { + const { clientStream, agentStream } = createInMemoryChannel(); + const agent = new FakeAgent({ + extMethodImpl: (method, params) => { + if (method === 'qwen/control/session/approval_mode') { + return Promise.resolve({ + previous: 'default', + current: (params as { mode: string }).mode, + }); + } + return Promise.resolve({}); + }, + }); + new AgentSideConnection(() => agent as Agent, 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, + }); + + await bridge.setSessionApprovalMode( + session.sessionId, + ApprovalMode.YOLO, + { persist: false }, + ); + + const it2 = iter[Symbol.asyncIterator](); + const next = await it2.next(); + expect(next.value?.type).toBe('approval_mode_changed'); + expect((next.value?.data as { next: string }).next).toBe( + ApprovalMode.YOLO, + ); + abort.abort(); + await bridge.shutdown(); + }); + }); + + describe('extNotification — in-session mode update (A2)', () => { + it('promotes current_mode_update to approval_mode_changed when no bridge roundtrip is in flight', 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/mode-update', { + v: 1, + sessionId: session.sessionId, + currentModeId: 'auto-edit', + }); + + 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('approval_mode_changed'); + expect((collected[0]?.data as { next: string }).next).toBe('auto-edit'); + abort.abort(); + await bridge.shutdown(); + }); + + it('suppresses current_mode_update while a bridge approval-mode roundtrip is in flight', async () => { + let releaseMode: (() => void) | undefined; + let capturedConn: AgentSideConnection | undefined; + const factory: ChannelFactory = async () => { + const { clientStream, agentStream } = createInMemoryChannel(); + const fakeAgent = new FakeAgent({ + extMethodImpl: (method) => { + if (method.includes('approval_mode')) { + return new Promise>((res) => { + releaseMode = () => + res({ previous: 'default', current: 'yolo' }); + }); + } + return {}; + }, + }); + capturedConn = new AgentSideConnection( + () => fakeAgent as Agent, + 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, + }); + + const modeChange = bridge + .setSessionApprovalMode(session.sessionId, ApprovalMode.YOLO, { + persist: false, + }) + .catch(() => {}); + await new Promise((r) => setTimeout(r, 10)); + + void capturedConn!.extNotification('qwen/notify/session/mode-update', { + v: 1, + sessionId: session.sessionId, + currentModeId: 'auto', + }); + await new Promise((r) => setTimeout(r, 10)); + + releaseMode!(); + await modeChange; + + const seen: string[] = []; + for await (const e of iter) { + seen.push(e.type); + if (e.type === 'approval_mode_changed') break; + } + // Only the bridge's own approval_mode_changed (yolo) — the suppressed + // 'auto' notification did NOT produce a second event. + expect(seen.filter((t) => t === 'approval_mode_changed')).toHaveLength(1); + abort.abort(); + await bridge.shutdown(); + }); + + it('drops malformed mode-update params without throwing', 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, + }); + + // Missing currentModeId + void capturedConn!.extNotification('qwen/notify/session/mode-update', { + v: 1, + sessionId: session.sessionId, + }); + // Non-string currentModeId + void capturedConn!.extNotification('qwen/notify/session/mode-update', { + v: 1, + sessionId: session.sessionId, + currentModeId: 42, + }); + await new Promise((r) => setTimeout(r, 50)); + + // No events should have been produced (no approval_mode_changed). + // Send a known good one to break the iterator. + void capturedConn!.extNotification('qwen/notify/session/model-update', { + v: 1, + sessionId: session.sessionId, + currentModelId: 'qwen-max', + }); + + const seen: string[] = []; + for await (const e of iter) { + seen.push(e.type); + if (e.type === 'model_switched') break; + } + expect(seen.filter((t) => t === 'approval_mode_changed')).toEqual([]); + abort.abort(); + await bridge.shutdown(); + }); + + it('drops a current_mode_update with an unknown mode id (enum guard)', async () => { + // The agent can reach this receive path without `Session.setMode`'s + // enum validation, so a bogus mode id must be dropped here before it + // fans out to SSE clients / the SDK reducer's state.approvalMode. + 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, + }); + + // Well-formed string, but not a known approval mode. + void capturedConn!.extNotification('qwen/notify/session/mode-update', { + v: 1, + sessionId: session.sessionId, + currentModeId: 'totally-bogus', + }); + await new Promise((r) => setTimeout(r, 50)); + + // A known good model-update breaks the iterator; the bogus mode must + // not have produced an approval_mode_changed (or a legacy dual-emit). + void capturedConn!.extNotification('qwen/notify/session/model-update', { + v: 1, + sessionId: session.sessionId, + currentModelId: 'qwen-max', + }); + + const seen: string[] = []; + for await (const e of iter) { + seen.push(e.type); + if (e.type === 'model_switched') break; + } + expect(seen.filter((t) => t === 'approval_mode_changed')).toEqual([]); + expect(seen.filter((t) => t === 'session_update')).toEqual([]); + abort.abort(); + await bridge.shutdown(); + }); + + it('dual-emits a legacy session_update on the setMode path (no legacyFrameSent)', async () => { + // The ACP `session/set_mode` path has no `sendUpdate`, so the demux + // owns the IDE-companion compat frame: one approval_mode_changed plus + // one legacy session_update{current_mode_update}. + 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/mode-update', { + v: 1, + sessionId: session.sessionId, + currentModeId: 'auto-edit', + }); + + const collected: Array<{ type: string; data: unknown }> = []; + for await (const e of iter) { + collected.push({ type: e.type, data: e.data }); + if (e.type === 'session_update') break; + } + expect(collected.map((c) => c.type)).toEqual([ + 'approval_mode_changed', + 'session_update', + ]); + // Canonical ACP-nested shape so the companion's standard + // data.update.sessionUpdate switch recognises it. + const update = ( + collected[1]?.data as { + update?: { sessionUpdate?: string; currentModeId?: string }; + } + ).update; + expect(update?.sessionUpdate).toBe('current_mode_update'); + expect(update?.currentModeId).toBe('auto-edit'); + abort.abort(); + await bridge.shutdown(); + }); + + it('suppresses the legacy dual-emit when legacyFrameSent is true (exit_plan_mode path)', async () => { + // `Session.sendCurrentModeUpdateNotification` already published the + // legacy session_update via `sendUpdate` before this extNotification, + // so the demux must promote to approval_mode_changed only — emitting + // its own dual-emit would deliver the legacy frame to the companion + // twice for one mode change. + 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/mode-update', { + v: 1, + sessionId: session.sessionId, + currentModeId: 'auto-edit', + legacyFrameSent: true, + }); + await new Promise((r) => setTimeout(r, 50)); + + // A known good model-update breaks the iterator; assert exactly one + // approval_mode_changed and NO legacy session_update from this path. + void capturedConn!.extNotification('qwen/notify/session/model-update', { + v: 1, + sessionId: session.sessionId, + currentModelId: 'qwen-max', + }); + + const seen: string[] = []; + for await (const e of iter) { + seen.push(e.type); + if (e.type === 'model_switched') break; + } + expect(seen.filter((t) => t === 'approval_mode_changed')).toHaveLength(1); + expect(seen.filter((t) => t === 'session_update')).toEqual([]); + abort.abort(); + await bridge.shutdown(); + }); + }); + + describe('A5 — session snapshot on attach', () => { + it('yields session_snapshot after replay_complete when snapshot=true', 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 }); + + // Promote a model change to populate the cache. + void capturedConn!.extNotification('qwen/notify/session/model-update', { + v: 1, + sessionId: session.sessionId, + currentModelId: 'qwen-turbo', + }); + await new Promise((r) => setTimeout(r, 20)); + + // Subscribe with snapshot=true (triggers replay_complete + snapshot). + const abort = new AbortController(); + const iter = bridge.subscribeEvents(session.sessionId, { + signal: abort.signal, + lastEventId: 0, + snapshot: true, + }); + + const collected: BridgeEvent[] = []; + for await (const e of iter) { + collected.push(e); + if (e.type === 'session_snapshot') break; + } + const rc = collected.find((e) => e.type === 'replay_complete'); + const snap = collected.find((e) => e.type === 'session_snapshot'); + expect(rc).toBeDefined(); + expect(snap).toBeDefined(); + expect(collected.indexOf(snap!)).toBeGreaterThan(collected.indexOf(rc!)); + expect( + (snap!.data as { currentModelId: string | null }).currentModelId, + ).toBe('qwen-turbo'); + abort.abort(); + await bridge.shutdown(); + }); + + it('carries currentApprovalMode in the snapshot when an approval-mode change was promoted', async () => { + // The other A5 tests only seed currentModelId, so the + // publishApprovalModeChanged → entry.currentApprovalMode → snapshot + // pipeline is otherwise untested at the bridge level: a typo writing + // the wrong field would leave currentApprovalMode null and slip past. + 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 }); + + // Promote an in-session mode change to populate currentApprovalMode + // (flows through onModePromoted → publishApprovalModeChanged). + void capturedConn!.extNotification('qwen/notify/session/mode-update', { + v: 1, + sessionId: session.sessionId, + currentModeId: 'auto-edit', + }); + await new Promise((r) => setTimeout(r, 20)); + + const abort = new AbortController(); + const iter = bridge.subscribeEvents(session.sessionId, { + signal: abort.signal, + lastEventId: 0, + snapshot: true, + }); + + const collected: BridgeEvent[] = []; + for await (const e of iter) { + collected.push(e); + if (e.type === 'session_snapshot') break; + } + const snap = collected.find((e) => e.type === 'session_snapshot'); + expect(snap).toBeDefined(); + expect( + (snap!.data as { currentApprovalMode: string | null }) + .currentApprovalMode, + ).toBe('auto-edit'); + abort.abort(); + await bridge.shutdown(); + }); + + it('does NOT yield session_snapshot when snapshot is not set', 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 }); + + // Promote a model change so there IS cache state. + void capturedConn!.extNotification('qwen/notify/session/model-update', { + v: 1, + sessionId: session.sessionId, + currentModelId: 'qwen-turbo', + }); + await new Promise((r) => setTimeout(r, 20)); + + // Subscribe WITHOUT snapshot. + const abort = new AbortController(); + const iter = bridge.subscribeEvents(session.sessionId, { + signal: abort.signal, + lastEventId: 0, + }); + + // After replay_complete, send a known event to break the loop. + const collected: BridgeEvent[] = []; + // Publish something after a short delay so the iterator eventually yields. + setTimeout(() => { + void capturedConn!.extNotification('qwen/notify/session/model-update', { + v: 1, + sessionId: session.sessionId, + currentModelId: 'qwen-max', + }); + }, 30); + + for await (const e of iter) { + collected.push(e); + // Stop after we see replay_complete + one more real event. + if ( + collected.some((c) => c.type === 'replay_complete') && + collected.some((c) => c.type === 'model_switched') + ) + break; + } + expect( + collected.find((e) => e.type === 'session_snapshot'), + ).toBeUndefined(); + abort.abort(); + await bridge.shutdown(); + }); + + it('yields session_snapshot up front on a fresh connection (no Last-Event-ID)', async () => { + // Regression for the A5 primary use case: a fresh attach has no + // `Last-Event-ID`, so the bus never emits `replay_complete` (the whole + // replay block is gated on `lastEventId !== undefined`). Keying the + // snapshot solely off `replay_complete` made it silently no-op exactly + // when a client most needs to seed state — on initial attach. + 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 }); + + void capturedConn!.extNotification('qwen/notify/session/model-update', { + v: 1, + sessionId: session.sessionId, + currentModelId: 'qwen-turbo', + }); + await new Promise((r) => setTimeout(r, 20)); + + // Fresh subscribe — snapshot=true, NO lastEventId. + const abort = new AbortController(); + const iter = bridge.subscribeEvents(session.sessionId, { + signal: abort.signal, + snapshot: true, + }); + + const it2 = iter[Symbol.asyncIterator](); + const first = await it2.next(); + // The very first frame must be the snapshot (no replay precedes it). + expect(first.value?.type).toBe('session_snapshot'); + expect( + (first.value?.data as { currentModelId: string | null }).currentModelId, + ).toBe('qwen-turbo'); + abort.abort(); + await bridge.shutdown(); + }); + + it('seeds snapshot from newSession response without any intermediate notification (cold attach)', async () => { + // F7qEJ: seedSnapshotCaches fills the cache from the newSession + // response alone — no extNotification or setSessionModel needed. + const factory: ChannelFactory = async () => { + const { clientStream, agentStream } = createInMemoryChannel(); + const fakeAgent = new FakeAgent({ + newSessionImpl: (p) => + Promise.resolve({ + sessionId: `sess:${p.cwd}`, + models: { + currentModelId: 'qwen-plus', + availableModels: [{ modelId: 'qwen-plus', name: 'Qwen Plus' }], + }, + modes: { + currentModeId: 'auto-edit', + availableModes: [ + { modeId: 'auto-edit', id: 'auto-edit', name: 'Auto Edit' }, + ], + }, + }), + }); + new AgentSideConnection(() => fakeAgent as Agent, 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 }); + + // Subscribe with snapshot=true, no lastEventId — pure cold attach. + const abort = new AbortController(); + const iter = bridge.subscribeEvents(session.sessionId, { + signal: abort.signal, + snapshot: true, + }); + const it2 = iter[Symbol.asyncIterator](); + const first = await it2.next(); + expect(first.value?.type).toBe('session_snapshot'); + const data = first.value?.data as { + currentModelId: string | null; + currentApprovalMode: string | null; + }; + expect(data.currentModelId).toBe('qwen-plus'); + expect(data.currentApprovalMode).toBe('auto-edit'); + abort.abort(); + await bridge.shutdown(); + }); + }); + + describe('§2.2 — post-roundtrip reconciliation', () => { + const makeReconcileFactory = + ( + sessionContextModelId: string | undefined, + opts: { throwOnStatus?: boolean } = {}, + ): ChannelFactory => + async () => { + const { clientStream, agentStream } = createInMemoryChannel(); + const fakeAgent = new FakeAgent({ + extMethodImpl: (method) => { + if (method === 'qwen/status/session/context') { + if (opts.throwOnStatus) { + throw new Error('status read failed'); + } + return Promise.resolve({ + state: { models: { currentModelId: sessionContextModelId } }, + }); + } + return Promise.resolve({}); + }, + }); + const augmented = new Proxy(fakeAgent, { + get(target, prop) { + if (prop === 'unstable_setSessionModel') { + return async () => ({}); + } + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return (target as any)[prop]; + }, + }); + new AgentSideConnection(() => augmented as Agent, agentStream); + return { + stream: clientStream, + exited: new Promise< + | { exitCode: number | null; signalCode: NodeJS.Signals | null } + | undefined + >(() => {}), + kill: async () => {}, + killSync: () => {}, + }; + }; + + it('publishes a corrective model_switched when the agent state drifted from cache', async () => { + // Switch to qwen-max, but the agent's real state is qwen-turbo (e.g. + // an agent-side override). Reconciliation must emit a corrective + // model_switched so peers converge on the agent's truth. + const bridge = makeBridge({ + channelFactory: makeReconcileFactory('qwen-turbo'), + }); + const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + const abort = new AbortController(); + const iter = bridge.subscribeEvents(session.sessionId, { + signal: abort.signal, + }); + + await bridge.setSessionModel( + session.sessionId, + { sessionId: session.sessionId, modelId: 'qwen-max' }, + undefined, + ); + + const seen: Array<{ type: string; modelId?: string }> = []; + for await (const e of iter) { + seen.push({ + type: e.type, + modelId: (e.data as { modelId?: string })?.modelId, + }); + if (seen.filter((s) => s.type === 'model_switched').length === 2) break; + } + const switches = seen.filter((s) => s.type === 'model_switched'); + // First the requested change, then the corrective one from reconcile. + expect(switches[0]?.modelId).toBe('qwen-max'); + expect(switches[1]?.modelId).toBe('qwen-turbo'); + abort.abort(); + await bridge.shutdown(); + }); + + it('does NOT publish a corrective event when agent state matches cache', async () => { + // Stateful agent: `sessionContext` echoes the last model the bridge + // set, so reconciliation always finds cache == agent truth and never + // emits a corrective. Two distinct changes must therefore produce + // exactly two model_switched events, with no duplicates in between. + let lastModel: string | undefined; + const factory: ChannelFactory = async () => { + const { clientStream, agentStream } = createInMemoryChannel(); + const fakeAgent = new FakeAgent({ + extMethodImpl: (method) => { + if (method === 'qwen/status/session/context') { + return Promise.resolve({ + state: { models: { currentModelId: lastModel } }, + }); + } + return Promise.resolve({}); + }, + }); + const augmented = new Proxy(fakeAgent, { + get(target, prop) { + if (prop === 'unstable_setSessionModel') { + return async (p: { modelId: string }) => { + lastModel = p.modelId; + return {}; + }; + } + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return (target as any)[prop]; + }, + }); + new AgentSideConnection(() => augmented as Agent, 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, + }); + + await bridge.setSessionModel( + session.sessionId, + { sessionId: session.sessionId, modelId: 'qwen-max' }, + undefined, + ); + // Second distinct change terminates the iterator; a spurious + // corrective would surface as a duplicate model_switched. + await bridge.setSessionModel( + session.sessionId, + { sessionId: session.sessionId, modelId: 'qwen-plus' }, + undefined, + ); + + const switches: string[] = []; + for await (const e of iter) { + if (e.type === 'model_switched') { + switches.push((e.data as { modelId: string }).modelId); + if (switches.includes('qwen-plus')) break; + } + } + expect(switches).toEqual(['qwen-max', 'qwen-plus']); + abort.abort(); + await bridge.shutdown(); + }); + + it('swallows a failed status read without crashing or masking the original change', async () => { + const bridge = makeBridge({ + channelFactory: makeReconcileFactory(undefined, { + throwOnStatus: true, + }), + }); + const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + const abort = new AbortController(); + const iter = bridge.subscribeEvents(session.sessionId, { + signal: abort.signal, + }); + + await expect( + bridge.setSessionModel( + session.sessionId, + { sessionId: session.sessionId, modelId: 'qwen-max' }, + undefined, + ), + ).resolves.toBeDefined(); + + const it2 = iter[Symbol.asyncIterator](); + const next = await it2.next(); + // The original model_switched is delivered; reconcile failure stays in + // the operator log (no bus event the SDK cannot decode). + expect(next.value?.type).toBe('model_switched'); + expect((next.value?.data as { modelId: string }).modelId).toBe( + 'qwen-max', + ); + abort.abort(); + await bridge.shutdown(); + }); + + it('does NOT reconcile when the model roundtrip itself fails', async () => { + // The agent's unstable_setSessionModel rejects, so publishModelSwitched + // never runs and the cache is unchanged. Reconciliation must be skipped + // (no status read), and the only bus event is model_switch_failed — + // never a corrective model_switched paired with the failure. + let statusReads = 0; + const factory: ChannelFactory = async () => { + const { clientStream, agentStream } = createInMemoryChannel(); + const fakeAgent = new FakeAgent({ + extMethodImpl: (method) => { + if (method === 'qwen/status/session/context') { + statusReads += 1; + return Promise.resolve({ + state: { models: { currentModelId: 'qwen-turbo' } }, + }); + } + return Promise.resolve({}); + }, + }); + const augmented = new Proxy(fakeAgent, { + get(target, prop) { + if (prop === 'unstable_setSessionModel') { + return async () => { + throw new Error('agent refused model switch'); + }; + } + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return (target as any)[prop]; + }, + }); + new AgentSideConnection(() => augmented as Agent, 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, + }); + + await expect( + bridge.setSessionModel( + session.sessionId, + { sessionId: session.sessionId, modelId: 'qwen-max' }, + undefined, + ), + ).rejects.toThrow(); + + const it2 = iter[Symbol.asyncIterator](); + const next = await it2.next(); + expect(next.value?.type).toBe('model_switch_failed'); + // Give any (incorrectly) scheduled reconcile a tick to fire. + await new Promise((r) => setTimeout(r, 10)); + expect(statusReads).toBe(0); + abort.abort(); + await bridge.shutdown(); + }); + + it('re-runs reconcile when a newer change publishes during the status read (generation rerun)', async () => { + // Anti-lost-reconcile: while reconcile for change A awaits its status + // RPC, a second change B publishes (bumping the generation). B's own + // reconcile bails on the in-flight guard, so without the `rerun` path + // B would never be reconciled. Gate the FIRST status read until B has + // published; assert the FIRST read is discarded (generation changed) + // and a SECOND read fires after the guard releases, whose corrective + // reflects the agent's truth read AFTER B — not a stale read for A. + let statusReads = 0; + let releaseFirstStatus: (() => void) | undefined; + const firstStatusGate = new Promise((res) => { + releaseFirstStatus = res; + }); + const factory: ChannelFactory = async () => { + const { clientStream, agentStream } = createInMemoryChannel(); + const fakeAgent = new FakeAgent({ + extMethodImpl: (method) => { + if (method === 'qwen/status/session/context') { + statusReads += 1; + // Agent truth drifts from both A and B, so the post-rerun + // read produces an observable corrective. + const payload = { + state: { models: { currentModelId: 'qwen-turbo' } }, + }; + return statusReads === 1 + ? firstStatusGate.then(() => payload) + : Promise.resolve(payload); + } + return Promise.resolve({}); + }, + }); + const augmented = new Proxy(fakeAgent, { + get(target, prop) { + if (prop === 'unstable_setSessionModel') { + return async () => ({}); + } + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return (target as any)[prop]; + }, + }); + new AgentSideConnection(() => augmented as Agent, 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, + }); + + // A: publishes gen=1; its reconcile starts and blocks on the gate. + await bridge.setSessionModel( + session.sessionId, + { sessionId: session.sessionId, modelId: 'qwen-max' }, + undefined, + ); + // B: publishes gen=2 while A's reconcile is still awaiting the gated + // status read; B's own reconcile bails on the in-flight guard. + await bridge.setSessionModel( + session.sessionId, + { sessionId: session.sessionId, modelId: 'qwen-plus' }, + undefined, + ); + // Now let A's status read resolve — it must detect the generation + // change, discard its (stale) read, and re-run. + releaseFirstStatus!(); + + const switches: string[] = []; + for await (const e of iter) { + if (e.type === 'model_switched') { + switches.push((e.data as { modelId: string }).modelId); + if (switches.includes('qwen-turbo')) break; + } + } + // The two requested changes, then ONE corrective from the rerun. + expect(switches).toEqual(['qwen-max', 'qwen-plus', 'qwen-turbo']); + // Two reads total: the gated (discarded) one + the rerun. + expect(statusReads).toBe(2); + abort.abort(); + await bridge.shutdown(); + }); + + it('publishes a corrective approval_mode_changed when the agent mode drifted from cache', async () => { + // approvalMode analog of the model drift test. The bridge sets YOLO, + // but the agent's real mode is `plan` (e.g. an agent-side exit_plan_mode + // restore). Reconciliation reads `state.modes.currentModeId` — a + // DIFFERENT status shape from the model branch — and must emit a + // corrective approval_mode_changed with next:'plan' so peers converge + // on the agent's truth. + const factory: ChannelFactory = async () => { + const { clientStream, agentStream } = createInMemoryChannel(); + const fakeAgent = new FakeAgent({ + extMethodImpl: (method, params) => { + if (method === 'qwen/control/session/approval_mode') { + return Promise.resolve({ + previous: 'default', + current: (params as { mode: string }).mode, + }); + } + if (method === 'qwen/status/session/context') { + return Promise.resolve({ + state: { modes: { currentModeId: 'plan' } }, + }); + } + return Promise.resolve({}); + }, + }); + new AgentSideConnection(() => fakeAgent as Agent, 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, + }); + + await bridge.setSessionApprovalMode( + session.sessionId, + ApprovalMode.YOLO, + { persist: false }, + undefined, + ); + + const nexts: string[] = []; + for await (const e of iter) { + if (e.type === 'approval_mode_changed') { + nexts.push((e.data as { next: string }).next); + if (nexts.length === 2) break; + } + } + // First the requested change, then the corrective one from reconcile. + expect(nexts[0]).toBe('yolo'); + expect(nexts[1]).toBe('plan'); + abort.abort(); + await bridge.shutdown(); + }); + + it('does NOT reconcile when the approval-mode roundtrip itself fails', async () => { + // approvalMode analog of the model roundtrip-fail test. The agent's + // approval_mode ext rejects, so publishApprovalModeChanged never runs + // and the cache is unchanged. Reconciliation must be skipped (no status + // read) and no corrective approval_mode_changed must reach the bus. + let statusReads = 0; + const factory: ChannelFactory = async () => { + const { clientStream, agentStream } = createInMemoryChannel(); + const fakeAgent = new FakeAgent({ + extMethodImpl: (method) => { + if (method === 'qwen/control/session/approval_mode') { + throw new Error('agent refused approval-mode switch'); + } + if (method === 'qwen/status/session/context') { + statusReads += 1; + return Promise.resolve({ + state: { modes: { currentModeId: 'plan' } }, + }); + } + return Promise.resolve({}); + }, + }); + new AgentSideConnection(() => fakeAgent as Agent, 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, + }); + const nexts: string[] = []; + const collecting = (async () => { + for await (const e of iter) { + if (e.type === 'approval_mode_changed') { + nexts.push((e.data as { next: string }).next); + } + } + })(); + + await expect( + bridge.setSessionApprovalMode( + session.sessionId, + ApprovalMode.YOLO, + { persist: false }, + undefined, + ), + ).rejects.toThrow(); + + // Give any (incorrectly) scheduled reconcile a tick to fire. + await new Promise((r) => setTimeout(r, 10)); + expect(statusReads).toBe(0); + expect(nexts).toEqual([]); + abort.abort(); + await collecting; + await bridge.shutdown(); + }); + + it('drops unknown agent-returned approval mode without publishing a corrective event', async () => { + // F7qEL / F8E2h: when the agent returns a mode not in + // KNOWN_APPROVAL_MODES, the approvalMode reconcile branch should + // drop it (action=dropped reason=unknown_mode) instead of + // broadcasting an invalid approval_mode_changed. We trigger the + // approvalMode reconcile via setSessionApprovalMode (not via + // modelServiceId, which only reconciles the model branch). + let statusReads = 0; + const factory: ChannelFactory = async () => { + const { clientStream, agentStream } = createInMemoryChannel(); + const fakeAgent = new FakeAgent({ + extMethodImpl: (method, params) => { + if (method === 'qwen/control/session/approval_mode') { + return Promise.resolve({ + previous: 'default', + current: (params as { mode: string }).mode, + }); + } + if (method === 'qwen/status/session/context') { + statusReads += 1; + // Agent claims a mode that's NOT in KNOWN_APPROVAL_MODES. + return Promise.resolve({ + state: { modes: { currentModeId: 'super-yolo' } }, + }); + } + return Promise.resolve({}); + }, + }); + new AgentSideConnection(() => fakeAgent as Agent, 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, + }); + const modeEvents: string[] = []; + const collecting = (async () => { + for await (const e of iter) { + if (e.type === 'approval_mode_changed') { + modeEvents.push((e.data as { next: string }).next); + } + } + })(); + + // setSessionApprovalMode triggers reconcileAfterRoundtrip(entry, + // 'approvalMode'). The status read returns 'super-yolo' which + // isn't in KNOWN_APPROVAL_MODES — reconcile must DROP it. + await bridge.setSessionApprovalMode( + session.sessionId, + ApprovalMode.YOLO, + { persist: false }, + undefined, + ); + + // Wait for reconcile to fire (async microtask chain). + await new Promise((r) => setTimeout(r, 50)); + // Positive assertion: reconcile DID execute (status was read). + // Without this, a future refactor that disables reconcile would + // make the modeEvents assertion pass vacuously. + expect(statusReads).toBe(1); + // Only the original mode change should appear — no corrective + // for the unknown 'super-yolo' value from the agent. + expect(modeEvents).toEqual(['yolo']); + abort.abort(); + await collecting; + await bridge.shutdown(); + }); + + it('syncs peer session cache on persisted approval-mode change (snapshot reflects new mode)', async () => { + // F7qEK: when session A persists a mode change, peer session B's + // cache should be updated so a subsequent snapshot on B reports + // the new workspace default — not the stale pre-change value. + const factory: ChannelFactory = async () => { + const { clientStream, agentStream } = createInMemoryChannel(); + const fakeAgent = new FakeAgent({ + extMethodImpl: (method, params) => { + if (method === 'qwen/control/session/approval_mode') { + return Promise.resolve({ + previous: 'default', + current: (params as { mode: string }).mode, + }); + } + if (method === 'qwen/status/session/context') { + // Status RPC returns agent's authoritative mode. After the + // persist, the agent is on 'yolo' — return it so reconcile + // sees no drift and does not emit a corrective. + return Promise.resolve({ + state: { modes: { currentModeId: 'yolo' } }, + }); + } + return Promise.resolve({}); + }, + }); + new AgentSideConnection(() => fakeAgent as Agent, agentStream); + return { + stream: clientStream, + exited: new Promise< + | { exitCode: number | null; signalCode: NodeJS.Signals | null } + | undefined + >(() => {}), + kill: async () => {}, + killSync: () => {}, + }; + }; + const bridge = makeBridge({ + channelFactory: factory, + sessionScope: 'thread', + persistApprovalMode: async () => {}, + }); + // Two sessions in the same workspace (thread scope → each attach + // creates a new session). + const sessionA = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + const sessionB = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + expect(sessionA.sessionId).not.toBe(sessionB.sessionId); + + // Persist a mode change on A. + await bridge.setSessionApprovalMode( + sessionA.sessionId, + ApprovalMode.YOLO, + { persist: true }, + undefined, + ); + + // Subscribe on B with snapshot — should reflect the persisted mode. + const abort = new AbortController(); + const iter = bridge.subscribeEvents(sessionB.sessionId, { + signal: abort.signal, + snapshot: true, + }); + const it2 = iter[Symbol.asyncIterator](); + const first = await it2.next(); + expect(first.value?.type).toBe('session_snapshot'); + expect( + (first.value?.data as { currentApprovalMode: string | null }) + .currentApprovalMode, + ).toBe('yolo'); + abort.abort(); + await bridge.shutdown(); + }); + }); +}); + describe('channelIdleTimeoutMs', () => { it('kills the channel immediately when timeout is 0 (default)', async () => { const handle = makeChannel(); diff --git a/packages/acp-bridge/src/bridge.ts b/packages/acp-bridge/src/bridge.ts index f5a374f5264..dc122e9d897 100644 --- a/packages/acp-bridge/src/bridge.ts +++ b/packages/acp-bridge/src/bridge.ts @@ -26,7 +26,12 @@ import { } from '@qwen-code/qwen-code-core'; import type { ShellCommandResult } from './bridgeTypes.js'; import type { AcpChannel } from './channel.js'; -import { EventBus, DEFAULT_RING_SIZE, type BridgeEvent } from './eventBus.js'; +import { + EventBus, + DEFAULT_RING_SIZE, + EVENT_SCHEMA_VERSION, + type BridgeEvent, +} from './eventBus.js'; import { TurnBoundaryCompactionEngine } from './compactionEngine.js'; import { BridgeChannelClosedError, @@ -37,6 +42,7 @@ import { SERVE_STATUS_EXT_METHODS, STATUS_SCHEMA_VERSION, type ServeSessionStatsStatus, + type ServeSessionContextStatus, type ServeSessionTasksStatus, } from './status.js'; import { @@ -72,7 +78,7 @@ import type { BridgeOptions, BridgeTelemetry } from './bridgeOptions.js'; import { MCP_RESTART_SERVER_DEADLINE_MS } from './mcpTimeouts.js'; import { defaultSpawnChannelFactory } from './spawnChannel.js'; import { writeStderrLine } from './internal/stderrLine.js'; -import { BridgeClient } from './bridgeClient.js'; +import { BridgeClient, KNOWN_APPROVAL_MODES } from './bridgeClient.js'; import { CANCEL_VOTE_SENTINEL, createNoOpPermissionAuditPublisher, @@ -234,6 +240,20 @@ interface SessionEntry { * `/model` (no bridge roundtrip) sees this false and IS promoted. */ modelRoundtripInFlight?: boolean; + /** A2: true while the bridge drives an approval-mode roundtrip. */ + approvalModeRoundtripInFlight?: boolean; + /** §2.3: cached model id, updated by every `publishModelSwitched` call. */ + currentModelId?: string; + /** §2.3: cached approval mode, updated by every `publishApprovalModeChanged` call. */ + currentApprovalMode?: string; + /** §2.3: monotonic counter bumped on every `model_switched` publish. */ + modelPublishGeneration: number; + /** §2.3: monotonic counter bumped on every `approval_mode_changed` publish. */ + approvalModePublishGeneration: number; + /** §2.2: true while a model reconciliation read is in flight. */ + modelReconciliationInFlight?: boolean; + /** §2.2: true while an approval-mode reconciliation read is in flight. */ + approvalModeReconciliationInFlight?: boolean; /** * Per-session approval-mode FIFO. Mirrors `modelChangeQueue`: * serializes concurrent `setSessionApprovalMode` calls so two @@ -820,6 +840,7 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { }, timeoutMs); idleTimer.unref(); } + // BkUyD: superset of `channelInfo` covering channels // that are dying but not yet OS-reaped. `killSession` / // `doSpawn`-newSession-failure / `shutdown` mark a channel as @@ -1048,6 +1069,24 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { // consumers + channels / IDE companion omit it; BridgeClient // falls back to its inline fs proxy. opts.fileSystem, + // §2.3: centralised model_switched publish — keeps cache + generation + // update atomic. BridgeClient calls this instead of inlining publish. + (entry, modelId, originator) => + publishModelSwitched(entry as SessionEntry, modelId, originator), + // A2: centralised approval_mode_changed publish on in-session mode + // promotion. `previous` is read from the bridge state cache. + (entry, modeId, originator) => { + const se = entry as SessionEntry; + publishApprovalModeChanged( + se, + { + previous: se.currentApprovalMode ?? 'default', + next: modeId, + persisted: false, + }, + originator, + ); + }, ); const connection = new ClientSideConnection(() => client, channel.stream); @@ -1267,7 +1306,11 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { // `ensureChannel`, never spawning a fresh one. Tear down the // empty channel so the next attempt gets a clean spawn. const ci = await ensureChannel(); - let newSessionResp: { sessionId: string }; + let newSessionResp: { + sessionId: string; + models?: { currentModelId?: unknown } | null; + modes?: { currentModeId?: unknown } | null; + }; try { newSessionResp = await telemetry.withSpan( 'session.new', @@ -1316,6 +1359,7 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { newSessionResp.sessionId, boundWorkspace, ); + seedSnapshotCaches(entry, newSessionResp); const clientId = registerClient(entry, requestedClientId); // `defaultEntry` is the single-scope attach target — only sessions // SPAWNED UNDER `'single'` may claim it. A thread-scope spawn must @@ -1402,6 +1446,11 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { // `Session.setModel`, which emits it) is suppressed by the demux — // the authoritative `model_switched` is published below. entry.modelRoundtripInFlight = true; + // Mirror setSessionModel: only reconcile after a change that landed. A + // rejected roundtrip leaves the cache unchanged (often still unset on + // the create/attach path), so reconciling would emit a corrective + // model_switched right beside the model_switch_failed below. + let succeeded = false; try { await Promise.race([ withTimeout( @@ -1414,15 +1463,13 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { ), transportClosed, ]); - entry.events.publish({ - type: 'model_switched', - data: { sessionId: entry.sessionId, modelId }, - ...(originatorClientId ? { originatorClientId } : {}), - }); + publishModelSwitched(entry, modelId, originatorClientId); + succeeded = true; } catch (err) { // Surface the failure to ALL attached clients, not just the // caller — a shared session swallowing a denied model change - // silently would surprise the others. + // silently would surprise the others. `publish()` never throws + // (see `publishModelSwitched`), so no wrapper. entry.events.publish({ type: 'model_switch_failed', data: { @@ -1435,6 +1482,13 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { throw err; } finally { entry.modelRoundtripInFlight = false; + if (succeeded) { + void reconcileAfterRoundtrip(entry, 'model'); + } else { + writeStderrLine( + `[reconcile] session=${entry.sessionId} target=model action=skipped reason=roundtrip_failed`, + ); + } } }); // Tail swallows failures so subsequent model changes still run; the @@ -1679,6 +1733,152 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { const createSessionEventBus = (): EventBus => new EventBus(eventRingSize, undefined, new TurnBoundaryCompactionEngine()); + // §2.3 publish helpers — centralise cache + generation + bus publish so + // every `model_switched` / `approval_mode_changed` site stays atomic. + + const publishModelSwitched = ( + entry: SessionEntry, + modelId: string, + originatorClientId: string | undefined, + ): void => { + entry.currentModelId = modelId; + entry.modelPublishGeneration++; + // `EventBus.publish` never throws (a closed bus is a return-undefined + // no-op); per its documented contract we don't wrap it — a try/catch + // here would be dead code for "bus closed" and would mislabel a real + // programming error (e.g. a `TypeError`) as a benign bus-closed swallow. + entry.events.publish({ + type: 'model_switched', + data: { sessionId: entry.sessionId, modelId }, + ...(originatorClientId ? { originatorClientId } : {}), + }); + }; + + const publishApprovalModeChanged = ( + entry: SessionEntry, + payload: { previous: string; next: string; persisted: boolean }, + originatorClientId: string | undefined, + ): void => { + entry.currentApprovalMode = payload.next; + entry.approvalModePublishGeneration++; + // See `publishModelSwitched`: `publish()` never throws, so no wrapper. + entry.events.publish({ + type: 'approval_mode_changed', + data: { + sessionId: entry.sessionId, + previous: payload.previous, + next: payload.next, + persisted: payload.persisted, + }, + ...(originatorClientId ? { originatorClientId } : {}), + }); + }; + + // §2.2 post-roundtrip reconciliation — after a bridge-driven model or + // approval-mode change settles, re-read the agent's actual state and + // emit a corrective event if it drifted from the cached value. + const reconcileAfterRoundtrip = async ( + entry: SessionEntry, + target: 'model' | 'approvalMode', + ): Promise => { + const flagKey = + target === 'model' + ? 'modelReconciliationInFlight' + : 'approvalModeReconciliationInFlight'; + const genOf = () => + target === 'model' + ? entry.modelPublishGeneration + : entry.approvalModePublishGeneration; + if (entry[flagKey]) return; + entry[flagKey] = true; + const genBefore = genOf(); + // Set when a newer change published while our status read was in + // flight; we re-run once after releasing the guard (see `finally`). + let rerun = false; + try { + const status = await requestSessionStatus( + entry.sessionId, + SERVE_STATUS_EXT_METHODS.sessionContext, + ); + if (genOf() !== genBefore) { + // A newer change published during our RPC; its own + // `reconcileAfterRoundtrip` bailed on the in-flight guard above, + // so without a re-run the latest change would never be + // reconciled. Skip this (now-stale) read and re-run once. The + // re-run is gated on this generation-change signal — NOT on a + // bare `genOf() !== genBefore` at `finally` time — because a + // corrective publish below bumps the generation itself and would + // otherwise self-trigger an unbounded reconcile loop. + rerun = true; + writeStderrLine( + `[reconcile] session=${entry.sessionId} target=${target} action=skipped reason=generation_changed genBefore=${genBefore} genAfter=${genOf()}`, + ); + return; + } + + if (target === 'model') { + const actual = ( + status?.state?.models as { currentModelId?: string } | undefined + )?.currentModelId; + if ( + typeof actual === 'string' && + actual && + actual !== entry.currentModelId + ) { + writeStderrLine( + `[reconcile] session=${entry.sessionId} target=model action=corrected cached=${entry.currentModelId ?? ''} actual=${actual}`, + ); + publishModelSwitched(entry, actual, undefined); + } + } else { + const actual = ( + status?.state?.modes as { currentModeId?: string } | undefined + )?.currentModeId; + // Same enum backstop as the demux path (`handleInSessionModeUpdate`): + // `actual` is an agent-supplied id typed `unknown`, and the SDK's + // `isApprovalModeChangedData` is a structural check (deliberately + // forward-compatible with a future 5th mode), NOT an enum gate. An + // unknown id here would fan out to every SSE client and land in the + // reducer's `state.approvalMode`, so drop it before publishing. + if (actual && !KNOWN_APPROVAL_MODES.has(actual)) { + writeStderrLine( + `[reconcile] session=${entry.sessionId} target=approvalMode action=dropped reason=unknown_mode mode=${actual}`, + ); + } else if (actual && actual !== entry.currentApprovalMode) { + writeStderrLine( + `[reconcile] session=${entry.sessionId} target=approvalMode action=corrected cached=${entry.currentApprovalMode ?? ''} actual=${actual}`, + ); + publishApprovalModeChanged( + entry, + { + previous: entry.currentApprovalMode ?? 'default', + next: actual, + persisted: false, + }, + undefined, + ); + } + } + } catch (err) { + // The status read failed — drift can be neither confirmed nor + // corrected. Keep the signal in the operator log rather than + // emitting a bus event no client can decode: `reconciliation_failed` + // is not a known SDK event type, so `asKnownDaemonEvent` drops it + // and the reducer never sees it. Long-lived SSE connections that + // never disconnect will hold their last-seen state until the next + // successful roundtrip triggers another reconcile; reconnecting + // clients get a fresh `session_snapshot` on attach. + writeStderrLine( + `[reconcile] session=${entry.sessionId} target=${target} action=failed error=${ + err instanceof Error ? err.message : String(err) + }`, + ); + } finally { + entry[flagKey] = false; + if (rerun) void reconcileAfterRoundtrip(entry, target); + } + }; + const createSessionEntry = ( ci: ChannelInfo, sessionId: string, @@ -1695,6 +1895,8 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { promptQueue: Promise.resolve(), modelChangeQueue: Promise.resolve(), approvalModeQueue: Promise.resolve(), + modelPublishGeneration: 0, + approvalModePublishGeneration: 0, pendingPermissionIds: new Set(), clientIds: new Map(), clientLastSeenAt: new Map(), @@ -1712,6 +1914,42 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { return entry; }; + // A5: seed the snapshot caches from the agent's session-create response + // (`newSession` / `loadSession` / `resumeSession` all return `models` + + // `modes`). Without this the caches stay unset until the first change, so a + // cold `?snapshot=1` attach to a session that never switched would return + // `{ currentModelId: null, currentApprovalMode: null }` and the SDK reducer's + // `!= null` guard would leave the client unseeded — defeating A5's primary + // (initial-attach) use case. The agent's `currentModelId` is already the + // canonical `model(authType)` form (acpAgent `formatAcpModelId`), matching + // what `reconcileAfterRoundtrip` reads back, so seeding it keeps the model + // comparison format-stable. Mode ids pass the same `KNOWN_APPROVAL_MODES` + // backstop the demux/reconcile paths use. + const seedSnapshotCaches = ( + entry: SessionEntry, + resp: { + models?: { currentModelId?: unknown } | null; + modes?: { currentModeId?: unknown } | null; + }, + ): void => { + const model = resp.models?.currentModelId; + if (typeof model === 'string' && model.length > 0) { + entry.currentModelId = model; + } else if (model != null) { + writeStderrLine( + `[seed] session=${entry.sessionId} target=model action=dropped value=${JSON.stringify(model)} reason=invalid_type`, + ); + } + const mode = resp.modes?.currentModeId; + if (typeof mode === 'string' && KNOWN_APPROVAL_MODES.has(mode)) { + entry.currentApprovalMode = mode; + } else if (mode != null) { + writeStderrLine( + `[seed] session=${entry.sessionId} target=approvalMode action=dropped value=${JSON.stringify(mode)} reason=${typeof mode !== 'string' ? 'invalid_type' : 'unknown_mode'}`, + ); + } + }; + const isAcpSessionResourceNotFound = ( err: unknown, sessionId: string, @@ -1975,6 +2213,7 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { restoreEvents, ); entry.restoreState = state; + seedSnapshotCaches(entry, state); const clientId = registerClient(entry, req.clientId); // Fold synchronous coalesce reservations into the new entry's // `attachCount`. By this point all coalescers that beat us must @@ -2505,7 +2744,46 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { subscribeEvents(sessionId, subOpts) { const entry = byId.get(sessionId); if (!entry) throw new SessionNotFoundError(sessionId); - return entry.events.subscribe(subOpts); + const raw = entry.events.subscribe(subOpts); + if (!subOpts?.snapshot) return raw; + + // A5: wrap the iterator to inject a synthetic `session_snapshot` + // frame so a freshly attached / reconnecting client can seed its + // side-channel reducer without an extra round-trip. Captures cached + // state synchronously at yield time. + // + // The bus only emits `replay_complete` on the `Last-Event-ID` + // resume path (`eventBus.subscribe` gates the whole replay block on + // `opts.lastEventId !== undefined`). A fresh connection has no + // `Last-Event-ID`, so it never sees `replay_complete` — keying the + // snapshot solely off that sentinel silently no-ops on the primary + // use case (initial attach). So inject up front when there is no + // resume cursor, and otherwise after `replay_complete` so the + // client applies replayed deltas before the snapshot seeds state. + const snapshotFrame = (): BridgeEvent => ({ + v: EVENT_SCHEMA_VERSION, + type: 'session_snapshot', + data: { + sessionId: entry.sessionId, + currentModelId: entry.currentModelId ?? null, + currentApprovalMode: entry.currentApprovalMode ?? null, + }, + }); + async function* withSnapshot(): AsyncIterable { + let injected = false; + if (subOpts?.lastEventId === undefined) { + yield snapshotFrame(); + injected = true; + } + for await (const event of raw) { + yield event; + if (!injected && event.type === 'replay_complete') { + yield snapshotFrame(); + injected = true; + } + } + } + return withSnapshot(); }, getSessionLastEventId(sessionId) { @@ -3209,6 +3487,12 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { // `applyModelServiceId`, so the agent notification can never slip // through after the flag clears even if transport ordering changes. entry.modelRoundtripInFlight = true; + // Only reconcile after a change that actually landed. If the + // roundtrip rejects (timeout / transport close) `publishModelSwitched` + // never ran and the cache is unchanged, so a reconcile would just emit + // a confusing corrective `model_switched` alongside the + // `model_switch_failed` the catch block already publishes. + let succeeded = false; try { const result = await Promise.race([ withTimeout( @@ -3218,14 +3502,26 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { ), transportClosed, ]); - entry.events.publish({ - type: 'model_switched', - data: { sessionId: entry.sessionId, modelId: req.modelId }, - ...(originatorClientId ? { originatorClientId } : {}), - }); + // Cache the model id as received from the caller. The bridge + // layer does not have access to the CLI's `formatAcpModelId` + // (which requires `authType`), so it cannot canonicalize here. + // In practice callers always send canonical ids (from + // `buildAvailableModels`); any residual raw→canonical drift is + // corrected by the `reconcileAfterRoundtrip` below, which reads + // the agent's authoritative canonical id and re-publishes if it + // differs. + publishModelSwitched(entry, req.modelId, originatorClientId); + succeeded = true; return result; } finally { entry.modelRoundtripInFlight = false; + if (succeeded) { + void reconcileAfterRoundtrip(entry, 'model'); + } else { + writeStderrLine( + `[reconcile] session=${entry.sessionId} target=model action=skipped reason=roundtrip_failed`, + ); + } } }); // Tail-swallow on the queue so a model-change failure doesn't poison @@ -3241,20 +3537,17 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { // Mirror `applyModelServiceId`'s observability contract: surface // failed model changes on the SSE bus so subscribers can update // their UI / retry. Without this the only signal is the HTTP - // 5xx, which doesn't reach passive viewers. - try { - entry.events.publish({ - type: 'model_switch_failed', - data: { - sessionId: entry.sessionId, - requestedModelId: req.modelId, - error: err instanceof Error ? err.message : String(err), - }, - ...(originatorClientId ? { originatorClientId } : {}), - }); - } catch { - /* bus closed */ - } + // 5xx, which doesn't reach passive viewers. `publish()` never + // throws (see `publishModelSwitched`), so no wrapper. + entry.events.publish({ + type: 'model_switch_failed', + data: { + sessionId: entry.sessionId, + requestedModelId: req.modelId, + error: err instanceof Error ? err.message : String(err), + }, + ...(originatorClientId ? { originatorClientId } : {}), + }); throw err; } // model_switched is published inside the work callback above (while the @@ -3298,76 +3591,117 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { // the queued work means the next change can't start its `extMethod` until // this change's side effects are fully done. Mirrors `modelChangeQueue`. const approvalWork = entry.approvalModeQueue.then(async () => { - const response = (await Promise.race([ - withTimeout( - entry.connection.extMethod( + // A2: suppress the agent's current_mode_update notification while + // the bridge owns the change. Mirrors `modelRoundtripInFlight`. + // The flag stays true through persist + publish so the notification + // cannot slip through during the persist phase (review finding #3). + entry.approvalModeRoundtripInFlight = true; + // See setSessionModel: only reconcile after a change that landed, so + // a rejected roundtrip can't pair a corrective event with the failure. + let succeeded = false; + try { + const response = (await Promise.race([ + withTimeout( + entry.connection.extMethod( + SERVE_CONTROL_EXT_METHODS.sessionApprovalMode, + { sessionId, mode }, + ), + initTimeoutMs, SERVE_CONTROL_EXT_METHODS.sessionApprovalMode, - { sessionId, mode }, ), - initTimeoutMs, - SERVE_CONTROL_EXT_METHODS.sessionApprovalMode, - ), - getTransportClosedReject(entry), - ])) as { previous: ApprovalMode; current: ApprovalMode }; - - let persisted = false; - if (opts.persist) { - try { - await withTimeout( - persistApprovalMode?.(boundWorkspace, mode) ?? Promise.resolve(), - PERSIST_TIMEOUT_MS, - 'persistApprovalMode', - ); - persisted = persistApprovalMode !== undefined; - } catch (err) { - // Persist failure is non-fatal — the in-process change already - // took effect inside the ACP child. Log but don't fail the route. - writeStderrLine( - `setSessionApprovalMode: persist failed: ${ - err instanceof Error ? err.message : String(err) - }`, + getTransportClosedReject(entry), + ])) as { previous: ApprovalMode; current: ApprovalMode }; + + if ( + typeof response.current !== 'string' || + !KNOWN_APPROVAL_MODES.has(response.current) + ) { + // Throw so the HTTP caller sees a 500 instead of a misleading + // 200 OK with the requested mode echoed back. Without this, + // the HTTP client thinks the mode changed while the cache and + // SSE bus still show the old value. + throw new Error( + `Agent returned unknown approval mode: ${JSON.stringify(response.current)}`, ); } - } - try { - entry.events.publish({ - type: 'approval_mode_changed', - data: { - sessionId: entry.sessionId, + + let persisted = false; + if (opts.persist) { + try { + await withTimeout( + persistApprovalMode?.(boundWorkspace, mode) ?? + Promise.resolve(), + PERSIST_TIMEOUT_MS, + 'persistApprovalMode', + ); + persisted = persistApprovalMode !== undefined; + } catch (err) { + writeStderrLine( + `setSessionApprovalMode: persist failed: ${ + err instanceof Error ? err.message : String(err) + }`, + ); + } + } + publishApprovalModeChanged( + entry, + { previous: response.previous, next: response.current, persisted, }, - ...(originatorClientId ? { originatorClientId } : {}), - }); - } catch { - /* bus closed */ - } - // A persisted change becomes the workspace default, so fan out a - // workspace-scoped mirror for peer sessions. Skip the requesting - // session (its own bus already got the publish above) to avoid - // double-counting in the reducer. - if (persisted) { - broadcastWorkspaceEvent( - { - type: 'approval_mode_changed', - data: { - sessionId: entry.sessionId, - previous: response.previous, - next: response.current, - persisted, - }, - ...(originatorClientId ? { originatorClientId } : {}), - }, - entry.sessionId, + originatorClientId, ); + // #4282 fold-in 4 (S2): a persisted change becomes the workspace + // default, so fan out a workspace-scoped mirror for peer sessions. + // #4297 fold-in 1: skip the requesting session (its own bus already + // got the publish above) to avoid double-counting in the reducer. + if (persisted) { + broadcastWorkspaceEvent( + { + type: 'approval_mode_changed', + data: { + sessionId: entry.sessionId, + previous: response.previous, + next: response.current, + persisted, + }, + ...(originatorClientId ? { originatorClientId } : {}), + }, + entry.sessionId, + ); + // F3Qgp: a persisted change rewrites the workspace default, so the + // peers we just notified now hold a stale `currentApprovalMode` in + // their SessionEntry cache. Their GET status / session_snapshot + // would report the pre-change mode until their own next roundtrip. + // `byId` is the per-workspace session map (the bridge is bound per + // workspace), so mirror the new default into every peer's cache; + // skip the originator, whose cache `publishApprovalModeChanged` + // already updated. + for (const peer of byId.values()) { + if (peer.sessionId === entry.sessionId) { + continue; + } + peer.currentApprovalMode = response.current; + } + } + succeeded = true; + return { + sessionId: entry.sessionId, + mode: response.current, + previous: response.previous, + persisted, + }; + } finally { + entry.approvalModeRoundtripInFlight = false; + if (succeeded) { + void reconcileAfterRoundtrip(entry, 'approvalMode'); + } else { + writeStderrLine( + `[reconcile] session=${entry.sessionId} target=approvalMode action=skipped reason=roundtrip_failed`, + ); + } } - return { - sessionId: entry.sessionId, - mode: response.current, - previous: response.previous, - persisted, - }; }); // Tail-swallow so a failed change doesn't poison subsequent ones. entry.approvalModeQueue = approvalWork.then( diff --git a/packages/acp-bridge/src/bridgeClient.test.ts b/packages/acp-bridge/src/bridgeClient.test.ts index 57978a9bc00..3f77dda4c3b 100644 --- a/packages/acp-bridge/src/bridgeClient.test.ts +++ b/packages/acp-bridge/src/bridgeClient.test.ts @@ -17,12 +17,12 @@ * proxy is fully bypassed — no `fs.writeFile` syscall); * 2. when `fileSystem` is omitted, the inline proxy runs and * reads / writes real disk (sanity check that the fallback - * path the 7-arg constructor's positional slot opt-outs to + * path the 8-arg constructor's positional slot opt-outs to * still works). * - * Regression guard: the constructor takes 7 positional args; the - * 7th (`fileSystem`) is optional and at the tail. A subtle re- - * ordering (or dropping the arg from `bridge.ts:773` factory's + * Regression guard: the constructor takes 8 positional args; the + * 6th (`fileSystem`) is optional. A subtle re-ordering (or + * dropping the arg from `bridge.ts`'s factory * `new BridgeClient(..., opts.fileSystem)` call) would silently * bypass the adapter in production. Test #1 + #2 catch that * because the mock fileSystem would never be called. diff --git a/packages/acp-bridge/src/bridgeClient.ts b/packages/acp-bridge/src/bridgeClient.ts index 6e851a16ddf..cafc1d03e5f 100644 --- a/packages/acp-bridge/src/bridgeClient.ts +++ b/packages/acp-bridge/src/bridgeClient.ts @@ -119,6 +119,19 @@ const MAX_EARLY_EVENTS_PER_SESSION = 32; const MAX_SUGGESTION_LENGTH = 500; const EARLY_EVENT_TTL_MS = 60_000; +// Known approval-mode ids accepted on the in-session `current_mode_update` +// demux path. Mirrors the `modeMap` keys in `Session.setMode` (CLI); an id +// outside this set is dropped before it fans out to SSE clients / the SDK +// reducer. Keep the two in lockstep. Exported so the bridge's reconcile and +// snapshot-seed paths apply the same enum backstop to agent-supplied mode ids. +export const KNOWN_APPROVAL_MODES: ReadonlySet = new Set([ + 'plan', + 'default', + 'auto-edit', + 'auto', + 'yolo', +]); + /** * Human-readable label for a `fs.Stats` object's kind, used in the * `readTextFile` "not a regular file" rejection message (BX8YO). @@ -195,6 +208,8 @@ export interface BridgeClientSessionEntry { * in `bridge.ts`; surfaced here for the demux. */ modelRoundtripInFlight?: boolean; + /** A2: mirrors `modelRoundtripInFlight` for approval-mode roundtrips. */ + approvalModeRoundtripInFlight?: boolean; } /** @@ -271,6 +286,28 @@ export class BridgeClient implements Client { * companion — preserves the inline proxy behavior. */ private readonly fileSystem?: BridgeFileSystem, + /** + * §2.3 callback: centralised `model_switched` publish through the + * bridge factory's cache-updating helper. The BridgeClient calls + * this instead of inlining `entry.events.publish(...)` so the + * cache update + generation bump stays atomic in one place. + */ + private readonly onModelPromoted?: ( + entry: BridgeClientSessionEntry, + modelId: string, + originatorClientId: string | undefined, + ) => void, + /** + * §2.3 / A2 callback: centralised `approval_mode_changed` publish. + * Called by the A2 `current_mode_update` demux when the agent + * switches approval mode in-session (exit_plan_mode, ProceedAlways, + * /mode). `previous` is read from the bridge state cache. + */ + private readonly onModePromoted?: ( + entry: BridgeClientSessionEntry, + modeId: string, + originatorClientId: string | undefined, + ) => void, ) {} async requestPermission( @@ -438,6 +475,10 @@ export class BridgeClient implements Client { this.handleInSessionModelUpdate(params); return; } + if (method === 'qwen/notify/session/mode-update') { + this.handleInSessionModeUpdate(params); + return; + } if (method === 'qwen/notify/session/prompt-suggestion') { const sessionId = params['sessionId']; const suggestion = params['suggestion']; @@ -532,7 +573,15 @@ export class BridgeClient implements Client { ); return; } - try { + if (this.onModelPromoted) { + this.onModelPromoted( + entry, + currentModelId, + entry.activePromptOriginatorClientId, + ); + } else { + // `EventBus.publish` never throws (closed bus → undefined no-op); per + // its documented contract we don't wrap it. entry.events.publish({ type: 'model_switched', data: { sessionId, modelId: currentModelId }, @@ -540,12 +589,135 @@ export class BridgeClient implements Client { ? { originatorClientId: entry.activePromptOriginatorClientId } : {}), }); + } + writeStderrLine( + `[demux] session=${sessionId} type=current_model_update action=promoted model=${currentModelId}`, + ); + } + + /** + * A2: promote an in-session `current_mode_update` extNotification to + * `approval_mode_changed`. Uses the same suppression pattern as + * `handleInSessionModelUpdate` — suppressed while the bridge is driving + * its own approval-mode roundtrip (`entry.approvalModeRoundtripInFlight`) + * — but diverges with two additions the model handler lacks: enum + * validation against `KNOWN_APPROVAL_MODES`, and a legacy + * `session_update{current_mode_update}` dual-emit for IDE companion + * compat (transition — see §6 of the design doc), itself deduped via the + * `legacyFrameSent` flag. + */ + private handleInSessionModeUpdate(params: Record): void { + const sessionId = params['sessionId']; + const currentModeId = params['currentModeId']; + if (typeof sessionId !== 'string' || typeof currentModeId !== 'string') { + return; + } + // Validate against the known approval-mode enum before it fans out. + // `Session.setMode` guards the symmetric send path with the same set + // ("an unknown id would call setApprovalMode(undefined), leaving the + // permission system undefined"); this is the receive path the agent + // can reach without that validation, so an unknown id here would + // propagate through `approval_mode_changed` to every SSE client and + // land in the SDK reducer's `state.approvalMode`. Keep in lockstep + // with `Session.setMode`'s `modeMap` keys (includes `auto`). + if (!KNOWN_APPROVAL_MODES.has(currentModeId)) { + writeStderrLine( + `[demux] session=${sessionId} type=current_mode_update action=dropped reason=unknown_mode mode=${currentModeId}`, + ); + return; + } + const entry = this.resolveEntry(sessionId); + if (!entry) { + writeStderrLine( + `[demux] session=${sessionId} type=current_mode_update action=dropped reason=no_entry`, + ); + return; + } + if (entry.approvalModeRoundtripInFlight) { + writeStderrLine( + `[demux] session=${sessionId} type=current_mode_update action=suppressed reason=bridge_roundtrip_in_flight`, + ); + return; + } + if (this.onModePromoted) { + this.onModePromoted( + entry, + currentModeId, + entry.activePromptOriginatorClientId, + ); + } else { + // Fallback path (no `onModePromoted` injected — tests / non-bridge + // consumers; production always wires the bridge callback). Mirror + // the main path's full payload: the SDK's + // `isApprovalModeChangedData` requires `previous` (non-empty + // string) and `persisted` (boolean), so a `{ sessionId, next }` + // shape fails validation and `asKnownDaemonEvent` drops the event. + // `previous` is unavailable on this path (the cache lives on the + // bridge's `SessionEntry`, not the demux interface), so seed it + // with the protocol default. + // + // `EventBus.publish` never throws (a closed bus is a return-undefined + // no-op and subscriber-enqueue failures are caught internally), so + // per its documented contract we don't wrap it in try/catch. + entry.events.publish({ + type: 'approval_mode_changed', + data: { + sessionId, + previous: 'default', + next: currentModeId, + persisted: false, + }, + ...(entry.activePromptOriginatorClientId + ? { originatorClientId: entry.activePromptOriginatorClientId } + : {}), + }); + } + // TODO(dual-emit-removal): also emit the legacy generic + // `session_update{current_mode_update}` for one release cycle so the + // VS Code IDE companion's existing `case 'current_mode_update'` + // handler keeps working. Remove this block (and its tracking issue) + // once the companion ships an `approval_mode_changed` handler. + // + // Skip it when the producer already sent the legacy frame itself: the + // `exit_plan_mode` path (`Session.sendCurrentModeUpdateNotification`) + // calls `sendUpdate` before this extNotification, which + // `BridgeClient.sessionUpdate` already fanned onto the bus as the same + // `session_update{current_mode_update}` frame. Dual-emitting here would + // deliver it twice. The `setMode` path omits the flag (it has no + // `sendUpdate`), so its dual-emit still fires. + // + // Use the canonical ACP-nested shape (`data.update.sessionUpdate`), + // matching what `BridgeClient.sessionUpdate` publishes for a real + // `current_mode_update` notification. A flat + // `{ sessionId, sessionUpdate, currentModeId }` would (a) not be + // recognised by the companion's standard `data.update.sessionUpdate` + // switch, and (b) collide structurally with the real `session_update` + // the agent already emits on the `exit_plan_mode` path — leaving two + // incompatible shapes on the bus for one change. + if (params['legacyFrameSent'] === true) { writeStderrLine( - `[demux] session=${sessionId} type=current_model_update action=promoted model=${currentModelId}`, + `[demux] session=${sessionId} type=current_mode_update action=promoted mode=${currentModeId} legacy_frame=skipped`, ); - } catch { - /* bus closed */ + return; } + // `EventBus.publish` never throws (closed bus → undefined no-op); per its + // documented contract we don't wrap it in try/catch. + entry.events.publish({ + type: 'session_update', + data: { + sessionId, + update: { + sessionUpdate: 'current_mode_update', + currentModeId, + }, + }, + ...(entry.activePromptOriginatorClientId + ? { originatorClientId: entry.activePromptOriginatorClientId } + : {}), + }); + writeStderrLine( + `[demux] session=${sessionId} type=current_mode_update action=promoted mode=${currentModeId}`, + ); } /** diff --git a/packages/acp-bridge/src/bridgeTypes.ts b/packages/acp-bridge/src/bridgeTypes.ts index 6afc521b088..dac9f613fe3 100644 --- a/packages/acp-bridge/src/bridgeTypes.ts +++ b/packages/acp-bridge/src/bridgeTypes.ts @@ -239,7 +239,10 @@ export interface AcpSessionBridge { */ subscribeEvents( sessionId: string, - opts?: SubscribeOptions, + opts?: SubscribeOptions & { + /** Yield a synthetic `session_snapshot` frame after replay completes. */ + snapshot?: boolean; + }, ): AsyncIterable; /** diff --git a/packages/cli/src/acp-integration/session/Session.test.ts b/packages/cli/src/acp-integration/session/Session.test.ts index 7fb252dcbcc..272f9c5e109 100644 --- a/packages/cli/src/acp-integration/session/Session.test.ts +++ b/packages/cli/src/acp-integration/session/Session.test.ts @@ -352,6 +352,68 @@ describe('Session', () => { expect(mockConfig.setApprovalMode).toHaveBeenCalledWith(expected); }); + + it('emits a current_mode_update extNotification after switching (A2)', async () => { + await session.setMode({ + sessionId: 'test-session-id', + modeId: 'auto-edit', + }); + + expect(mockClient.extNotification).toHaveBeenCalledWith( + 'qwen/notify/session/mode-update', + expect.objectContaining({ + v: 1, + sessionId: 'test-session-id', + currentModeId: 'auto-edit', + }), + ); + }); + + it('rejects an unknown modeId and does NOT touch approval mode (A2)', async () => { + await expect( + session.setMode({ + sessionId: 'test-session-id', + // eslint-disable-next-line @typescript-eslint/no-explicit-any + modeId: 'totally-bogus' as any, + }), + ).rejects.toThrow(/Unknown approval mode/); + + expect(mockConfig.setApprovalMode).not.toHaveBeenCalled(); + expect(mockClient.extNotification).not.toHaveBeenCalledWith( + 'qwen/notify/session/mode-update', + expect.anything(), + ); + }); + }); + + describe('sendCurrentModeUpdateNotification', () => { + // The exit_plan_mode / edit-ProceedAlways path publishes the legacy + // `session_update{current_mode_update}` frame itself (via sendUpdate), + // so its extNotification must carry `legacyFrameSent: true` to stop the + // bridge demux from emitting a second, duplicate legacy frame. Unlike + // `setMode` (which omits the flag), a regression dropping it here would + // double-publish to the IDE companion. (A2) + it('marks the extNotification legacyFrameSent so the demux skips its dual-emit', async () => { + await ( + session as unknown as { + sendCurrentModeUpdateNotification: ( + outcome: core.ToolConfirmationOutcome, + ) => Promise; + } + ).sendCurrentModeUpdateNotification( + core.ToolConfirmationOutcome.ProceedAlways, + ); + + expect(mockClient.extNotification).toHaveBeenCalledWith( + 'qwen/notify/session/mode-update', + expect.objectContaining({ + v: 1, + sessionId: 'test-session-id', + currentModeId: 'auto-edit', + legacyFrameSent: true, + }), + ); + }); }); describe('rewindToTurn', () => { diff --git a/packages/cli/src/acp-integration/session/Session.ts b/packages/cli/src/acp-integration/session/Session.ts index 1fd0d240df6..5d43a12df14 100644 --- a/packages/cli/src/acp-integration/session/Session.ts +++ b/packages/cli/src/acp-integration/session/Session.ts @@ -1812,8 +1812,34 @@ export class Session implements SessionContext { yolo: ApprovalMode.YOLO, }; + // `modeId` arrives over the wire (ACP `session/set_mode`, or + // `setSessionConfigOption` casting an unknown `value` to string), so + // validate at this boundary. An unknown id would otherwise call + // `setApprovalMode(undefined)` — leaving the permission system in an + // undefined state — and the A2 broadcast below would fan the bogus id + // out to every attached SSE client. const approvalMode = modeMap[params.modeId as ApprovalModeValue]; + if (approvalMode === undefined) { + throw RequestError.invalidParams( + undefined, + `Unknown approval mode: ${params.modeId}`, + ); + } this.config.setApprovalMode(approvalMode); + + // A2 (#4511): notify attached clients of an in-session mode switch. + // Mirrors the model-update extNotification in `setModel`. + void this.client + .extNotification('qwen/notify/session/mode-update', { + v: 1, + sessionId: this.sessionId, + currentModeId: params.modeId, + }) + .catch((error) => { + // Advisory only; a failed notification must not fail the mode + // switch. Matches the model-update extNotification in `setModel`. + debugLogger.debug('mode-update extNotification failed', error); + }); } /** @@ -1870,8 +1896,9 @@ export class Session implements SessionContext { sessionId: this.sessionId, currentModelId: effectiveModelId, }) - .catch(() => { + .catch((error) => { // Advisory only; a failed notification must not fail the model switch. + debugLogger.debug('model-update extNotification failed', error); }); if (options.persistDefault ?? true) { @@ -1927,6 +1954,29 @@ export class Session implements SessionContext { }; await this.sendUpdate(update); + + // A2 (#4511): promote the mode change to the bridge side-channel so + // it reaches `approval_mode_changed` on the SSE bus, matching the + // extNotification in `setMode`. + // + // Unlike `setMode`, this path already published the legacy + // `session_update{current_mode_update}` frame via `sendUpdate` above + // (BridgeClient.sessionUpdate fans it onto the bus). Tell the demux to + // skip its compat dual-emit so the IDE companion sees exactly one + // legacy frame for this change, not two. `setMode` omits the flag, so + // its dual-emit still fires (it has no `sendUpdate`). + void this.client + .extNotification('qwen/notify/session/mode-update', { + v: 1, + sessionId: this.sessionId, + currentModeId: newModeId, + legacyFrameSent: true, + }) + .catch((error) => { + // Advisory only; a failed notification must not fail the mode + // change. Matches the model-update extNotification in `setModel`. + debugLogger.debug('mode-update extNotification failed', error); + }); } /** diff --git a/packages/cli/src/serve/server.ts b/packages/cli/src/serve/server.ts index 7da85e27e2a..1569e2acfe3 100644 --- a/packages/cli/src/serve/server.ts +++ b/packages/cli/src/serve/server.ts @@ -2584,10 +2584,12 @@ export function createServeApp( let iter: AsyncIterator | undefined; const abort = new AbortController(); try { + const snapshot = req.query['snapshot'] === '1'; const iterable = bridge.subscribeEvents(sessionId, { signal: abort.signal, lastEventId, ...(maxQueued !== undefined ? { maxQueued } : {}), + ...(snapshot ? { snapshot: true } : {}), }); iter = iterable[Symbol.asyncIterator](); } catch (err) { diff --git a/packages/sdk-typescript/src/daemon/events.ts b/packages/sdk-typescript/src/daemon/events.ts index 1e02981a2c8..0559e9916b8 100644 --- a/packages/sdk-typescript/src/daemon/events.ts +++ b/packages/sdk-typescript/src/daemon/events.ts @@ -110,6 +110,11 @@ export const DAEMON_KNOWN_EVENT_TYPE_VALUES = [ 'turn_error', 'session_rewound', 'session_branched', + // A5 (#4511): synthetic side-channel snapshot yielded after + // `replay_complete` when `?snapshot=1` is set on the SSE endpoint. + // Carries `currentModelId` and `currentApprovalMode` so reconnecting + // clients can seed their reducer without an extra round-trip. + 'session_snapshot', ] as const; const DAEMON_KNOWN_EVENT_TYPES: ReadonlySet = new Set( @@ -686,6 +691,12 @@ export type DaemonMcpServerRemovedEvent = DaemonEventEnvelope< DaemonMcpServerRemovedData >; +export interface DaemonSessionSnapshotData { + sessionId: string; + currentModelId: string | null; + currentApprovalMode: string | null; + [key: string]: unknown; +} export type DaemonSessionUpdateEvent = DaemonEventEnvelope< 'session_update', DaemonSessionUpdateData @@ -825,6 +836,10 @@ export type DaemonSessionRewoundEvent = DaemonEventEnvelope< 'session_rewound', DaemonSessionRewoundData >; +export type DaemonSessionSnapshotEvent = DaemonEventEnvelope< + 'session_snapshot', + DaemonSessionSnapshotData +>; export type DaemonSessionBranchedEvent = DaemonEventEnvelope< 'session_branched', DaemonSessionBranchedData @@ -909,7 +924,8 @@ export type KnownDaemonEvent = | DaemonWorkspaceMutationEvent | DaemonAuthEvent | DaemonAssistEvent - | DaemonTurnEvent; + | DaemonTurnEvent + | DaemonSessionSnapshotEvent; export interface DaemonSessionViewState { lastEventId?: number; @@ -1134,6 +1150,11 @@ const RESYNC_PASSTHROUGH_TYPES = new Set([ 'session_closed', 'client_evicted', 'stream_error', + // A5 (#4511): the snapshot is a full-state authoritative frame, not a + // delta, so it is safe to apply during resync — and it is exactly what + // lets a client that reconnected past the ring recover currentModelId / + // approvalMode without waiting for the next loadSession. + 'session_snapshot', ]); export function createDaemonSessionViewState( @@ -1344,7 +1365,10 @@ export function asKnownDaemonEvent( : undefined; case 'settings_changed': return event.data != null && typeof event.data === 'object' - ? (event as DaemonEventEnvelope<'settings_changed', Record>) + ? (event as DaemonEventEnvelope< + 'settings_changed', + Record + >) : undefined; case 'workspace_initialized': return isWorkspaceInitializedData(event.data) @@ -1382,6 +1406,10 @@ export function asKnownDaemonEvent( return isSessionRewoundData(event.data) ? (event as DaemonSessionRewoundEvent) : undefined; + case 'session_snapshot': + return isSessionSnapshotData(event.data) + ? (event as DaemonSessionSnapshotEvent) + : undefined; case 'session_branched': return isSessionBranchedData(event.data) ? (event as DaemonSessionBranchedEvent) @@ -1760,6 +1788,17 @@ export function reduceDaemonSessionEvent( rewindCount: base.rewindCount + 1, lastRewind: mergeOriginator(event.data, event), }; + case 'session_snapshot': + return { + ...base, + sessionId: event.data.sessionId, + ...(event.data.currentModelId != null + ? { currentModelId: event.data.currentModelId } + : {}), + ...(event.data.currentApprovalMode != null + ? { approvalMode: event.data.currentApprovalMode } + : {}), + }; case 'session_branched': return { ...base, @@ -2515,6 +2554,23 @@ function isSessionBranchedData( ); } +function isSessionSnapshotData( + value: unknown, +): value is DaemonSessionSnapshotData { + // `currentModelId` / `currentApprovalMode` are `string | null` on the + // wire. Validate the types here, not just `sessionId`: the reducer + // propagates these into `state.currentModelId` / `state.approvalMode` + // on a `!= null` check alone, so an unchecked non-string (e.g. `42`, + // `{}`) would land in state and crash downstream `.trim()`-style calls. + if (!isRecord(value) || !isNonEmptyString(value['sessionId'])) return false; + const model = value['currentModelId']; + const mode = value['currentApprovalMode']; + return ( + (model === null || typeof model === 'string') && + (mode === null || typeof mode === 'string') + ); +} + function isPermissionOption(value: unknown): value is DaemonPermissionOption { return isRecord(value) && isNonEmptyString(value['optionId']); } diff --git a/packages/sdk-typescript/src/daemon/index.ts b/packages/sdk-typescript/src/daemon/index.ts index 25e90d2e851..b3832e0fe70 100644 --- a/packages/sdk-typescript/src/daemon/index.ts +++ b/packages/sdk-typescript/src/daemon/index.ts @@ -251,6 +251,9 @@ export type { DaemonTurnCompleteEvent, DaemonTurnErrorData, DaemonTurnErrorEvent, + // A5 — side-channel session snapshot + DaemonSessionSnapshotData, + DaemonSessionSnapshotEvent, DaemonDeviceFlowReducerState, DaemonAuthState, KnownDaemonEvent, diff --git a/packages/sdk-typescript/test/unit/daemonEvents.test.ts b/packages/sdk-typescript/test/unit/daemonEvents.test.ts index a158b4dff79..b0327b92153 100644 --- a/packages/sdk-typescript/test/unit/daemonEvents.test.ts +++ b/packages/sdk-typescript/test/unit/daemonEvents.test.ts @@ -2485,6 +2485,42 @@ describe('PR 21 — auth device-flow events', () => { expect(state.unrecognizedKnownEventCount).toBe(1); expect(state.awaitingResync).toBe(false); }); + + it('still applies session_snapshot while awaitingResync (RESYNC_PASSTHROUGH_TYPES)', () => { + // session_snapshot is in RESYNC_PASSTHROUGH_TYPES — a reconnecting + // client that missed ring events still needs to seed its side-channel + // model/mode state. Without passthrough, the auto-skip gate would + // drop the snapshot and the client would remain on stale null/null. + const afterResync = reduceDaemonSessionEvent( + createDaemonSessionViewState(), + { + v: 1, + type: 'state_resync_required', + data: { + reason: 'ring_evicted', + lastDeliveredId: 5, + earliestAvailableId: 12, + }, + }, + ); + expect(afterResync.awaitingResync).toBe(true); + + const afterSnapshot = reduceDaemonSessionEvent(afterResync, { + id: 13, + v: 1, + type: 'session_snapshot', + data: { + sessionId: 's-1', + currentModelId: 'qwen-max', + currentApprovalMode: 'auto-edit', + }, + }); + // The snapshot must have applied — model/mode state is populated. + expect(afterSnapshot.currentModelId).toBe('qwen-max'); + expect(afterSnapshot.approvalMode).toBe('auto-edit'); + // awaitingResync stays true (consumer hasn't explicitly recovered). + expect(afterSnapshot.awaitingResync).toBe(true); + }); }); describe('followup_suggestion (daemon assist push)', () => { @@ -2601,4 +2637,94 @@ describe('PR 21 — auth device-flow events', () => { expect(state.lastFollowupSuggestion).toBeUndefined(); }); }); + + describe('session_snapshot (A5 #4511)', () => { + it('asKnownDaemonEvent narrows session_snapshot', () => { + const event: DaemonEvent = { + v: 1, + type: 'session_snapshot', + data: { + sessionId: 's-1', + currentModelId: 'qwen-turbo', + currentApprovalMode: 'auto', + }, + }; + const known = asKnownDaemonEvent(event); + expect(known).toBeDefined(); + expect(known!.type).toBe('session_snapshot'); + }); + + it('reducer seeds currentModelId and approvalMode from snapshot', () => { + const state = reduceDaemonSessionEvent(createDaemonSessionViewState(), { + v: 1, + type: 'session_snapshot', + data: { + sessionId: 's-1', + currentModelId: 'qwen-turbo', + currentApprovalMode: 'yolo', + }, + }); + expect(state.sessionId).toBe('s-1'); + expect(state.currentModelId).toBe('qwen-turbo'); + expect(state.approvalMode).toBe('yolo'); + }); + + it('reducer does not overwrite model/mode with null snapshot values', () => { + const initial = { + ...createDaemonSessionViewState(), + currentModelId: 'existing-model', + approvalMode: 'default', + }; + const state = reduceDaemonSessionEvent(initial, { + v: 1, + type: 'session_snapshot', + data: { + sessionId: 's-1', + currentModelId: null, + currentApprovalMode: null, + }, + }); + expect(state.currentModelId).toBe('existing-model'); + expect(state.approvalMode).toBe('default'); + }); + + it('drops malformed session_snapshot (missing sessionId)', () => { + const state = reduceDaemonSessionEvent(createDaemonSessionViewState(), { + v: 1, + type: 'session_snapshot', + data: { currentModelId: 'qwen-turbo' }, + }); + expect(state.unrecognizedKnownEventCount).toBe(1); + }); + + it('drops session_snapshot with a non-string currentModelId', () => { + // Guards the reducer's `!= null` propagation: an unchecked non-string + // would land in `state.currentModelId` and crash downstream string ops. + const state = reduceDaemonSessionEvent(createDaemonSessionViewState(), { + v: 1, + type: 'session_snapshot', + data: { + sessionId: 's1', + currentModelId: 42 as unknown as string, + currentApprovalMode: null, + }, + }); + expect(state.unrecognizedKnownEventCount).toBe(1); + expect(state.currentModelId).toBeUndefined(); + }); + + it('drops session_snapshot with a non-string currentApprovalMode', () => { + const state = reduceDaemonSessionEvent(createDaemonSessionViewState(), { + v: 1, + type: 'session_snapshot', + data: { + sessionId: 's1', + currentModelId: null, + currentApprovalMode: {} as unknown as string, + }, + }); + expect(state.unrecognizedKnownEventCount).toBe(1); + expect(state.approvalMode).toBeUndefined(); + }); + }); });