agentHost: restore authentication on transport reconnect - #339454
Open
Osvaldo Ortega (osortega) wants to merge 1 commit into
Open
Osvaldo Ortega (osortega) wants to merge 1 commit into
Osvaldo Ortega (osortega) wants to merge 1 commit into
Conversation
Re-establish authentication on replacement transports even when reconnect resumes an existing client. Restore protected subscriptions after authentication before reporting readiness or draining queued actions. Add regression coverage for replay and snapshot recovery with cached or resolved credentials, response continuity, and genuinely unavailable subscriptions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Authentication and subscription recovery ordering needs final human review, with live mobile background/foreground validation still pending.
Review effort: Balanced
Findings: None
What changed in this PR
Follow-up to #339229 that restores authentication on replacement transports in the shared Agent Host Protocol client.
Changes:
- Re-authenticates during replay and snapshot recovery.
- Restores protected subscriptions before releasing queued actions, while preserving terminal subscription errors.
- Adds six regressions covering credentials, recovery ordering, response continuity, and subscription failures.
| File | Description |
|---|---|
| src/vs/platform/agentHost/test/electron-browser/agentHostProtocolClient.test.ts | Adds authentication and subscription recovery regressions. |
| src/vs/platform/agentHost/browser/agentHostProtocolClient.ts | Coordinates authentication, subscription restoration, and recovery readiness. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Screenshot ChangesBase: Changed (1)
|
| Before | After |
|---|---|
 |
 |
2 insignificant change(s) omitted (≤20 px, Δ≤2). See CI logs for details.
Osvaldo Ortega (osortega)
marked this pull request as ready for review
October 3, 2026 07:41
Osvaldo Ortega (osortega)
enabled auto-merge (squash)
October 3, 2026 07:42
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #339229. The transport can now reconnect, but the client still assumed that a remembered client ID meant authentication survived on the replacement connection. A host that requires authentication again can reject later requests with
AuthRequired, while local authentication caching prevents a new authentication request from being sent.Validation
Manual verification pending
On mobile, open a sandbox chat, send a message, switch to another app, return, and send a follow-up. Verify recovery authenticates before messages are released and that responses remain visible after completion.
The supplied logs establish the post-reconnect authentication failure. They do not identify the chat-state update that removed an earlier streamed reply, so this PR does not claim that the separate disappearing-response symptom is conclusively resolved. Live mobile validation remains pending. The separately reported regular-tunnel recovery issue is also outside this fix.