feat(cursor): migrate remote sessions to ACP with model/variant pickers#799
Open
swear01 wants to merge 8 commits into
Open
feat(cursor): migrate remote sessions to ACP with model/variant pickers#799swear01 wants to merge 8 commits into
swear01 wants to merge 8 commits into
Conversation
…ffort pickers Move stream-json remote launcher to legacy path and add ACP launcher with set_config_option model/mode sync, optimistic keepalive on config changes, and shared catalog caching. Web gets dual base/effort Cursor pickers for session and new-session flows; hide composer status bar when Cursor sends no usage_update. Co-authored-by: Cursor <cursoragent@cursor.com>
Enrich the web/mobile picker with agent --list-models SKUs grouped under ACP wire bases, fix session-open base highlight, and keep catalog discovery safe while the ACP transport holds the CLI lock. Co-authored-by: Cursor <cursoragent@cursor.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Findings
- [Major] Resetting Cursor model to Default does not update ACP backend — the web Default row sends
model: null, butapplyLiveModelreturns before making any ACPsession/set_config_optioncall, so HAPI clearssession.modelwhile Cursor keeps the previously selected explicit model. Evidence:web/src/lib/sessionChatCursorModel.ts:46maps Default towireId: null, andcli/src/cursor/cursorAcpRemoteLauncher.ts:316exits before applying a backend default. Users who switch from a model variant back to Default will see Default in HAPI, but subsequent prompts can still run on the old Cursor model.
Suggested fix:if (!requested || isSpawnDefaultModel(requested)) { const modelOption = backend.getConfigOptionByCategory?.(acpSessionId, 'model'); const defaultWire = modelOption?.options.find((option) => isSpawnDefaultModel(option.value))?.value; if (modelOption && defaultWire) { await backend.setConfigOption(acpSessionId, modelOption.id, defaultWire); backend.pinSessionModelWireId(acpSessionId, defaultWire); } else if (options.throwOnFailure) { throw new Error('Cursor default model is not available via ACP'); } this.currentBackendModel = null; previousSetModel(undefined); this.session.pushKeepAlive(); syncCursorModelsFromAcp(backend, acpSessionId); return null; }
Questions
- None.
Summary
- Review mode: initial
- One live Cursor ACP model reset regression found. Add a regression test that starts with an explicit Cursor model, calls
session.setModel(null)/applyModelConfig(null), and asserts the backend receives the ACP default model option.
Testing
- Not run (automation; static PR review only).
HAPI Bot
| const previousModel = this.currentBackendModel ?? this.session.model ?? null; | ||
| const applySeq = ++this.modelApplySeq; | ||
|
|
||
| if (!requested || isSpawnDefaultModel(requested)) { |
There was a problem hiding this comment.
[MAJOR] Resetting Cursor model to Default does not update ACP backend. The web Default row sends model: null, but this branch returns before making any ACP session/set_config_option call, so HAPI clears session.model while Cursor keeps the previously selected explicit model. Users who switch from a model variant back to Default will see Default in HAPI, but subsequent prompts can still run on the old Cursor model.
Suggested fix:
if (!requested || isSpawnDefaultModel(requested)) {
const modelOption = backend.getConfigOptionByCategory?.(acpSessionId, 'model');
const defaultWire = modelOption?.options.find((option) => isSpawnDefaultModel(option.value))?.value;
if (modelOption && defaultWire) {
await backend.setConfigOption(acpSessionId, modelOption.id, defaultWire);
backend.pinSessionModelWireId(acpSessionId, defaultWire);
} else if (options.throwOnFailure) {
throw new Error('Cursor default model is not available via ACP');
}
this.currentBackendModel = null;
previousSetModel(undefined);
this.session.pushKeepAlive();
syncCursorModelsFromAcp(backend, acpSessionId);
return null;
}Web sends model: null for Default; push session/set_config_option with the ACP default[] wire so Cursor backend matches hub state. Regression tests for setModel(null) and applyModelConfig(null). Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Findings
- [Major] Stale ACP lock can permanently disable Cursor model discovery — the new cross-process guard treats any existing lock directory as an active
agent acptransport, but the lock is only removed by normalclose/exithandlers. If the session child is SIGKILLed, the machine reboots, or the process crashes beforeunregisterActiveAcpTransport(),isAgentAcpTransportActive()keeps returning true forever, solistCursorModels()takes the active-ACP path and will not run the cold ACP/CLI probes. Users then see no Cursor model catalog until they manually delete$HAPI_HOME/locks/agent-acp-active. Evidence:cli/src/agent/backends/acp/agentCliGuard.ts:47checks only directory existence.
Suggested fix:function staleLockPid(): number | null { const raw = readFileSync(join(getAcpLockDir(), 'pid'), 'utf8').trim(); const pid = Number(raw); if (!Number.isInteger(pid) || pid <= 0) return null; try { process.kill(pid, 0); return null; } catch { return pid; } } export function isAgentAcpTransportActive(): boolean { if (activeAcpTransportCount > 0) return true; const lockDir = getAcpLockDir(); if (!existsSync(lockDir)) return false; if (staleLockPid() !== null) { rmSync(lockDir, { recursive: true, force: true }); return false; } return true; }
Questions
- None.
Summary
- Review mode: follow-up after new commits
- One lock-lifetime regression remains in the Cursor ACP model discovery path. Add a regression test that writes a lock pid for a non-running process and verifies
listCursorModels()can leave the active-ACP path and probe again.
Testing
- Not run (automation; static PR review only).
HAPI Bot
| } | ||
|
|
||
| export function isAgentAcpTransportActive(): boolean { | ||
| return activeAcpTransportCount > 0 || existsSync(getAcpLockDir()); |
There was a problem hiding this comment.
[MAJOR] Stale ACP lock can permanently disable Cursor model discovery. The new cross-process guard treats any existing lock directory as an active agent acp transport, but the lock is only removed by normal close/exit handlers. If the session child is SIGKILLed, the machine reboots, or the process crashes before unregisterActiveAcpTransport(), this keeps returning true forever, so listCursorModels() takes the active-ACP path and will not run the cold ACP/CLI probes. Users then see no Cursor model catalog until they manually delete $HAPI_HOME/locks/agent-acp-active.
Suggested fix:
function staleLockPid(): number | null {
const raw = readFileSync(join(getAcpLockDir(), 'pid'), 'utf8').trim();
const pid = Number(raw);
if (!Number.isInteger(pid) || pid <= 0) return null;
try {
process.kill(pid, 0);
return null;
} catch {
return pid;
}
}
export function isAgentAcpTransportActive(): boolean {
if (activeAcpTransportCount > 0) return true;
const lockDir = getAcpLockDir();
if (!existsSync(lockDir)) return false;
if (staleLockPid() !== null) {
rmSync(lockDir, { recursive: true, force: true });
return false;
}
return true;
}Check lock pid with signal 0; remove orphaned lock dirs after SIGKILL or crash so listCursorModels can run cold probes again. Regression tests for guard and catalog discovery. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Findings
- [Major] Cursor model discovery is disabled by unrelated ACP sessions —
AcpStdioTransportnow callsregisterActiveAcpTransport()for every ACP process, butisAgentAcpTransportActive()is used only to decide whether it is safe to spawn Cursor'sagent --list-models. Gemini/OpenCode/Kimi also constructAcpSdkBackend, so any live non-Cursor ACP session creates$HAPI_HOME/locks/agent-acp-active; the runner then treats Cursor ACP as active andlistCursorModels()returns only a snapshot/shared cache or an empty catalog instead of probing Cursor. That blocks Cursor model pickers while unrelated ACP sessions are running. Evidencecli/src/agent/backends/acp/AcpStdioTransport.ts:76.
Suggested fix:const shouldGuardAgentCli = options.command === 'agent' if (shouldGuardAgentCli) { registerActiveAcpTransport() } private releaseAgentCliGuard(): void { if (!shouldGuardAgentCli || this.guardReleased) return this.guardReleased = true unregisterActiveAcpTransport() }
Questions
- None.
Summary
- Review mode: follow-up after new commits
- The stale-lock issue from the prior HAPI Bot review appears addressed, but the guard is now applied too broadly and can regress Cursor model discovery whenever other ACP agents are active.
Testing
- Not run (automation; static PR review only).
HAPI Bot
| buildAcpStdioSpawnOptions(options.env) | ||
| ) as ChildProcessWithoutNullStreams; | ||
|
|
||
| registerActiveAcpTransport(); |
There was a problem hiding this comment.
[MAJOR] Cursor model discovery is disabled by unrelated ACP sessions. AcpStdioTransport now calls registerActiveAcpTransport() for every ACP process, but isAgentAcpTransportActive() is used only to decide whether it is safe to spawn Cursor's agent --list-models. Gemini/OpenCode/Kimi also construct AcpSdkBackend, so any live non-Cursor ACP session creates $HAPI_HOME/locks/agent-acp-active; the runner then treats Cursor ACP as active and listCursorModels() returns only a snapshot/shared cache or an empty catalog instead of probing Cursor.
Suggested fix:
const shouldGuardAgentCli = options.command === 'agent'
if (shouldGuardAgentCli) {
registerActiveAcpTransport()
}
private releaseAgentCliGuard(): void {
if (!shouldGuardAgentCli || this.guardReleased) return
this.guardReleased = true
unregisterActiveAcpTransport()
}Stale-lock cleanup clears dead pids; handler tests must simulate an active lock with the current process pid to avoid cold probes/timeouts. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Findings
- [Major] ACP lock blocks Cursor model discovery during unrelated ACP sessions —
AcpStdioTransportnow registers the globalagent-acp-activelock for every ACP subprocess, butlistCursorModels()treats that lock as meaning Cursor'sagent acpis active and refuses to spawnagent --list-models. Gemini/OpenCode/Kimi all constructAcpSdkBackendwith non-agentcommands, so any live session for those backends can make Cursor model pickers return only stale cache or an empty list. Evidencecli/src/agent/backends/acp/AcpStdioTransport.ts:76.
Suggested fix:private readonly shouldGuardAgentCli: boolean; constructor(options: { command: string; args?: string[]; env?: Record<string, string> }) { this.shouldGuardAgentCli = options.command === 'agent'; // ... spawn setup ... if (this.shouldGuardAgentCli) { registerActiveAcpTransport(); } } private releaseAgentCliGuard(): void { if (!this.shouldGuardAgentCli || this.guardReleased) return; this.guardReleased = true; unregisterActiveAcpTransport(); }
Questions
- None.
Summary
- Review mode: follow-up after new commits
- The stale-lock test update does not address the remaining regression: the Cursor-specific CLI guard is still applied to all ACP transports. I found no other high-confidence issues in the latest diff.
Testing
- Not run (automation; static PR review only).
HAPI Bot
| buildAcpStdioSpawnOptions(options.env) | ||
| ) as ChildProcessWithoutNullStreams; | ||
|
|
||
| registerActiveAcpTransport(); |
There was a problem hiding this comment.
[MAJOR] ACP lock blocks Cursor model discovery during unrelated ACP sessions. This registers the global agent-acp-active lock for every ACP subprocess, but listCursorModels() treats that lock as meaning Cursor's agent acp is active and refuses to spawn agent --list-models. Gemini/OpenCode/Kimi construct AcpSdkBackend with gemini, opencode, and kimi, so any live non-Cursor ACP session can make Cursor model pickers return only stale cache or an empty list.
Suggested fix:
private readonly shouldGuardAgentCli: boolean;
constructor(options: { command: string; args?: string[]; env?: Record<string, string> }) {
this.shouldGuardAgentCli = options.command === 'agent';
// ... spawn setup ...
if (this.shouldGuardAgentCli) {
registerActiveAcpTransport();
}
}
private releaseAgentCliGuard(): void {
if (!this.shouldGuardAgentCli || this.guardReleased) return;
this.guardReleased = true;
unregisterActiveAcpTransport();
}Gemini/OpenCode/Kimi ACP sessions must not register agent-acp-active; that blocked listCursorModels while unrelated backends were running. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Findings
- [Major] Cursor model changes are still exposed for local sessions — the new Cursor config RPC can only apply model changes after the ACP remote launcher installs its model apply handler, but the hub route still allows
/sessions/:id/modelfor locally controlled Cursor sessions. In local modecanApplyModelConfig()stays false, so the UI can send a model change for an active local Cursor session and consistently get a 409 instead of hiding/rejecting the unsupported action at the boundary. Evidencecli/src/cursor/runCursor.ts:139; related route gaphub/src/web/routes/sessions.ts:358.
Suggested fix:if ((flavor === 'codex' || flavor === 'cursor') && sessionResult.session.agentState?.controlledByUser === true) { return c.json({ error: 'Model selection can only be changed for remote sessions' }, 409) }
Questions
- None.
Summary
- Review mode: follow-up after new commits
- The prior ACP guard issue is fixed in the latest head. I found one remaining regression around exposing Cursor model changes while the session is locally controlled.
Testing
- Not run (automation; static PR review only).
HAPI Bot
| if (config.model !== undefined) { | ||
| const requestedModel = resolveNullableSessionModel(config.model); | ||
| const sessionInstance = sessionWrapperRef.current; | ||
| if (!sessionInstance?.canApplyModelConfig()) { |
There was a problem hiding this comment.
[MAJOR] Cursor model changes are still exposed for local sessions. This new guard makes model application depend on the ACP remote launcher's model apply handler, but the hub route still allows /sessions/:id/model for locally controlled Cursor sessions. In local mode the handler is never installed, so the UI can send a model change for an active local Cursor session and consistently get Cursor ACP session is not ready to apply model changes instead of hiding/rejecting the unsupported action at the boundary.\n\nSuggested fix:\nts\nif ((flavor === 'codex' || flavor === 'cursor') && sessionResult.session.agentState?.controlledByUser === true) {\n return c.json({ error: 'Model selection can only be changed for remote sessions' }, 409)\n}\n
Hub returns 409 when controlledByUser is set, matching Codex. Web hides model and variant pickers for local Cursor sessions so users do not hit a dead RPC path. Document pre-push-review in AGENTS.md. Verified: bun typecheck; bun run test (919 cli + 243 hub + 768 web + 46 shared). Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Findings
- [Major] Cursor ask_question responses send labels/indexes instead of ACP ids — the new Cursor parser drops each question
idand optionid, then the shared footer submits answers keyed byString(i)with selected option labels.CursorExtensionAdapterforwards those values asquestionIdandselectedOptionIds, so ACP receives0/Option Ainstead of the original ids likeapproach/a; prompts with choices can be answered incorrectly or rejected. Evidenceweb/src/components/ToolCard/cursorAskQuestion.ts:20; related formattercli/src/cursor/utils/cursorExtensionAdapter.ts:108.
Suggested fix:export type CursorAskQuestionQuestion = AskUserQuestionQuestion & { id: string options: Array<AskUserQuestionQuestion['options'][number] & { id: string }> } const questionId = typeof raw.id === 'string' && raw.id.trim() ? raw.id.trim() : String(questions.length) const optionId = typeof opt.id === 'string' && opt.id.trim() ? opt.id.trim() : label options.push({ id: optionId, label, description: null }) // When submitting CursorAskQuestion, key by q.id and send option.id values. answers[question.id] = selectedOptionIndices .map((idx) => question.options[idx]?.id) .filter((id): id is string => Boolean(id))
Questions
- None.
Summary
- Review mode: follow-up after new commits
- The previous local Cursor model-change issue is fixed in this head. I found one remaining regression in the new Cursor ACP ask-question flow.
Testing
- Not run (automation; static PR review only). Add a web-side test that a Cursor question
{ id: 'approach', options: [{ id: 'a', label: 'Option A' }] }submits{ approach: ['a'] }, plus the existing adapter test already covers forwarding those ids.
HAPI Bot
| for (const raw of rawQuestions) { | ||
| if (!isObject(raw)) continue | ||
|
|
||
| const question = typeof raw.prompt === 'string' |
There was a problem hiding this comment.
[MAJOR] This parser drops Cursor's stable question/option ids. The footer later submits answers keyed by question index with selected labels, and CursorExtensionAdapter forwards those values as ACP questionId / selectedOptionIds, so ACP receives values like 0 and Option A instead of approach and a.
Suggested fix:
const questionId = typeof raw.id === 'string' && raw.id.trim() ? raw.id.trim() : String(questions.length)
const optionId = typeof opt.id === 'string' && opt.id.trim() ? opt.id.trim() : label
options.push({ id: optionId, label, description: null })
// In AskUserQuestionFooter for CursorAskQuestion:
answers[question.id] = selectedOptionIndices
.map((idx) => question.options[idx]?.id)
.filter((id): id is string => Boolean(id))
Parse and submit question.id and option.id so ACP receives keys like
{ approach: ['a'] } instead of index/label. Verified: bun typecheck && bun run test.
Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Findings
- No issues found at >=80% confidence in the latest diff.
Questions
- None.
Summary
- Review mode: follow-up after new commits
- Previous HAPI Bot finding about Cursor ask_question stable ids appears addressed in this head. Residual risk is mainly integration coverage: this PR changes Cursor remote ACP startup, live model switching, model discovery cache/locks, and web model pickers across CLI/hub/web boundaries.
Testing
- Not run (automation; static PR review only). Existing added tests cover the reviewed model picker, Cursor ACP launcher, hub route/cache, and ask_question id paths.
HAPI Bot
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Migrates HAPI Cursor remote integration from
agent -p --output-format stream-jsontoagent acp, with legacy stream-json resume only for pre-migration sessions. Adds Zed-style permission/plan/question flows and a two-tier model catalog (ACP wire ids + CLIagent --list-modelsSKUs) for New Session and in-session pickers.Closes #781
Fixes #705
Fixes #687
Design heuristics
Protocol & routing
cursorSessionProtocol: 'acp'. No silent fallback to stream-json if ACP init/load fails (fail loud; operator starts fresh).cursorSessionIdwithoutacpkeepscursorLegacyRemoteLauncheruntil removed in a follow-up.configOptions+ hubsession.model. Match Zed: prefersession/set_config_option(model/mode categories), not ad-hoc CLI flags mid-session.Model catalog (two sources, one UI)
session/new/ snapshotbase[param=…])set_config_optionagent --list-modelsgpt-5.5-high-fast, …)cursorCliSkuBaseId(slug); only attach SKUs whose base matches an ACP wire base.context=272k,reasoning=medium) and show probe/display names instead.matchCliSkuToAcpWireIdbeforeset_config_option.-high-fast,-low, …) only for mapping slug → wire.Picker UX (Web)
defaultValue: 'auto'; in-session:null(Default = agent default).shouldApply: false).cursorSelectedBaseis still'auto'butpicker.baseKeyis known fromsession.model, highlight the session base (avoids flashing Default).Catalog discovery (CLI / Hub)
agent --list-models; read live snapshot → shared on-disk cache (HAPI_HOME/cache/cursor-models.json) → in-memory cache.agent acpwire probe first; then optional CLI probe only to attachcliModelSkus(wires remain authoritative).useCursorModelsForMachinewhenmetadata.machineIdis set so in-session picker gets SKUs even when session RPC is lock-bound.Keepalive / optimistic UI
setModelupdates session state immediately;update-metadatakeepalive pushesmodelto hub; rollback if ACP rejects.Explicit non-goals (this PR)
usage_updateonly.Test plan
bun typecheckbun run test(cli + hub + web unit)session.modelupdatesFollow-up
cursorLegacyRemoteLauncheronce legacy sessions are migrated or expired.usage_updatebefore first prompt (upstream/agent dependent).Made with Cursor