Repository navigation
Conversation
![opencodejr[bot]](https://avatars.githubusercontent.com/in/5497873?s=60&v=4){kind=small}
There was a problem hiding this comment.
Looks good. I ran opencode acp from source on the base and with this PR. After renaming a session through the HTTP API, session/resume and session/load sent no session_info_update on the base. With the PR, each sent one carrying the stored title, before the response. The live-rename path needs a real model, so I didn't run it end to end, but the new wire tests cover it. bun test test/acp/ passes (104 tests) and tsc -b is clean.
Public API impact: no SDK, HTTP, config or CLI changes. The only change is that the ACP adapter now sends a standard session_info_update notification with title on rename and on load/resume/fork of a titled session. Clients that don't handle this update can ignore it, so it is backwards compatible.
One small robustness point, in the inline comment: a failed rename notification would stop the shared event stream.
| Stream.runForEach((event) => { | ||
| const entry = sessions.get(event.data.sessionID) | ||
| if (!entry) return Effect.void | ||
| if (event.type === "session.renamed") { |
There was a problem hiding this comment.
This makes the shared event stream depend on connection.sessionUpdate, which can fail with a RequestError. Before this change, the stream only ran Queue.offer, which can't fail. If one rename notification fails now, the catchCause below logs a warning and the stream ends. After that, model and agent changes made outside the client stop reaching it for every attached session, and so do later renames. Ignoring a failed notification here (for example Effect.ignore, or logging it) keeps the stream running.
There was a problem hiding this comment.
Good catch — fixed in 32f5eb6. The rename notification is now best-effort: it catches the cause, logs a warning, and keeps the shared stream running, so one failed sessionUpdate can no longer take down selection forwarding for every attached session. Interruption is still allowed through (Cause.hasInterruptsOnly), matching the handling of the other background notifications in this package (usage updates, child session updates). The full ACP suite (104 tests) and typecheck pass.
There was a problem hiding this comment.
The earlier point is addressed. A failed rename notification is now caught and logged, so it no longer stops the shared event stream that carries model and agent changes. I re-ran bun test test/acp/ (104 pass) and tsc -b (clean), and loading a renamed session still sends the stored title as a session_info_update. Nothing new to raise. The public API is unchanged by this commit.
There was a problem hiding this comment.
The new commit only merges dev, and the ACP changes are the same as at my last review. Re-checked on the merged head: bun test test/acp/ passes (104 tests), tsc -b is clean, and resuming a renamed session over opencode acp still sends the stored title as a session_info_update. Nothing new to raise. The public API is unchanged by this commit.
Issue for this PR
Closes #56894
Type of change
What does this PR do?
opencode acpnever told clients about generated session titles. Core generates a title after the first user message (SessionTitle.generate, durable eventsession.renamed), andsession/listreturns stored titles, but nothing pushed title changes mid-session. ACP clients that display dynamic session names fromsession_info_update(e.g. the ACP Pro extension) were stuck with a fallback label likeopencode - Session xxxxuntil they re-listed sessions.Changes:
packages/cli/src/acp/sessions.ts: the existing global event subscription now also handlessession.renamedand forwards it for attached sessions as{sessionUpdate: "session_info_update", title}. This is global rather than per-turn because title generation runs in a detached fiber that can settle after the triggering turn has ended; it also covers renames from other sources (e.g.session/fork). Renames for sessions the adapter does not have attached are ignored.packages/cli/src/acp/service.ts: loading, resuming, or forking a session that already carries a non-fallback title now announces it once viasession_info_update. A titled session never triggers title generation again, so without this the client would never learn the stored title. Fallback titles (New session - <timestamp>) are left alone so clients can replace them on the first rename.How did you verify your code works?
New wire tests in
packages/cli/test/acp/session.test.ts:session_info_updatewith the titlesession/prompt) is forwardedAdjusted
replay.test.tsand the command-ordering expectations to account for the new notification.bun test test/acp/(104 tests),tsc -b, and the repo'slint+typecheckall pass.Screenshots / recordings
N/A — protocol-level change, no UI.
Checklist