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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/acp-set-mode-plan.md
Original file line number Diff line number Diff line change
@@ -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.
51 changes: 43 additions & 8 deletions packages/acp-server/src/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -1102,14 +1111,22 @@ export class AcpSession {
/** Switch the ACP mode (plan mode + permission mode). */
async setMode(id: AcpModeId): Promise<void> {
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;
Expand All @@ -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<boolean> {
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;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve plan cancellation when status reads fail

When getPlan() rejects while the engine is actually in plan mode (for example, status() hits a non-missing plan-file I/O error), returning false treats the state as inactive. That makes init() report default, and more importantly makes setMode('default' | 'auto' | 'yolo') skip cancelPlan() before updating permission/currentModeId and emitting a non-plan mode, leaving the engine in plan mode while ACP says it exited; propagate the unknown state or still issue the cancel for non-plan targets.

Useful? React with 👍 / 👎.

}
}

/** Push a fresh `config_option_update` to the client. */
private async emitConfigOptionUpdate(): Promise<void> {
try {
Expand Down
26 changes: 26 additions & 0 deletions packages/acp-server/test/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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');
},
Expand Down