feat (ui): reconnect desktop ACP sessions after sleep and connection loss - #10411
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ecd99c9c9
ℹ️ 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".
| } catch (error) { | ||
| if (generation !== connectionGeneration) { | ||
| throw error; | ||
| } | ||
| } |
There was a problem hiding this comment.
Re-throw permanent reconnect failures
When the goose serve backend has exited, the get-acp-url IPC path calls GooseServeLeaseRegistry.getAcpUrl, which throws GOOSE_SERVE_EXITED_USER_MESSAGE instead of returning a URL. This catch treats that permanent error like a transient socket failure and retries forever, so after an unexpected backend exit the renderer stays in recovering and every getAcpClient() caller waits on a never-settling pendingConnection instead of surfacing the close-window error.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c2609cb4a2
ℹ️ 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".
| if (isAcpRecovering()) { | ||
| return; |
There was a problem hiding this comment.
Do not drop auto-submitted prompts during recovery
When a session with an initialMessage/shouldStartAgent is mounted while ACP recovery is in progress, useAutoSubmit marks the prompt as submitted and clears the initial message immediately after calling handleSubmit; this new guard then silently returns before queuing or sending anything. After reconnect, the auto-submit effect will not retry because hasAutoSubmittedRef is already true, so deep-link/fork/continue flows can lose the user's prompt instead of sending it once the connection is restored.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4924819ec6
ℹ️ 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".
| if (isAcpRecovering()) { | ||
| return; |
There was a problem hiding this comment.
Preserve append submissions during reconnect
Fresh evidence beyond the earlier auto-submit case is that BaseChat still wires recipe activities and message-list append actions directly to handleSubmit, and those controls are not disabled by queueProcessingBlocked. When a user clicks one of those append controls while ACP recovery is active, this guard returns without adding a local message or retrying after recovery, so the prompt is silently lost; please either disable those append entry points while reconnecting or keep the submitted input pending instead of dropping it here.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
this is a valid edge case. Normal submissions are blocked and auto-submit is delayed during recovery. Recipe/MCP append actions are rare during the brief reconnect window, so we’re leaving them out of scope to avoid additional queueing complexity.
Summary
Added desktop ACP recovery after laptop sleep or connection loss.
Testing
Unit testing and manual
Related issue
Fix #10368