Skip to content

fix(agents): let Back and Forward shortcuts traverse provider dialogs - #2509

Merged
matt2e merged 3 commits into
mainfrom
broken-7
Nov 5, 2026
Merged

fix(agents): let Back and Forward shortcuts traverse provider dialogs#2509
matt2e merged 3 commits into
mainfrom
broken-7

Conversation

@matt2e

@matt2e matt2e commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Problem

⌘[ / ⌘] (Ctrl+[ / Ctrl+] on Windows and Linux) were reported to do nothing while Settings was open. #2273 has since fixed that for Settings sections: Settings is now a navigation visit, and the shortcut dispatcher lets the reserved Back/Forward chords through over a visit.

One case still failed. With a provider dialog open from Settings → Agent setup, ⌘[ did nothing, although browser Back already closed the provider because it is a routed history entry. A provider opened from an Agents-page link had the same gap.

Root cause

ProviderDialog rendered the shared Dialog without visit. With a provider on top, modalOpen() in src/features/shortcuts/service.ts returned true (a plain modal) instead of "visit", so the dispatcher skipped navigation-back and navigation-forward.

Changes

  • 80f5dcf: ProviderDialog takes a visit prop (default false) and passes it to both its managed and fallback Dialog renders. AgentDefaults passes visit={routed}, so only the Settings-routed provider is a visit. This commit also withheld the visit while the provider reported blocking. The next commit removes that guard.
  • 3817460: Drops the blocking guard. docs/navigation-history.md:59-63 says Back can hide blocking provider controls while the operation continues in the provider service, and Forward restores them without restarting it. The guard was also partial: in the web build, browser Back still closed a blocking provider. Keeping the visit matches ChannelMembersDialog, which stays a visit while share.busy prevents close. X, Escape and backdrop dismissal stay blocked by preventClose. AgentsPage now passes visit={routed} as well.
  • d4d4520: Test-only. Tightens the blocking-provider tests (see below).

Intentionally unchanged: the migration ProviderDialog in AgentSettingsHost.tsx stays a plain modal. It is local state stacked on the non-visit "Move agent?" dialog, so making it a visit would let ⌘[ pop the history entry underneath it.

Some commit messages refer to pre-rebase SHAs: eb0b16ca83 is 80f5dcf4aa and 1f96145645 is 3817460390.

Tests

  • src/app/navigation.test.tsx: Back closes a Settings provider and leaves Settings open on Agent setup. Forward reopens the provider.
  • src/bundled/agents/AgentDefaults.test.tsx (new):
    • An unrouted provider is not a visit, and Ctrl+[ over it is inert and not prevented.
    • With a real memory-history controller, Ctrl+[ hides a blocking routed provider. Ctrl+] restores it with Close still disabled and without calling check() again, so nothing restarts.
  • src/bundled/agents/AgentsPage.test.tsx: the same blocking journey with a real controller. The Agents grid stays visible after Ctrl+[. Ctrl+] returns to the same visit, which shows the unmount was not treated as Close.
  • tests/browser/navigation.spec.mjs: both provider journeys gain Ctrl+[ and Ctrl+] steps after the existing goBack/goForward steps. The Agents journey fails at the Ctrl+[ step without the AgentsPage change.

Verification

Run at d4d4520:

  • vitest run on the three touched test files: 111/111 passed.
  • pnpm typecheck passes, and biome check --error-on-warnings passes on the changed files.
  • Playwright navigation.spec.mjs in Chromium and WebKit: 66/66 passed. The file filter also matched message-navigation.spec.mjs.

Not tested

  • The Tauri desktop app.
  • macOS ⌘ chords. The tests ran on Linux, where ControlOrMeta resolves to Ctrl.
  • Mouse back buttons and trackpad swipes.
  • A browser test that presses Ctrl+[ while a provider is blocking. Only the unit tests cover that path.

Before / after

provider-ctrl-bracket-before-after.mp4

Left is before the fix (its parent commit) and right is after (the first fix commit). The header labels show the pre-rebase SHAs 0b2dfbf988 and eb0b16ca83. Both sides open the Fixture provider from Settings → Agent setup, then press Ctrl+[, Ctrl+] and Ctrl+[ at 5.6 s, 8.3 s and 11.0 s. On the left, the provider never closes and the caption shows defaultPrevented: false. On the right, it closes onto Agent setup, reopens, then closes again, and the caption shows defaultPrevented: true. Settings stays open on both sides throughout. Recorded in headless Chromium on Linux.

If the video does not play, these frames from it show both sides at the same instant:

![Just after the first Ctrl+: left (before) the Fixture provider is still open over Settings with defaultPrevented false; right (after) it has closed onto Agent setup with Settings still open and defaultPrevented true

Just after the third chord (second Ctrl+[): left (before) the provider is still open, nothing happened across all three chords; right (after) the provider that Ctrl+] reopened is closed again

🤖 Generated with Claude Code

@matt2e
matt2e marked this pull request as ready for review November 4, 2026 22:32
@matt2e
matt2e requested a review from a team as a code owner November 4, 2026 22:32
matt2e and others added 3 commits November 5, 2026 01:13
@matt2e @claude
fix(agents): let Back and Forward shortcuts traverse a Settings provider
55f22e4 
With a provider dialog open from Settings → Agent setup, ⌘[/Ctrl+[ did
nothing and ⌘]/Ctrl+] only worked from Settings itself. The provider is a
routed history entry, and browser Back already closed it, but it rendered
the shared Dialog without `visit`. The shortcut dispatcher therefore
treated it as a plain modal and skipped the reserved Back/Forward chords.

ProviderDialog now takes a `visit` prop and passes it to both its managed
and fallback Dialog renders. AgentDefaults passes `visit={routed}`, so
only the Settings-routed provider becomes a visit. Back closes it and
leaves Settings on Agent setup, and Forward reopens it, as
docs/navigation-history-behavior.md describes. While the provider reports
`blocking` (install or sign-in work that must be cancelled before the
dialog closes), the visit is withheld. That keeps Back, mouse back and
trackpad swipes from closing it, matching its existing preventClose
protection.

Unchanged: the local provider in AgentSettingsHost's migration flow and
any unrouted AgentDefaults keep blocking Back. The Agents page provider
is left for a separate change.

Tests:
- App test: the routed provider is a visit, Ctrl+[ closes it onto Agent
  setup with Settings open, and Ctrl+] reopens it.
- AgentDefaults tests: an unrouted provider has no visit and Ctrl+[ stays
  inert and unprevented over it, and a routed provider withholds its
  visit while blocking.
- Browser journey: Ctrl+[/Ctrl+] steps after the existing goBack and
  goForward steps in "Settings provider closes before its parent...".

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Signed-off-by: mtoohey <[email protected]>
@matt2e @claude
fix(agents): keep routed providers a history visit, including while b…
3dbf44a 
…locking

Follow-up to eb0b16ca83, addressing its review.

eb0b16ca83 withheld the Settings provider's visit while the provider
reported `blocking`, so ⌘[/Ctrl+[ did nothing during an install or
sign-in. That contradicts docs/navigation-history.md: Back can still hide
blocking provider controls while the operation continues in the provider
service, and Forward restores them without restarting it. The guard was
also partial. `data-navigation-visit` is read only by the shortcut
dispatcher, so browser Back in the web build still closed a blocking
provider. ProviderDialog now passes `visit` through unchanged. X, Escape
and backdrop dismissal stay blocked by `preventClose`, as before. This
matches ChannelMembersDialog, which keeps its visit while `share.busy`
prevents close.

The Agents page rendered the same routed ProviderDialog without `visit`.
So ⌘[/Ctrl+[ could not close a provider opened from an Agents-page link,
although browser Back could. AgentsPage now passes `visit={routed}`, as
AgentDefaults does. docs/navigation-history-behavior.md already treats
both providers as visits, so the docs are unchanged.

Tests:
- AgentDefaults: the blocking test is inverted and now drives a real
  memory-history controller. With the routed provider open, it flips to
  blocking. Ctrl+[ is prevented and closes it onto Agent setup. Ctrl+]
  restores it, still guarded (Close disabled). The unrouted test drops
  its final "dialog still open" assertion, which could not fail because
  nothing was wired to close the dialog.
- AgentsPage: a routed provider stays a visit while blocking, and Ctrl+[
  is prevented and calls Back.
- Browser journey "Agents provider Back is reversible…": after the
  existing goBack/goForward steps, Ctrl+[ closes the provider onto Agents
  and Ctrl+] reopens it. Without the AgentsPage change it fails at the
  Ctrl+[ step.

Mouse back buttons and trackpad swipes were not tested.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Signed-off-by: mtoohey <[email protected]>
@matt2e @claude
test(agents): prove Back/Forward over a blocking provider neither res…
5ec4bb6 
…tarts nor retires it

Follow-up to 1f96145645, applying both nits from its review. Test-only;
no production code changes.

AgentDefaults: the "lets Back hide a routed provider's blocking work and
Forward restore it" test checked the "without restarting it" half only
through Close staying disabled after Forward. That held no matter what,
because the fixture's state is never reset, `check()` was a no-op and
DialogBody is static. The fixture's `check` is now a vi.fn(). The test
records its call count once the provider is blocking and asserts the
count is unchanged after Ctrl+[ and Ctrl+].

AgentsPage: the blocking-provider test mocked `back`, so the route never
changed and the dialog never unmounted. It showed the dispatcher calls
Back, not that the page hides the dialog without treating the unmount
as a Close, which calls navigator.dismiss and retires the visit Forward
returns to. The test now uses the memory-history setup from
AgentDefaults.test.tsx: a real controller, the host's navigation
shortcuts and a host wrapper that renders AgentsPage from each visit. It
opens the provider link and makes the provider block. Ctrl+[ then hides
the dialog and leaves the Agents page and its grid showing, and Ctrl+]
restores the dialog as a visit with Close still disabled.

Each tightened test passes at HEAD. Each was checked against temporary
regressions, reverted before this commit:
- ManagedProviderDialog calls provider.check() on mount: the
  AgentDefaults test fails ("expected 2 times, but got 3 times"). It
  also fails if the fixture's DialogBody calls check() on mount.
- ProviderDialog calls onClose when it unmounts, and AgentsPage's
  setProviderKey drops its stale-visit guard (aborted signal or
  entry-id mismatch): the AgentsPage test fails at Ctrl+], because the
  dismissal retired the provider visit. With the guard left in place,
  the unmount-time close alone is still ignored and the test passes.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Signed-off-by: mtoohey <[email protected]>
kazemek21
kazemek21 approved these changes Nov 5, 2026
View reviewed changes

@kazemek21 kazemek21 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.

Kyle reviewed this PR and the Nexus review-tracker notes and approved with no comments.

Posted by Codex (runtime model ID unavailable) on behalf of kylez.

@matt2e
matt2e merged commit 9abfe54 into main Nov 5, 2026
83 of 85 checks passed
@matt2e
matt2e deleted the broken-7 branch November 5, 2026 05:52
ljknight added a commit to ljknight/buzz-app that referenced this pull request Nov 5, 2026
@ljknight
Merge origin/main into lauraknight/quote-selection
ccbdb11 
* origin/main:
  Keep /leave commands out of channel history (block#2476)
  fix(agents): ask before switching or closing drops unsaved edits (block#2503)
  fix(agents): keep the Model settings row full width (block#2577)
  feat(github): show a pull request's changes beside its summary (block#2492)
  fix(search): open an existing DM from a ⌘K person row without a relay open; cover hidden-DM previews (block#2519)
  feat(search): show channel type icon in search result subtitles (block#2521)
  test(browser): bind port-0 fixture servers where the OS assigns (block#2564)
  test(browser): fix six journey sync flakes from the 2026-11-04 sweep (block#2566)
  fix(channels): ask before Escape, Close or an outside click drops a template draft (block#2501)
  fix(channels): render the channel details description as linked Markdown with a label-only Edit target (block#2506)
  fix(agents): let Back and Forward shortcuts traverse provider dialogs (block#2509)
  Fix Inbox recovery after a delayed Retry (block#2496)

# Conflicts:
#	docs/buzz-help.md
wpfleger96 added a commit that referenced this pull request Nov 5, 2026
@wpfleger96
Merge remote-tracking branch 'origin/main' into elrond/cli-credential…
f8d3423 
…-service

* origin/main: (68 commits)
  fix(messages): keep decoded images across virtualized remounts (#2474)
  Keep /leave commands out of channel history (#2476)
  fix(agents): ask before switching or closing drops unsaved edits (#2503)
  fix(agents): keep the Model settings row full width (#2577)
  feat(github): show a pull request's changes beside its summary (#2492)
  fix(search): open an existing DM from a ⌘K person row without a relay open; cover hidden-DM previews (#2519)
  feat(search): show channel type icon in search result subtitles (#2521)
  test(browser): bind port-0 fixture servers where the OS assigns (#2564)
  test(browser): fix six journey sync flakes from the 2026-11-04 sweep (#2566)
  fix(channels): ask before Escape, Close or an outside click drops a template draft (#2501)
  fix(channels): render the channel details description as linked Markdown with a label-only Edit target (#2506)
  fix(agents): let Back and Forward shortcuts traverse provider dialogs (#2509)
  Fix Inbox recovery after a delayed Retry (#2496)
  feat(composer): optional ⌘/Ctrl+Enter to send (#2331)
  test: use keyboard activation for reply caret setup (#2557)
  test: settle member search before invitation expansion (#2532)
  test(agent-command): isolate the copied executable writer (#2537)
  test(sidebar): wait for the rolled-back row to leave Starred before collapsing Work (#2574)
  test(browser): click timeline rows only after the timeline settles (#2560)
  ci: split Vitest across six shards so 3/4 stops hitting the job cap (#2571)
  ...

# Conflicts:
#	src/app/Settings.tsx
#	src/bundled/goose/sessions.test.ts
agude-sq added a commit to agude-sq/buzz-app that referenced this pull request Nov 5, 2026
@agude-sq
Merge remote-tracking branch 'origin/main' into agude/prefer-full-names
13ff9a1 
* origin/main: (43 commits)
  feat(settings): follow the system clock and let people hide message times (block#2440)
  feat(plugins): repository access rules for Git imports (block#2484)
  feat(desktop): add Settings to the macOS app menu (block#2594)
  Preserve and renew external app approval (block#2464)
  fix(direct-messages): show verified identity in new message recipient search (block#2585)
  Offer plugin updates from the Buzz shell (block#2491)
  fix(messages): keep decoded images across virtualized remounts (block#2474)
  Keep /leave commands out of channel history (block#2476)
  fix(agents): ask before switching or closing drops unsaved edits (block#2503)
  fix(agents): keep the Model settings row full width (block#2577)
  feat(github): show a pull request's changes beside its summary (block#2492)
  fix(search): open an existing DM from a ⌘K person row without a relay open; cover hidden-DM previews (block#2519)
  feat(search): show channel type icon in search result subtitles (block#2521)
  test(browser): bind port-0 fixture servers where the OS assigns (block#2564)
  test(browser): fix six journey sync flakes from the 2026-11-04 sweep (block#2566)
  fix(channels): ask before Escape, Close or an outside click drops a template draft (block#2501)
  fix(channels): render the channel details description as linked Markdown with a label-only Edit target (block#2506)
  fix(agents): let Back and Forward shortcuts traverse provider dialogs (block#2509)
  Fix Inbox recovery after a delayed Retry (block#2496)
  feat(composer): optional ⌘/Ctrl+Enter to send (block#2331)
  ...

# Conflicts:
#	docs/buzz-help.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kazemek21 kazemek21 kazemek21 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@matt2e @kazemek21