You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(web): focus the primary action when confirmation dialogs open - #18187
Selected update and install confirmation dialogs focus Cancel even though the primary action is visually emphasized. Pressing Enter therefore cancels. This change lets those prompts opt into primary-action focus while keeping generic and destructive confirmations Cancel-first.
A shared DialogAction marker selects the enabled primary button on popup open. Explicit caller focus wins, caller refs remain composed, and touch opening retains popup focus. Desktop update prompts in Settings, current and legacy sidebars, and remote single/batch update actions opt in. Relay install and browser-import quit/retry screens use the same marker. The broad Update everything dialog keeps its existing behavior.
Based on @rgrvlsk’s original contribution. Contributor history is preserved; main was merged and the implementation rebuilt against current primitives. The rebuild fixes caller refs overriding the internal focus ref and removes redundant comments and runtime variant bookkeeping.
Focused suites: 35 passed in 3 files, versus 35 in 4 files before test cleanup. Seven jsdom cases with fixed sleeps were replaced with six Chromium interaction cases; existing remote-update tests exercise the real host with Enter/Space, and the existing coordinator suite covers queued preferences. Restoring main’s host produces two expected focus failures; restoring the original hook produces a caller-ref regression failure.
Web, contracts, desktop, and mobile typechecks passed. Scoped lint and formatting passed. An independent reviewer also ran the local API suite: 43 passed in 4 files.
One full-diff review round, Claude Opus 5.5 and Codex GPT-6 Sol: no substantive findings. Both rechecked the earlier concern about MCP approvals, checkpoint rollback, and sign-out becoming Enter-to-approve; those callers retain Cancel focus.
Live headless Chromium, isolated dev state: opted-in update Confirm receives focus, Enter and Space confirm, Tab reaches Cancel; generic and destructive confirmations focus Cancel and Enter cancels, including destructive plus explicit confirm opt-in. The real relay prompt focuses Download and install and resolves on Enter. The actual BrowserImportWizard, mounted with synthetic callbacks, focuses and activates its initial quit/retry actions.
Browser-import later steps retain the persistent popup: after asynchronous retry failure, focus returns to the popup. This change only chooses focus when opening and does not claim automatic refocusing between wizard steps.
The screenshots use the same synthetic app data, 1280×800 viewport, dialog text, and Liberation Sans fallback font. Baseline web focus code matches main ab6be13d43; head is 0c37cce149. The live coordinator was invoked with the real update message/options; no real updates, relay installation, cookie import, or tool approval ran.
Not verified: packaged Electron desktop, actual update execution from Settings/sidebar or remote batch, macOS, Windows, physical touch/soft keyboards, screen readers, iOS, or Android. Mobile uses its separate native dialogs. Synthetic Chromium touch tests verify popup focus; they do not verify OS keyboard behavior.
Implementation: Codex GPT-6.1 Sol; tests: Codex GPT-6 Astra; reviews: Claude Opus 5.5 and Codex GPT-6 Sol. Harness: T3 Code through Codex.
Dialogs focused Cancel on open, so pressing Enter cancelled even though
Confirm is the highlighted action. Popups now focus the AlertDialogAction
or DialogAction when they open, so Enter and Space confirm and Tab reaches
Cancel, with native button semantics. Destructive actions aren't marked,
so those dialogs keep focus on Cancel. Touch opens still focus the popup.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Macroscope's review found this PR requires human approval — Existing update, installation, and import dialogs now initially focus their primary action, changing the default keyboard interaction. Human review is required to accept that default change, including for update confirmations that close and relaunch applications.
You can add or adjust custom eligibility rules. Learn more.
No actionable comments were generated in the recent review.
Walkthrough
Confirmation options now carry initial-focus preferences through queued dialog states. Dialog components select focus targets, and selected update, relay install, and browser import flows use action components that participate in focus selection.
ConfirmDialogOptions adds initialFocus choices for Confirm and Cancel. Confirmation state preserves the setting when a dialog opens, closes, or advances from the queue.
Dialog popups focus enabled marked actions, or the popup on touch opens. The modules add DialogAction and AlertDialogAction. Browser tests cover focus choices, disabled actions, refs, and touch opens.
pingdotgg/t3code#28842: Changes the confirmation-dialog contract and implementation, including ConfirmDialogHost and desktop update and relay-install flows.
Suggested labels:bugfix
Suggested reviewers:juliusmarminge
Merge Risk: Minimal · up to 93291
The intended confirmation dialogs gain primary-action focus, while “Update everything” remains Cancel-first. No identified issue prevents merging after normal checks.
Security Architecture Review
Security architecture risk:🔵 Low · up to 6f46e
The change selects an initial focus target rather than granting new permissions or removing confirmation. Ordinary and destructive confirmations retain Cancel unless an eligible action explicitly opts in. No security bypass was established in the inspected flows, but external-caller and runtime coverage remains incomplete.
Retained concerns
No architecture-level concerns identified.
Security review details
Security Blast Radius
inferred — The most consequential inspected focus change reaches confirmations for selected remote desktop machines, including a batch-update path. Their existing target selection and confirmation remain in place; the PR changes the keyboard-default choice rather than introducing additional execution targets. Broader tenant or deployment exposure was not established.
Trust Boundaries and Controls
observed — The inspected MCP approval bridge passes only a message to requestConfirmDialog, not focus options. ConfirmDialogHost renders message-derived content as text and marks Confirm only when the state explicitly requests it and the variant is default. This path therefore retains its previous Cancel-first approval behavior.
observed — Shared action selection excludes natively disabled elements. Destructive-action exclusion in DialogAction is a compile-time contract, whereas ConfirmDialogHost also enforces a runtime default-variant condition. These are focus-selection safeguards, not substitutes for operation authorization.
Resilience and Maintainability Implications
observed — The confirmation transition assessment found focus metadata preserved across active, closing, and queued states without changing response ownership or cleanup. The inspected relay confirmation handler requires a confirming state and a live resolver, clears that resolver before resolution, and rejects overlapping installation requests.
Pre-merge checks: 4
Check name
Status
Explanation
Title check
The title clearly and concisely describes the primary change: focusing the primary action when confirmation dialogs open.
Description check
The description explains the problem, implementation, scope, linked issue, verification results, screenshots, recording, and known limitations. It provides the required information even though it does…
Linked Issues check
Issue [#18167] requires targeted primary-action focus for desktop and server update prompts, relay installation, and browser-import quit/retry actions. The reviewed changes pass `initialFocus: "confir…
Out of Scope Changes check
The changed dialog primitive, confirmation-state plumbing, targeted update call sites, relay and browser-import actions, and focused tests directly support [#18167]. The default Cancel focus, destruct…
Full details: Description check
Explanation
The description explains the problem, implementation, scope, linked issue, verification results, screenshots, recording, and known limitations. It provides the required information even though it does not repeat the template headings explicitly.
Full details: Linked Issues check
Explanation
Issue [#18167] requires targeted primary-action focus for desktop and server update prompts, relay installation, and browser-import quit/retry actions. The reviewed changes pass initialFocus: "confirm" through ConfirmDialogState and apply DialogAction to the relay and browser-import actions. Update call sites in Settings, sidebars, and remote single/batch actions opt in. The shared dialog remains Cancel-first by default, and destructive confirmations remain Cancel-first. The summary reports focused browser coverage for default, opt-in, disabled, caller-focus, destructive, touch, relay, and browser-import behavior.
Full details: Out of Scope Changes check
Explanation
The changed dialog primitive, confirmation-state plumbing, targeted update call sites, relay and browser-import actions, and focused tests directly support [#18167]. The default Cancel focus, destructive-confirmation behavior, caller ref composition, and touch handling preserve required confirmation safety and support the opt-in behavior. No unrelated change is demonstrated.
Finishing Touches
Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
ConfirmDialogHost serves every requestConfirmDialog and dialogs.confirm
caller, so focusing Confirm there also made Enter approve MCP tool
requests, roll back checkpoints and sign out providers. The host now
keeps Cancel focused unless the caller passes initialFocus: "confirm",
and only the desktop and server update prompts do.
DialogAction now rejects destructive Button variants at compile time
instead of matching the variant name, which missed ghost-destructive.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Re CodeRabbit's security note: good catch. ConfirmDialogHost serves every requestConfirmDialog / dialogs.confirm caller, and MCP tool approvals, checkpoint rollback and provider sign-out all use the default variant, so they would have become Enter-to-confirm too. Fixed in 6f46e56: the host now keeps Cancel focused unless the caller passes initialFocus: "confirm", and only the update prompts opt in. Focus preference is now separate from destructive styling, and DialogAction rejects destructive Button variants at compile time. Added host tests for the default, the opt-in, and destructive + opt-in.
Taking over this one. I’ll rebuild on current main, trim the tests to meaningful focus behavior, and push to this branch while keeping generic and destructive confirmations Cancel-first.
Linux Electron, light theme, 1280×900, fresh isolated state on each commit. Settings → About → Install, opened by mouse and keyboard; press Enter or Space without changing dialog focus. Then Settings → General → change In-app notifications → Restore device defaults → Enter.
The update action resides under About on both commits, rather than General as stated in the supplied reproduction. A synthetic downloaded update was supplied through Electron IPC. Install requests were counted; the fixture then reported Up to Date. No release was downloaded or installed.
AI-assisted; verified with the tests and tools listed above.
Not shown:
Actual release download, installation, and restart: the updater IPC was seeded and install requests intercepted to keep both apps available for repeated captures.
Touch opening and explicit caller focus precedence: these were not exercised in the desktop reproduction.
Generic non-destructive confirmations, disabled primary actions, and other changed dialog consumers: only the update confirmation and destructive restore-defaults control were exercised.
…ngdotgg#18187)
Co-authored-by: Roger Vilaseca <roger@tres65.es>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Julius Marminge <julius0216@outlook.com>
(cherry picked from commit 4382138)
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
macroscope-reviewOpt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviewspr:needs-juliusNeeds Julius: route gate, unverifiable surface, or blockedsize:L100-499 changed lines (additions + deletions).vouch:unvouchedPR author is not yet trusted in the VOUCHED list.
2 participants
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.
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
juliusmarminge
added
the
macroscope-review
ChatGPT Codex Connector
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Selected update and install confirmation dialogs focus Cancel even though the primary action is visually emphasized. Pressing Enter therefore cancels. This change lets those prompts opt into primary-action focus while keeping generic and destructive confirmations Cancel-first.
A shared DialogAction marker selects the enabled primary button on popup open. Explicit caller focus wins, caller refs remain composed, and touch opening retains popup focus. Desktop update prompts in Settings, current and legacy sidebars, and remote single/batch update actions opt in. Relay install and browser-import quit/retry screens use the same marker. The broad Update everything dialog keeps its existing behavior.
Based on @rgrvlsk’s original contribution. Contributor history is preserved; main was merged and the implementation rebuilt against current primitives. The rebuild fixes caller refs overriding the internal focus ref and removes redundant comments and runtime variant bookkeeping.
Verification for rebuilt head 0c37cce149:
The screenshots use the same synthetic app data, 1280×800 viewport, dialog text, and Liberation Sans fallback font. Baseline web focus code matches main ab6be13d43; head is 0c37cce149. The live coordinator was invoked with the real update message/options; no real updates, relay installation, cookie import, or tool approval ran.
Head recording: Enter confirms the opted-in update, cancels generic approval, and cancels destructive confirmation
Not verified: packaged Electron desktop, actual update execution from Settings/sidebar or remote batch, macOS, Windows, physical touch/soft keyboards, screen readers, iOS, or Android. Mobile uses its separate native dialogs. Synthetic Chromium touch tests verify popup focus; they do not verify OS keyboard behavior.
Implementation: Codex GPT-6.1 Sol; tests: Codex GPT-6 Astra; reviews: Claude Opus 5.5 and Codex GPT-6 Sol. Harness: T3 Code through Codex.
Fixes #18167