Skip to content

Add keyboard shortcuts for agent model and effort picker - #375

Open
aAtila wants to merge 5 commits into
mainfrom
feat/agent-model-picker-shortcut
Open

Add keyboard shortcuts for agent model and effort picker#375
aAtila wants to merge 5 commits into
mainfrom
feat/agent-model-picker-shortcut

Conversation

@aAtila

@aAtila aAtila commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator

The agent composer's model picker and provider effort picker can now be opened from remappable keyboard shortcuts, routed to the currently focused composer.

Changes

  • Add remappable global shortcuts that open the active composer's model picker and effort picker.
  • Route shortcut requests through the focused window to the active composer, with teardown-safe menu presentation guards so a menu never opens on a composer that's tearing down.
  • Snapshot Codex effort options into composer props so the shortcut-opened effort menu has stable state.
  • Align Codex and Claude effort picker trigger styling with the model picker.
  • Cover shortcut catalog registration and composer open-request filtering with tests.

The model picker keeps its default binding (Cmd+Shift+M) and the effort picker ships unbound; both remain remappable and scoped to the active composer, so triggering a picker never fires a menu on the wrong window. Provider effort controls now follow the same focused-composer shortcut path as the model picker.

Testing

All green via the coordinated daemon:

  • make guardrails — passed (SwiftPM notice inventory matches Package.resolved, 50 packages).
  • make dev-lint — passed (0 violations, 0 formatting diffs across 1386 files).
  • make dev-test for the related suites — 55 tests, 0 failures:
    • AgentModelPickerOpenRequestGuardTests (7)
    • AgentEffortPickerOpenRequestGuardTests (13)
    • KeyboardShortcutCatalogTests (2)
    • AgentModeChatSwitchActivationTests (4)
    • AgentModeStopSubmitTargetTests (24)
    • WorktreeMergeReviewStateTests (5)

Fixes #363

@aAtila
aAtila force-pushed the feat/agent-model-picker-shortcut branch from 3e951dd to d0167db Compare August 9, 2026 20:39

@baron baron left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approved on exact head d0167db9d855da950e5fa26dc9e10d37e66e276f. The shortcut routing is guarded by the target window/tab and the detached menu presentation has a stable ownership guard; I found no code blocker. App shard 4 still timed out in unrelated WorktreeAPISmokeHarnessTests, so rerun the required check successfully on current main before merge—no retries or timeout inflation.

aAtila added 5 commits August 13, 2026 19:06
- route the remappable shortcut through the focused window to the active composer
- add a stable menu open request path with teardown-safe presentation guards
- cover shortcut catalog registration and composer open request filtering
- add an unbound global shortcut for opening the active effort picker
- route shortcut requests through focused-window composer guards
- snapshot Codex effort options into composer props for stable menu state

This extends the model picker shortcut flow to provider effort controls while keeping shortcut-triggered menus scoped to the active composer.
@aAtila
aAtila force-pushed the feat/agent-model-picker-shortcut branch from 91c0b00 to 2f79656 Compare August 13, 2026 17:10
@aAtila

aAtila commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks, Baron. Since your review of d0167db9, I ran an additional code-review pass with PRO (web) and addressed its findings:

  • deferred picker presentation now revalidates composer ownership/lifecycle before opening;
  • picker menus now use fresh state instead of state captured before presentation;
  • focused tests cover these stale-state and teardown paths.

Regarding the unrelated WorktreeAPISmokeHarnessTests timeout: after several CI reruns, the shard finally passed unchanged. The same intermittent timeout also appears on unrelated main runs, while three focused local runs passed. I added no retry logic or timeout inflation.

All required checks are now green on the current head (2f796567). Since the reviewed head has advanced, another look would be appreciated.

@aAtila
aAtila requested a review from baron August 14, 2026 06:03

morluto commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

Audit disposition — merge candidate after final current-base checks (2026-08-14)

I did not find a code-level blocker in the focused-window routing, active-composer filtering, teardown-safe menu presentation, or stable effort-option snapshot. The design is appropriately scoped to the focused composer rather than a global menu owner.

Final gates: refresh onto current main, obtain exact-head CI, and manually exercise two windows, rapid focus transfer, disabled/unavailable effort controls, composer teardown, and remapped/unbound shortcuts. Subject to those checks, this looks suitable to merge.

morluto commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

Deep-review assessment — 2026-08-14

Disposition: merge candidate after current-head gates. The focused-composer routing, window/tab identity checks, teardown-safe presentation guards, and stable effort-option snapshot form a coherent contract. I did not find a code-level blocker.

Final validation should include a manual multi-window smoke: change focus between composers, invoke both shortcuts during/after tab teardown, verify disabled or unavailable effort controls do not open stale menus, and confirm remapping/unbound behavior. With exact-head CI green, this is appropriately narrow and ready for non-author approval.

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.

Keyboard shortcuts for the Agent/Model and Effort pickers

3 participants