Repository navigation
feat(settings): make automatic terminal creation optional - #22504
trentshapiro wants to merge 7 commits into
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe change adds a default-enabled setting that controls automatic terminal creation when selecting a workspace. Terminal seeding and web-runtime bootstrap paths read the setting, while explicit startup work remains handled separately. When automatic creation is disabled and a tab group has no active content, the panel can display terminal, markdown, and policy-controlled browser actions. Floating-terminal actions now use a shared action component. Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: 🟡 Moderate · up to With automatic terminal creation disabled, launches configured with default tabs can open a generic terminal without the requested tabs. This core workflow should be corrected before merging. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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: 1
🧹 Nitpick comments (1)
src/renderer/src/components/settings/GeneralWorkspaceSettingsSection.tsx (1)
106-106: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse intent-named IDs for the new localization strings.
The new setting and search strings use newly minted hash IDs. The repository policy does not grandfather these IDs. Rename the setting title, description, search title, description, and keyword IDs in an adjacent dedicated localization change, with matching English catalog entries.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: a2093a8a-8909-4159-b34d-a291d488424d
📒 Files selected for processing (29)
src/renderer/src/components/WorkspaceEmptyStateAction.tsxsrc/renderer/src/components/floating-terminal/FloatingTerminalEmptyState.tsxsrc/renderer/src/components/floating-terminal/FloatingTerminalPanel.empty-state.test.tsxsrc/renderer/src/components/settings/GeneralWorkspaceSettingsSection.test.tsxsrc/renderer/src/components/settings/GeneralWorkspaceSettingsSection.tsxsrc/renderer/src/components/settings/general-search.tssrc/renderer/src/components/tab-group/TabGroupEmptyState.tsxsrc/renderer/src/components/tab-group/TabGroupPanel.tsxsrc/renderer/src/components/terminal/initial-terminal-structured-launch.test.tsxsrc/renderer/src/components/terminal/initial-terminal-wiring.test.tssrc/renderer/src/components/terminal/initial-terminal.test.tssrc/renderer/src/components/terminal/initial-terminal.tssrc/renderer/src/components/use-terminal-watcher-effects.tssrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/lib/can-inspect-agent-activation-inventory.tssrc/renderer/src/lib/web-runtime-worktree-terminal-after-wake.tssrc/renderer/src/lib/worktree-activation-created-agent.test.tssrc/renderer/src/lib/worktree-activation-default-tabs.test.tssrc/renderer/src/lib/worktree-activation-emptied-workspace-reseed.test.tssrc/renderer/src/lib/worktree-activation-empty-remote.test.tssrc/renderer/src/lib/worktree-activation-store-contract.tssrc/renderer/src/lib/worktree-activation.tssrc/renderer/src/lib/worktree-creation-flow-execute.tssrc/renderer/src/lib/worktree-initial-terminal-seeding.tssrc/renderer/src/runtime/web-session-tabs-sync-terminal-bootstrap.test.tssrc/renderer/src/runtime/web-session-tabs-sync/active-session-subscription.tssrc/renderer/src/runtime/web-session-tabs-sync/tracking-decisions.tssrc/shared/default-global-settings.tssrc/shared/global-settings-types.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Turning off Open a Terminal When Selecting a Workspace silently drops configured default tabs — and, on the web-runtime path, non-agent startup work — in the backgrounded and activation-recovery create paths. The primary activation path preserves them, so the same workspace behaves differently depending on whether the create was backgrounded.
Reviewed changes
- New setting —
autoCreateTerminalOnWorkspaceActivation(default on) added toGlobalSettings,buildDefaultSettings, the General workspace pane, and the general search catalog. - Activation gate —
shouldAutoCreateInitialTerminalgains a thirdautomaticCreationEnabledargument, threaded through local/folder/SSH activation, the seeding helper, web-runtime after-wake, worktree creation, and the web-session bootstrap. - Empty-workspace surface — new
WorkspaceEmptyStateAction+TabGroupEmptyState, rendered byTabGroupPanelonly when the setting is off;FloatingTerminalEmptyStateis refactored onto the shared action. - Empty
defaultTabsno longer counts as activation work —hasActivationWorknow usesopts?.defaultTabs?.tabs.length. - Extraction —
canInspectAgentActivationInventorymoved to its own module; behavior tests added for local, folder, SSH/remote, web-runtime, and the settings toggle.
ℹ️ Nitpicks
TabGroupEmptyStatealways renders the New Browser action, whileFloatingTerminalEmptyStategates it behindshowNewBrowser. On workspaces where managed browser creation is disabled (e.g. some SSH/remote hosts), clicking it throws and surfaces an error toast; consider computing availability viagetClientCreationActionPolicyand hiding the action.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Fixes the prior review's default-tab/web-runtime findings, gates the empty-state Browser action, and renames the new localization IDs. New commits since the last pullfrog review (97ca8897df):
- Default tabs preserved when the setting is off —
ensureWorktreeHasInitialTerminalnow counts a non-emptydefaultTabspayload as explicit launch work, soapplyDefaultTerminalTabsstill runs withautomaticCreationEnabled: false. - Web-runtime non-agent startup preserved —
ensureWebRuntimeWorktreeTerminalAfterWakeskips the passive-seed gate when an explicitstartupis supplied. - Browser action gated in the empty state —
TabGroupEmptyStategains ashowNewBrowserprop, computed inTabGroupPanelfromgetClientCreationActionPolicy, matchingFloatingTerminalEmptyState. - Intent-named localization IDs — the setting/search strings were renamed off hashed IDs, and JSDoc was added to the touched exports.
- Tests — coverage added for preserved default tabs, preserved remote startup, the disabled-setting empty workspace, and the Browser action gating.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
|
@coderabbitai review |
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: c6c929dd-d6a4-4201-a3a5-e01cde99f49d
📒 Files selected for processing (19)
src/renderer/src/components/WorkspaceEmptyStateAction.tsxsrc/renderer/src/components/floating-terminal/FloatingTerminalEmptyState.tsxsrc/renderer/src/components/settings/GeneralWorkspaceSettingsSection.tsxsrc/renderer/src/components/settings/general-search.tssrc/renderer/src/components/tab-group/TabGroupEmptyState.test.tsxsrc/renderer/src/components/tab-group/TabGroupEmptyState.tsxsrc/renderer/src/components/tab-group/TabGroupPanel.tsxsrc/renderer/src/components/terminal/initial-terminal.tssrc/renderer/src/components/use-terminal-watcher-effects.tssrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/lib/can-inspect-agent-activation-inventory.tssrc/renderer/src/lib/web-runtime-worktree-terminal-after-wake.tssrc/renderer/src/lib/worktree-activation-default-tabs.test.tssrc/renderer/src/lib/worktree-activation-empty-remote.test.tssrc/renderer/src/lib/worktree-activation.tssrc/renderer/src/lib/worktree-creation-flow-execute.tssrc/renderer/src/lib/worktree-initial-terminal-seeding.tssrc/renderer/src/runtime/web-session-tabs-sync/tracking-decisions.tssrc/shared/default-global-settings.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- src/renderer/src/components/terminal/initial-terminal.ts
- src/renderer/src/runtime/web-session-tabs-sync/tracking-decisions.ts
- src/renderer/src/components/use-terminal-watcher-effects.ts
- src/shared/default-global-settings.ts
- src/renderer/src/lib/can-inspect-agent-activation-inventory.ts
- src/renderer/src/i18n/locales/en.json
- src/renderer/src/components/WorkspaceEmptyStateAction.tsx
- src/renderer/src/lib/worktree-creation-flow-execute.ts
- src/renderer/src/components/floating-terminal/FloatingTerminalEmptyState.tsx
- src/renderer/src/lib/worktree-activation.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.
|
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Follow-up fix for duplicate activation terminals. New commits since the last pullfrog review (a71d53d13b):
- Applied default tabs no longer count as explicit launch work —
ensureWorktreeHasInitialTerminalcomputeshasPendingDefaultTabs(a non-emptydefaultTabspayload with no persisted applied marker) and uses it for bothhasExplicitLaunchWorkand thecallerProvidesSurfaceearly return, so re-activating an already-seeded workspace can no longer fall through to a bare fallback terminal when automatic creation is off. - Web-runtime wake duplicate guards restored for non-agent startup —
ensureWebRuntimeWorktreeTerminalAfterWake's live-PTY / mirrored-host-tabs / last-known-count guards run again whenopts.startupis set; only the empty-workspace auto-create gate stays skipped for startup. - Tests — applied default tabs with the setting off create nothing; non-agent startup does not repeat when a terminal is already live.
The prior review's open thread (worktree-creation-flow-execute.ts) is resolved and its concern remains addressed by this refinement.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
Important
The flag simplification in f22910ee68 drops explicit non-startup launch work (setup, issueCommand, non-empty defaultTabs) on the web-runtime after-wake path when the setting is off — the host terminal is never requested, so the queued commands have no tab to attach to. Existing tests do not cover this arm.
Reviewed changes — incremental review of f22910ee68 ("refactor: simplify initial terminal activation") since the prior pullfrog review at 20ac964a75:
- Setting read at the leaf —
ensureWorktreeHasInitialTerminalandensureWebRuntimeWorktreeTerminalAfterWakenow readsettings.autoCreateTerminalOnWorkspaceActivationdirectly instead of accepting the caller-threadedautomaticCreationEnabledoption;InitialTerminalOptionsand the store contract drop the field. - Activation derivation removed —
activateAndRevealWorktreeno longer computeshasActivationWork || setting !== false; the agent-activation gate reads the setting inline (equivalent, since those gates are already guarded by!hasActivationWork/!opts.startup). - Inventory helper inlined —
can-inspect-agent-activation-inventory.tswas deleted andcanInspectAgentActivationInventorymoved back intoworktree-activation.ts. - Test rewrites —
worktree-activation-empty-remote.test.tsrebuilt on aseedRemoteWorktreeTestStatehelper;TabGroupEmptyState.test.tsxmoved to React Testing Library; JSDoc reworded across the touched modules.
The single existing review thread (worktree-creation-flow-execute.ts) is already resolved, and the new commits do not reopen it.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — incremental review of 9314f5c9b9 ("fix: preserve explicit web-runtime launch work") since the prior pullfrog review at f22910ee68:
- Explicit-work bypass restored on the web-runtime after-wake path —
ensureWebRuntimeWorktreeTerminalAfterWakegainshasExplicitLaunchWork; when set, it skips the passive empty-workspace gate so the host terminal is requested even with the setting off. - Callers forward the signal —
activateAndRevealWorktreepasseshasActivationWork, andexecuteWorktreeCreationcomputeshasExplicitTerminalWorkonce and passes it to both the activation-recovery and backgrounded after-wake sites. defaultTabstruthiness tightened — the creation-flow flag now countsresult.defaultTabs?.tabs.length, matching the seeding helper's pending-tab semantics.- Tests — after-wake coverage for the explicit-work bypass with the setting off, plus an assertion that a setup create forwards
hasExplicitLaunchWork: true.
The prior review's regression (explicit non-startup work stranded on a live web runtime with the setting off) is resolved and its thread is closed. The remaining live-host limitation — defaultTabs are not materialized on a live host authority regardless of the setting — is pre-existing from 20ac964a75 and outside this delta.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
|
Thank you so much, @trentshapiro, for the thoughtful implementation, tests, and before/after evidence here, and for crediting @yiuf63’s earlier work in #9960. We referred to your PR while comparing the community approaches with the empty-pane launch UI that has now landed in #32182. I have explicitly credited you and linked this PR in #32182. The empty-state action surface is now available there, including file, terminal, browser, and host-aware agent launches. Your automatic-terminal preference and its activation/creation/import/bootstrap coverage go beyond what #32182 landed; that remaining request is still tracked in #11108. We really appreciate the care you put into this and the concrete coverage you provided. Thank you again for helping move this workflow forward. |
|
Closing as superseded. |
ELI5
Orca can now leave an empty workspace empty. A new setting controls whether selecting, creating, or importing an empty workspace opens a terminal automatically. When automatic creation is disabled, the workspace shows explicit actions for opening a terminal, Markdown note, or browser.
What Changed
Open a Terminal When Selecting a Workspaceunder General workspace settings. Existing behavior remains enabled by default.Why
Workspace navigation should be able to remain passive. Users who intentionally keep a workspace empty should not have a terminal or process created merely because they selected, created, or imported it. A default-on preference preserves current behavior while allowing explicit control.
This implementation builds on the approach explored in #9960. It also incorporates the automated review feedback from that PR by using intent-named localization IDs and reusing the parent workspace model commands in the empty state.
Linked Issue
Fixes #11108
Visual Proof
Before, selecting an empty workspace with automatic creation enabled opens
Terminal 1.After, selecting an empty workspace with automatic creation disabled leaves it empty and presents explicit actions.
Testing
pnpm typecheckpasses.pnpm run check:code-quality:changedpasses with zero findings.pnpm run build:desktoppasses.pnpm lintwas run and reaches unrelated existing import-cycle findings in unchanged mobile files.pnpm testwas run and reaches unrelated local environment and integration failures, including unavailable Playwright browser binaries and real CLI timeouts. The finalized feature-focused suite passes.pnpm buildcompletes desktop and web output, then the native macOS helper build fails locally when Xcode reportsUnknown error parsing property list.AI Disclosure
OpenAI Codex was used to inspect the stalled implementation, implement and test the feature, review the branch, incorporate prior automated feedback, and prepare this PR.
Review
origin/main. No remaining feature-blocking issues were found.Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
N/AChecklist
pnpm lint,pnpm typecheck,pnpm test, andpnpm buildall pass locally. Local failures are documented above and CI will cover the full matrix.