From e4cc3916c8f1e971f9e15bbd67f42158be70fc47 Mon Sep 17 00:00:00 2001 From: HeavyGee <133152184+heavygee@users.noreply.github.com> Date: Mon, 27 Jul 2026 05:57:31 +0100 Subject: [PATCH] =?UTF-8?q?fix(cursor):=20CreatePlan=20Yes=20accepts=20and?= =?UTF-8?q?=20continues=20task=20(nested=20outcome=20+=20plan=E2=86=92exec?= =?UTF-8?q?ute)=20(#1097)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(cursor): nest ACP extension outcome so plan approvals aren't cancelled Cursor's ACP blocking extension methods (cursor/ask_question, cursor/create_plan) expect the JSON-RPC result to nest the outcome under an `outcome` key, e.g. `{ outcome: { outcome: "accepted" } }`. The adapter returned it flat (`{ outcome: "accepted" }`), so Cursor read `response.outcome.outcome` as undefined and fell back to a cancellation — an approved plan was relayed to the agent as `User cancelled`, making plan mode unusable over HAPI for Cursor sessions. Wrap every ask_question / create_plan response in the nested envelope and add regression tests asserting the exact wire shape for the affirmative approval -> CreatePlan path (plus approved_for_session, reject, and abort). Fixes #79 Co-authored-by: Cursor * fix(cursor): continue task after CreatePlan Yes (plan→execute) Nested ACP accept alone is not a complete fix: Yes unblocked create_plan but the prompt turn still ended with the plan "done" and no execution. Mirror Claude ExitPlanMode: on accept, leave plan/ask for an executable mode and queue a continue prompt so the original user task keeps going. Co-authored-by: Cursor * fix(cursor): sync runCursor enqueue mode after CreatePlan accept Codex Major on #1097: setPermissionMode alone left runCursor's currentPermissionMode stale, so the next user message could re-enter plan/ask after Yes. Notify onPermissionModeChanged from CursorSession so the enqueue source of truth stays aligned with the session. Co-authored-by: Cursor --------- Co-authored-by: Debian Co-authored-by: Cursor --- cli/src/cursor/cursorAcpRemoteLauncher.ts | 36 +++++- cli/src/cursor/loop.ts | 5 +- cli/src/cursor/runCursor.ts | 3 + cli/src/cursor/session.test.ts | 27 +++++ cli/src/cursor/session.ts | 6 + .../utils/cursorExtensionAdapter.test.ts | 110 +++++++++++++++--- .../cursor/utils/cursorExtensionAdapter.ts | 47 ++++++-- cli/src/cursor/utils/cursorModeConfig.test.ts | 16 +++ cli/src/cursor/utils/cursorModeConfig.ts | 14 +++ cli/src/cursor/utils/cursorPlanContinue.ts | 8 ++ 10 files changed, 245 insertions(+), 27 deletions(-) create mode 100644 cli/src/cursor/utils/cursorPlanContinue.ts diff --git a/cli/src/cursor/cursorAcpRemoteLauncher.ts b/cli/src/cursor/cursorAcpRemoteLauncher.ts index ef2d8cfa..8cbe7af2 100644 --- a/cli/src/cursor/cursorAcpRemoteLauncher.ts +++ b/cli/src/cursor/cursorAcpRemoteLauncher.ts @@ -24,8 +24,10 @@ import { applyCursorAcpMode, applyCursorAcpModel, isCursorAutoReviewMode, + resolveCursorModeAfterPlanApproval, wireIdForCursorSessionState } from './utils/cursorModeConfig'; +import { CURSOR_PLAN_CONTINUE } from './utils/cursorPlanContinue'; import { cursorPassThroughStatusMessage, parseCursorSpecialCommand } from './cursorSpecialCommands'; import { buildCursorModelsSeedPayload, seedCursorModelsCache } from '@/modules/common/cursorModels'; import { readSharedCursorModelsCache } from '@/modules/common/cursorModelsSharedCache'; @@ -34,6 +36,7 @@ import { registerAcpSessionTitleSync } from '@/agent/acpSessionTitle'; class CursorAcpRemoteLauncher extends RemoteLauncherBase { private readonly session: CursorSession; private backend: ReturnType | null = null; + private acpSessionId: string | null = null; private permissionAdapter: PermissionAdapter | null = null; private extensionAdapter: CursorExtensionAdapter | null = null; private happyServer: { stop: () => void } | null = null; @@ -115,7 +118,8 @@ class CursorAcpRemoteLauncher extends RemoteLauncherBase { const extensionAdapter = new CursorExtensionAdapter( session.client, backend, - (message) => this.handleAgentMessage(message) + (message) => this.handleAgentMessage(message), + () => this.handleCreatePlanAccepted() ); this.extensionAdapter = extensionAdapter; @@ -155,6 +159,7 @@ class CursorAcpRemoteLauncher extends RemoteLauncherBase { mcpServers: mcpServerList }); } + this.acpSessionId = acpSessionId; if (acpSessionId !== resumeSessionId) { session.onSessionFoundWithProtocol(acpSessionId, 'acp'); @@ -304,6 +309,35 @@ class CursorAcpRemoteLauncher extends RemoteLauncherBase { setCursorAcpModelsSnapshot(null); } + private handleCreatePlanAccepted(): void { + const backend = this.backend; + const acpSessionId = this.acpSessionId; + if (!backend || !acpSessionId) { + logger.warn('[cursor-acp] CreatePlan accepted but ACP session is not ready; skip continue handoff'); + return; + } + + const session = this.session; + const executeMode = resolveCursorModeAfterPlanApproval( + session.getPermissionMode() as PermissionMode + ) as PermissionMode; + + // Leave plan/ask for an executable mode, then queue a continue prompt so + // Yes means "keep going on the user task" (Claude ExitPlanMode parallel). + session.setPermissionMode(executeMode); + void applyCursorAcpMode(backend, acpSessionId, executeMode).then(() => { + this.applyDisplayMode(executeMode); + }); + + session.queue.unshiftIsolated(CURSOR_PLAN_CONTINUE, { + permissionMode: executeMode, + model: session.model + }); + logger.debug('[cursor-acp] CreatePlan accepted — queued continue prompt', { + executeMode + }); + } + private handleAgentMessage(message: AgentMessage): void { const converted = convertAgentMessage(message); if (converted) { diff --git a/cli/src/cursor/loop.ts b/cli/src/cursor/loop.ts index ecffca76..1bb0d7af 100644 --- a/cli/src/cursor/loop.ts +++ b/cli/src/cursor/loop.ts @@ -31,6 +31,8 @@ interface LoopOptions { model?: string; sessionMetadata?: Metadata | null; onSessionReady?: (session: CursorSession) => void; + /** Keep runCursor's enqueue mode in sync when the session leaves plan/ask. */ + onPermissionModeChanged?: (mode: PermissionMode) => void; } export async function loop(opts: LoopOptions): Promise { @@ -52,7 +54,8 @@ export async function loop(opts: LoopOptions): Promise { cursorWorktree: opts.cursorWorktree, cursorAddDirs: opts.cursorAddDirs, model: opts.model, - permissionMode: opts.permissionMode ?? 'default' + permissionMode: opts.permissionMode ?? 'default', + onPermissionModeChanged: opts.onPermissionModeChanged }); await runLocalRemoteSession({ diff --git a/cli/src/cursor/runCursor.ts b/cli/src/cursor/runCursor.ts index 8bc3a1af..b52d9977 100644 --- a/cli/src/cursor/runCursor.ts +++ b/cli/src/cursor/runCursor.ts @@ -183,6 +183,9 @@ export async function runCursor(opts: { model: opts.model, sessionMetadata: bootstrap.metadata, onModeChange: createModeChangeHandler(session), + onPermissionModeChanged: (permissionMode) => { + currentPermissionMode = permissionMode; + }, onSessionReady: (instance) => { sessionWrapperRef.current = instance; syncSessionMode(); diff --git a/cli/src/cursor/session.test.ts b/cli/src/cursor/session.test.ts index c1b89533..777592c2 100644 --- a/cli/src/cursor/session.test.ts +++ b/cli/src/cursor/session.test.ts @@ -38,4 +38,31 @@ describe('CursorSession', () => { cursorSessionProtocol: 'acp' }); }); + + it('notifies onPermissionModeChanged so runCursor enqueue mode stays in sync', () => { + const onPermissionModeChanged = vi.fn(); + const session = new CursorSession({ + api: {} as never, + client: { + updateMetadata: vi.fn(), + keepAlive: vi.fn(), + emitMessagesConsumed: vi.fn() + } as never, + path: '/tmp', + logPath: '/tmp/log', + sessionId: null, + messageQueue: new MessageQueue2(() => 'hash'), + onModeChange: vi.fn(), + startedBy: 'runner', + startingMode: 'remote', + mode: 'remote', + permissionMode: 'plan', + onPermissionModeChanged + }); + + session.setPermissionMode('default'); + + expect(session.getPermissionMode()).toBe('default'); + expect(onPermissionModeChanged).toHaveBeenCalledWith('default'); + }); }); diff --git a/cli/src/cursor/session.ts b/cli/src/cursor/session.ts index 6fbfc4a9..69e1a8e3 100644 --- a/cli/src/cursor/session.ts +++ b/cli/src/cursor/session.ts @@ -11,6 +11,7 @@ type LocalLaunchFailure = { }; type CursorModelApplyHandler = (model: string | null | undefined) => Promise; +type CursorPermissionModeChangedHandler = (mode: PermissionMode) => void; export class CursorSession extends AgentSessionBase { readonly cursorArgs?: string[]; @@ -23,6 +24,7 @@ export class CursorSession extends AgentSessionBase { readonly startingMode: 'local' | 'remote'; localLaunchFailure: LocalLaunchFailure | null = null; private modelApplyHandler: CursorModelApplyHandler | null = null; + private permissionModeChangedHandler: CursorPermissionModeChangedHandler | null = null; constructor(opts: { api: ApiClient; @@ -40,6 +42,7 @@ export class CursorSession extends AgentSessionBase { cursorAddDirs?: readonly string[]; model?: string; permissionMode?: PermissionMode; + onPermissionModeChanged?: CursorPermissionModeChangedHandler; }) { super({ api: opts.api, @@ -67,10 +70,13 @@ export class CursorSession extends AgentSessionBase { this.startedBy = opts.startedBy; this.startingMode = opts.startingMode; this.permissionMode = opts.permissionMode; + this.permissionModeChangedHandler = opts.onPermissionModeChanged ?? null; } setPermissionMode = (mode: PermissionMode): void => { this.permissionMode = mode; + // Keep runCursor's enqueue source of truth in sync (CreatePlan accept, ACP mode sync). + this.permissionModeChangedHandler?.(mode); }; setModel = (model: string | null | undefined): void => { diff --git a/cli/src/cursor/utils/cursorExtensionAdapter.test.ts b/cli/src/cursor/utils/cursorExtensionAdapter.test.ts index be2b7c37..ed3d8d85 100644 --- a/cli/src/cursor/utils/cursorExtensionAdapter.test.ts +++ b/cli/src/cursor/utils/cursorExtensionAdapter.test.ts @@ -7,7 +7,7 @@ import { CursorExtensionAdapter } from './cursorExtensionAdapter'; type ExtensionHandler = (params: unknown, requestId: string | number | null) => Promise; -function createHarness() { +function createHarness(options?: { onCreatePlanAccepted?: () => void }) { const handlers = new Map(); let agentState: AgentState = { requests: {}, completedRequests: {} }; const messages: AgentMessage[] = []; @@ -24,9 +24,14 @@ function createHarness() { } } as unknown as AcpSdkBackend; - const adapter = new CursorExtensionAdapter(session, backend, (message) => { - messages.push(message); - }); + const adapter = new CursorExtensionAdapter( + session, + backend, + (message) => { + messages.push(message); + }, + options?.onCreatePlanAccepted + ); return { handlers, @@ -74,9 +79,12 @@ describe('CursorExtensionAdapter', () => { answers: { q1: ['opt-a'] } }); expect(handled).toBe(true); + // Cursor ACP expects the outcome nested under `outcome` (see cursor.com/docs/cli/acp). await expect(pending).resolves.toEqual({ - outcome: 'answered', - answers: [{ questionId: 'q1', selectedOptionIds: ['opt-a'] }] + outcome: { + outcome: 'answered', + answers: [{ questionId: 'q1', selectedOptionIds: ['opt-a'] }] + } }); }); @@ -90,11 +98,16 @@ describe('CursorExtensionAdapter', () => { decision: 'denied' }); - await expect(pending).resolves.toEqual({ outcome: 'cancelled' }); + await expect(pending).resolves.toEqual({ outcome: { outcome: 'cancelled' } }); }); - it('resolves create_plan approval as accepted', async () => { - const { handlers, adapter } = createHarness(); + it('resolves create_plan approval as accepted with nested outcome envelope', async () => { + // Regression for the plan-approval bug: operator clicks "Yes" on a Cursor + // CreatePlan approval, but the agent received `User cancelled` because the + // response outcome was returned flat instead of nested. Cursor reads + // `response.outcome.outcome`, so the envelope MUST be nested. + const onCreatePlanAccepted = vi.fn(); + const { handlers, adapter } = createHarness({ onCreatePlanAccepted }); const pending = handlers.get('cursor/create_plan')!({ toolCallId: 'plan-1', plan: '# Plan' @@ -106,7 +119,65 @@ describe('CursorExtensionAdapter', () => { decision: 'approved' }); - await expect(pending).resolves.toEqual({ outcome: 'accepted' }); + await expect(pending).resolves.toEqual({ outcome: { outcome: 'accepted' } }); + expect(onCreatePlanAccepted).toHaveBeenCalledOnce(); + }); + + it('resolves create_plan approved_for_session as accepted with nested envelope', async () => { + const onCreatePlanAccepted = vi.fn(); + const { handlers, adapter } = createHarness({ onCreatePlanAccepted }); + const pending = handlers.get('cursor/create_plan')!({ + toolCallId: 'plan-1b', + plan: '# Plan' + }, null); + + await adapter.handlePermissionResponse({ + id: 'plan-1b', + approved: true, + decision: 'approved_for_session' + }); + + await expect(pending).resolves.toEqual({ outcome: { outcome: 'accepted' } }); + expect(onCreatePlanAccepted).toHaveBeenCalledOnce(); + }); + + it('does not invoke create-plan continue handoff on denial or abort', async () => { + const onCreatePlanAccepted = vi.fn(); + const { handlers, adapter } = createHarness({ onCreatePlanAccepted }); + const denied = handlers.get('cursor/create_plan')!({ toolCallId: 'plan-deny' }, null); + const aborted = handlers.get('cursor/create_plan')!({ toolCallId: 'plan-abort' }, null); + + await adapter.handlePermissionResponse({ + id: 'plan-deny', + approved: false, + decision: 'denied' + }); + await adapter.handlePermissionResponse({ + id: 'plan-abort', + approved: false, + decision: 'abort' + }); + + await expect(denied).resolves.toEqual({ outcome: { outcome: 'rejected' } }); + await expect(aborted).resolves.toEqual({ outcome: { outcome: 'cancelled' } }); + expect(onCreatePlanAccepted).not.toHaveBeenCalled(); + }); + + it('does not invoke create-plan continue handoff for ask_question answers', async () => { + const onCreatePlanAccepted = vi.fn(); + const { handlers, adapter } = createHarness({ onCreatePlanAccepted }); + const pending = handlers.get('cursor/ask_question')!({ toolCallId: 'q-ok' }, null); + + await adapter.handlePermissionResponse({ + id: 'q-ok', + approved: true, + answers: { q1: ['a'] } + }); + + await expect(pending).resolves.toMatchObject({ + outcome: { outcome: 'answered' } + }); + expect(onCreatePlanAccepted).not.toHaveBeenCalled(); }); it('resolves create_plan denial as rejected', async () => { @@ -119,7 +190,20 @@ describe('CursorExtensionAdapter', () => { decision: 'denied' }); - await expect(pending).resolves.toEqual({ outcome: 'rejected' }); + await expect(pending).resolves.toEqual({ outcome: { outcome: 'rejected' } }); + }); + + it('resolves create_plan abort as cancelled', async () => { + const { handlers, adapter } = createHarness(); + const pending = handlers.get('cursor/create_plan')!({ toolCallId: 'plan-3' }, null); + + await adapter.handlePermissionResponse({ + id: 'plan-3', + approved: false, + decision: 'abort' + }); + + await expect(pending).resolves.toEqual({ outcome: { outcome: 'cancelled' } }); }); it('returns false from handlePermissionResponse for unrelated permission ids', async () => { @@ -198,8 +282,8 @@ describe('CursorExtensionAdapter', () => { await adapter.cancelAll('User aborted'); - await expect(askPending).resolves.toEqual({ outcome: 'cancelled' }); - await expect(planPending).resolves.toEqual({ outcome: 'cancelled' }); + await expect(askPending).resolves.toEqual({ outcome: { outcome: 'cancelled' } }); + await expect(planPending).resolves.toEqual({ outcome: { outcome: 'cancelled' } }); expect(getAgentState().requests).toEqual({}); expect(getAgentState().completedRequests).toMatchObject({ 'q-cancel': { status: 'canceled', decision: 'abort' }, diff --git a/cli/src/cursor/utils/cursorExtensionAdapter.ts b/cli/src/cursor/utils/cursorExtensionAdapter.ts index cad638bf..f452a4d3 100644 --- a/cli/src/cursor/utils/cursorExtensionAdapter.ts +++ b/cli/src/cursor/utils/cursorExtensionAdapter.ts @@ -21,13 +21,17 @@ type PermissionResponseMessage = { export type CursorExtensionMessageHandler = (message: AgentMessage) => void; +/** Invoked when the operator accepts a CreatePlan request (Yes / Yes for session). */ +export type CursorCreatePlanAcceptedHandler = () => void; + export class CursorExtensionAdapter { private readonly pending = new Map(); constructor( private readonly session: ApiSessionClient, private readonly backend: AcpSdkBackend, - private readonly onMessage: CursorExtensionMessageHandler + private readonly onMessage: CursorExtensionMessageHandler, + private readonly onCreatePlanAccepted?: CursorCreatePlanAcceptedHandler ) { this.registerHandlers(); } @@ -103,19 +107,29 @@ export class CursorExtensionAdapter { const decision = response.decision ?? (response.approved ? 'approved' : 'denied'); if (pending.tool === 'CursorAskQuestion') { if (decision === 'abort' || decision === 'denied') { - pending.respond({ outcome: 'cancelled' }); + pending.respond(wrapOutcome({ outcome: 'cancelled' })); } else { - pending.respond({ + pending.respond(wrapOutcome({ outcome: 'answered', answers: formatQuestionAnswers(pending.arguments, response.answers) - }); + })); } } else if (decision === 'abort') { - pending.respond({ outcome: 'cancelled' }); + pending.respond(wrapOutcome({ outcome: 'cancelled' })); } else if (decision === 'denied') { - pending.respond({ outcome: 'rejected' }); + pending.respond(wrapOutcome({ outcome: 'rejected' })); } else { - pending.respond({ outcome: 'accepted' }); + // Accept first so Cursor unblocks, then hand off to execute (mode + // switch + continue prompt). Without the handoff, Yes ends the turn + // with "plan done" instead of continuing the user's task. + pending.respond(wrapOutcome({ outcome: 'accepted' })); + if (pending.tool === 'CursorCreatePlan') { + try { + this.onCreatePlanAccepted?.(); + } catch (error) { + logger.warn('[cursor-acp] onCreatePlanAccepted failed', error); + } + } } const status = response.approved ? 'approved' : 'denied'; @@ -205,11 +219,7 @@ export class CursorExtensionAdapter { this.pending.clear(); for (const [id, pending] of entries) { - pending.respond( - pending.tool === 'CursorAskQuestion' - ? { outcome: 'cancelled' } - : { outcome: 'cancelled' } - ); + pending.respond(wrapOutcome({ outcome: 'cancelled' })); this.session.updateAgentState((currentState) => { const requestEntry = currentState.requests?.[id]; @@ -235,6 +245,19 @@ export class CursorExtensionAdapter { } } +/** + * Cursor's ACP blocking extension methods (`cursor/ask_question`, + * `cursor/create_plan`) expect the JSON-RPC result to nest the outcome under an + * `outcome` key, e.g. `{ outcome: { outcome: "accepted" } }`. Returning the + * outcome object flat (`{ outcome: "accepted" }`) makes Cursor read + * `response.outcome.outcome` as undefined and fall back to a cancellation, so an + * approved plan is relayed to the agent as `User cancelled`. See + * https://cursor.com/docs/cli/acp (CursorCreatePlanResponse / CursorAskQuestionResponse). + */ +function wrapOutcome(outcome: T): { outcome: T } { + return { outcome }; +} + function extractToolCallId(params: unknown): string | null { if (!isObject(params)) return null; return asString(params.toolCallId); diff --git a/cli/src/cursor/utils/cursorModeConfig.test.ts b/cli/src/cursor/utils/cursorModeConfig.test.ts index e827a4ba..df85c58b 100644 --- a/cli/src/cursor/utils/cursorModeConfig.test.ts +++ b/cli/src/cursor/utils/cursorModeConfig.test.ts @@ -5,6 +5,7 @@ import { applyCursorAcpMode, isCursorAutoReviewMode, resolveCursorAcpWireId, + resolveCursorModeAfterPlanApproval, toCursorAcpMode, wireIdForCursorSessionState } from './cursorModeConfig'; @@ -30,6 +31,21 @@ describe('toCursorAcpMode', () => { }); }); +describe('resolveCursorModeAfterPlanApproval', () => { + it('leaves plan/ask for default so Yes can execute the task', () => { + expect(resolveCursorModeAfterPlanApproval('plan')).toBe('default'); + expect(resolveCursorModeAfterPlanApproval('ask')).toBe('default'); + expect(resolveCursorModeAfterPlanApproval(undefined)).toBe('default'); + }); + + it('preserves executable modes (yolo, default, debug, autoReview)', () => { + expect(resolveCursorModeAfterPlanApproval('yolo')).toBe('yolo'); + expect(resolveCursorModeAfterPlanApproval('default')).toBe('default'); + expect(resolveCursorModeAfterPlanApproval('debug')).toBe('debug'); + expect(resolveCursorModeAfterPlanApproval('autoReview')).toBe('autoReview'); + }); +}); + describe('applyCursorAcpMode', () => { it('prefers set_config_option for mode changes', async () => { const setConfigOption = vi.fn(async () => {}); diff --git a/cli/src/cursor/utils/cursorModeConfig.ts b/cli/src/cursor/utils/cursorModeConfig.ts index 77488bdd..36a5693b 100644 --- a/cli/src/cursor/utils/cursorModeConfig.ts +++ b/cli/src/cursor/utils/cursorModeConfig.ts @@ -18,6 +18,20 @@ export function toCursorAcpMode(mode: CursorPermissionMode | undefined): CursorA return 'agent'; } +/** + * Permission mode to use after the operator accepts a CreatePlan request. + * Plan/ask are read-only planning modes — leave them for an executable mode so + * "Yes" means continue the task, not "plan complete, stop". + */ +export function resolveCursorModeAfterPlanApproval( + mode: CursorPermissionMode | undefined +): CursorPermissionMode { + if (mode === 'plan' || mode === 'ask' || mode === undefined) { + return 'default'; + } + return mode; +} + /** True when HAPI permission mode should spawn/toggle Cursor Auto-review. */ export function isCursorAutoReviewMode(mode: CursorPermissionMode | undefined): boolean { return mode === 'autoReview'; diff --git a/cli/src/cursor/utils/cursorPlanContinue.ts b/cli/src/cursor/utils/cursorPlanContinue.ts new file mode 100644 index 00000000..de8f920f --- /dev/null +++ b/cli/src/cursor/utils/cursorPlanContinue.ts @@ -0,0 +1,8 @@ +/** + * Queued after the operator accepts a Cursor `create_plan` request so the + * session continues toward the original user task (mirror of Claude's + * PLAN_FAKE_RESTART after ExitPlanMode approval). Without this, Yes only + * unblocks ACP and the prompt turn ends — plan complete, task abandoned. + */ +export const CURSOR_PLAN_CONTINUE = + 'The plan was approved. Continue executing it now toward completing the user\'s original request. Do not stop solely because the plan was accepted.';