Skip to content

fix(sidebar): activate workspace number shortcuts by rendered sidebar order - #9560

Closed
Hanxcellent wants to merge 1 commit into
stablyai:mainfrom
Hanxcellent:fix/9497-workspace-shortcut-order
Closed

Hanxcellent wants to merge 1 commit into
stablyai:mainfrom
Hanxcellent:fix/9497-workspace-shortcut-order

Conversation

@Hanxcellent

Copy link
Copy Markdown

What

Fixes #9497.

Cmd/Ctrl+1..9 workspace shortcuts now activate the Nth row exactly as rendered in the sidebar, and folder workspaces activate through the dedicated folder path instead of the plain worktree path.

Why

The shortcut handler previously recomputed workspace order from the Zustand store (sorting/grouping/filtering). That order could disagree with the rows actually on screen — component-local state such as host drag collapse, collapsed groups, and import cards is invisible to the store — so pressing a number could jump to the wrong workspace. Folder workspaces were also mis-activated as regular worktrees.

How

  • WorktreeList publishes its rendered row order from a layout effect (setVisibleWorktreeIds), and registers itself as the order publisher while mounted (registerVisibleWorktreeIdsPublisher).
  • While the sidebar is mounted, the shortcut path trusts the published order directly (O(1) lookup). Only after unmount does it fall back to reconciling against the store, so shortcuts still work with the sidebar closed while dropping rows that no longer exist.
  • workspace-number-shortcut-activation.ts routes folder workspaces through activateAndRevealFolderWorkspace.

No visual change.

Tests

  • New regression tests: visible-worktree-shortcut-order.test.ts (published-order trust while mounted, reconciliation after unmount, empty-order handling) and workspace-number-shortcut-activation.test.ts (folder vs worktree activation routing). All were written red-first.
  • Focused suites: 133/133 passed. Full pnpm typecheck passed. pnpm build passed. Full pnpm test: identical pass/fail set to main on the same machine (remaining failures are pre-existing local-environment issues on main, e.g. missing optional windows-native-registry).

AI code review summary

Reviewed by a Codex reviewer agent over the full diff; final verdict: no actionable findings.

  • Cross-platform: shortcut handling keeps the existing metaKey/ctrlKey runtime platform check; no hardcoded modifiers or paths.
  • SSH/remote/local: activation goes through the existing worktree-activation paths, which are host-agnostic; no assumptions of local-only execution.
  • Agents/integrations & git providers: no provider-specific logic touched; ordering is purely renderer-side.
  • Performance: mounted path is O(1) per keypress; full reconstruction only runs when the sidebar is unmounted.
  • UI: no visual change; behavior verified via unit/integration tests.
  • Security: no new inputs, IPC surfaces, or shell/git command construction.

Platform / remote notes

Verified on macOS. The change is renderer-only ordering logic behind the existing platform-checked shortcut handler, so no platform- or SSH-specific behavior differences are expected.

X handle: (contributor: add your handle here)

… order

Cmd/Ctrl+N previously recomputed workspace order from the store, which
could disagree with the rows actually rendered in the sidebar (drag
state, collapsed groups, import cards), and folder workspaces were
activated through the plain worktree path.

The sidebar now publishes its rendered row order in a layout effect,
and while it is mounted the shortcut path trusts that published order
directly (O(1)); only after unmount does it reconcile against the
store. Folder workspaces activate through the dedicated folder path.

Fixes stablyai#9497
@Hanxcellent
Hanxcellent force-pushed the fix/9497-workspace-shortcut-order branch from 2e7877b to 52f0f1a Compare July 20, 2026 11:06
@coderabbitai

coderabbitai Bot commented Jul 20, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: a7f684c1-40a8-43da-971f-ae19092d5515

📥 Commits

Reviewing files that changed from the base of the PR and between 2e7877bdb9e56f246771711fb4d85a1aa2334dfc and 52f0f1a.

📒 Files selected for processing (7)
  • src/renderer/src/components/sidebar/WorktreeList.tsx
  • src/renderer/src/components/sidebar/rendered-workspace-shortcut-order.ts
  • src/renderer/src/components/sidebar/visible-worktree-shortcut-order.test.ts
  • src/renderer/src/components/sidebar/visible-worktrees.ts
  • src/renderer/src/hooks/useIpcEvents.ts
  • src/renderer/src/lib/workspace-number-shortcut-activation.test.ts
  • src/renderer/src/lib/workspace-number-shortcut-activation.ts

📝 Walkthrough

Walkthrough

Sidebar shortcut ordering now centralizes agent-send eligibility, collapsed-group handling, host visibility filtering, and rendered workspace ordering. Visible-worktree caching tracks published order and catalog entries while supporting publisher registration and live reconciliation. WorktreeList uses the shared helpers and preserves published IDs during unmount. Workspace-number navigation now activates folder workspaces or worktrees according to the shortcut key format, with tests covering ordering, caching, filtering, and activation routing.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: workspace number shortcuts now follow the rendered sidebar order.
Description check ✅ Passed The description covers summary, rationale, implementation, tests, review notes, and no visual change; only some template sections are abbreviated.
Linked Issues check ✅ Passed The changes address #9497 by using rendered sidebar order while mounted and reconciling after unmount, keeping Cmd/Ctrl+number consistent.
Out of Scope Changes check ✅ Passed The added tests and folder-workspace activation support are directly tied to the shortcut-order fix and are not out of scope.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@nwparker

Copy link
Copy Markdown
Contributor

For the record, since this stayed closed and unreviewed: @Hanxcellent, your approach here was substantially correct, and one piece of it is genuinely better than what shipped.

#10693 has now fixed the ordering half of #9497 the same way you did — deriving the shortcut order from the sidebar's rendered rows rather than a flat list.

The part you had that #10693 does not: workspace-number-shortcut-activation.ts, which branches on parseWorkspaceKey and routes folder workspaces to activateAndRevealFolderWorkspace instead of activateAndRevealWorktree. I checked this before deciding, and it isn't a crash — activateAndRevealWorktree does resolve a folder:* key (findKnownWorktreeById has a folder branch), activates the workspace and creates its terminal. So numbering a folder workspace works today.

But your split is still the more correct call: the plain path skips the getFreshFolderWorkspacePathStatus / folderWorkspaceActivationBlocked gate that activateAndRevealFolderWorkspace enforces, so Cmd+N on a folder whose path is missing or unmounted activates it silently instead of showing the error toast a sidebar click would. That's a real (if narrow) inconsistency between clicking a card and pressing its number.

I've left that out of #10693 deliberately to keep it to one reviewable concern, and filed it separately so it doesn't get lost. If you'd like to open a focused PR for just that activation split, it'd be welcome — credited to you.

Sorry this sat unreviewed after you closed it; the idea was a good one.

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.

[Bug]: Cmd+number selects a different workspace when the sidebar is closed

2 participants