feat(acp): re-enable change_title for ACP launchers with manual title precedence - #1672
feat(acp): re-enable change_title for ACP launchers with manual title precedence#1672junmo-kim wants to merge 4 commits into
Conversation
… precedence opencode/cursor/kimi disabled the hapi change_title MCP tool (tiann#1028), relying solely on native ACP session-title sync. When the agent never generates a native title there was no way to set one short of hub API calls. Re-enable change_title on those launchers. The shared title-sync controller marks manual titles, so later native sessionInfoUpdate titles no longer overwrite them.
Copilot has the same ACP title-sync shape as opencode/cursor/kimi, so it gets the same shared controller wiring. Add a cursor regression test asserting one controller instance is shared across backend (re)creation paths and cover the onChangeTitle failure path.
There was a problem hiding this comment.
Findings
- [Major] Manual-title precedence is lost when the remote launcher is recreated — the new
manualflag only lives in one controller closure. HAPI creates a new remote launcher after a local↔remote round trip, and a resumed process also constructs a fresh controller, so the next ACPsessionInfoUpdatecan overwrite the title set throughchange_title. Evidence:cli/src/agent/acpSessionTitle.ts:18(related lifecycle context:cli/src/agent/loopBase.ts:59,cli/src/opencode/opencodeRemoteLauncher.ts:1129).
Suggested fix:Add the optional marker totype AcpSessionTitleClient = Pick< ApiSessionClient, "sendClaudeSessionMessage" | "getMetadata" | "updateMetadata" >; let manual = client.getMetadata()?.acpManualTitle === true; // in markManualTitle manual = true; client.updateMetadata((metadata) => ({ ...metadata, acpManualTitle: true }));
MetadataSchemaand preserve it in resumed-session metadata.
Summary
- Review mode: initial
- One Major issue found: manual precedence currently survives Cursor backend recreation, but not normal launcher/session recreation.
Testing
- Not run (automation; PR code was not executed). GitHub integration check passed; main test check was pending at review time.
- Add a regression test that marks a title manual, recreates the controller from the same persisted metadata, then verifies a later native title is ignored. Add one remote→local→remote launcher test.
HAPI Bot
The manual flag lived only in one controller closure, so a recreated remote launcher or a resumed session started fresh and let the next native ACP sessionInfoUpdate overwrite a change_title rename. Store acpManualTitle in session metadata and initialize new controllers from it.
There was a problem hiding this comment.
Findings
- [Major] Process resume still discards the manual-title marker — the new controller reads
acpManualTitle, butbootstrapExistingSession()rebuilds metadata through an allowlist that omits it and immediately writes that rebuilt object back. Evidencecli/src/agent/acpSessionTitle.ts:22; related contextcli/src/agent/sessionFactory.ts:121andcli/src/agent/sessionFactory.ts:357.
Suggested fix:if (metadata.acpManualTitle !== undefined) { preserved.acpManualTitle = metadata.acpManualTitle }
Summary
Review mode: follow-up after new commits
The prior Major remains on actual process resume. The added schema field and controller-recreation unit test only work when the marker is already present in the newly constructed client's metadata; the existing-session bootstrap drops it first.
Testing
Not run (automation; PR code was not executed). At review time, integration and drift-gate passed; the main test check was pending. Add a bootstrapExistingSession regression test with acpManualTitle: true and verify both returned and written metadata retain it.
HAPI Bot
bootstrapExistingSession rebuilds metadata through an allowlist, so the manual-title marker was dropped on process resume and native ACP title sync could overwrite a change_title rename.
There was a problem hiding this comment.
Findings
- None.
Summary
Review mode: follow-up after new commits
No issues found in the latest full diff. The prior Major is resolved by preserving acpManualTitle in existing-session metadata (cli/src/agent/sessionFactory.ts:123) and asserting returned, written, and runner-reported metadata retain it (cli/src/agent/sessionFactory.test.ts:167).
Residual risk: local execution was unavailable because Bun is not installed in this review runner; the GitHub test check remained in progress at review time.
Testing
Not run locally (bunx: command not found). At review time, integration and drift-gate passed; test was pending.
HAPI Bot
Why
ACP-based launchers (opencode, cursor, kimi, copilot) disable the hapi
change_titleMCP tool (#1028) and rely solely on native ACP session-title sync. When the agent never generates a native title (e.g. short sessions), there is no way to set a title short of hub API calls.What
change_titleon those four ACP launchers.change_titlemarks a shared title-sync controller (createAcpSessionTitleSync), so later nativesessionInfoUpdatetitles no longer overwrite a manual rename. Cursor's three backend-(re)creation paths share one controller instance.Verification
onChangeTitlesuccess/failure paths; full suites green (cli 2452 / web 2845 / shared / relay) + typecheck all packages.change_titlein--tools, and the MCP server'stools/listexposes it.