feat(actions): render Global Actions in the tab strip - #80
Merged
Conversation
Global Actions apply to every project instead of one, so they cannot live in projects.customCommandsJson the way Project Actions do. Add a daemon-owned global_sidebar_commands table (migration 0018) so mobile, web, and every desktop build read one list rather than mirroring a per-project column. The four default actions (dev/build/test/setup) stay project-scoped, so every global action is user-created. That removes the whole default-resurrection and tombstone branch: the stored rows are the entire list. readSidebarHud attaches globalCommands after the fact, the same way it already handles commandsByProject, so read_sidebar_hud stays a pure projection of project rows. mutateSidebarHudSettings gains a globalCommand target; SidebarHudSettingsMutation was project-only, so it grows one optional field and a global write schedules no projectUpdated delta because it touches no project row. Project and Global saves validate through one shared path — the payloads are identical and only ownership differs, so splitting the validation would let the two lists drift into accepting different action shapes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Global Actions draw as icons in the Agents tab strip action cluster, and the built-in New Terminal / New Chat / New Browser Tab buttons each become hideable. Cluster width is now derived from what is actually drawn rather than a fixed four-button constant. Global actions render before the built-ins so the user's own actions sit closest to the tabs and the built-in controls stay against the right edge where muscle memory expects them. The pane overflow button has no toggle: it is the only route to the remaining pane actions. A new sidebar bridge channel carries the list to gpui. The payload is action id, display name, and icon slug only — no command text or URLs reach gpui. A strip click sends the id back through the existing Action selector, which resolves the trusted definition on the sidebar side, so the tab strip never holds anything executable. Settings > Actions splits into Global Actions and Project Actions over one list implementation; only the bridge message types and copy differ. Note that desktop settings writes are handled in Rust rather than the sidebar TypeScript runtime, so the global message types are routed there too — an unrouted type is silently dropped, and the Rust HUD fetch feeds the settings modal, so both had to learn about global actions or saving and listing would each fail on their own. Built-in button visibility is a local setting, matching hideProjectHeaderDiffStats: it is per-machine window chrome. It is cached in app state and refreshed on the settings hook because the tab strip draws every frame and must not stat the settings file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
🚧 Files skipped from review as they are similar to previous changes (9)
📝 WalkthroughWalkthroughGlobal sidebar commands now support daemon persistence, scoped settings management, HUD synchronization, and GPUI tab-strip rendering. Built-in tab-strip buttons have independent visibility settings. The CEF bridge carries bounded, validated global-action payloads. ChangesGlobal sidebar actions
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Settings
participant Gxserver
participant SidebarRuntime
participant GPUI
Settings->>Gxserver: save or reorder global command
Gxserver->>SidebarRuntime: return refreshed globalCommands
SidebarRuntime->>GPUI: publish GlobalActions payload
GPUI->>GPUI: validate and update tab-strip state
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.45.0)gpui/src/main.rsast-grep timed out on this file 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
sidebar/settings-modal.tsx (1)
9203-9225: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd search keys for the new Actions toggles.
ActionsSettingsTabrenders fromextraSettingsTabSearches.actions, and its current section only definesterminalAction,browserAction, andactionShortcuts. Add search entries forhideTabStripNewTerminalButton,hideTabStripNewChatButton, andhideTabStripNewBrowserButtonso queries such as “New Terminal button” find the Tab Strip Buttons controls.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sidebar/settings-modal.tsx` around lines 9203 - 9225, The Actions settings search configuration is missing entries for the three Tab Strip Buttons toggles. Update extraSettingsTabSearches.actions, alongside terminalAction, browserAction, and actionShortcuts, to add search keys for hideTabStripNewTerminalButton, hideTabStripNewChatButton, and hideTabStripNewBrowserButton using terms that match their labels and descriptions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@gpui/sidebar/gxserver-runtime.ts`:
- Around line 4893-4905: Update the method containing postedGlobalActionsPayload
so it caches the payload only when postGlobalActions(payload) explicitly returns
true. When the bridge is unavailable or returns false, clear the cached value
and schedule a bounded retry, preserving the existing no-log behavior and
preventing repeated retries from becoming unbounded.
- Around line 14182-14195: Preserve the originating scope when resolving
commands: update the native click selector and its resolution path so Global
Actions carry an explicit global scope, then have the command-resolution method
select exclusively from globalCommands for that scope while retaining
project-first behavior for project actions. Use the existing command-resolution
symbols around normalizedCommandId and globalCommands, and do not rely on
globally unique command IDs.
In `@gxserver-rs/src/domain.rs`:
- Around line 594-642: Update order_global_sidebar_commands to deduplicate
command_ids while preserving their first-occurrence order, then acquire a writer
transaction before reading stored_ids so concurrent saves or deletes cannot
interleave with the reorder. Execute the complete read-and-update sequence
within one transaction, committing only after every update succeeds and rolling
back on any error, matching the BEGIN IMMEDIATE/COMMIT/ROLLBACK pattern used by
update_session_order.
In `@gxserver-rs/src/sidebar_hud.rs`:
- Around line 785-806: Update global_sidebar_command_save_mutation to reject
command IDs recognized by is_default_sidebar_command_id before constructing or
returning the global save mutation. Preserve the existing generated-ID behavior
for omitted commandId values, and return the established validation/domain error
for reserved IDs so global actions cannot be saved under default command
identifiers.
In `@sidebar/settings-modal.tsx`:
- Around line 9203-9225: Update the three tab-strip ToggleField rows in
ActionsSettingsTab—hideTabStripNewTerminalButton, hideTabStripNewChatButton, and
hideTabStripNewBrowserButton—to spread getSettingModificationProps for their
corresponding setting keys. Add the required getSettingModificationProps prop to
ActionsSettingsTab’s props and pass it from SettingsModal so each row exposes
advanced, modified, and reset behavior.
---
Nitpick comments:
In `@sidebar/settings-modal.tsx`:
- Around line 9203-9225: The Actions settings search configuration is missing
entries for the three Tab Strip Buttons toggles. Update
extraSettingsTabSearches.actions, alongside terminalAction, browserAction, and
actionShortcuts, to add search keys for hideTabStripNewTerminalButton,
hideTabStripNewChatButton, and hideTabStripNewBrowserButton using terms that
match their labels and descriptions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 523b302d-8458-4571-84f1-4320efac2051
📒 Files selected for processing (17)
gpui/sidebar/gxserver-runtime.tsgpui/src/cef/shell.rsgpui/src/cef/sidebar_bridge_manifest.rsgpui/src/cef/unsupported.rsgpui/src/main.rsgpui/src/shared_settings.rsgxserver-rs/src/constants.rsgxserver-rs/src/domain.rsgxserver-rs/src/server.rsgxserver-rs/src/sidebar_hud.rsgxserver-rs/src/storage.rsshared/ghostex-settings.tsshared/gxserver-protocol.tsshared/session-grid-contract-sidebar.tsshared/sidebar-commands.test.tsshared/sidebar-commands.tssidebar/settings-modal.tsx
Reorder issued one UPDATE per row with no surrounding transaction, so a failure partway left some rows on the new sortOrder and others on the old — an interleaved list that the server would then echo back as the confirmed order. Wrap it in BEGIN IMMEDIATE / COMMIT / ROLLBACK, mirroring update_session_order, which guards the identical case. Taking the writer reservation before the read also closes the read-then-write race against a concurrent save or delete, whose row would otherwise keep a stale sortOrder. Deduplicate the requested ids as well: a repeated id consumed two index positions, because the append guard skipped the duplicate while the write loop still assigned it twice. Reject the reserved built-in ids (dev/build/test/setup) for global actions. The read projection recomputes isDefault from the id, so a stored global action called "dev" came back marked as a default however it was written. Rejecting at save is what makes the invariant hold on both paths, and it keeps the global and project id spaces from colliding on the reserved names. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A run-by-id selector could not tell a Global Action from a Project Action sharing the same id, so a tab strip click resolved the project command first and could launch something the user did not click. Selectors now carry a scope and resolution reads that list exclusively. Scope is optional and absent means project, so the Command Palette and every existing sender are unchanged. The Global Actions bridge cached its payload before calling into CEF. An absent bridge function makes the optional call return undefined WITHOUT throwing, and a rejected payload returns false, so an undelivered payload was recorded as sent and the tab strip stayed empty until an unrelated HUD change produced a different payload. Cache only what the bridge confirms; the next HUD refresh retries on its own. Give the three tab strip toggles the same modification props every neighbouring row has, so they show a modified indicator and a per-row reset like the rest of Settings. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reorder issued one UPDATE per row with no surrounding transaction, so a failure partway left some rows on the new sortOrder and others on the old — an interleaved list that the server would then echo back as the confirmed order. Wrap it in BEGIN IMMEDIATE / COMMIT / ROLLBACK, mirroring update_session_order, which guards the identical case. Taking the writer reservation before the read also closes the read-then-write race against a concurrent save or delete, whose row would otherwise keep a stale sortOrder. Deduplicate the requested ids as well: a repeated id consumed two index positions, because the append guard skipped the duplicate while the write loop still assigned it twice. Reject the reserved built-in ids (dev/build/test/setup) for global actions. The read projection recomputes isDefault from the id, so a stored global action called "dev" came back marked as a default however it was written. Rejecting at save is what makes the invariant hold on both paths, and it keeps the global and project id spaces from colliding on the reserved names. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
maddada#75 landed the showOnProjectRow surface, which touches the same HUD contract. Both sides are additive, so the resolution keeps each: - readSidebarHud and mutateSidebarHudSettings call maddada#75's new apply_commands_by_project_if_requested helper and still attach globalCommands. - The shared save validator carries show_on_project_row, so global and project saves keep taking the identical action definition. - The mutation target union widens to "command" | "globalCommand" alongside maddada#75's showOnProjectRow field. Also updates two normalizeStoredSidebarCommands expectations that maddada#75 left asserting the pre-showOnProjectRow shape; they fail on main as merged, independently of this branch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…strip Resolves the two surfaces that both features touch: - runSidebarCommand now branches on scope. A global selector resolves against the global list and carries no group id, because a Global Action belongs to no project row; project selectors keep maddada#75's per-project resolution and project activation. The scopes pick different lists instead of falling through to each other. - The GPUI HUD state carries both maddada#75's commandsByProject and globalCommands, and global commands run through the same boundary normalization so a legacy daemon's missing showOnProjectRow becomes a real boolean. - The Settings save payload carries showOnProjectRow for both scopes, and the gpui command write enum carries both scope and show_on_project_row. Also adds Settings search entries for Global Actions and the three tab strip button toggles, so a query like "New Terminal button" finds them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Second of two PRs for Global Actions. Stacked on #79 — that PR's commit is the first one here, so review only
feat(actions): render Global Actions in the tab strip. Once #79 merges this diff shrinks to just that commit.This is the half you flagged as the hard part ("The tabs bar in gpui is harder to modify").
What it does
Cluster width is now derived from what's actually drawn rather than the fixed four-button constant. Global actions render before the built-ins so the user's own actions sit closest to the tabs and the built-in controls stay against the right edge. The pane overflow button has no toggle — it's the only route to the remaining pane actions.
Security shape of the bridge
The new sidebar bridge channel carries action id, display name and icon slug only. No command text or URLs reach gpui. A strip click sends the id back through the existing Action selector, which resolves the trusted definition on the sidebar side — so the tab strip never holds anything executable. Same posture as the status-indicator bridge: strict version/type, key allowlist, bounded list.
Two things worth knowing for review
globalCommandsseparately — otherwise saving and listing each fail on their own.hideProjectHeaderDiffStats, since it's per-machine window chrome. It's cached in app state and refreshed on the settings hook because the tab strip draws every frame and must not stat the settings file.Verification
cargo checkclean both crates; typecheck clean; 1279 TS tests pass. Per AGENTS.md no tests were added undergpui/.Confirmed by hand in a real dev build: a global action saves, its icon appears in the strip, clicking it opens a command terminal running in the active project's cwd, and the built-in toggles hide/show cleanly.
Open question: does #75 (per-project actions on sidebar rows) still stand now that Global Actions exist, or would you rather drop that surface? Happy either way.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements