Handle session/setTitle and emit SessionInfoUpdate#286
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: df61470c54
ℹ️ 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".
| [dependencies] | ||
| agent-client-protocol = { version = "=0.11.1", features = ["unstable"] } | ||
| # Temporary local override until `session/setTitle` is released in the Rust SDK. | ||
| agent-client-protocol = { version = "=0.11.1", path = "../rust-sdk/src/agent-client-protocol", features = ["unstable"] } |
There was a problem hiding this comment.
Avoid committing agent-client-protocol as a local path dependency here, because path = "../rust-sdk/src/agent-client-protocol" requires a sibling checkout that is not part of this repository. In a normal clone (including CI and downstream builds), Cargo cannot resolve that path, so cargo build --locked fails before compilation. This effectively makes the commit non-buildable outside the author’s local workspace.
Useful? React with 👍 / 👎.
Previously the title was persisted to session_index.jsonl but the connected client (Zed) only saw the change on the next session/list refresh — usually after restart. Send a SessionUpdate::SessionInfoUpdate carrying the new title so the client can update its session list in place, no restart needed. Covers both rename surfaces: the session/setTitle JSON-RPC and the /rename builtin slash command both flow through handle_set_title.
daniel-agentee
force-pushed
the
acp-set-title-handler
branch
from
df61470 to
24ce239
Compare
|
Thanks @chatgpt-codex-connector — addressed in 24ce239. Replaced the
|
|
To use Codex here, create an environment for this repo. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 24ce239c69
ℹ️ 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".
| ) -> Result<SetSessionTitleResponse, Error> { | ||
| info!("Setting session title for session: {}", args.session_id); | ||
|
|
||
| let thread = self.get_thread(&args.session_id)?; |
There was a problem hiding this comment.
session/setTitle now fails fast on self.get_thread(&args.session_id), which only resolves sessions in the in-memory self.sessions map. After session/close (or before session/load), a session can still appear in session/list but is no longer in that map, so rename requests for listed historical sessions return resource_not_found instead of persisting the new title. This breaks the main rename flow for non-active sessions; the handler should allow a DB/index-only rename path when no live thread is attached.
Useful? React with 👍 / 👎.
1 participant
Summary
Implements the agent side of the new ACP
session/setTitlemethod so ACP clients (e.g. Zed) can persist a session rename through codex-acp. After a successful rename, codex-acp emits aSessionInfoUpdatenotification so the client can repaint its session list inline — no restart, nosession/listround-trip required.Why
Zed's agent panel already has a title editor for ACP sessions (zed#54689), but it can't persist anything for ACP threads because there was no protocol method to push the new title down. With the protocol method now defined upstream, codex-acp needs a handler that:
session/listreturns the new title even after a fresh process)custom-titleentry to the JSONL rollout (sofind_thread_name_by_idreturns the new value)session/listalso needs to consult the rollout'scustom-titleentries when assembling each session's title, otherwise the rename is invisible until the next codex restart.What's in this PR
codex_agent.rs— registers aSetSessionTitleRequesthandler that dispatches toThread::set_title. AdjustslistSessionsto consultfind_thread_name_by_idfirst and fall back to the formatted first user message.thread.rs— addsThread::set_title+ThreadMessage::SetTitle+ThreadActor::handle_set_title, which callsappend_thread_name(codex_home, thread_id, title)to persist the rename. Then emitsSessionUpdate::SessionInfoUpdate { title }so the client knows.test_set_session_title_renames_underlying_threadcovering both the JSONL persistence and the SessionInfoUpdate emit.Tests
21 lib tests pass on the rebuilt branch (24ce239).
Upstream dependency chain
This PR sits on top of two upstream PRs that must merge first. To keep the build green here in the meantime,
Cargo.tomlcarries a temporary[patch.crates-io]block pointing at branch revs on my fork — see the inline comment inCargo.tomlfor removal steps once releases ship.SetSessionTitleRequest/SetSessionTitleResponseto the protocol schema. Patched in here viaagent-client-protocol-schema = { git = ..., rev = b9739d6 }.agent-client-protocol = { git = ..., rev = 34523b3 }.Once #1199 and rust-sdk#164 merge and an
agent-client-protocolrelease ships (likely 0.12.0 or 0.14.0), drop both lines of the[patch.crates-io]block and bump theagent-client-protocolversion on line 21 — that's the entire cleanup.Companion PRs
/renameas a builtin slash command, on top of Expose Codex skills as ACP slash commands #279 + this PR — keeping rename infrastructure and the inline slash command as separate reviewable concerns.Draft
Drafted while waiting on the two upstream PRs above. Ready for design review on the codex-acp side; will mark ready-for-review once the dependency chain unblocks the temp patch removal.