Skip to content

accounts: preserve sign-in during session replacement - #333206

Merged
Vritant Bhardwaj (vritant24) merged 6 commits into
mainfrom
agents/investigate-issue-333197-root-cause
Aug 31, 2026
Merged

accounts: preserve sign-in during session replacement#333206
Vritant Bhardwaj (vritant24) merged 6 commits into
mainfrom
agents/investigate-issue-333197-root-cause

Conversation

@vritant24

@vritant24 Vritant Bhardwaj (vritant24) commented Aug 28, 2026

Copy link
Copy Markdown
Member

Summary

  • Keeps the default account signed in while GitHub authentication replaces a session in a combined event.
  • Prevents the Agent Host sign-in banner from appearing during an active session when the replacement credential remains valid.
  • Adds privacy-safe diagnostics that distinguish interactive-login replacements from shared secret-storage reconciliation.
  • Preserves immediate sign-out behavior for removal-only events, including while replacement reconciliation is in flight.

Fixes #333197

Technical context for AI-assisted review

Intent and previous behavior

GitHub Authentication can publish one session-change event containing both the removed current session and its replacement. The default-account listener treated the removal as a complete sign-out, cleared the account, and skipped reconciliation. That temporary null account propagated through chat entitlement state and caused the Agent Host sign-in banner to appear even though the replacement credential was valid.

Runtime logs from the reported occurrence show the GitHub removal and addition in the same event cycle, followed within milliseconds by the Sessions signed-out path. Subsequent authentication using the replacement credential succeeded.

Implementation

The default-account listener now distinguishes removal-only events from events containing added or changed replacement candidates. Removal-only events still clear the account immediately. Replacement events retain the current account while the existing throttled refresh retrieves, validates, and selects the actual default session.

Each asynchronous refresh captures the current session before it starts. If that session changes while the refresh is pending, the stale result is discarded so an earlier replacement reconciliation cannot overwrite a later removal-only sign-out.

GitHub Authentication records replacement events at info level with only the source category, event kind, and added/removed/changed counts. Routine session changes remain at trace level. The logs intentionally exclude account labels and identifiers, session identifiers, scopes, and credentials.

Regression coverage drives a combined replacement and a genuine removal-only event through the authentication service fixture. A blocked-refresh case verifies that removal remains final when replacement reconciliation was already in flight.

Behavior and constraints

  • Added or changed sessions are reconciliation candidates; they are not assigned directly.
  • If reconciliation finds no valid replacement, the normal refresh clears the default account.
  • Events from unrelated authentication providers remain ignored.
  • This change addresses the false default-account transition and sign-in banner. Remote Agent Host tunnel reconnect churn from the same authentication event remains separate follow-up work.

Copilot AI balanced review requested due to automatic review settings August 28, 2026 17:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

Review tier: Balanced
Findings: 1 High severity

New issues introduced by this change (1)
Severity Finding
High severity src/​vs/​workbench/​services/​accounts/​browser/​defaultAccount.ts — A removal-only event can be undone by an already-running replacement refresh. That refresh captures…
What changed in this PR

Preserves default-account continuity while GitHub Authentication replaces sessions.

Changes:

  • Reconciles combined removal/replacement events without an intermediate sign-out.
  • Retains immediate clearing for removal-only events.
  • Adds regression coverage for replacement, removal, and later addition.
File Description
src/​vs/​workbench/​services/​accounts/​browser/​defaultAccount.ts Distinguishes replacement events from removal-only events.
src/​vs/​workbench/​services/​accounts/​test/​browser/​defaultAccount.test.ts Tests session transition behavior and adds authentication fixture overrides.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/vs/workbench/services/accounts/browser/defaultAccount.ts Outdated
@vs-code-engineering

Copy link
Copy Markdown
Contributor

📬 CODENOTIFY

The following users are being notified based on files changed in this PR:

TylerLeonhardt

Matched files:

  • extensions/github-authentication/src/github.ts

Comment thread src/vs/workbench/services/accounts/browser/defaultAccount.ts Outdated
Keep the default account available while authentication providers replace a
removed session with a new candidate in the same event. Add privacy-safe logs
to distinguish interactive and secret-storage replacements.

Fixes #333197

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

Review tier: Balanced
Findings: 1 High severity · 1 Medium severity

New issues introduced by this change (2)
Severity Finding
High severity src/​vs/​workbench/​services/​accounts/​browser/​defaultAccount.ts — A removal-only event can be overwritten by the reconciliation started for a preceding replacement…
Medium severity src/​vs/​workbench/​services/​accounts/​test/​browser/​defaultAccount.test.ts — The PR description says the regression coverage exercises changed-session candidates and recovery…
Issues resolved since last review (1)
Severity Finding
High severity src/​vs/​workbench/​services/​accounts/​browser/​defaultAccount.ts — A removal-only event can be undone by an already-running replacement refresh. That refresh captures… View resolved comment

Comment thread src/vs/workbench/services/accounts/browser/defaultAccount.ts
Do not publish an asynchronous refresh result after the current session changes.
This keeps a removal-only sign-out final when replacement reconciliation was
already in flight.

Refs #333197

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Base: 193cdecf Current: bce499fe

No screenshot changes.

@vritant24
Vritant Bhardwaj (vritant24) marked this pull request as ready for review August 31, 2026 20:38
Model the Agents web workbench in the account test fixture so browser runs do
not skip provider initialization and session-change listener registration.

Refs #333197

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Model a local editor in Electron and a remote editor in browser tests so the
shared default-account replacement behavior is covered outside the Agents
window as well.

Refs #333197

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@vritant24
Vritant Bhardwaj (vritant24) merged commit 57341dd into main Aug 31, 2026
37 checks passed
@vritant24
Vritant Bhardwaj (vritant24) deleted the agents/investigate-issue-333197-root-cause branch August 31, 2026 23:14
@vs-code-engineering vs-code-engineering Bot added this to the 1.137.0 milestone Aug 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sign in bar popped up in the middle of an active session

4 participants