Skip to content

sessions: use generic sign out confirmation copy#307727

Merged
hawkticehurst merged 2 commits intomainfrom
copilot/wide-rodent
Apr 4, 2026
Merged

sessions: use generic sign out confirmation copy#307727
hawkticehurst merged 2 commits intomainfrom
copilot/wide-rodent

Conversation

@hawkticehurst
Copy link
Copy Markdown
Member

@hawkticehurst hawkticehurst commented Apr 3, 2026

Summary

Fixes #307465.

Updates the Agents app sign-out flow to show a generic confirmation dialog instead of referencing the GitHub Copilot Chat extension.

What Changed

  • keeps the shared workbench authentication sign-out action unchanged
  • handles the Agents-specific confirmation dialog directly in the sessions account menu sign-out action
  • performs the existing sign-out cleanup from the sessions action after confirmation, including removing matching auth sessions and clearing stored usage/access state

Validation

  • npm run compile-check-ts-native
  • npm run valid-layers-check
  • node --experimental-strip-types build/hygiene.ts src/vs/sessions/contrib/accountMenu/browser/account.contribution.ts src/vs/workbench/contrib/authentication/browser/actions/signOutOfAccountAction.ts

Notes

  • the scope is intentionally limited to the sessions app so other authentication dialogs keep their existing workbench behavior
  • the Agents detail text remains account-specific: This will sign out '{0}' from the Agents app.

Allow the shared account sign-out action to accept dialog copy overrides so the Agents app can show a generic confirmation instead of listing extension usage.

Adds a focused test for the override path while preserving the default workbench confirmation behavior.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 3, 2026 21:11
@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:

  • src/vs/workbench/contrib/authentication/browser/actions/signOutOfAccountAction.ts
  • src/vs/workbench/contrib/authentication/test/browser/signOutOfAccountAction.test.ts

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the Agents (sessions) sign-out confirmation flow to use generic, Agents-specific dialog copy by routing through the shared _signOutOfAccount command and allowing that command to accept optional dialog message/detail overrides.

Changes:

  • Adds optional dialogMessage / dialogDetail overrides to the shared workbench sign-out action’s confirmation dialog.
  • Updates the Agents account menu “Sign Out” action to invoke _signOutOfAccount with Agents-specific confirmation copy.
  • Adds a unit test covering both the override path and the default (extension-usage) confirmation behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/vs/workbench/contrib/authentication/test/browser/signOutOfAccountAction.test.ts Adds unit coverage for custom dialog override copy and default fallback copy.
src/vs/workbench/contrib/authentication/browser/actions/signOutOfAccountAction.ts Extends command args to accept optional confirmation dialog message/detail overrides.
src/vs/sessions/contrib/accountMenu/browser/account.contribution.ts Routes Agents sign-out through _signOutOfAccount with Agents-specific confirmation strings.

Keep the Agents-specific sign-out confirmation flow inside the sessions account menu instead of extending the shared authentication sign-out action.

This restores the core workbench auth dialog behavior and limits the copy change to the sessions app surface.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@hawkticehurst
Copy link
Copy Markdown
Member Author

hawkticehurst commented Apr 3, 2026

📬 CODENOTIFY

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

@TylerLeonhardt

Matched files:

  • src/vs/workbench/contrib/authentication/browser/actions/signOutOfAccountAction.ts
  • src/vs/workbench/contrib/authentication/test/browser/signOutOfAccountAction.test.ts

PR was reimplemented so they no longer touch these lower level workbench auth implementation files.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Comment on lines +210 to +214
const allSessions = await authenticationService.getSessions(providerId);
const sessions = allSessions.filter(session => session.account.label === accountLabel);
await Promise.all(sessions.map(session => authenticationService.removeSession(providerId, session.id)));
authenticationUsageService.removeAccountUsage(providerId, accountLabel);
authenticationAccessService.removeAllowedExtensions(providerId, accountLabel);
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

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

The sign-out implementation here duplicates the session-removal and cleanup logic from SignOutOfAccountAction (workbench). This increases the risk of behavior drift if the shared sign-out logic changes (e.g., additional per-account cleanup). Consider extracting the shared “remove all sessions for account + clear usage/access” logic into a common helper/command that both call, keeping only the confirmation dialog copy different in Sessions.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member Author

@hawkticehurst hawkticehurst Apr 3, 2026

Choose a reason for hiding this comment

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

cc @osortega @joshspicer @TylerLeonhardt –– would love any thoughts/opinions on this. The original PR did update workbench level logic, but I got a bit concerned about doing that (I've been generally trying to avoid touching non session files while working on updates for the app) and updated the implementation to be scoped to sessions app at the cost of some duplicated logic.

If any of you feel I should update workbench files though please let me know and I'll revert

@hawkticehurst hawkticehurst merged commit a240f2a into main Apr 4, 2026
23 checks passed
@hawkticehurst hawkticehurst deleted the copilot/wide-rodent branch April 4, 2026 04:14
@vs-code-engineering vs-code-engineering bot added this to the 1.115.0 milestone Apr 4, 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.

Agents: The sign-out dialog shouldn't talk about extensions

4 participants