From 752f531b0a42da798e28407314fed9f2dbe7fe03 Mon Sep 17 00:00:00 2001 From: Kelvin Amoaba <97001695+AmoabaKelvin@users.noreply.github.com> Date: Thu, 22 Oct 2026 21:10:01 +0000 Subject: [PATCH] fix(chat): send pi and acp messages after a model-only change (#32004) --- src/main/acp/acp-message-execution-options.ts | 3 +- src/main/acp/acp-structured-options.ts | 5 ++- .../acp-structured-session-adapter.test.ts | 29 +++++++++++++++ .../acp/acp-structured-session-adapter.ts | 1 + src/main/pi/rpc-message-dispatch.ts | 3 +- .../pi/rpc-session-adapter.test-fixture.ts | 12 ++++-- src/main/pi/rpc-session-adapter.test.ts | 37 +++++++++++++++++++ 7 files changed, 83 insertions(+), 7 deletions(-) diff --git a/src/main/acp/acp-message-execution-options.ts b/src/main/acp/acp-message-execution-options.ts index c684ef15553..07e06db30be 100644 --- a/src/main/acp/acp-message-execution-options.ts +++ b/src/main/acp/acp-message-execution-options.ts @@ -24,7 +24,8 @@ export async function applyAcpMessageOptions( for (const key of ['model', 'effort'] as const) { const value = options?.[key] ?? (key === 'model' ? initial.model : defaultEffort) if (value === undefined) { - if (session.options.picked.has(key) || session.options.reported()[key] !== undefined) { + const applied = session.options.applied.get(key) + if (applied !== undefined && applied !== initial[key]) { throw new AgentSessionOptionRejectedError( new Error(`The default ${key} for this model is unknown`) ) diff --git a/src/main/acp/acp-structured-options.ts b/src/main/acp/acp-structured-options.ts index 8bfcd642c6c..8c7821d7caf 100644 --- a/src/main/acp/acp-structured-options.ts +++ b/src/main/acp/acp-structured-options.ts @@ -65,6 +65,8 @@ export class AcpStructuredOptions { // A loaded session keeps the model it ran; only a new one resolves the agent's own config. private resolvesConfig = false readonly picked = new Set() + /** Each key's value from Orca's last successful write; never the agent's report. */ + readonly applied = new Map() private initialChoice: Readonly> = {} currentProof = new Set() writeRevision = 0 @@ -250,7 +252,7 @@ export function writeAcpSessionOption( connection: Pick, options: AcpStructuredOptions, write: AcpOptionWrite, - bound: { agent: string; timeoutMs: number; signal?: AbortSignal; key: string } + bound: { agent: string; timeoutMs: number; signal?: AbortSignal; key: string; value: string } ): Promise { const { agent, timeoutMs, signal } = bound if (signal?.aborted) { @@ -267,6 +269,7 @@ export function writeAcpSessionOption( } apply() options.currentProof.add(bound.key) + options.applied.set(bound.key, bound.value) } const applied = write.method === 'config' diff --git a/src/main/acp/acp-structured-session-adapter.test.ts b/src/main/acp/acp-structured-session-adapter.test.ts index b470ab562c3..89c9d4faf48 100644 --- a/src/main/acp/acp-structured-session-adapter.test.ts +++ b/src/main/acp/acp-structured-session-adapter.test.ts @@ -180,6 +180,35 @@ describe('ACP structured session adapter: turns', () => { }) }) + it('sends model-only choices away from the opening model, back, and away again', async () => { + const rig = await openAcpAdapterRig() + await rig.acquire() + for (const [index, [clientMessageId, model]] of [ + ['away', 'grok-4.6'], + ['back', 'grok-4.7'], + ['again', 'grok-4.6'] + ].entries()) { + await expect( + rig.adapter.dispatch({ + sessionId: SESSION, + fence: 1, + clientMessageId, + body: hello, + executionOptions: { model } + }) + ).resolves.toEqual({ state: 'admitted' }) + rig.child().agent.reply(await rig.frame('session/prompt', index), { stopReason: 'end_turn' }) + await waitFor(() => expect(rig.adapter.holdsDispatch(SESSION)).toBe(false)) + } + // Back on the opening model, its opening effort is restored; elsewhere the agent keeps its own. + expect(rig.sent('session/set_config_option').map((frame) => frame.params)).toMatchObject([ + { configId: 'model', value: 'grok-4.6' }, + { configId: 'model', value: 'grok-4.7' }, + { configId: 'reasoning_effort', value: 'high' }, + { configId: 'model', value: 'grok-4.6' } + ]) + }) + it("sends Grok's prompt identity only to an agent whose dialect echoes it", async () => { const rig = await openAcpAdapterRig({ deps: { spec: { ...GROK, dialect: GENERIC_ACP_DIALECT } } diff --git a/src/main/acp/acp-structured-session-adapter.ts b/src/main/acp/acp-structured-session-adapter.ts index 6a72b9dad6c..c135829df7a 100644 --- a/src/main/acp/acp-structured-session-adapter.ts +++ b/src/main/acp/acp-structured-session-adapter.ts @@ -263,6 +263,7 @@ export class AcpStructuredSessionAdapter implements StructuredAgentSessionAdapte // Bounded, and abandoned by a close or Stop: the session's queue waits on it. await writeAcpSessionOption(session.connection, session.options, write, { key: input.key, + value: input.value, agent: session.spec.agent, timeoutMs: this.deps.optionWriteTimeoutMs ?? ACP_OPTION_WRITE_TIMEOUT_MS, ...(input.signal ? { signal: input.signal } : {}) diff --git a/src/main/pi/rpc-message-dispatch.ts b/src/main/pi/rpc-message-dispatch.ts index 807f52b37ae..d301516ffcd 100644 --- a/src/main/pi/rpc-message-dispatch.ts +++ b/src/main/pi/rpc-message-dispatch.ts @@ -73,7 +73,8 @@ export async function dispatchPiRpcMessage( (key === 'model' || model === session.initialOptions.model ? session.initialOptions[key] : undefined) - if (value === undefined && session.optionProof.writes.has(key)) { + const applied = session.selected.get(key) + if (value === undefined && applied !== undefined && applied !== session.initialOptions[key]) { throw new AgentSessionOptionRejectedError( new Error(`The default ${key} for this model is unknown`) ) diff --git a/src/main/pi/rpc-session-adapter.test-fixture.ts b/src/main/pi/rpc-session-adapter.test-fixture.ts index 52a50e5c2af..78e326d3d62 100644 --- a/src/main/pi/rpc-session-adapter.test-fixture.ts +++ b/src/main/pi/rpc-session-adapter.test-fixture.ts @@ -44,7 +44,10 @@ export class FakeConnection implements PiRpcConnection { private readonly exitListeners: (() => void)[] = [] requestOverride?: (command: string) => Promise | undefined abortRequest?: () => void - constructor(readonly handlers: JsonlRpcAgentConnectionOptions) {} + constructor( + readonly handlers: JsonlRpcAgentConnectionOptions, + private readonly reported: { thinkingLevel?: string } = {} + ) {} async request(command: string): Promise { this.requests.push(command) const override = this.requestOverride?.(command) @@ -52,7 +55,7 @@ export class FakeConnection implements PiRpcConnection { return override } if (command === 'get_state') { - return state + return { ...state, ...this.reported } } if (command === 'get_available_models') { return { models: [state.model] } @@ -121,7 +124,8 @@ export function reportedEvent( export async function setup( options: Readonly> = {}, - eventSink?: (sink: StructuredAgentSessionEventSink) => StructuredAgentSessionEventSink + eventSink?: (sink: StructuredAgentSessionEventSink) => StructuredAgentSessionEventSink, + reported?: ConstructorParameters[1] ) { const rig = await openProviderTimelineRig({ agent: 'pi', sessionId }) const connections: FakeConnection[] = [] @@ -152,7 +156,7 @@ export async function setup( })) const openConnection = vi.fn( (_launch: ProviderProcessLaunch, handlers: JsonlRpcAgentConnectionOptions) => { - const connection = new FakeConnection(handlers) + const connection = new FakeConnection(handlers, reported) connections.push(connection) return connection } diff --git a/src/main/pi/rpc-session-adapter.test.ts b/src/main/pi/rpc-session-adapter.test.ts index a0416a9ed56..13d46a3b47c 100644 --- a/src/main/pi/rpc-session-adapter.test.ts +++ b/src/main/pi/rpc-session-adapter.test.ts @@ -200,6 +200,43 @@ describe('Pi RPC session ownership and delivery', () => { expect(h.connection.sent).toMatchObject([{ type: 'prompt', message: 'chosen' }]) }) + it('sends model-only choices away from the opening model, back, and away again', async () => { + const h = await setup({}, undefined, { thinkingLevel: 'medium' }) + h.connection.requestOverride = (command) => + command === 'set_thinking_level' ? Promise.resolve({}) : undefined + for (const [clientMessageId, model] of [ + ['away', 'anthropic/model-2'], + ['back', 'anthropic/model-1'], + ['again', 'anthropic/model-2'] + ]) { + await expect( + h.adapter.dispatch({ + sessionId, + fence: 7, + clientMessageId, + body: { + kind: 'message', + role: 'user', + blocks: [{ type: 'text', text: clientMessageId }] + }, + executionOptions: { model } + }) + ).resolves.toEqual({ state: 'admitted' }) + } + // Back on the opening model, its opening effort is restored; elsewhere Pi keeps its own. + expect(h.connection.requests.filter((command) => command.startsWith('set_'))).toEqual([ + 'set_model', + 'set_model', + 'set_thinking_level', + 'set_model' + ]) + expect(h.connection.sent).toMatchObject([ + { message: 'away' }, + { message: 'back' }, + { message: 'again' } + ]) + }) + it('rejects a known-unsent prompt when an option times out and preserves Stop reason', async () => { const h = await setup() const timeout = new Error('Pi did not answer set_model')