fix(acp): confirm pending steer message on queuedSteer notification - #10532
Conversation
The server sends a session_info_update with _meta.goose.queuedSteer carrying the server-assigned messageId whenever a steer is queued. The UI never read this signal, so pending steer messages could be prematurely discarded before the agent processed them — particularly for image-only steers that have no text chunks to trigger confirmation. Add getGooseQueuedSteer to extract the messageId from the meta field, and emit localSteerConfirmed in the session_info_update handler so the steer message is confirmed as soon as the server acknowledges it.
![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: e6851dde15
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…gLocalSteerMessage The server emits queuedSteer before returning the steer RPC response, so localSteerConfirmed could fire before the UI called addPendingLocalSteerMessage. The delete was a no-op and the message was then added as pending afterward, still vulnerable to discarding. Track early confirmations in preConfirmedSteerMessageIds. When addPendingLocalSteerMessage arrives for a pre-confirmed ID, skip adding it to pending so it is never eligible for discarding.
michaelneale
left a comment
michaelneale
left a comment
There was a problem hiding this comment.
LGTM. Nicely handled — the race condition where queuedSteer can arrive before addPendingLocalSteerMessage is the tricky part, and the preConfirmedSteerMessageIds set handles it symmetrically in both directions and is cleared on replay reset. Good coverage: the race test plus steer-only / title+steer / empty adapter cases.
One minor note (non-blocking): shared.ts adds a gooseMode?: string field to the sessionInfo change type that doesn't appear to be used in this diff — is that intentional/leftover?
|
Merging this to unblock, but the interface to me is becoming confusing between:
with lots of concepts spread across these three. Could the steering process be orchestrated almost entirely in the server with one simple method to add a steering message? More broadly as well @jamadeo pointed out a couple weeks ago that sending a message in zed when a tool calling loop is in flight already continues the loop taking the new message into account without anything needed atop default ACP. I think it issues a cancel, waits for the current tool execution (if one is dispatched to respond to a tool call request) and then sends the new message and off the agent goes... Do we need anything more than this? |
* origin/main: fix(ui): clear stale pending ACP connection after terminal recovery failure (#10552) Make provider smoke tests faster and more reliable (#10605) fix(acp): confirm pending steer message on queuedSteer notification (#10532) chore(deps): bump body-parser from 1.20.5 to 1.20.6 in /documentation (#10601) chore(deps): bump webpack-dev-server from 5.2.5 to 5.2.6 in /documentation (#10593) feat(compaction): structured summary output with template rendering (#10471) feat(skills): allow disabling built-in skills (#10600) fix: apply hermit env directly in node shims so a fish login shell doesn't break MCP startup (#10028) chore(release): bump version to 1.44.0 (minor) (#10597) fix(extensions): preserve command arguments through forms (#10527) fix(permissions): enforce manual approval for code mode (#10528) fix(apps): confine app file operations (#10481) fix(local-inference): preserve featured model size on delete and backfill missing sizes (#10422) refactor(acp): extract tool call handling from server (#10574)
3 participants
Summary
The server sends a session_info_update with _meta.goose.queuedSteer carrying the server-assigned messageId whenever a steer is queued. The UI never read this signal, so pending steer messages could be prematurely discarded before the agent processed them — particularly for image-only steers that have no text chunks to trigger confirmation.
Add getGooseQueuedSteer to extract the messageId from the meta field, and emit localSteerConfirmed in the session_info_update handler so the steer message is confirmed as soon as the server acknowledges it.
Testing
Manual