Skip to content

fix(editor): a file from a server no longer reloads once after each restart - #31196

Merged
Jinwoo-H merged 7 commits into
mainfrom
fix/server-editor-file-stable-id
Oct 20, 2026
Merged

Jinwoo-H merged 7 commits into
mainfrom
fix/server-editor-file-stable-id

Conversation

@Jinwoo-H

@Jinwoo-H Jinwoo-H commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 14 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​470 $\color{#cf222e}{\Huge{\mathbf{−}}}$​68 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​402
Prod 9 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​105 $\color{#cf222e}{\Huge{\mathbf{−}}}$​54 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​51

ELI5

When you open a file that lives on a paired server, Orca gives it an internal name. It used to give the file one name when you first opened it and a different name when Orca restarted. Because the name changed, the editor reloaded that file once after every restart. Now the file gets its final name the first time you open it, so a restart has nothing to rename and the file does not reload.

What Changed

Before: a file from a server reloads once after each restart.
After: it keeps its identity across restarts, and there is no reload.

How it works:

  • Every open editor file has an id. A file on this computer is named by its path. A file from a server (one with a runtime environment) is supposed to be named by an "owned" id that includes its workspace and server. Restore already used the owned id for every server file (hydrated-editor-session-state.ts). First open did not: it used the bare path unless another owner already held that path. So at the first restart the id changed, and the editor tab rebuilt itself. Unsaved drafts were carried over, but the editor still reloaded.
  • resolveEditorFileIdForOwner (editor-file-ids.ts) now returns the owned id for any server file the first time it is opened. Files on this computer still use their path. The desktop's live mirror of a server's tabs uses the same naming (mirrored-editor-file-identity.ts). Because a server file is now always given an owned id, the mirror's lookup for a bare-path collision could never matter any more, so it was removed.
  • In builds with the layout switch on, a server's layout frames name the desktop's copy of each server editor tab the same way (server-layout-frame-ids.ts). That way a workspace drawn from frames and the same workspace drawn by the mirror never disagree on a file's id. That code runs only with the switch on. Its reverse direction has no caller yet, and its comment now says that a tab the server named by path comes back under the server's owned id.

Owner decision Q15. Owner-visible row 24: "server file no longer reloads once after restart". This change applies to both builds and is not behind the layout switch.

Sweep: code that might assume a server file's id equals its path

Key point: every id this change produces already appears today after any restart. So the sweep looked for code that only worked during the first session.

Place Verdict
Open, and reuse of an already-open file: open-file-apply.ts:72-89 Safe. It reuses a file matched by path and owner, not by id.
Mirror reuse: mirrored-editor-file-identity.ts:57-67, tab-builders.ts:118-150 Fixed. Changed and tested. A file that is already open keeps its id for the rest of the session.
Restore and the old-id migration map: hydrated-editor-session-state.ts:66-84, hydrated-editor-file-ids.ts:53-63 Safe, unchanged. Tested: an older saved session with path ids still moves its tabs, active file and draft to the owned id.
Drafts and dirty state: workspace-session.ts:137, editorDrafts[id] everywhere Safe. Keyed by id and saved per file record. Tested: a draft survives a restart.
Same path, two workspaces, or local and server: isEditorFileIdOccupiedByOtherOwner (editor-file-ids.ts:105-118) Tested. Ids stay distinct. A local file opened after a server file with the same path now keeps its path id, where before it got an owned id that restore later renamed.
Rename and move remap: remap-open-editor-tabs-for-path-change.ts:241-266 Safe. A server file now simply never claims the bare-path slot. The existing remap tests pass.
Markdown preview source id: markdown-preview-actions.ts:106-114, EditorContent.tsx:216, EditorPanel.tsx:300, editor-panel-render-model.ts:169 Safe. The source id comes from the resolver, and the ?? filePath fallbacks only apply to old records that lack a source id.
Link and preview "open edit tab" lookups: getOpenedEditFileIdAfterOpen (editor-file-ids.ts:160-180), markdown-preview-source-routing.ts:41-64 Safe. They match by path and workspace and fall back to the path only when nothing is found.
Search results, check annotations, review notes, CLI or terminal link open: search-match-open.ts:52-76, check-annotation-open.ts:35-55, use-note-opening.ts:74-90, terminal-file-open-routing.ts Safe. All go through openFile. The line reveal matches by path when no id is given (EditorEditFileSurface.tsx:327).
Explorer highlight: FileExplorerVirtualRows.tsx:236 (activeFileId === n.path) Safe. Auto-reveal selects the row by the file's path (useFileExplorerAutoReveal.ts:70-85).
Phone markdown bridge and phone tab close: mobile-markdown-bridge.ts:177-195, mobile-session-tab-close.ts:14 Safe. Ids come from the tab and go back unchanged. Neither compares an id with a path.
Diff tabs: buildDiffEditorFileId, open-history-diff.ts Safe. They use a separate id scheme that this change does not touch.
Conflict review, selecting a file: open-conflict-review.ts Fixed and tested. It used to look the file up by its path, so in a first session it would open a second copy of an already-open server file. It now works out the workspace's owner the way openFile does and uses the same owner-aware id. If the owner cannot be worked out, it falls back to the old path lookup rather than guessing. A server file it opens itself carries its owner, so a later open reuses it. A local file is unchanged.
Conflict review overview (all conflicted files stacked): EditorConflictReviewSurface.tsx, useEditorPanelActiveTabContentLoad.ts, editor-panel-draft-selector.ts Fixed and tested. All three now show a file under its open tab's id, so a server file's unsaved edits still appear there. Rare case not covered: a stale tab from a different owner holding the same path in the same workspace. That only affects what is displayed.
Shared layout and saved-session readers: workspace-layout-editor-tab-file.ts:10-16, orcad-source-left-behind-session-rows.ts:151, profile-session-markdown-transfer.ts:79 Safe. They accept both the path and the owned id. With the switch off, they read the server's own local files, which keep path ids.
Local chat-session mirror (local-structured-session-tabs-sync) Safe. It applies only chat tabs and never editor files.

Why

The other fix would have been to make restore keep the bare path for a server file. That would bring back two owners colliding on one id: a local file and a server file with the same path, or the same server path in two workspaces. The owned id was introduced to prevent exactly that. Naming the file correctly at the single place ids are minted (resolveEditorFileIdForOwner, used by open, the live mirror and the rename remap) makes the first open match what restore already produces, and adds no new rule.

Linked Issue

Owner decision Q15 (no tracker issue).

Visual Proof

N/A. The visible symptom is a one-time editor reload just after a restart with a paired server. The new tests check the cause directly: the file id and the tab ids are unchanged across a save-and-restore round trip. I did not run the app with a paired server.

Testing

  • I manually tested these changes locally
  • Automated tests added/updated, or explained why not below

New server-editor-file-restart-identity.test.ts:

  1. Opening a server file gives the owned id. After a restart the file id, the tab ids and the active file are unchanged, and nothing that restore renames is named by a saved tab. The same holds for a file the live mirror opens.
  2. A draft survives the restart.
  3. A local file still gets its path, including after a server file with the same path is opened.
  4. The same server path in two workspaces stays distinct, with separate drafts, before and after a restart.
  5. An older saved session with path ids for server files still restores and moves to the owned id, including its tab, active file and draft.

On base, tests 1 to 4 fail and test 5 passes, as intended.

Conflict review (editor-git-status-reconciliation.test.ts): (a) a server file is already open and then picked in conflict review: the existing copy is reused, with no duplicate, and conflict review selects it. (b) conflict review opens a server file first, and a later normal open reuses it. (c) a local file is unchanged. Before the conflict-review fix, (a) and (b) fail. Overview drafts (editor-panel-draft-selector.test.ts): an open server file's unsaved edits show in the overview. This fails before the overview fix.

Server layout frames: a path held by two owners on the server still maps to the one owned id for that path, whichever record is matched. That id depends only on the path, so the code now checks only the path. Existing tests that opened a server file and then used its path as its id now use the id openFile returns, or the owned id the mirror gives. No assertion was dropped. Local runs: 82 directly affected test files pass (911 tests), plus 136 more files that touch server-owned editor files (1509 tests), plus every test file that touches conflicts or the editor panel (172 files, 2148 tests). The CI static gate (ci-static-gate.mjs) passed at this head. No src/main changes.

Review

An independent review found no blocking bugs. It raised three points, all handled: conflict review is now fixed in this PR (see the sweep table), the reverse frame mapping's comment is corrected, and the now-unused collision lookup in the mirror was removed. Each later change also had its own review, and none found blocking bugs.

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

  • SSH and remote: affects only files owned by a paired server. Nothing changes on the wire: ids are desktop-local, and the server never sees the desktop's editor file ids with the switch off.
  • Compatibility: older saved sessions restore exactly as before (test 5).
  • Cross-platform: no path handling changed. The owned id URI-encodes the path as before.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

@coderabbitai

coderabbitai Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

📝 Walkthrough

Runtime-owned editor files now use owner-scoped IDs during identity resolution and mirrored-tab synchronization. Server layout conversion matches editor entities by owned ID or path. Conflict-review flows resolve open server files by ID. Tests cover editor operations, conflict drafts, and identity persistence across restart, including migration from legacy path-based sessions.

Priority: ➖ Normal

Merge Risk: 🟡 Moderate · up to 52686

Fix owner-aware conflict-review lookups and preserve distinct layout identities before merging to avoid showing the wrong content or losing access to one owner’s file state.

Pre-merge checks: Passed 3 · Failed 2

Failed Failed checks: 2 warnings

Check name Status Explanation Resolution
Description check Warning The description is detailed and covers the change, rationale, testing, compatibility, and checklist items. However, the required Linked Issue section does not provide an actual issue link or issue ref… Add a valid linked issue under the Linked Issue section, such as Fixes #<issue-number>. If no issue exists, create or attach the required tracker issue before merging.
Docstring Coverage Warning Docstring coverage is 29.17% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 24 functions across 22 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 3 passed
Check name Status Explanation
Title check Passed The title clearly identifies the primary user-facing fix: server files no longer reload after restarts.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description is detailed and covers the change, rationale, testing, compatibility, and checklist items. However, the required Linked Issue section does not provide an actual issue link or issue reference; it only cites owner decision Q15.

Resolution

Add a valid linked issue under the Linked Issue section, such as Fixes #&lt;issue-number&gt;. If no issue exists, create or attach the required tracker issue before merging.

✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: a7c64a7f-a9a8-41d4-a2b4-89798f08453b
📥 Commits

Reviewing files that changed from the base of the PR and between 9714e79 and d0d9374.

📒 Files selected for processing: 11
  • config/renderer-layout-writer-baseline.txt
  • src/renderer/src/components/editor/EditorConflictReviewSurface.tsx
  • src/renderer/src/components/editor/editor-panel-draft-selector.test.ts
  • src/renderer/src/components/editor/editor-panel-draft-selector.ts
  • src/renderer/src/components/editor/useEditorPanelActiveTabContentLoad.ts
  • src/renderer/src/components/editor/useEditorPanelContentState.ts
  • src/renderer/src/runtime/server-layout-frame-ids.test.ts
  • src/renderer/src/runtime/server-layout-frame-ids.ts
  • src/renderer/src/store/slices/editor-git-status-reconciliation.test.ts
  • src/renderer/src/store/slices/editor/actions/open-conflict-review.ts
  • src/renderer/src/store/slices/editor/file-ids/editor-file-ids.ts
💤 Files with no reviewable changes: 1
  • config/renderer-layout-writer-baseline.txt

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.

Comment on lines 99 to 107
}

const absolutePath = joinPath(activeFile.filePath, entry.path)
if (needsFileRead(absolutePath)) {
void loadFileContent(absolutePath, absolutePath, activeFile.worktreeId, entry.path)
const fileId = conflictOverviewFileId(openFiles, activeFile.worktreeId, absolutePath)
if (needsFileRead(fileId)) {
void loadFileContent(absolutePath, fileId, activeFile.worktreeId, entry.path)
}
}
return

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🔎 Supported by static analysis

🏁 Script executed:

sed -n '1,200p' src/renderer/src/components/editor/useEditorPanelActiveTabContentLoad.ts
sed -n '125,205p' src/renderer/src/components/editor/useEditorPanelContentState.ts

Repository: stablyai/orca

Length of output: 7368


🏁 Script executed:

set -o pipefail
printf '%s\n' '--- conflictOverviewFileId definitions/usages ---'
rg -n -F --glob '*.ts' --glob '*.tsx' -e 'conflictOverviewFileId' -e 'resolveEditorFileIdForOwner' src/ || test "$?" -eq 1
printf '%s\n' '--- openFiles and conflict-review state references ---'
rg -n -F --glob '*.ts' --glob '*.tsx' -e 'setOpenFiles' -e 'openFiles:' -e 'selectedConflictReviewFile' -e "mode: 'conflict-review'" src/renderer/src/components/editor src/renderer/src/store src/renderer/src | head -n 240
printf '%s\n' '--- helper source ---'
helper=$(rg -l -F --glob '*.ts' --glob '*.tsx' 'export function conflictOverviewFileId' src/ | head -n 1)
if [ -n "$helper" ]; then
  sed -n '1,240p' "$helper"
fi
printf '%s\n' '--- owner hook context ---'
sed -n '1,180p' src/renderer/src/components/editor/useEditorPanelContentState.ts
printf '%s\n' '--- relevant tests ---'
rg -n -F --glob '*.{test,spec}.ts' --glob '*.{test,spec}.tsx' -e 'conflictOverviewFileId' -e 'conflict-review' -e 'openFiles' src/renderer/src | head -n 240

Repository: stablyai/orca

Length of output: 42134


🏁 Script executed:

set -o pipefail
printf '%s\n' '--- editor-file-ids ---'
nl -ba src/renderer/src/store/slices/editor/file-ids/editor-file-ids.ts | sed -n '100,215p'
printf '%s\n' '--- open conflict review action ---'
nl -ba src/renderer/src/store/slices/editor/actions/open-conflict-review.ts | sed -n '1,180p'
printf '%s\n' '--- open conflict file action ---'
file=$(rg -l -F --glob '*.ts' 'resolveEditorFileIdForOwner(' src/renderer/src/store/slices/editor/actions | head -n 1)
printf 'candidate=%s\n' "$file"
if [ -n "$file" ]; then nl -ba "$file" | sed -n '1,150p'; fi
printf '%s\n' '--- content state derivation ---'
nl -ba src/renderer/src/components/editor/useEditorPanelContentState.ts | sed -n '1,210p'
printf '%s\n' '--- conflict surface lookup and callbacks ---'
nl -ba src/renderer/src/components/editor/EditorConflictReviewSurface.tsx | sed -n '1,125p'
nl -ba src/renderer/src/components/editor/EditorConflictReviewSurface.tsx | sed -n '230,280p'

Repository: stablyai/orca

Length of output: 35999


🏁 Script executed:

set -o pipefail
printf '%s\n' '--- open-file-apply ---'
nl -ba src/renderer/src/store/slices/editor/actions/open-file-apply.ts | sed -n '1,180p'
printf '%s\n' '--- workspace editor item ---'
nl -ba src/renderer/src/store/slices/editor/tabs/workspace-editor-item.ts | sed -n '1,240p'
printf '%s\n' '--- callers of openWorkspaceEditorItem and openFile actions ---'
rg -n -F --glob '*.ts' --glob '*.tsx' -e 'openWorkspaceEditorItem(' -e 'openFile(' -e 'openFileApply' src/renderer/src | head -n 260
printf '%s\n' '--- file content loader ---'
nl -ba src/renderer/src/components/editor/useEditorPanelFileContentLoader.ts | sed -n '1,260p'

Repository: stablyai/orca

Length of output: 43057


🏁 Script executed:

set -o pipefail
printf '%s\n' '--- non-activating production open calls ---'
rg -n -F --glob '*.ts' --glob '*.tsx' -g '!*.test.*' -g '!*.spec.*' -e "selection: 'none'" -e "selection: 'background'" -e 'openFile({' -e 'openFile(' src/renderer/src | head -n 320
printf '%s\n' '--- openFile action declaration and wrapper ---'
rg -n -F --glob '*.ts' -e 'openFile:' -e 'createOpenFile' -e 'applyOpenFileToState' src/renderer/src/store/slices/editor
printf '%s\n' '--- editor synchronization mutations ---'
rg -n -F --glob '*.ts' --glob '*.tsx' -g '!*.test.*' -g '!*.spec.*' -e 'openFiles:' -e 'set((s)' -e 'setState({' src/renderer/src/runtime src/renderer/src/store | grep -E 'openFiles|migrat|mirror|sync|batch' | head -n 320
printf '%s\n' '--- migration implementation ---'
nl -ba src/renderer/src/components/editor/migrate-restored-editor-file-owner.ts | sed -n '1,240p'

Repository: stablyai/orca

Length of output: 18046


🏁 Script executed:

set -o pipefail
printf '%s\n' '--- open-file action ---'
nl -ba src/renderer/src/store/slices/editor/actions/open-file-action.ts | sed -n '1,125p'
printf '%s\n' '--- OpenFileOptions and selection call sites ---'
nl -ba src/renderer/src/store/slices/editor/types/editor-files-slice.ts | sed -n '1,90p'
rg -n -C 4 --glob '*.ts' --glob '*.tsx' -g '!*.test.*' -g '!*.spec.*' -e 'selection:' -e 'openFile\(' src/renderer/src/components src/renderer/src/hooks src/renderer/src/runtime src/renderer/src/lib | head -n 360
printf '%s\n' '--- sync final patch ---'
nl -ba src/renderer/src/runtime/web-session-tabs-sync/apply-final-patch.ts | sed -n '1,180p'
printf '%s\n' '--- reparent implementation ---'
rg -n -F --glob '*.ts' -e 'reparentRestoredEditorFileOwner' src/renderer/src

Repository: stablyai/orca

Length of output: 41948


🏁 Script executed:

set -o pipefail
printf '%s\n' '--- restored owner reparent ---'
nl -ba src/renderer/src/store/slices/editor/actions/restored-editor-owner.ts | sed -n '1,240p'
printf '%s\n' '--- nextOpenFiles producers ---'
rg -n -C 5 --glob '*.ts' -g '!*.test.*' -e 'nextOpenFiles' src/renderer/src/runtime src/renderer/src/store | head -n 360
printf '%s\n' '--- sync tab builders ---'
nl -ba src/renderer/src/runtime/web-session-tabs-sync/tab-builders.ts | sed -n '1,180p'

Repository: stablyai/orca

Length of output: 30132


🏁 Script executed:

set -o pipefail
printf '%s\n' '--- mirrored editor identity ---'
nl -ba src/renderer/src/runtime/web-session-tabs-sync/mirrored-editor-file-identity.ts | sed -n '1,260p'
printf '%s\n' '--- active-state reconciliation ---'
rg -n -C 6 --glob '*.ts' -g '!*.test.*' -e 'function applyActiveStateUpdates' -e 'nextActiveFileId' -e 'activeFileIdByWorktree' src/renderer/src/runtime/web-session-tabs-sync | head -n 320

Repository: stablyai/orca

Length of output: 19590


Rerun the conflict overview load when openFiles changes.

The overview effect uses openFiles to select the content ID, but openFiles is not a dependency. If synchronization adds the matching server-owned edit tab while the review tab remains active, the overview switches from the path ID to the owned ID without loading content for that ID. The overview can remain stuck in its loading state.

Suggested fix
     isVisible,
-    gitStatusEntries
+    gitStatusEntries,
+    openFiles
   ])

Comment on lines +33 to +46
let conflictFileId = absolutePath
set((s) => {
const conflict = toOpenConflictMetadata(entry)
const existing = s.openFiles.find((f) => f.id === absolutePath)
const runtimeEnvironmentId = conflictFileRuntimeOwner(s, worktreeId)
// Why: a server's file is open under its owned id; looking up the bare path would open a second copy.
const fileId = resolveEditorFileIdForOwner(
s,
absolutePath,
worktreeId,
runtimeEnvironmentId,
['edit']
)
conflictFileId = fileId
const existing = s.openFiles.find((f) => f.id === fileId)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🔎 Supported by static analysis

🏁 Script executed:

sed -n '1,115p' src/renderer/src/store/slices/editor/actions/open-conflict-review.ts
sed -n '105,205p' src/renderer/src/store/slices/editor/file-ids/editor-file-ids.ts
rg -n 'openConflictReviewFile|resolveEditorOperation.*Provenance|editorOperation.*Provenance' src/renderer/src/store/slices src/renderer/src/components/editor | head -100

Repository: stablyai/orca

Length of output: 9149


🏁 Script executed:

set -o pipefail
printf '%s\n' '--- provenance references ---'
rg -n -F -- 'captureEditorFileOperationProvenance' src shared || true
printf '%s\n' '--- provenance helper files ---'
rg -n -F -- 'export function captureEditorFileOperationProvenance' src shared || true
printf '%s\n' '--- conflict surface ---'
sed -n '1,130p' src/renderer/src/components/editor/EditorConflictReviewSurface.tsx
printf '%s\n' '--- conflict action remainder ---'
sed -n '100,260p' src/renderer/src/store/slices/editor/actions/open-conflict-review.ts
printf '%s\n' '--- conflict tests around callers ---'
sed -n '430,610p' src/renderer/src/store/slices/editor-git-status-reconciliation.test.ts

Repository: stablyai/orca

Length of output: 20165


🏁 Script executed:

set -o pipefail
printf '%s\n' '--- provenance helper ---'
sed -n '1,230p' src/renderer/src/lib/editor-file-operation-owner.ts
printf '%s\n' '--- provenance tests ---'
sed -n '1,220p' src/renderer/src/lib/editor-file-operation-owner.test.ts
printf '%s\n' '--- conflict reuse tests remainder ---'
sed -n '560,700p' src/renderer/src/store/slices/editor-git-status-reconciliation.test.ts
printf '%s\n' '--- editor owner tests around route behavior ---'
sed -n '1,210p' src/renderer/src/store/slices/editor-tab-host-ownership.test.ts

Repository: stablyai/orca

Length of output: 25743


🏁 Script executed:

set -o pipefail
printf '%s\n' '--- file-id owner predicates ---'
sed -n '1,125p' src/renderer/src/store/slices/editor/file-ids/editor-file-ids.ts
printf '%s\n' '--- route resolution references ---'
rg -n -F -e 'function resolveExplicitWorktreeOperationRouteResult' -e 'function resolveWorktreeOperationRoute' -e 'kind: '\''ambiguous'\''' src/renderer/src shared || true
printf '%s\n' '--- route implementation candidates ---'
sed -n '1,260p' src/renderer/src/store/slices/editor/file-ids/editor-file-ids.ts
printf '%s\n' '--- focused diff ---'
git diff 997d34b04389665795966ec4c0325b5cbfabf9ee d0d9374a06355a8e2f549105fe96c4ce64e9b56d -- src/renderer/src/store/slices/editor/actions/open-conflict-review.ts src/renderer/src/store/slices/editor/file-ids/editor-file-ids.ts

Repository: stablyai/orca

Length of output: 23099


🏁 Script executed:

set -o pipefail
printf '%s\n' '--- file-id owner predicates ---'
sed -n '1,125p' src/renderer/src/store/slices/editor/file-ids/editor-file-ids.ts
printf '%s\n' '--- route resolution references ---'
rg -n -F -e 'function resolveExplicitWorktreeOperationRouteResult' -e 'function resolveWorktreeOperationRoute' -e "kind: 'ambiguous'" src/renderer/src shared || true
printf '%s\n' '--- focused diff ---'
git diff 997d34b04389665795966ec4c0325b5cbfabf9ee d0d9374a06355a8e2f549105fe96c4ce64e9b56d -- src/renderer/src/store/slices/editor/actions/open-conflict-review.ts src/renderer/src/store/slices/editor/file-ids/editor-file-ids.ts

Repository: stablyai/orca

Length of output: 15653


🏁 Script executed:

set -o pipefail
printf '%s\n' '--- analogous open-file provenance handling ---'
sed -n '1,135p' src/renderer/src/store/slices/editor/actions/open-file-apply.ts
rg -n -F -e 'operationProvenance' -e 'captureEditorFileOperationProvenance' src/renderer/src/store/slices/editor/actions src/renderer/src/store/slices/editor | head -120
printf '%s\n' '--- catalog route implementation ---'
sed -n '1,110p' src/renderer/src/lib/worktree-operation-catalog-route.ts
printf '%s\n' '--- route ambiguity tests ---'
sed -n '50,95p' src/renderer/src/lib/worktree-operation-route.test.ts
sed -n '210,250p' src/renderer/src/lib/worktree-operation-route.test.ts
sed -n '600,640p' src/renderer/src/lib/worktree-operation-route.test.ts
sed -n '690,715p' src/renderer/src/lib/worktree-operation-route.test.ts

Repository: stablyai/orca

Length of output: 23720


Do not treat an unresolved owner as local.

When captureEditorFileOperationProvenance cannot resolve the worktree, conflictFileRuntimeOwner returns null. resolveEditorFileIdForOwner then does not match an existing server-owned file and can return the bare path. The action appends a second OpenFile for that path.

Keep the unresolved state distinct from a valid local owner and stop the conflict-file open until the owner resolves.

Suggested fix
-function conflictFileRuntimeOwner(s: AppState, worktreeId: string): string | null {
+function conflictFileRuntimeOwner(
+  s: AppState,
+  worktreeId: string
+): string | null | undefined {
   try {
     return captureEditorFileOperationProvenance(s, worktreeId, undefined, false).generation.route
       .runtimeEnvironmentId
   } catch {
-    return null
+    return undefined
   }
 }
...
         const conflict = toOpenConflictMetadata(entry)
         const runtimeEnvironmentId = conflictFileRuntimeOwner(s, worktreeId)
+        if (runtimeEnvironmentId === undefined) {
+          openedConflictFile = false
+          return s
+        }
         // Why: a server's file is open under its owned id; looking up the bare path would open a second copy.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to GitHub limitations.

⚠️ Outside diff range comments (1)

🟡 Minor · Make conflictOverviewFileId owner-aware. · editor-file-ids.ts:187-197

src/renderer/src/store/slices/editor/file-ids/editor-file-ids.ts:187-197
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Make conflictOverviewFileId owner-aware.

Before an entry is selected, the conflict-review tab renders its overview. The overview, loader, and draft selector all use conflictOverviewFileId, which currently returns the first edit tab matching only worktree and path, or the raw path. If a local edit tab precedes the server-owned edit tab, the overview can read the local buffer, content, or draft instead of the server-owned file.

Apply the same owner-aware resolution used for selectedFileId inside conflictOverviewFileId, using the review's resolved runtime owner. Keep all overview lookups routed through this shared helper.


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 12029c90-9377-48c9-a8bc-017303e52fed
📥 Commits

Reviewing files that changed from the base of the PR and between d0d9374 and 526869f.

📒 Files selected for processing: 1
  • src/renderer/src/runtime/web-session-tabs-sync-browser-tabs.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.

@Jinwoo-H
Jinwoo-H merged commit 695ac88 into main Oct 20, 2026
79 of 81 checks passed
nwparker added a commit that referenced this pull request Oct 20, 2026
…ping-check tests

After #31196 a server's tab is no longer keyed by its path, so these tests
flagged a tab that does not exist and no read was ever issued.

Co-Authored-By: Claude <noreply@anthropic.com>
nwparker added a commit that referenced this pull request Oct 20, 2026
… test-only editor-state guard (#31250)

* refactor(editor): drop isWholeEditorState and the diff-signature re-export; bound superseded restore re-reads

- Remove isWholeEditorState; its callers take the real AppState. Tests that
  mocked partial stores now build full states from createTestStore.
- Import getDiskBaselineSignature from shared/disk-baseline-signature
  directly and delete the re-export from diff-content-signature.ts.
- Restored-tab conflict scan: count superseded re-reads toward
  MAX_VERIFY_ATTEMPTS, so a read slower than the re-verify interval gives up
  with the conflict banner instead of looping forever. Adds a regression test.

Co-Authored-By: Claude <noreply@anthropic.com>

* fix(editor): keep the last read at the superseded re-read cap instead of giving up

A matching read at the cap cleared nothing and showed a false changed-on-disk
banner; it now resolves like any read. Only read failures give up.

Co-Authored-By: Claude <noreply@anthropic.com>

* fix(editor): back off on superseded restore re-reads instead of capping them

A superseded read may predate the newer check, so it never resolves the
verification, even at the attempt cap. Overlapping checks now delay the next
fresh read (bounded, one timer per tab) and keep verification pending; only
failed reads count toward giving up.

Co-Authored-By: Claude <noreply@anthropic.com>

* test(editor): address the restored tab by its owned id in the overlapping-check tests

After #31196 a server's tab is no longer keyed by its path, so these tests
flagged a tab that does not exist and no read was ever issued.

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
Jinwoo-H added a commit that referenced this pull request Oct 20, 2026
#31196 edited the removed renderer Markdown bridge and moved the editor save queue's
precondition check; the bridge stays deleted and the queue keeps main's order without the
'mobile' trigger.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant