Repository navigation
feat(floating-workspace): render the floating panel through the shared workspace surface - #22302
Conversation
…red session can run there
…-chat' into brennanb2025/floating-unify-impl
… workspace kind The host half removed the 'floating-workspace' blocker; this pin still asserted it and went red on the merge. Floating now routes like any workspace, so with capabilities un-negotiated the resolver answers 'runtime-capability-unknown'.
…ow without a worktree record The renderer had no Worktree row for the floating id: markdown creation, tab-bar path entry, native-chat file links, attachments and model discovery all resolve a workspace through getKnownWorktreeById and silently fail for it. The host already answers with a synthetic row minted from the resolved floating directory; this gives the renderer the same single seam. - The repo slice stores the host-resolved floating directory (floatingWorkspacePath); the panel's existing cwd resolution now writes it there instead of panel-local state, so there is one copy. - findKnownWorktreeById mints the shared floatingWorkspaceToWorktree row from that path, local host only, cached while the directory is unchanged — mirroring the folder-workspace branch it sits beside. - No consumer changes needed: every row-dependent path already goes through this resolver.
Shared editor/browser activation always wrote global selection (activeFileId, activeBrowserTabId, activeTabType). The floating panel is an overlay above the active workspace, and its active tab must never become global selection — the invariant documented in agent-auto-ack-targets.ts. Group-scoped activation is a prerequisite for rendering the floating workspace through the shared tab-group surface. - setActiveFile and setActiveBrowserTab take an optional target workspace, mirroring setActiveTabType: global selection moves only when the target IS the globally active workspace; per-workspace state always updates. Omitted keeps legacy behavior. - The shared tab-group activation/creation commands pass their workspace, which is a no-op for the main surface (hidden surfaces are inert, so commands only fire on the active workspace) and confines floating activation to floating state. - Terminal setActiveTab was already owner-scoped; now pinned by test.
… from main-window chrome
Preparation for hosting the floating workspace on the shared surface, whose shell owns its own
titlebar tab strip and cannot mount the main chrome:
- WorkspaceTabDragLayer: the one dnd-kit scope for a workspace's strips and tree, extracted from
TabGroupSplitLayout as a render-prop layer so a host can put shell-owned chrome (a titlebar
strip) inside the same drag scope as the tree.
- TabGroupSplitNodeTree: the chrome-free recursive group tree, with an explicit tabStrip
composition point on TabGroupPanel ('attached' keeps today's per-group strip; 'external' hosts
render no per-group strip at all — a hidden one would register duplicate drag sortables).
- WorkspacePaneOverlayLayers: the retained pane hosts extracted from WorktreeSplitSurface so any
tree host mounts the same pane lifecycle stack.
- TerminalPaneOverlayLayer gains ownsNativeChatToggleShortcut, following the existing rule that
only one live workspace registers the chord.
TabGroupSplitLayout and WorktreeSplitSurface recompose these parts; the main surface's DOM is
unchanged.
…mically An empty floating workspace has no tab-group layout at all (the slice starts empty and hydration skips tab-less workspaces), while the shared group tree requires one. This model resolves the two together: the panel shows its empty state exactly until the first tab exists, and mounts the tree only with a layout plus a live focused group (stored focus, else the group with an active tab, else the first group — matching the shell chrome's focused-group policy).
…d workspace surface The panel previously projected exactly one tab group and hand-rendered its panes, so any tab that left that group — 'Move Tab to Split' creates a second group — stayed alive in state but vanished from the panel. The body now mounts the shared group tree and retained pane overlay stack for the floating workspace id, so every group renders, splits work, and structured chat finally has a floating surface. - One drag scope: WorkspaceTabDragLayer wraps the titlebar strip AND the body tree, so exactly one DndContext owns floating tab drag; strip drops reorder, body-edge drops split. The floating-only FloatingWorkspaceTabDragContext is deleted. - The titlebar keeps the panel's single TabBar, now fed by the shared per-group workspace model of the focused group (tabStrip='external' keeps per-group strips out of the shell), with workspace-scoped activation commands so global selection never moves. - Panes render through WorkspacePaneOverlayLayers: terminals gate on the resolved cwd and a settled viewport exactly as before; browser guests ride the shared retention layer; agent session tabs appear in the strip and render through the structured overlay. - The empty state renders exactly until the first resolvable tab exists (stale unified entries still show it), per the atomic layout/emptiness model. - Deleted: the single-group projection, the hand-written pane maps, FloatingBrowserSlot, and the floating pane-handle registry. Known behavior deltas, both narrowing floating onto main-workspace semantics: Cmd/Ctrl+W with panel (not terminal) focus closes the active tab rather than one pane of a multi-pane tab, and a pane-owned close of the last tab no longer arms the panel focus reclaim (strip and shortcut closes still do).
The retained overlay stack moved into WorkspacePaneOverlayLayers and the floating chrome model became a guest-paint consumer; the structured-pane layering pin and the browser-guest retention census both name files, so they follow the seam.
|
Important Review skippedWe couldn't safely recover the incremental review. No full review was started, and the last reviewed checkpoint was preserved. Retry later, or explicitly request a full review by commenting You can disable this status message by setting the Use the checkbox below for a quick retry:
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 (29)
💤 Files with no reviewable changes (9)
Included review availability: Your plan provides up to 10 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughFloating workspaces can use structured sessions and resolve to local directories. Session records can retain launch directories, which structured launches and TUI handoffs use. The renderer stores floating workspace paths and panel state. It renders floating tabs through shared split-tree, drag, and overlay components. Workspace-scoped activation, attention, tab closing, and native-chat directory resolution also account for the floating panel. Priority: ➖ Normal Merge Risk: 🟡 Moderate · up to “Close All Files” can unexpectedly close structured chat sessions in the floating panel. Correct that behavior before merging; the split-pane test also leaves a gap in protection against a pane-rendering regression. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to The design pins chats to their original folder and refuses to resume when that folder is unavailable, which limits accidental folder switching. It also shares the folder path with session-status consumers. The access rules for those consumers and the validation of a path at initial session creation remain unconfirmed. Retained concerns
Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 19.05% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 84 functions across 102 files. (6 skipped: 6 unsupported.) ✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
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: 671921f1-4d2b-408c-89c0-057b76e63f10
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (69)
src/main/agent-launch/agent-launch-executor.test.tssrc/main/agent-launch/agent-launch-mode.tssrc/main/runtime/orca-runtime-persist-headless-terminal-title.tssrc/main/runtime/orca-runtime-resolve-browser-network-execution-host-for-worktree.tssrc/main/runtime/orca-runtime-resolve-recovered-structured-tui-transcript.tssrc/main/runtime/rpc/methods/agent-launch-floating-workspace.test.tssrc/renderer/src/components/TerminalWorktreeSplitSurface.tsxsrc/renderer/src/components/WorkspacePaneOverlayLayers.tsxsrc/renderer/src/components/activity/ActivityPrototypePage.tsxsrc/renderer/src/components/activity/activity-thread-actions.tssrc/renderer/src/components/activity/use-agent-pane-threads.tssrc/renderer/src/components/browser-pane/host-guest/browser-guest-retention-site-census.test.tssrc/renderer/src/components/floating-terminal/FloatingBrowserSlot.test.tsxsrc/renderer/src/components/floating-terminal/FloatingBrowserSlot.tsxsrc/renderer/src/components/floating-terminal/FloatingTerminalPanel.bounds.test.tsxsrc/renderer/src/components/floating-terminal/FloatingTerminalPanel.empty-state.test.tsxsrc/renderer/src/components/floating-terminal/FloatingTerminalPanel.focus.test.tsxsrc/renderer/src/components/floating-terminal/FloatingTerminalPanel.markdown-editor.test.tsxsrc/renderer/src/components/floating-terminal/FloatingTerminalPanel.shortcuts.test.tsxsrc/renderer/src/components/floating-terminal/FloatingTerminalPanel.tab-lifecycle.test.tsxsrc/renderer/src/components/floating-terminal/FloatingTerminalPanelSurface.tsxsrc/renderer/src/components/floating-terminal/FloatingTerminalWindowControls.test.tsxsrc/renderer/src/components/floating-terminal/FloatingWorkspaceTabDragContext.tsxsrc/renderer/src/components/floating-terminal/floating-terminal-panel-render-probe.tssrc/renderer/src/components/floating-terminal/floating-terminal-panel-test-fixtures.tssrc/renderer/src/components/floating-terminal/floating-terminal-panel-test-harness.tssrc/renderer/src/components/floating-terminal/floating-terminal-panel-test-module-mocks.tssrc/renderer/src/components/floating-terminal/floating-workspace-surface-model.test.tssrc/renderer/src/components/floating-terminal/floating-workspace-surface-model.tssrc/renderer/src/components/floating-terminal/terminal-pane-handle-registry.test.tssrc/renderer/src/components/floating-terminal/terminal-pane-handle-registry.tssrc/renderer/src/components/floating-terminal/use-floating-terminal-close-actions.tssrc/renderer/src/components/floating-terminal/use-floating-terminal-create-actions.tssrc/renderer/src/components/floating-terminal/use-floating-terminal-focus-lifecycle.tssrc/renderer/src/components/floating-terminal/use-floating-terminal-initial-focus-effects.tssrc/renderer/src/components/floating-terminal/use-floating-terminal-panel-controller.tssrc/renderer/src/components/floating-terminal/use-floating-terminal-panel-focus-reclaim.tssrc/renderer/src/components/floating-terminal/use-floating-terminal-panel-geometry.tssrc/renderer/src/components/floating-terminal/use-floating-terminal-panel-items.tssrc/renderer/src/components/floating-terminal/use-floating-terminal-panel-local-state.tssrc/renderer/src/components/floating-terminal/use-floating-terminal-panel-maximize.tssrc/renderer/src/components/floating-terminal/use-floating-terminal-panel-shortcuts.tssrc/renderer/src/components/floating-terminal/use-floating-terminal-panel-store-state.tssrc/renderer/src/components/floating-terminal/use-floating-workspace-chrome-model.tssrc/renderer/src/components/native-chat/native-chat-stop-layering.test.tssrc/renderer/src/components/sidebar/SidebarAgentsList.tsxsrc/renderer/src/components/tab-group/TabGroupPanel.tsxsrc/renderer/src/components/tab-group/TabGroupSplitLayout.test.tssrc/renderer/src/components/tab-group/TabGroupSplitLayout.tsxsrc/renderer/src/components/tab-group/TabGroupSplitNodeTree.tsxsrc/renderer/src/components/tab-group/WorkspaceTabDragLayer.tsxsrc/renderer/src/components/tab-group/useTabGroupActivationCommands.tssrc/renderer/src/components/tab-group/useTabGroupCreationCommands.tssrc/renderer/src/components/tab-group/useTabGroupWorkspaceModel.focus.test.tssrc/renderer/src/components/terminal-pane/TerminalPaneOverlayLayer.tsxsrc/renderer/src/lib/agent-launch-route-input.test.tssrc/renderer/src/lib/agent-launch-routing.test.tssrc/renderer/src/store/repos/repo-state.tssrc/renderer/src/store/slices/browser/browser-slice-contract.tssrc/renderer/src/store/slices/browser/browser-tab-focus-actions.tssrc/renderer/src/store/slices/editor/actions/open-file-mutations.tssrc/renderer/src/store/slices/editor/types/editor-files-slice.tssrc/renderer/src/store/slices/floating-workspace-row.test.tssrc/renderer/src/store/slices/repos.tssrc/renderer/src/store/slices/workspace-scoped-activation.test.tssrc/renderer/src/store/slices/worktrees/listing/detected-worktree-meta.tssrc/shared/floating-workspace-worktree.tssrc/shared/structured-native-chat-launch-route.test.tssrc/shared/structured-native-chat-launch-route.ts
💤 Files with no reviewable changes (9)
- src/renderer/src/components/floating-terminal/terminal-pane-handle-registry.test.ts
- src/renderer/src/components/floating-terminal/terminal-pane-handle-registry.ts
- src/renderer/src/components/floating-terminal/FloatingBrowserSlot.tsx
- src/renderer/src/components/floating-terminal/FloatingBrowserSlot.test.tsx
- src/renderer/src/components/floating-terminal/use-floating-terminal-panel-local-state.ts
- src/renderer/src/components/floating-terminal/FloatingWorkspaceTabDragContext.tsx
- src/renderer/src/components/floating-terminal/use-floating-terminal-panel-items.ts
- src/main/agent-launch/agent-launch-mode.ts
- src/shared/structured-native-chat-launch-route.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review.
| } else if (item.contentType === 'agent-session') { | ||
| // Mirrors the shared workspace close: a structured chat closes by its unified tab. | ||
| closeUnifiedTab(item.id) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '1,180p' src/renderer/src/components/floating-terminal/use-floating-terminal-close-actions.ts
rg -n "closeFloatingItemConfirmed|closeFloatingItems|onClose" src/renderer/src/components/floating-terminal/FloatingTerminalPanelSurface.tsx src/renderer/src/components/floating-terminal/use-floating-terminal-panel-shortcuts.tsRepository: stablyai/orca
Length of output: 9904
🏁 Script executed:
#!/bin/bash
printf '%s\n' '--- FloatingTerminalPanelSurface focused section ---'
sed -n '130,205p' src/renderer/src/components/floating-terminal/FloatingTerminalPanelSurface.tsx
printf '%s\n' '--- shortcut close paths ---'
sed -n '1,80p' src/renderer/src/components/floating-terminal/use-floating-terminal-panel-shortcuts.ts
sed -n '130,165p' src/renderer/src/components/floating-terminal/use-floating-terminal-panel-shortcuts.ts
printf '%s\n' '--- action declarations/usages ---'
rg -n -C 4 "closeFile\s*[:=]|closeUnifiedTab\s*[:=]|closeUnifiedTab\(|closeFile\(" src/renderer/src/store src/renderer/src/components src/renderer/src/lib
printf '%s\n' '--- agent-session tab model/type references ---'
rg -n -C 3 "agent-session|agentSessionItems" src/renderer/src/components/floating-terminal src/renderer/src/storeRepository: stablyai/orca
Length of output: 42377
🏁 Script executed:
#!/bin/bash
printf '%s\n' '--- closeFile implementation ---'
fd -i 'close-file-action.ts' src/renderer/src
close_file="$(fd -i 'close-file-action.ts' src/renderer/src | head -n 1)"
if [ -n "$close_file" ]; then
cat -n "$close_file"
fi
printf '%s\n' '--- closeUnifiedTab implementation ---'
cat -n src/renderer/src/store/slices/tabs/tabs-close-actions.ts | sed -n '1,125p'Repository: stablyai/orca
Length of output: 18725
Add the agent-session branch to closeFloatingItemConfirmed.
The tab-strip close action uses closeFloatingItemConfirmed. Its fallback calls closeFile(item.entityId), which does not close an agent-session unified tab. The tab remains open.
The alternate close actions use closeFloatingItems, so their agent-session handling remains correct. The panel close shortcut also uses the confirmed path.
🐛 Suggested fix in the confirmed-close chain
if (item.contentType === 'browser') {
destroyWorkspaceWebviews(latest.browserPagesByWorkspace, item.entityId)
closeBrowserTab(item.entityId)
- } else if (item.contentType === 'simulator') {
+ } else if (item.contentType === 'simulator' || item.contentType === 'agent-session') {
closeUnifiedTab(item.id)
} else {
Floating panel visual proofBoth builds launched hidden (
Pixel compare is of the floating panel. The tab strip (top 36px, 72px on the browser shot) matched on every paired shot. The pane body did not, except the empty state. 1. Empty state — identicalPanel pixels: 0 of 515,200 differed. Before:
After:
The full window differed by 169 pixels, all in the status-bar memory readout (919.9 MB vs 901.2 MB). 2. One terminal — body does not matchBefore shows the shell prompt ( After shows a black body and no prompt. The shared pane’s Before:
After:
3. Several tabs — tab strip matches, body does notThree terminals (Terminal 1–3). Tab strip pixels match. The body is the same failure as scenario 2 (~93% of panel pixels differ). Before:
After:
4. Maximized — same bounds, body does not matchBoth panels maximized to the same rect. Tab strip matches. Before still shows the prompt. After stays black, and the pane measures 0×0. Before:
After:
5. Markdown editor — chrome tab matches, editor is clippedBoth opened Before:
After:
6. Browser — toolbar matches, empty state does notBoth show the New Tab, Before:
After:
7. Move Tab to Split → Right — both panes do not renderRight-clicked Terminal 2, then Move Tab to Split → Right. The store on both sides ended with two groups (the moved terminal alone, and the other four tabs). On main, Terminal 2 disappears from the strip. The original group stays (Terminal 1, Terminal 3, untitled.md, New Tab) in a single pane. On this branch the strip shows only Terminal 2. The original group’s tabs are not on screen, and there is no second pane. Under the tab is a crushed browser toolbar ( Before:
After (includes the later Codex Chat tab):
8. Structured chat — session tab created, surface not visibleFlags set on this branch only: The new tab is Console warning:
That comes from This launch happened after the split, so the panel was already in the broken split layout. The 0-height pane was already true for the terminal and the editor before the split.
9. Floating launch does not move the main window’s active tabBefore the titlebar click, the main window’s active tab was ConsoleNo Not claimedThe floating panel is not identical to main once a tab is open. Only the empty state matched pixel for pixel. |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ No critical issues — a couple of things worth a look before merge.
Reviewed changes
- Host resolution for the floating workspace:
resolveRuntimeFileTargetgains a floating branch that mints a synthetic worktree from the configured directory,floatingWorkspaceToWorktree/isFloatingWorkspaceSelectorare added, and the'floating-workspace'structured blocker is deleted so floating routes by capability. - Shared surface in the floating panel: the panel body now renders the shared group tree, the extracted
WorkspacePaneOverlayLayers, and oneWorkspaceTabDragLayer; the private one-group projection, floating browser slot, floating drag context and pane-handle registry are deleted. - Workspace-scoped activation:
setActiveFile/setActiveBrowserTabtake an optional owning-workspace id so floating activation never moves global selection (setActiveTabTypealready had one). - Synthetic renderer row: a new
floatingWorkspacePathstore field plus a one-slot cache letsgetKnownWorktreeByIdanswer for the floating sentinel, local-only.
ℹ️ The floating session's TUI resume still resolves $HOME, not the configured directory
The new branch makes a structured session's location the configured floating directory, but the structured-to-TUI handoff resolves the same workspace through the terminal scope, which still hardcodes homedir() for the floating sentinel — and then trusts that path for the agent. So a resumed floating session runs its agent in $HOME while its record says the configured directory. Before this PR floating could not hold a structured session, so this path was unreachable for it; it is now reachable.
Technical details
# Floating session location and resumed TUI cwd disagree
## Affected sites
- `src/main/runtime/orca-runtime-persist-headless-terminal-title.ts:204-212` — new floating branch resolves the configured directory for `resolveRuntimeFileTarget`.
- `src/main/runtime/orca-runtime-resolve-browser-network-execution-host-for-worktree.ts:117-131` — unchanged `resolveTerminalWorkspaceLaunchTarget` floating arm returns `path: homedir()`.
- `src/main/runtime/orca-runtime-get-agent-session-execution-namespace.ts:108` — `ensureAgentSession` resolves the workspace through `resolveTerminalWorkspaceLaunchScope(request.worktree)`, then `createTerminal(\`id:${workspace.id}\`, …)` with no `cwd`, so it falls back to that scope path.
- `src/main/runtime/orca-runtime-get-agent-session-execution-namespace.ts:169` — `markWorkspaceTrustedForAgent(agent, connectionId, workspace.path)` trusts the resolved (`$HOME`) path.
## Required outcome
- A floating structured session's resumed TUI should start in the same directory the session is filed under, and only that directory should be marked trusted for the agent.
- Either give `resolveTerminalWorkspaceLaunchTarget`'s floating arm the same configured-directory resolution as the new file-target branch, or route the handoff through the file-target resolver.
## Open questions for the human
- Is the terminal-scope `homedir()` arm deliberately left alone as a follow-up, or is the intended fix to unify both resolvers on one floating-directory source?ℹ️ Nitpicks
src/shared/workspace-launch-kind.tsdoc still says the floating terminal "can host a PTY and nothing else";src/main/agent-launch/agent-launch-mode.tsstill carries the "The floating workspace has nowhere to keep a session" comment aboveresolveStructuredNativeChatSupport. Both are now false.workspaceKindis still accepted byresolveStructuredNativeChatSupport(src/shared/structured-native-chat-launch-route.ts:48) and passed by callers, but the function no longer reads it — the field is now dead input.- Deleting
FloatingBrowserSlot.test.tsxremoved the only assertions that a floating browser tab registers its overlay slot viewport. The sharedBrowserPaneOverlayLayerowns that now, but nothing pins it for the floating worktree.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
…so panes own their height The tree's nodes size themselves as flex items, and the floating shell mounted them in a block container, so every group body — and every pane anchored to one via anchor-size() — measured 0px tall. Wrap the tree and overlay layers in the same 'absolute inset-0 flex' frame the worktree split surface uses, record the host contract on the tree, pin the frame in the panel's element-tree test, and add a real-renderer e2e spec that measures group-body and pane boxes for the single-group and split layouts (jsdom computes no layout, which is how the 0px regression passed 50k tests).
…ate path calls floatingWorkspaceToResolvedWorktree called floatingWorkspaceToWorktree without importing it. The module is @ts-nocheck (mechanically split runtime), so tsc never saw the missing import and it only failed at call time — the structured createSupport probe for the floating workspace threw a ReferenceError. Add the import and a test that runs the real method, which fails on the missing import where the typechecker cannot.
…ebook navigation navigateBrowserPageToUrl looked the workspace up in allWorktrees(), which has no row for folder or floating workspaces, so their notebook navigations ran with an undefined workspace path. Use getKnownWorktreeById, the same resolver every other consumer was moved to.
Rendered proof after the fixes (head
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/e2e/floating-workspace-pane-height.spec.ts (1)
134-141: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover both split-pane overlays.
After the split, the test checks only group-body rectangles. It does not check that both
[data-emulator-pane]overlays are visible and sized. A regression that mounts an overlay only for the active group can pass these assertions while the other split is blank. Assert two visible emulator panes and check both bounding boxes.Proposed test addition
for (const body of await splitBodies.all()) { const box = await body.boundingBox() expect(box, 'each split group body must have a box').not.toBeNull() expect(box!.height, 'split group body height').toBeGreaterThan(MIN_PANE_EDGE_PX) expect(box!.width, 'split group body width').toBeGreaterThan(MIN_PANE_EDGE_PX) } + + const splitPanes = openPanel.locator('[data-emulator-pane]:visible') + await expect(splitPanes).toHaveCount(2) + for (const pane of await splitPanes.all()) { + const box = await pane.boundingBox() + expect(box, 'each split emulator pane must have a box').not.toBeNull() + expect(box!.height, 'split emulator pane height').toBeGreaterThan(MIN_PANE_EDGE_PX) + expect(box!.width, 'split emulator pane width').toBeGreaterThan(MIN_PANE_EDGE_PX) + }
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 47b5f2b3-ca78-425a-b62a-dc57ced50c84
📒 Files selected for processing (7)
src/main/runtime/orca-runtime-resolve-browser-network-execution-host-for-worktree.tssrc/main/runtime/rpc/methods/agent-launch-floating-workspace.test.tssrc/renderer/src/components/browser-pane/navigate/navigate-browser-page-url.tssrc/renderer/src/components/floating-terminal/FloatingTerminalPanel.tab-lifecycle.test.tsxsrc/renderer/src/components/floating-terminal/FloatingTerminalPanelSurface.tsxsrc/renderer/src/components/tab-group/TabGroupSplitNodeTree.tsxtests/e2e/floating-workspace-pane-height.spec.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/runtime/orca-runtime-resolve-browser-network-execution-host-for-worktree.ts
- src/renderer/src/components/tab-group/TabGroupSplitNodeTree.tsx
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.
There was a problem hiding this comment.
ℹ️ No new issues in the incremental delta — the three fix commits land cleanly.
Reviewed changes
- Panes no longer collapse to 0px: the floating panel's body now mounts
TabGroupSplitNodeTreeandWorkspacePaneOverlayLayersinside the sameabsolute inset-0 flexsurface frame the main worktree surface uses, with a unit pin on the class contract and a new real-renderer e2e spec that measures actual bounding boxes for single/split groups and the anchored pane. - Missing import restored in the
@ts-nocheckresolver mixin:floatingWorkspaceToWorktreeis imported again, and a new test executes the realfloatingWorkspaceToResolvedWorktree, which is the only kind of check that can catch a missing import in that file. - Notebook navigation resolves floating and folder workspaces:
navigate-browser-page-url.tsnow usesgetKnownWorktreeByIdinstead ofallWorktrees(), so those workspaces get the correct path instead ofundefined. - Cleanup: the orphaned
FloatingBrowserSlot.test.tsxis deleted and a straypnpm-lock.yamlrewrite is reverted.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
…hared workspace surface
…ost, and keep the controls' inset
There was a problem hiding this comment.
ℹ️ No critical issues — two small suggestions inline.
Reviewed changes
- Let the shared terminal create path own floating-tab activation:
createFloatingTerminalTabno longer passesactivate: falseor callsactivateTab, and the floating create tests now assertactivateTabis not called. - Re-check focus reclaim when the intent arms: the focus-reclaim hook subscribes to the reclaim intent via
useSyncExternalStoreand gates consumption on the armed snapshot, so a close that lands after the empty panel rendered can still reclaim the keyboard. - Keep the split surface mounted with an empty focused group: the surface model now returns empty only when no unified tab has a backing entity,
hasVisibleFloatingTabsderives from the surface, andactiveClosableTabchecks the visible tab order. - Added surface-model, empty-state, and focus-reclaim test cases covering those paths.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
| const terminalIds = new Set( | ||
| (state.tabsByWorktree[FLOATING_TERMINAL_WORKTREE_ID] ?? []).map((tab) => tab.id) | ||
| ) | ||
| const browserIds = new Set( | ||
| (state.browserTabsByWorktree[FLOATING_TERMINAL_WORKTREE_ID] ?? []).map((tab) => tab.id) | ||
| ) | ||
| const fileIds = new Set(state.openFiles.map((file) => file.id)) | ||
| const hasVisibleTab = tabs.some((tab) => { | ||
| if (tab.contentType === 'terminal') { | ||
| return terminalIds.has(tab.entityId) | ||
| } | ||
| if (tab.contentType === 'browser') { | ||
| return browserIds.has(tab.entityId) | ||
| } | ||
| if (tab.contentType === 'agent-session' || tab.contentType === 'simulator') { | ||
| return true | ||
| } | ||
| return fileIds.has(tab.entityId) | ||
| }) |
There was a problem hiding this comment.
hasVisibleTab restates the per-tab visibility rule already in selectFloatingVisibleTabCount (src/renderer/src/store/floating-workspace-panel-selector.ts:40), and the two now diverge: the selector counts an editor file only when file.worktreeId === FLOATING_TERMINAL_WORKTREE_ID, while fileIds here spans every worktree. That selector also caches its scan, so reusing it (or extracting one shared predicate) keeps the surface and empty-state decisions from drifting apart.
| expect(panelElement.focus).toHaveBeenCalledWith({ preventScroll: true }) | ||
| }) | ||
|
|
||
| it('reclaims focus when intent arms after the empty panel has rendered', async () => { |
There was a problem hiding this comment.
This test does not pin the subscription the fix relies on: the harness mocks useSyncExternalStore to just call getSnapshot() and ignores the subscribe callback (floating-terminal-panel-test-module-mocks.ts:17), while runEffects re-runs every registered effect — so the manual renderPanel(true) at line 516 is what makes it pass, and it would still pass if subscribeFloatingPanelReclaimIntent were dropped. Assert the subscription is registered (spy on it) or drive the re-render from the listener so removing the wiring turns this red.
Resolve 71 conflicts against main 8127d94 (1091 commits). Main removed the terminal handoff and made hold a no-op, so the PR's handoff launch-folder plumbing and hold-refusal UI are dropped; the pinned launch folder now rides the /clear founding identity instead of a replacement attach. Unify main's and the PR's revealFloatingWorkspacePanel into one store-backed helper.
…nch directory Port what the merge left on main's newer shapes: - The floating chat's "folder is gone" refusal becomes a typed reason, launchFolderMissing, worded like every other start refusal (copy, desktop words, five translations). Older clients drop the unknown reason and show the code's generic words. - The PR's launch-folder tests run on main's journal-database record store; the attach-level case is a row in the pre-spawn refusal table. - Tests main added since the PR's base learn the PR's close-queue settling, workspace-scoped notebook lookup and tab-stamped chat host. - Move the store's visible-tab-index reads into the tab table, so the record store stays under the line cap with the pin writer in it. - Drop a duplicate Worktree import the merge left in a @ts-nocheck file. Rename the persisted field, the status summary field and the renderer slice from workspacePath to launchDirectory: it records the folder a session first launched in, not its workspace's path, and it has not shipped yet.
The floating panel now closes through the shared unsaved-changes prompt, so its own dialog's six strings had no caller and failed the runtime-catalog check.
Attach now reads params.location to decide whether a new record is a floating founder, so the partial fixture needs the location real attach params always carry.
The cast tripped the changed-code type-assertion gate.
… it was created in The model picker read the floating setting's current folder, so a chat pinned to another folder could show that folder's project default.
|
Problem. With Chat UI as the default view, Codex or Claude from the floating workspace's "+" menu opened the old chat drawn over a terminal, because the floating workspace had its own tab renderer and no folder or saved place a real chat could belong to. What changes for you. The floating panel now uses the same tab groups and panes as a regular workspace. "+" → Codex or Claude opens a real chat there: send, stop, close, and pick up where you left off after a restart. Each floating chat keeps the folder it started in. Changing the floating folder setting later does not move it, and if that folder is deleted the chat says so instead of running somewhere else. Also changes in regular workspaces. "Close All Editor Tabs" from a tab's menu now keeps pinned tabs, like the shortcut and Close Others. Fixed during review.
Deferred (P3, no current user impact). A client that sends its own location when creating a floating chat gets a raw error instead of a typed refusal if the folder can't be resolved. No current client does this. Verified.
Not verified.
|
…-unify-impl # Conflicts: # src/main/claude/claude-structured-launch-resolution.ts
…e the resolver takes
…agent-tree-job-object Main (#22302) gave the Codex launch resolver a launch-directory pin, changed its test store fakes and added a test passing the creation-time seam this branch removed. Kept main's store shape and new test without that seam, and kept this branch's Windows test that resolves a launch without creation times.
…t-launch-instant-tab Conflicts resolved against #24934's rebuild (82544cb), which this branch was built on, so each keeps main's squashed #24934 plus this branch's change. agent-launch-floating-workspace.test.ts takes main's #22302 routing (a structured preference opens a Codex chat) with this branch's host-derived viewMode on the terminal branch. The record-store slot test gets the resolveLaunchArgs dep #25721 made required, the same line as main's fix.
ELI5
With Chat UI as the default view, picking Codex or Claude from the "+" menu in the floating workspace opened the old chat drawn over a terminal, while the same "+" in a normal workspace opened the real chat.
The floating workspace had nowhere a real chat could live: no folder Orca could start the agent in and no place to keep the chat between restarts. This PR gives it one, so both "+" menus now open the same chat.
What Changed
Before: in the floating panel, "+" → Codex or Claude opened a terminal tab running the agent's command-line tool, shown as chat over the terminal. The panel also showed one tab group at a time, so moving a tab into a split could make it disappear.
After: in the floating panel, "+" → Codex or Claude opens the same chat a normal workspace gets: start, send, stop, close, and pick up where it left off after Orca restarts. The panel shows split groups side by side, using the same tab groups as a normal workspace.
Where a floating chat runs: the floating workspace's folder is a setting the user can change. A chat starts in the folder set at launch and records it on its saved session. After a restart it resumes in that same folder, even if the setting has changed since.
If that folder is gone: the chat says "The folder this chat ran in no longer exists. Restore it to continue this chat." It does not quietly start somewhere else.
Mechanism: the host now resolves the floating workspace like a folder workspace whose path is the floating-folder setting. The rule that sent every floating launch to a terminal is removed, so the "+" menu and
agent.launch(CLI, phone) take the same route as everywhere else.Mechanism, saved state: a floating chat's saved session gains an optional
launchDirectory. The host resolves the floating folder when it creates the chat and writes it in the same step that creates the saved session, so the folder can't be lost to a separate write failing. Worktree and folder chats don't store it; their workspace already names a fixed folder. A/clearcontinues in the same folder. Floating chats saved before this field existed get it filled in on their next launch.Mechanism, renderer: the floating panel renders through the shared tab-group surface instead of its own single-group renderer. Tab selection belongs to the workspace that owns the tab, so the panel never moves the main window's selection.
Also changes in regular workspaces
This comes from regular workspaces and the floating panel now closing tabs through one shared path:
Scope note: this is broader than the original launcher PR (#21390). It also replaces the floating panel's separate tab renderer with the shared one (the reason splits and chat panes work there) and deletes the old floating-only tab code. It is about 230 files against current main, many of them tests.
Why
The terminal fallback came from a rule that said the floating workspace "can host a terminal and nothing else", because it had no folder or saved place a chat could belong to. Giving it a real launch scope removes the reason for the rule, rather than adding a special case beside it.
Keeping the launch folder on the session answers "where did this chat run?" after the floating setting moves. Re-reading the setting on each resume would have silently moved a chat to a different folder.
Rendering the panel through the shared tab surface removes a second tab renderer that had drifted (no splits, no chat panes) instead of teaching it each missing feature.
Differences from the common pattern:
Linked Issue
Follow-up to merged #21390. No separate issue.
Visual Proof
Hidden dev builds of main (before) and this branch (after), isolated profile and home, stand-in Codex and Claude. Floating panel, "+" → Codex, then "+" → Claude:
On main both open a "Terminal" tab with chat drawn over it; on this branch they open "Codex Chat" and "Claude Chat" and reply.
Stop on a running Codex turn, and Claude resumed in the same conversation after the app relaunched:
A floating chat keeps its folder: after the floating folder setting moved to another folder and Orca restarted, the chat resumed the same conversation in its original folder (a new chat started in the new folder). If that original folder is deleted, the chat refuses instead of running elsewhere:
Split groups side by side in the floating panel (a terminal and the Codex chat), and a normal workspace's "+" → Codex unchanged:
Testing
Merged current main (1,091 commits); every conflict was resolved by hand, keeping main's changes and this PR's intent.
Typecheck (node, web, cli) passes; oxlint and the changed-code quality gate (including React Doctor) report zero findings.
1,472 named test files (every changed test, plus tests that import a changed module): all pass except tests that timed out while the machine was overloaded. Those pass when rerun on their own, apart from the cross-version suites, which could not unpack old releases within their time limit locally; CI runs them.
New and ported tests cover: the floating workspace resolving to its folder, floating
agent.launchtaking the structured route, the launch folder being pinned once and reused on resume, the missing-folder refusal and its wording,/clearcarrying the folder, the status feed publishing it, and the renderer using it only for local chats.Live macOS QA, main vs this branch, in hidden dev builds with an isolated home and profile and stand-in Codex and Claude (never the real tools):
After the review fixes, two more QA passes on the final behavior:
What QA did not show: Orca's own in-app restart can't run in a dev build (it stops the dev page server), so resume was checked by quitting and relaunching. Real Codex and Claude accounts were not used.
The real
~/.codexand~/.claudesettings files were hashed before, during and after QA and did not change.SSH: not exercised. No container runtime was running on this laptop or on the remote test Mac, and Docker Desktop was not started.
Linux, Windows and mobile were not exercised by hand. The floating workspace is local-only. On Windows, a floating folder on a WSL path gets "could not start" rather than a terminal, which is what a folder workspace on a WSL path does on main today, so it is not a regression.
I manually tested these changes locally
Automated tests added/updated, or explained why not below
Review
Two review rounds ran on this branch; what they found, what was fixed and what was deferred is in the PR comment below.
Agent skill upstream boundary
Notes
launchDirectoryon a saved session and ignore it. Older clients ignore it on the status feed, and show the generic "could not start" wording for the new missing-folder reason. No new message type is sent between client and host.Author X handle: @BrennanKB5
Checklist