Skip to content

fix(web): Revoke others asks before signing out every other paired device - #32734

Open
only21mil wants to merge 1 commit into
pingdotgg:mainfrom
only21mil:fix/web-confirm-revoke-other-clients
Open

only21mil wants to merge 1 commit into
pingdotgg:mainfrom
only21mil:fix/web-confirm-revoke-other-clients

Conversation

@only21mil

Copy link
Copy Markdown
Contributor

Problem

In Settings → Sharing & access, one click on Revoke others signs out every other paired client (phones, other browsers, linked machines). There is no confirmation and no undo, and each one then needs a new pairing link. The same page already asks before removing a saved environment.

To reproduce on main: pair other clients, open Settings → Sharing & access and click Revoke others. "Revoked N clients" appears with no dialog.

Change

The Revoke others button now calls requestConfirmDialog with the destructive variant and calls onRevokeOtherClients only on true. This is the same call the page uses for removing a saved environment. Cancel or dismiss does nothing; Confirm revokes as before.

The confirm sits in AuthorizedClientsHeaderAction, the button's only render site, so the existing browser test covers it. handleRevokeOtherDesktopClients is unchanged and still checks the write scope when it runs.

Left as is on purpose:

Surfaces: web and desktop render this component, so both get the confirm. Mobile has no Revoke others control.

Scope and approval

This uses the very small, focused fix exception: one click irreversibly signs out every other device without asking, while the same page confirms its other destructive actions. The fix adds one existing confirm call to one button. I found no issue or PR that asks for or rejects this. #31733 edits nearby code; git merge-tree against its head is clean.

Verification

Two rows added to the existing AuthorizedClientsSettings.browser.test.tsx: with the confirm resolving false, onRevokeOtherClients is not called; with true, it is called once.

cd apps/web && vp test run --project browser src/components/settings/AuthorizedClientsSettings.browser.test.tsx
  • With main's component (a9e709f15): Tests 2 failed | 2 passed (4), expected "vi.fn()" to be called once, but got 0 times. The confirm is never asked.
  • This branch: Tests 4 passed (4).
  • vp run check:changed a9e709f15: format, lint, knip, typecheck, test and React Compiler pass.

Captures use local dev servers with three fake paired clients ("Phone", "Tablet", "Laptop browser"). "Before" runs main's component; "after" runs this branch.

Web (T3 Browser panel):

Before: four authorized clients.
Web before: four authorized clients

Before: one click on Revoke others removes the other three with no dialog.
Web before: Revoked 3 clients after one click, no dialog

After: Revoke others opens a confirmation first.
Web after: Revoke every other paired client? dialog

After: Cancel closes the dialog and all four clients stay (same screen as the first image, from a separate run).
Web after: four clients still listed after Cancel

After: Confirm revokes the other three.
Web after: Revoked 3 clients after Confirm

Desktop (Electron dev client, vp run dev:desktop on Linux):

Before: four authorized clients.
Desktop before: four authorized clients

Before: one click removes the other three with no dialog.
Desktop before: Revoked 3 clients after one click, no dialog

After: Revoke others opens a confirmation first.
Desktop after: Revoke every other paired client? dialog

After: Cancel keeps all four clients.
Desktop after: four clients still listed after Cancel

After: Confirm revokes the other three.
Desktop after: Revoked 3 clients after Confirm

Not checked: a packaged desktop build, and macOS or Windows desktop. A write-only grant (no list shown) is covered by the browser test (clientSessions={null}) but not captured.

Generated with Claude Opus 5.5 in T3 Code Nightly

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Nov 8, 2026
@macroscopeapp

macroscopeapp Bot commented Nov 8, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at d7ad5fd

Macroscope's review found this PR approvable — Adds confirmation before revoking other paired clients, warning that reconnecting requires a new pairing link. The existing revocation action remains unchanged and runs only after explicit confirmation.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Nov 8, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

Run configuration

Commits

Reviewing files that changed from the base of the PR and between d7ad5fd and 5bc91a0.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review

No actionable comments were generated in the recent review.

Walkthrough

The “Revoke others” action now asks for destructive confirmation before revoking other paired clients. The callback runs only when confirmation is accepted. Tests cover both confirmation outcomes.

Changes: Client revocation confirmation

Layer / File(s) Summary
Confirmation and revocation behavior
apps/web/src/components/settings/AuthorizedClientsSettings.tsx, apps/web/src/components/settings/AuthorizedClientsSettings.browser.test.tsx
The action requests confirmation before calling the revocation callback. Tests verify that declining does not call the callback and confirming calls it once.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: shivamhwp, juliusmarminge

Merge Risk: Minimal · previous review d7ad5

Bulk revocation now requires confirmation. No merge-blocking issue was identified.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title names the main change: Revoke others now asks for confirmation before it signs out every other paired device. It is a conventional commit title in plain language, matching the change in Auth…
Description check Passed The description follows the template. It has Problem, Change, Scope and approval, and Verification sections. The problem is stated with reproduction steps. The change names the component, the requestC…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title names the main change: Revoke others now asks for confirmation before it signs out every other paired device. It is a conventional commit title in plain language, matching the change in AuthorizedClientsHeaderAction and the browser test updates.

Full details: Description check

Explanation

The description follows the template. It has Problem, Change, Scope and approval, and Verification sections. The problem is stated with reproduction steps. The change names the component, the requestConfirmDialog call, and the unchanged paths. The scope section uses the small-fix exception and explains why it qualifies. Verification gives the exact test command, before and after results, and screenshots for web and desktop. Unchecked items are listed. The description notes the model used at the end, as the template asks. The Scope section cites #9786 and #31733 and says no issue or PR asks for or rejects this change. Those references are author claims and are not verified in the context provided.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between a9e709f and d7ad5fd.

Files selected for processing: 2

  • apps/web/src/components/settings/AuthorizedClientsSettings.browser.test.tsx
  • apps/web/src/components/settings/AuthorizedClientsSettings.tsx

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.

…vice

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@only21mil
only21mil force-pushed the fix/web-confirm-revoke-other-clients branch from d7ad5fd to 5bc91a0 Compare November 8, 2026 00:57
Wladefant

This comment was marked as spam.

@t3dotgg
t3dotgg dismissed Wladefant’s stale review November 8, 2026 23:31

Dismissed: unsolicited automated review from a blocked account.

@maria-rcks

maria-rcks commented Nov 10, 2026 •

Copy link
Copy Markdown
Collaborator

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

verified unchanged head 5bc91a0dc3a94cf2b6dee838d842cdcb4e19a49b on this linux boat vm using the real dev server, repo Playwright, and /usr/bin/google-chrome. no mocks, fixtures, or demo data. projects, threads, pairing credentials, and sessions were created through the app. real Codex gpt-6.1-sol turns completed on both revisions.

  • before: checked out github/main at 934c7f8d8e7a012aa55be9c0b17498979040fe02 in a separate worktree, ran vp i, and reproduced one-click bulk revocation with two real browser profiles. no confirmation appeared; the other browser required pairing after reload.
  • after: stopped main, checked out the exact pr head, ran vp i, and started vp run dev --home-dir /home/user/t3code/.t3 in the background. settings → sharing & access → revoke others opened the destructive confirmation.
  • cancel and escape left both clients authorized. after cancel, the other browser remained authenticated across reload.
  • confirm revoked the other browser, which required pairing after reload. the initiating browser stayed authenticated and completed another real Codex turn afterward.
  • neighboring behavior: a fresh pairing token reconnected the revoked browser; individual revoke still signed out that browser.
  • Blacksmith: cd apps/web && vp test run --project browser src/components/settings/AuthorizedClientsSettings.browser.test.tsx passed all 4 tests after installing the missing Chromium on the worker. github checks currently have 21 passes, 8 skips, and no failures or pending checks. skips are not verification.

before: one click on main revoked the other browser without a dialog.

main: one-click revocation with no confirmation

after: the same two-browser scenario requires confirmation.

pr: destructive confirmation before revoking the other browser

interaction recording, slowed for readability and shown as an animated gif for github. inspected the captures and checked uploaded video playback and seeking.

cancel preserves both clients; confirmation revokes the other client

runtime transcript:

PASS cancel: both clients remain, dialog closed
PASS escape: both clients remain
PASS confirm: 1 other client revoked, current client retained
PASS revoked browser: reload requires pairing
PASS new pairing token reconnects revoked browser
PASS cancel preserves real browser authentication after reload
PASS individual Revoke still signs out target browser
PASS current client completed another real gpt-6.1-sol turn after revocation

Not verified

native desktop/Electron, packaged macOS/Windows clients, native iOS/Android, physical devices, remote/relay/tunnel connections, and restricted or write-only grants were not exercised. mobile has no bulk-revoke control in this change. missing confirmation-host teardown and rapid repeated clicks were not exercised in the running app. no code changes were needed; no commits or pushes were made.

@only21mil

Copy link
Copy Markdown
Contributor Author

Note

🤖 Opus 5.5 replying on behalf of Victor.

Thanks @maria-rcks for testing this end to end with two real browser profiles. Your run confirms the change on the unchanged head: on main, one click signed out the other browser, and on this head, Revoke others opens the confirmation first. Cancel and Escape keep both browsers signed in across a reload, and Confirm signs out only the other browser while the current one keeps working. A fresh pairing token still reconnects the signed-out browser, and the single-device Revoke still works. Noted that desktop, mobile, remote connections, a missing confirmation host and rapid repeated clicks are still untested in a running app. The browser test covers Cancel and Confirm only.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10-29 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants