Repository navigation
fix(workspaces): seed shells only for blank selection - #19940
Conversation
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
worktree-activation-surface-selection.ts— newWorktreeActivationSurfaceSelectionwithagent?: TuiAgent | null, andactivationProvidesInitialSurface()collapsing "navigation caller" and "agent selection" into one surface-ownership signal.worktree-activation.ts— folds allopts?.providesInitialSurfacereads into a single computed boolean; thenotifyHostRuntimewake-respawn gate now additionally requiresopts?.agent == null.worktree-creation-flow-execute.ts— threadspreparedRequest.agentinto activation and widens the non-activating seed skip fromstructuredLaunchtoagent !== null.- Create-flow callers —
folder-workspace-composer-submit.ts,full-creation-execution.ts, andfull-creation-structured-launch.tspass the picker selection through activation (including the structured fallback). - Host-side guards —
runtime-local-worktree-terminal-startup.ts,runtime-folder-worktree-create.ts, andruntime-remote-managed-worktree-create.tsskip the idle-shell fallback whencreatedWithAgentis set.
The change is coherent and well-motivated: it removes the duplicate-seed at both owners (renderer and host) rather than adding a renderer-only band-aid, and keeps Blank Terminal behavior and navigation-caller providesInitialSurface semantics intact.
I traced the load-bearing seams and found no correctness issues: the widened renderer skip (agent !== null instead of structuredLaunch) is safe because an agent selection always materializes its terminal via one of structured session, backend spawn, or a renderer startupPlan (which keeps hasExplicitTerminalWork true, so the actual seed still happens); agent is non-null in the composer model (tuiAgent: TuiAgent), so the new non-null settleFullCreationStructuredLaunch.agent parameter is well-typed; and the new surface-selection seam is correctly registered in the providesInitialSurface caller census. The tests pin real behavior (exact call/tabCount assertions plus a pre-existing ablation), and the widened assertion in the cancellation test reflects the new behavior rather than masking it.
The one gap worth noting for context only: the two host-side one-line guards (runtime-folder-worktree-create.ts and runtime-remote-managed-worktree-create.ts) have no direct unit test (the sibling startRuntimeLocalWorktreeTerminals does). They mirror tested logic and are exercised by the orca-runtime integration specs, so this is not a blocker.
DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
|
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 (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 4 remain after this review. 📝 WalkthroughWalkthroughAgent selections propagate through folder and worktree creation into activation. Activation derives initial-surface availability from the selected agent or an explicit caller flag. Agent-created worktrees skip fallback terminal creation across local, folder, and remote runtime paths. Web-runtime recovery carries startup and agent data and suppresses duplicate terminal bootstrap. Tests cover these paths and cancelled creation handling. Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to The workspace startup changes have no identified merge-blocking risk. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 66c3c9e7-39cf-446a-8ddd-b4716fc67031
📒 Files selected for processing (17)
src/main/runtime/runtime-folder-worktree-create.tssrc/main/runtime/runtime-local-worktree-terminal-startup.test.tssrc/main/runtime/runtime-local-worktree-terminal-startup.tssrc/main/runtime/runtime-remote-managed-worktree-create.tssrc/renderer/src/components/sidebar/folder-workspace-composer-submit.test.tssrc/renderer/src/components/sidebar/folder-workspace-composer-submit.tssrc/renderer/src/hooks/composer-state/full-creation-execution.tssrc/renderer/src/hooks/composer-state/full-creation-structured-launch.test.tssrc/renderer/src/hooks/composer-state/full-creation-structured-launch.tssrc/renderer/src/lib/worktree-activation-emptied-workspace-reseed.test.tssrc/renderer/src/lib/worktree-activation-surface-caller-wiring.test.tssrc/renderer/src/lib/worktree-activation-surface-selection.tssrc/renderer/src/lib/worktree-activation-web-runtime.test.tssrc/renderer/src/lib/worktree-activation.tssrc/renderer/src/lib/worktree-creation-agent-seeding.test.tssrc/renderer/src/lib/worktree-creation-flow-execute.tssrc/renderer/src/lib/worktree-creation-flow.test.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review.
| args.startup && !didSpawnStartup ? args.startup : undefined | ||
| ) | ||
| } else if (deps.ptySpawnAvailable && !didSpawnStartup) { | ||
| } else if (deps.ptySpawnAvailable && !didSpawnStartup && !args.createdWithAgent) { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 8 '\b(createdWithAgent|draftPaste|startupDraftPaste)\b' srcRepository: stablyai/orca
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in \
src/main/runtime/runtime-folder-worktree-create.ts \
src/main/runtime/runtime-local-worktree-terminal-startup.ts \
src/main/runtime/runtime-remote-managed-worktree-create.ts \
src/main/runtime/runtime-worktree-agent-startup.ts \
src/main/runtime/runtime-managed-worktree-create-types.ts
do
echo "===== $f ====="
ast-grep outline "$f" 2>/dev/null | head -n 120 || true
done
echo "===== targeted references ====="
rg -n -C 18 'draftPaste|startupDraftPaste|createdWithAgent|didSpawnStartup|shouldActivate|canSpawn|ptySpawnAvailable' \
src/main/runtime/runtime-folder-worktree-create.ts \
src/main/runtime/runtime-local-worktree-terminal-startup.ts \
src/main/runtime/runtime-remote-managed-worktree-create.ts \
src/main/runtime/runtime-worktree-agent-startup.ts \
src/main/runtime/runtime-managed-worktree-create-types.tsRepository: stablyai/orca
Length of output: 50369
🤖 get_repo_knowledge executed:
get_repo_knowledge stablyai/orca /tmp/coderabbit-repo-knowledge/stablyai-orca-89dc44e4/architecture /tmp/coderabbit-repo-knowledge/stablyai-orca-89dc44e4/learnings
Length of output: 47161
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in \
src/main/runtime/runtime-folder-worktree-create.ts \
src/main/runtime/runtime-local-worktree-terminal-startup.ts \
src/main/runtime/runtime-remote-managed-worktree-create.ts
do
echo "===== $f relevant lines ====="
rg -n -C 30 'draftPaste|startupDraftPaste|createdWithAgent|didSpawnStartup|shouldActivate|canSpawn|ptySpawnAvailable' "$f"
doneRepository: stablyai/orca
Length of output: 29260
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '110,205p' src/main/runtime/runtime-folder-worktree-create.ts
sed -n '105,190p' src/main/runtime/runtime-local-worktree-terminal-startup.ts
sed -n '165,255p' src/main/runtime/runtime-remote-managed-worktree-create.ts
sed -n '1,145p' src/main/runtime/runtime-worktree-agent-startup.tsRepository: stablyai/orca
Length of output: 15378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in src/main/runtime/runtime-folder-worktree-create.ts src/main/runtime/runtime-local-worktree-terminal-startup.ts src/main/runtime/runtime-remote-managed-worktree-create.ts; do
echo "===== $f ====="
awk 'BEGIN{p=0} /draftPaste|startupDraftPaste|createdWithAgent|didSpawnStartup|shouldActivate|canSpawn|ptySpawnAvailable/{p=1; start=NR-12} p && NR>=start {print NR ":" $0} p && NR>start+24 {p=0}' "$f"
doneRepository: stablyai/orca
Length of output: 19242
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 20 \
'createRuntimeFolderWorktree|startRuntimeLocalWorktreeTerminals|createRuntimeRemoteManagedWorktree|startupDraftPaste|draftPaste:' \
src/main/runtime src/main/ipc \
-g '*.ts' \
| head -n 1200Repository: stablyai/orca
Length of output: 50369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 24 'createRuntimeFolderWorktree\(|startRuntimeLocalWorktreeTerminals\(|createRuntimeRemoteManagedWorktree\(' src/main -g '*.ts'
rg -n -C 16 'startupDraftPaste|draftPaste' src/main/runtime/orca-runtime.ts src/main/runtime -g '*.ts' | head -n 1000Repository: stablyai/orca
Length of output: 50369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 12 'startupDraftPaste|effectiveCreatedWithAgent|effectiveDraftPaste' \
src/main src/shared src/preload src/cli \
-g '*.ts' | head -n 1000Repository: stablyai/orca
Length of output: 32624
Include explicit draft-paste agents in fallback suppression.
The orchestrator derives effectiveCreatedWithAgent from draftStartup.agent only when startup is absent. With explicit startup, it forwards startupDraftPaste separately and leaves the marker equal to createdWithAgent. If startup creation fails, all three fallback branches can therefore create a plain terminal. Guard with the effective draft-paste agent.
📍 Affects 3 files
src/main/runtime/runtime-folder-worktree-create.ts#L175-L175(this comment)src/main/runtime/runtime-local-worktree-terminal-startup.ts#L166-L166src/main/runtime/runtime-remote-managed-worktree-create.ts#L225-L225
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
web-runtime-worktree-terminal-after-wake.ts—ensureWebRuntimeWorktreeTerminalAfterWakenow acceptsstartup/agent/activateand materializes the prepared agent terminal (or blank shell) on the paired host, guarded by alaunchAgentdedup against existing mirrored/live tabs.worktree-activation.ts— thenotifyHostRuntimegate moved fromopts?.agent == nulltoopts?.providesInitialSurface !== true, now threadingstartup/agentinto the wake-respawn for both worktree and folder activation.worktree-creation-flow-execute.ts/worktree-creation-structured-session.ts— background and structured-fallback creation route agent selections throughensureWebRuntimeWorktreeTerminalAfterWake(activate: false) instead of leaving them surface-less.web-runtime-terminal-create-operation.ts/web-runtime-session-types.ts— new client-onlypreparedAgentCommandflag forces the legacysession.tabs.createTerminalpath, skipping host authority so a command that already encodes the agent startup is not rebuilt.active-session-subscription.ts— the wake-respawn skip flag now also gates the initial-terminal bootstrap, preventing a competing seed while the host-aware surface is in flight.
This commit closes the gap the prior commit left on paired web-runtime workspaces: an agent selection with no structured session, backend spawn, or startup plan previously produced no surface at all, since the renderer never asked the host to create the agent terminal. All affected flows (active reveal, background create, structured fallback, folder activation) now converge on one host-aware creation point, and Blank Terminal keeps its original shell-seeding path unchanged.
The load-bearing seams check out. launchAgent/launchToken/command are pre-existing params on session.tabs.createTerminal (the legacy path already forwarded them); the only wire-adjacent addition is preparedAgentCommand, a client-side flag that is never serialized. The opts?.providesInitialSurface !== true gate intentionally uses the raw navigation flag so agent selections still route through the wake-respawn while navigation (and structured-launch) callers stay excluded. The removed getKnownWorktreeById null-check in ensureWebRuntimeWorktreeTerminalAfterWake is safe because getRuntimeEnvironmentIdForWorktree returns null for unknown ids, and every call site passes a freshly-created worktree/folder key. New tests pin the exact host request (command/env/launchAgent/launchToken), the backend-spawned no-op, and the blank-vs-agent surface count.
DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
web-runtime-worktree-terminal-after-wake.ts— a failed host terminal-creation request now surfaces atoast.errorwith a stable per-workspace id, chained between thecreateWebRuntimeSessionTerminalcall and the existingfinallyrespawn teardown.worktree-activation-empty-remote.test.ts— mockssonnerand pins the failure surfacing (toast fires with the host message and stable id, exactly one host call, and the success path does not toast).
This commit closes the silent-failure gap the previous commit left open: when the paired host refused an agent-terminal request, a selected agent ended up with no surface and no explanation. The toast turns that into a concrete signal.
The seam checks out. createWebRuntimeSessionTerminal resolves a WebRuntimeTerminalCreateOutcome ({ status: 'created' } | { status: 'failed'; message: string }), so outcome.status === 'failed' narrows correctly before reading .message; createWebRuntimeSessionTerminalResult catches and converts every internal error into a resolved failed outcome, so the promise never rejects and the finally teardown always runs. The toast id is stable per runtime/worktree, so repeated failures update one toast rather than stacking. The new test is meaningful — it fails if the toast, the message, or the single-call guarantee regresses.
DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
Final review status: clean / merge-readyReviewed and fixed through a same-model review loop at head Functional correctnessThe root failure was treating picker selection intent as proof that an initial agent surface existed. That could suppress fallback terminal creation even when the execution host had not created an agent surface, leaving active or background workspaces empty. The final implementation makes backend create receipts authoritative, keeps surface creation on the execution host, and covers active/background creation, git worktrees and folder workspaces, paired runtimes, structured refusal, and in-flight create fencing. The final follow-up also handles a definite pre-create failure: it shows one environment/worktree-keyed error toast, performs no automatic retry, and still treats post-create reconciliation failures as created so an ambiguous retry cannot duplicate a terminal. Architecture and precedentThe implementation reuses Orca's existing Material deviation/risk: mixed-version new-client/old-host sessions can still retain an idle host shell next to the selected agent. That is a pre-existing compatibility limitation and should be capability-gated separately rather than broadening this PR. Validation
Remaining validation gaps
Recommendation: merge-ready based on functional review, architectural fit, focused validation, and green CI, with the live-platform and visual-proof gaps above recorded explicitly. |
Live platform validation follow-up: Windows, WSL, and SSH passValidated exact head Windows native — PASSWindows High Spec, exact-head disposable child checkout and isolated profile:
Windows screenshotsAgent singleton:
Same agent after re-entry:
Blank Terminal output:
Folder agent initial surface:
Authenticated folder-agent response:
WSL — PASSWindows High Spec with real Ubuntu 24.04 / Git 2.43:
WSL screenshots
Real SSH — PASSDisposable Ubuntu 22.04 arm64 SSH container, Node 20.20.2, real
SSH screenshots
Cleanup and final status
Recommendation: land. |
Electron validation statusValidated at exact PR head macOS Electron (fresh pass)Using
Screenshots: Other Electron/remote coverage
Remaining validation limits
No concrete release-blocking regression was found. Status: LAND for the reviewed and tested scope, with the limits above explicitly documented. |
…nt selection (#20182) * fix(workspaces): re-seed a gate-reported empty workspace after an agent selection #19940 routed two different questions through one predicate. An agent selection justifies skipping the *pre-emptive* shell at create time, but it was also suppressing the async activation gate's fail-closed re-seed. The gate returns `empty` only after adoption, structured inventory and resume all produced nothing, so it is positive evidence the agent surface never arrived (dead PTY, unreadable census, null startup plan) — exactly when the re-seed is needed. Suppressing it there left the workspace with zero tabs and no recovery: ensureWebRuntimeWorktreeTerminalAfterWake returns early for a plain local workspace, so nothing else seeds one. `gatedEmptyOutcomeReseedSuppressed` now answers the re-seed question on its own, honouring only an explicit `providesInitialSurface` caller promise — restoring v1.4.200's behaviour for that path. The create-time skip keeps using the agent-inclusive `activationProvidesInitialSurface` unchanged. * refactor(workspaces): make gated reseed policy explicit * fix(workspaces): harden gated empty reseeding * chore(workspaces): split duplicate-folder routing fix * refactor(workspaces): keep reseed helper outside caller census --------- Co-authored-by: Merge Sim <sim@local>
ELI5
When the user picks Blank Terminal, Orca opens a shell. When the user picks an agent, that agent owns the initial surface, so Orca does not open a competing idle shell first.
What Changed
providesInitialSurfacefor navigation callers.Workspace Creation Entry Paths
worktree.createThe PR also updates the shared completion paths used by those entry points:
providesInitialSurfacefor editor/diff surfaces.Reopening or navigating to an existing workspace is not a new creation entry point; it is touched only because the same activation/reseed code finishes newly created and restored workspaces.
Why
Renderer activation and host-side background creation could each seed a default shell before the selected agent opened. Enforcing the same ownership rule at both points prevents duplicate tabs and visible surface swaps while keeping Blank Terminal behavior unchanged.
Linked Issue
N/A — standalone Task 1 from the reviewed agent-launch surface-ownership plan.
Visual Proof
N/A — no visual styling or layout changed; the interaction routing is covered by focused automated tests and ablation.
Testing
I manually tested these changes locally
Automated tests added/updated, or explained why not below
Focused suite: 8 files, 92/92 tests passed.
Supporting launch-plan/census suite: 3 files, 29/29 tests passed.
Full-creation execution characterization: 1/1 test passed.
Sequential typechecks passed: web, Node, and CLI.
Direct
oxlinton all 17 changed files: 0 errors and 0 warnings.oxfmt --check,git diff --check, and changed-file NUL scan passed.Ablation: reverted the nine production files to
origin/mainwhile retaining the tests; 11/92 tests failed and 81/92 passed. Restoring the implementation returned the same suite to 92/92 passing.Review
The change removes the duplicate-seeding mechanism at both owners instead of adding a renderer-only guard. Existing navigation semantics remain owned by
providesInitialSurface; no remote wire contract changes.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
No inaccuracies were found in Task 1 of the plan. The repository does not contain
config/tsconfig.tc.node.json, so Node validation used the existingconfig/tsconfig.node.jsonas required by the project scripts.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (focused tests, direct lint/format, and sequential project typechecks passed; CI will cover the full suite and build)