Repository navigation
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📥 CommitsReviewing files that changed from the base of the PR and between bcf31c81e3095a5ea39e7dbb6ac01807390fad51 and c0888d4a237fdf094ceefeab0fe5b014245f423f. 📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughLegacy editor file routing now clears active worktree and workspace execution host selections before resolving worktree routes. It preserves the active runtime environment ID. Newly created editor tabs use the execution host from a matching open file, with operation provenance taking precedence over SSH target, runtime environment, and local host. Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to The reviewed changes are mergeable based on the available evidence. Architecture SummaryArchitecture risk: 🔵 Low · up to The change affects 1 system. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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: 556fc499-af29-4706-8765-a4929096f815
📥 Commits
Reviewing files that changed from the base of the PR and between 9f5a8a5 and 07a46dd927450c0783d507c1aea75131c6e4d0e8.
📒 Files selected for processing (4)
src/renderer/src/lib/editor-file-operation-owner.test.tssrc/renderer/src/lib/editor-file-operation-owner.tssrc/renderer/src/store/slices/editor-tab-placement.test.tssrc/renderer/src/store/slices/editor/tabs/workspace-editor-item.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.
| activeWorktreeId: null, | ||
| activeWorkspaceExecutionHostId: null, | ||
| settings: { ...state.settings, activeRuntimeEnvironmentId: null } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
set -euo pipefail
route=src/renderer/src/lib/worktree-operation-route.ts
owner=src/renderer/src/lib/editor-file-operation-owner.ts
ast-grep outline "$route" --match 'resolveWorktreeOperationRouteResult|resolveExplicitWorktreeOperationRouteResult' --view expanded
rg -n -C 10 'resolveWorktreeOperationRouteResult|resolveExplicitWorktreeOperationRouteResult|activeWorktreeId|activeWorkspaceExecutionHostId|activeRuntimeEnvironmentId|parseWorkspaceKey' "$route"
sed -n '83,109p;118,139p' "$owner"Repository: stablyai/orca
Length of output: 10657
🏁 Script executed:
#!/bin/bash
set -euo pipefail
route=src/renderer/src/lib/worktree-operation-route.ts
folder=src/renderer/src/lib/folder-workspace-runtime-owner.ts
printf '%s\n' '--- worktree route folder section ---'
sed -n '126,184p;203,276p' "$route"
printf '%s\n' '--- folder owner outline ---'
ast-grep outline "$folder" --view expanded
printf '%s\n' '--- folder owner implementation ---'
rg -n -C 12 'findFolderWorkspaceOwner|getExecutionHostIdForFolderWorkspace|FolderWorkspaceRuntimeOwnerState|activeWorktreeId|activeWorkspaceExecutionHostId|activeRuntimeEnvironmentId' "$folder"Repository: stablyai/orca
Length of output: 12887
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- folder owner remainder ---'
sed -n '137,210p' src/renderer/src/lib/folder-workspace-runtime-owner.ts
printf '%s\n' '--- folder route definition and imports ---'
rg -n -C 18 'resolveFolderWorkspaceOperationRoute|FolderWorkspaceRuntimeOwnerState|getExecutionHostIdForFolderWorkspace' src/renderer/src/lib/worktree-operation-route.tsRepository: stablyai/orca
Length of output: 8100
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- indexed folder owner selection ---'
rg -n -C 18 'findIndexedFolderWorkspaceOwner|findIndexedProjectGroupOwner' src/renderer/src/lib/worktree-runtime-owner-index.ts
printf '%s\n' '--- editor owner route comparison ---'
rg -n -C 20 'assertWorktreeOperationGenerationSnapshotCurrent|OWNER_CHANGED_MESSAGE|resolveCurrentEditorRoute|provenance.*route|generation.*route' src/renderer/src/lib/editor-file-operation-owner.tsRepository: stablyai/orca
Length of output: 11507
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 20 'captureWorktreeOperationGenerationSnapshot|assertWorktreeOperationGenerationSnapshotCurrent|WorktreeOperationGenerationSnapshot' src/renderer/src/lib/worktree-operation-generation.tsRepository: stablyai/orca
Length of output: 4528
Use owner-independent folder routing during validation.
Capture clears the focused host before resolving a legacy folder route. Validation passes the original focused host to resolveWorktreeOperationRoute. When multiple folder owners exist, this can select a different route after focus changes and trigger OWNER_CHANGED_MESSAGE, even though the file owner did not change.
Reuse the capture helper for the folder branch:
Suggested fix
if (parseWorkspaceKey(worktreeId)?.type === 'folder') {
- return resolveWorktreeOperationRoute(state, worktreeId)
+ return resolveLegacyEditorFileRoute(state, worktreeId)
}![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
resolveLegacyEditorFileRoute clears settings.activeRuntimeEnvironmentId, which is the only owner evidence a legacy pre-owner-projection remote runtime workspace has. Four existing tests now fail on this branch, so pnpm test is red as-is.
Reviewed changes
src/renderer/src/lib/editor-file-operation-owner.ts— legacy editor-file routing now resolves with UI focus (activeWorktreeId,activeWorkspaceExecutionHostId) and the global runtime selection (settings.activeRuntimeEnvironmentId) cleared.src/renderer/src/store/slices/editor/tabs/workspace-editor-item.ts— editable (contentType: 'editor') tabs are stamped withexecutionHostIdfrom the file's captured owner route instead of the active workspace host; the generic fallback stays for diff/review/check-details tabs.src/renderer/src/lib/editor-file-operation-owner.test.ts— new case pinning a legacy local worktree tolocalwhile another host is focused.src/renderer/src/store/slices/editor-tab-placement.test.ts— new case pinning the editor tab'sexecutionHostIdto the file owner.
resolveEditorItemExecutionHostId re-derives the same owner precedence that already exists in isOpenFileOwnedByWorktree (src/renderer/src/lib/unified-tab-host-ownership.ts:68). A shared getOpenFileExecutionHostId(file) helper would keep the tab stamp and the ownership predicates from drifting apart; it is a follow-up, not a blocker.
The regression is detailed inline. The fix direction I would suggest: clear only the UI-focus fields and keep the global runtime fallback, then re-add/adjust the new test to cover the local case without the runtime branch — or, if dropping the fallback is deliberate, update the four untitled-cleanup tests and justify the changed remote behavior.
ℹ️ Nitpicks
resolveEditorItemExecutionHostIdduplicates theoperationProvenance→externalSshTargetId→runtimeEnvironmentId→localprecedence inisOpenFileOwnedByWorktree(src/renderer/src/lib/unified-tab-host-ownership.ts:68); extract one shared id resolver.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
Important
The new commit fixes the four editor-close-file-cleanup failures but breaks the new editor-tab-placement test: with the global runtime fallback now retained, the unstamped worktree routes to runtime:hub-b, so pnpm test is still red.
Reviewed changes
Incremental re-review of the commit pushed after the prior pullfrog review (07a46dd).
- Retained the global runtime owner fallback —
resolveLegacyEditorFileRoutenow clears onlyactiveWorktreeId/activeWorkspaceExecutionHostIdand leavessettings.activeRuntimeEnvironmentIdintact, which restores the foureditor-close-file-cleanuptests (verified: 18/18 pass). - Pinned the legacy-owner test to a null global runtime — the
editor-file-operation-ownercase now setssettings.activeRuntimeEnvironmentId: null, so it exercises the focus-only path rather than the retained runtime branch.
The regression introduced by this commit is detailed inline.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Incremental re-review of the commit pushed after the prior pullfrog review (bcf31c8).
- Aligned the
editor-tab-placementfixture with the retained runtime fallback — the new ownership case now setssettings.activeRuntimeEnvironmentId: null, so its unstamped worktree resolves through the focus-only path and assertslocal, matching the retained global-runtime semantics theeditor-close-file-cleanupsuite relies on. Verified locally:editor-file-operation-owner.test.ts,editor-tab-placement.test.ts, andeditor-close-file-cleanup.test.tsall pass (35/35).
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
c0888d4 to
39a1ba6
Compare
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Incremental re-review of the commits since the prior pullfrog review (c0888d4). The branch was rebased onto a newer main, and the production code is unchanged from the approved revision. The only delta is test-only:
- Refactored both new ownership tests to use typed fixtures — the
as neverrepo/worktree/settings casts ineditor-file-operation-owner.test.tsandeditor-tab-placement.test.tsare replaced with the realmakeWorktree/TEST_REPOhelpers and a spread of the live settings object, so the fixtures now match the store shape instead of fabricating a partial one. - Verified the touched suites on the new head —
editor-file-operation-owner.test.ts,editor-tab-placement.test.ts, andeditor-close-file-cleanup.test.tsall pass locally (36/36). The production route resolver still nulls only the UI-focus fields, and the tests still fail if that behavior regresses.
deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏
2 participants
ELI5
Opening a file while a different machine is selected should not make that file belong to the selected machine. This keeps the file and its editor tab attached to the workspace that actually owns it.
What Changed
Before, an unstamped legacy worktree could inherit the active workspace host while opening a file, and the editor tab then inherited that same UI-focused host. Now editor-file ownership ignores active UI focus and global runtime selection when resolving a legacy worktree. New editable tabs use the captured file owner instead of the currently focused host.
Why
This prevents duplicate or incorrectly restored editor state when focus changes between local and remote workspaces. The generic tab host fallback remains unchanged for terminal, diff, and review tabs; only editable files with captured owner provenance override it.
Linked Issue
Relates to #21121
Visual Proof
N/A — this is an ownership-routing invariant with no standalone visual change.
Testing
I manually tested these changes locally
Automated tests added/updated
node node_modules/vitest/vitest.mjs run --config config/vitest.config.ts src/renderer/src/lib/editor-file-operation-owner.test.ts src/renderer/src/store/slices/editor-tab-placement.test.ts --reporter=verbose(17 tests passed)oxlinton changed filesoxfmton changed filestsc --noEmit -p config/tsconfig.tc.web.jsonis blocked by existing missing dependencies/types:remark-cjk-friendly/parseOnly,@xterm/addon-image, andORDERED_LIST_MARKER_PATTERN.Tested on Windows. The route is host-agnostic and explicitly preserves remote SSH ownership already captured on the file.
AI Disclosure
External contributor. Implemented with assistance from GPT-5 Codex; author reviewed the scope and automated coverage.
Review
Agent skill upstream boundary
Notes
The change is limited to editor-file ownership. It does not alter generic tab, terminal, diff, review, mobile, or SSH route selection.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (targeted checks above; full typecheck is blocked by existing dependency/type gaps)