fix: keep ACP session naming out of live conversations - #10963
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa15700fa0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let session_name_update_tx = self.config.session_name_update_tx.clone(); | ||
| let session_id_for_title = session_id.to_string(); | ||
| let runtime = tokio::runtime::Handle::current(); | ||
| provider.set_session_title_callback(Arc::new(move |title| { |
There was a problem hiding this comment.
When GOOSE_DISABLE_SESSION_NAMING/ACP disable_session_naming is set, reply_internal skips maybe_update_name and the ACP server also withholds session_name_update_tx, but this callback is still registered and always calls update_name_from_provider, so any ACP agent that emits a SessionInfoUpdate.title will still persistently rename the session despite the opt-out. Please gate this registration (or the callback body) on !self.config.disable_session_naming, not just the notification send.
Useful? React with 👍 / 👎.
lifeizhou-ap
left a comment
lifeizhou-ap
left a comment
There was a problem hiding this comment.
Looks good. There is a code review comment that seems valid and it is worthwhile addressing it.
* main: (32 commits) fix: keep turn-context in place for OpenAI Responses-stack models so prompt caching works (#10993) fix(developer): byte-bound the shell truncation preview (#10992) fix(openrouter): stop silently ignoring thinking effort off (#10991) fix: dispatch edited queued messages (#10933) fix: contain recipe template paths (#10930) fix: make shell approval titles faithful (#10986) fix: block MCP app form submissions (#10985) fix: migrate desktop routing to React Router 8.3.0 (#10971) fix: sanitize Bedrock tool errors (#10934) fix implicit OAuth during model discovery (#10929) fix: update React Router to 7.18.2 (#10967) test: early-exit code-exec smoke tests once tool invocation is observed (#10954) fix: keep ACP session naming out of live conversations (#10963) Bind MCP apps to trusted ownership metadata (#10747) tests: add recursion_limit attribute to remaining ACP test files (#10559) Sanitize Unicode tags in MCP resources (#10746) fix(oauth): preserve RFC 9207 iss from MCP OAuth callback (#10678) feat(installer): detect Termux and select musl portable build (#10568) feat: add Celeris provider (#10714) fix: shell ACP providers on desktop (#10907) ...
* main: (101 commits) fix: keep turn-context in place for OpenAI Responses-stack models so prompt caching works (#10993) fix(developer): byte-bound the shell truncation preview (#10992) fix(openrouter): stop silently ignoring thinking effort off (#10991) fix: dispatch edited queued messages (#10933) fix: contain recipe template paths (#10930) fix: make shell approval titles faithful (#10986) fix: block MCP app form submissions (#10985) fix: migrate desktop routing to React Router 8.3.0 (#10971) fix: sanitize Bedrock tool errors (#10934) fix implicit OAuth during model discovery (#10929) fix: update React Router to 7.18.2 (#10967) test: early-exit code-exec smoke tests once tool invocation is observed (#10954) fix: keep ACP session naming out of live conversations (#10963) Bind MCP apps to trusted ownership metadata (#10747) tests: add recursion_limit attribute to remaining ACP test files (#10559) Sanitize Unicode tags in MCP resources (#10746) fix(oauth): preserve RFC 9207 iss from MCP OAuth callback (#10678) feat(installer): detect Termux and select musl portable build (#10568) feat: add Celeris provider (#10714) fix: shell ACP providers on desktop (#10907) ...
2 participants
Summary
session_info_updatetitles and persist them for the matching Goose sessionRoot cause
Session naming called
Provider::completewith a hidden prompt. For ACP providers, the default completion implementation streams that prompt through the live stateful agent session, which could produce bad titles and visible phantom responses.Stateful providers now stay on the local naming path, so the hidden prompt never reaches their live session. ACP agents can replace the fallback by publishing a title through the standard session info update.
User impact
Codex ACP sessions receive a stable local title without an extra agent turn. Claude ACP titles can replace that fallback when the adapter publishes its generated title. User-provided titles always win.
Closes #10843
Verification
cargo test -p goose --lib session_title_publisher_forwards_non_empty_titlescargo test -p goose --lib test_maybe_update_name_uses_local_name_for_stateful_providercargo test -p goose --lib test_provider_name_replaces_generated_name_but_not_user_namecargo clippy -p goose --lib -- -D warningscargo fmt