Skip to content

feat(acp): re-enable change_title for ACP launchers with manual title precedence - #1672

Open
junmo-kim wants to merge 4 commits into
tiann:mainfrom
junmo-kim:feat/acp-change-title
Open

feat(acp): re-enable change_title for ACP launchers with manual title precedence#1672
junmo-kim wants to merge 4 commits into
tiann:mainfrom
junmo-kim:feat/acp-change-title

Conversation

@junmo-kim

@junmo-kim junmo-kim commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Why

ACP-based launchers (opencode, cursor, kimi, copilot) disable the hapi change_title MCP 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

  • Re-enable change_title on those four ACP launchers.
  • Manual titles win over native sync: change_title marks a shared title-sync controller (createAcpSessionTitleSync), so later native sessionInfoUpdate titles no longer overwrite a manual rename. Cursor's three backend-(re)creation paths share one controller instance.
  • Add the title-tool usage instruction back to the opencode prompt.

Verification

  • Unit: manual-precedence controller test, cursor controller-sharing test, onChangeTitle success/failure paths; full suites green (cli 2452 / web 2845 / shared / relay) + typecheck all packages.
  • Isolated E2E: spawned an opencode session against an isolated hub+runner — the bridge process now advertises change_title in --tools, and the MCP server's tools/list exposes it.

… 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.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Findings

  • [Major] Manual-title precedence is lost when the remote launcher is recreated — the new manual flag 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 ACP sessionInfoUpdate can overwrite the title set through change_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:
    type AcpSessionTitleClient = Pick<
        ApiSessionClient,
        "sendClaudeSessionMessage" | "getMetadata" | "updateMetadata"
    >;
    
    let manual = client.getMetadata()?.acpManualTitle === true;
    
    // in markManualTitle
    manual = true;
    client.updateMetadata((metadata) => ({ ...metadata, acpManualTitle: true }));
    Add the optional marker to MetadataSchema and 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

Comment thread cli/src/agent/acpSessionTitle.ts
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.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Findings

  • [Major] Process resume still discards the manual-title marker — the new controller reads acpManualTitle, but bootstrapExistingSession() rebuilds metadata through an allowlist that omits it and immediately writes that rebuilt object back. Evidence cli/src/agent/acpSessionTitle.ts:22; related context cli/src/agent/sessionFactory.ts:121 and cli/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

Comment thread cli/src/agent/acpSessionTitle.ts
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.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant