From ac44034f276c633a88eda198839765cec4275caa Mon Sep 17 00:00:00 2001 From: Yash Maheshwari Date: Mon, 31 Aug 2026 21:43:12 +0530 Subject: [PATCH] fix(acp): make session/set_mode plan idempotent on sessions created in plan mode --- .changeset/acp-set-mode-plan.md | 5 +++ packages/acp-server/src/session.ts | 51 +++++++++++++++++++++---- packages/acp-server/test/config.test.ts | 26 +++++++++++++ 3 files changed, 74 insertions(+), 8 deletions(-) create mode 100644 .changeset/acp-set-mode-plan.md diff --git a/.changeset/acp-set-mode-plan.md b/.changeset/acp-set-mode-plan.md new file mode 100644 index 00000000000..22558320b2f --- /dev/null +++ b/.changeset/acp-set-mode-plan.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Fix ACP `session/set_mode` failing with "Already in plan mode" right after creating a session when it was created in plan mode. \ No newline at end of file diff --git a/packages/acp-server/src/session.ts b/packages/acp-server/src/session.ts index 999c08e3fd2..c6cf582f24b 100644 --- a/packages/acp-server/src/session.ts +++ b/packages/acp-server/src/session.ts @@ -358,6 +358,15 @@ export class AcpSession { error: error instanceof Error ? error.message : String(error), }); } + // A session created while `default_plan_mode` is enabled enters plan mode + // at create time (`sessionLifecycleService`). Reflect that engine state in + // the ACP mode so `session/new` reports `plan` instead of the local + // `default` default — otherwise a client that sets an explicit mode right + // after creation re-enters plan and trips the engine's "Already in plan + // mode" guard. + if (await this.isPlanActive()) { + this.currentModeId = 'plan'; + } // Awaited: the post-`session/new` `available_commands_update` must already // carry the skills (see `activateSession`). await this.refreshSkills(); @@ -1102,14 +1111,22 @@ export class AcpSession { /** Switch the ACP mode (plan mode + permission mode). */ async setMode(id: AcpModeId): Promise { const { plan, permission } = acpModeToToggles(id); - if (plan) { - await this.agent.enterPlan(); - } else { - // KLIENT-GAP(plan): `exitPlan` (`planService.exit()`) is not on the - // klient surface; `cancelPlan` (`planModeCancel`) has the identical - // state effect (see `agent/plan/planOps.ts`) — only the persisted op - // name differs. - await this.agent.cancelPlan(); + // Idempotent plan toggle: only enter/cancel when the engine's plan state + // differs from the target, so a client re-asserting the active mode (e.g. + // `set_mode "plan"` right after `session/new` while the engine already + // entered plan mode via `default_plan_mode`) does not trip the engine's + // "Already in plan mode" guard. + const planActive = await this.isPlanActive(); + if (plan !== planActive) { + if (plan) { + await this.agent.enterPlan(); + } else { + // KLIENT-GAP(plan): `exitPlan` (`planService.exit()`) is not on the + // klient surface; `cancelPlan` (`planModeCancel`) has the identical + // state effect (see `agent/plan/planOps.ts`) — only the persisted op + // name differs. + await this.agent.cancelPlan(); + } } await this.agent.setPermission(permission); this.currentModeId = id; @@ -1121,6 +1138,24 @@ export class AcpSession { await this.emitConfigOptionUpdate(); } + /** + * Whether the engine's plan mode is currently active, as reported by the + * agent plan service's `status` (non-null while a plan is open). Best-effort: + * a transient read failure degrades to `false` so `setMode` falls back to the + * unconditional toggle rather than rejecting a call that previously worked. + */ + private async isPlanActive(): Promise { + try { + return (await this.agent.getPlan()) !== null; + } catch (error) { + log.warn('acp: could not read plan mode state', { + sessionId: this.sessionId, + error: error instanceof Error ? error.message : String(error), + }); + return false; + } + } + /** Push a fresh `config_option_update` to the client. */ private async emitConfigOptionUpdate(): Promise { try { diff --git a/packages/acp-server/test/config.test.ts b/packages/acp-server/test/config.test.ts index 2b6e14e84ca..65fa0289332 100644 --- a/packages/acp-server/test/config.test.ts +++ b/packages/acp-server/test/config.test.ts @@ -103,6 +103,31 @@ describe('acp-server config surface', () => { 30_000, ); + it( + 'session/set_mode plan succeeds when default_plan_mode already entered plan mode', + async () => { + homeDir = await mkdtemp(join(tmpdir(), 'acp-config-')); + await writeFile(join(homeDir, 'config.toml'), 'default_plan_mode = true\n', 'utf8'); + client = await createTestClient({ homeDir }); + await client.send('initialize', { protocolVersion: 1, clientCapabilities: {} }); + const { sessionId, modes } = (await client!.send('session/new', { + cwd: homeDir, + mcpServers: [], + })) as NewSessionResult; + // The engine entered plan mode at create time, so session/new must report + // it — a client that then sets an explicit plan mode must not hit the + // "Already in plan mode" guard. + expect(modes?.currentModeId).toBe('plan'); + + const result = await client!.send('session/set_mode', { + sessionId, + modeId: 'plan', + }); + expect(result).toEqual({}); + }, + 30_000, + ); + it( 'session/set_mode pushes current_mode_update alongside config_option_update', async () => { @@ -133,6 +158,7 @@ describe('acp-server config surface', () => { const session = Object.create(AcpSession.prototype) as AcpSession; const updates: unknown[] = []; const agent = { + getPlan: async () => null, enterPlan: async () => { throw new Error('plan toggle failed'); },