Repository navigation
Conversation
📝 Walkthrough
Priority: ➖ Normal Merge Risk: 🟡 Moderate · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 39.53% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 43 functions across 24 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly and concisely describes the primary change: sharing one editor buffer per physical file across tabs. Description check The description covers the required sections, explains the user impact and implementation, documents testing, addresses cross-platform and remote-host behavior, and identifies the linked issue excepti… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description covers the required sections, explains the user impact and implementation, documents testing, addresses cross-platform and remote-host behavior, and identifies the linked issue exception for a maintainer. The visual proof is correctly marked N/A because there is no visual change.
-
- Fix all pre-merge checks with AI
✨ 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.
Comment @coderabbitai help to get the list of available commands.
![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:
bd4f589b-3d35-4659-ab33-735eab5927d1
📒 Files selected for processing: 8
src/renderer/src/components/editor/closed-editor-view-state-retention.test.tssrc/renderer/src/components/editor/editor-model-host-isolation.test.tssrc/renderer/src/components/editor/editor-model-lifetime-fixture.tssrc/renderer/src/components/editor/editor-model-owner.test.tssrc/renderer/src/components/editor/editor-model-owner.tssrc/renderer/src/components/editor/editor-model-uri-language-service.test.tssrc/renderer/src/components/editor/editor-model-uri.test.tssrc/renderer/src/components/editor/editor-model-uri.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.
| function withoutOwner(path: string): string { | ||
| if (!path.startsWith(OWNER_PREFIX)) { | ||
| return path | ||
| } | ||
| const rest = path.indexOf('/', OWNER_PREFIX.length) | ||
| return rest === -1 ? path : path.slice(rest) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '1,180p' src/renderer/src/components/editor/editor-model-uri.ts
rg -n 'toEditorModelUri\(' src/renderer/src/components/editor
sed -n '1,130p' src/renderer/src/components/editor/editor-model-uri.test.tsRepository: stablyai/orca
Length of output: 8795
🏁 Script executed:
set -e
printf '%s\n' '--- owner-key references and declarations ---'
rg -n -F --glob '*.ts' --glob '*.tsx' -e 'getEditorModelOwnerKey' -e 'localEditorModelOwnerKey' -e 'modelOwnerKey' src/renderer/src/components/editor
printf '%s\n' '--- owner-key source candidates ---'
rg -n --glob '*.ts' --glob '*.tsx' 'function (getEditorModelOwnerKey|localEditorModelOwnerKey)|const (getEditorModelOwnerKey|localEditorModelOwnerKey)|export .*EditorModelOwner' src/renderer/src
printf '%s\n' '--- focused caller blocks ---'
sed -n '1,190p' src/renderer/src/components/editor/editor-model-owner.test.ts
sed -n '90,155p' src/renderer/src/components/editor/closed-editor-tab-controller.ts
sed -n '225,270p' src/renderer/src/components/editor/closed-editor-tab-controller.ts
sed -n '80,125p' src/renderer/src/components/editor/use-monaco-editor-mount.ts
sed -n '85,120p' src/renderer/src/components/editor/MonacoEditor.tsx
printf '%s\n' '--- file-path contract references ---'
rg -n -F --glob '*.ts' --glob '*.tsx' -e 'filePath:' -e 'filePath =' -e 'filePath,' src/renderer/src/components/editor | head -160Repository: stablyai/orca
Length of output: 38916
🏁 Script executed:
set -e
printf '%s\n' '--- editor-model-owner.ts ---'
nl -ba src/renderer/src/components/editor/editor-model-owner.ts
printf '%s\n' '--- OpenFile declaration ---'
rg -n -F --glob '*.ts' --glob '*.tsx' -e 'export interface OpenFile' -e 'interface OpenFile' -e 'type OpenFile' src/renderer/src
printf '%s\n' '--- OpenFile declaration context ---'
file=$(rg -l -F --glob '*.ts' --glob '*.tsx' 'export interface OpenFile' src/renderer/src | head -1)
if [ -n "$file" ]; then
nl -ba "$file" | sed -n '1,180p'
fi
printf '%s\n' '--- file-path normalization/opening references ---'
rg -n -F --glob '*.ts' --glob '*.tsx' -e 'openFiles' -e 'filePath' src/renderer/src/store src/renderer/src/components/editor src/renderer/src/lib | rg -F -e 'path' -e 'filePath' -e 'openFiles' | head -220Repository: stablyai/orca
Length of output: 33111
🏁 Script executed:
set -e
printf '%s\n' '--- OpenFile type ---'
nl -ba src/renderer/src/store/slices/editor/types/open-file.ts
printf '%s\n' '--- open-file action and apply path handling ---'
nl -ba src/renderer/src/store/slices/editor/actions/open-file-action.ts
nl -ba src/renderer/src/store/slices/editor/actions/open-file-apply.ts | sed -n '1,220p'
printf '%s\n' '--- repository-wide toEditorModelUri callers ---'
rg -n -F --glob '*.{ts,tsx}' 'toEditorModelUri(' .
printf '%s\n' '--- repository-wide imports ---'
rg -n -F --glob '*.{ts,tsx}' 'editor-model-uri' .
printf '%s\n' '--- changed file history-independent diff context ---'
git diff 92ed331184b65bf9dc9c9ce2d88e93d8181c3ee6 8195b6829f9913a1a65dfbc73439bdd3fd7f8bd5 -- src/renderer/src/components/editor/editor-model-uri.ts src/renderer/src/components/editor/editor-model-uri.test.tsRepository: stablyai/orca
Length of output: 33448
Preserve the owner prefix in raw file paths.
OpenFile.filePath is an absolute filesystem path, and owner-key callers pass it directly to toEditorModelUri. A valid path such as /.orca-owner/a/src/file.ts is therefore treated as an existing model key. /.orca-owner/a/src/file.ts and /.orca-owner/b/src/file.ts both become /src/file.ts before the same workspace owner segment is added. Their tabs can share one Monaco model and mix draft or undo state.
Do not infer model-key inputs from the /.orca-owner/ prefix. Use an explicit distinction at the boundary, such as separate helpers or an input flag, so raw OpenFile.filePath values keep their full path. The correction is localized because the normal owner-key callers already know whether they have a filesystem path or a model key.
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:
ccbba033-62ef-4d4a-87a3-3be4b68e3f48
📥 Commits
Reviewing files that changed from the base of the PR and between 8195b68 and 925ce418ae94105d07b42eced6bfade825177cdf.
📒 Files selected for processing: 7
src/renderer/src/components/editor/editor-autosave.tssrc/renderer/src/components/editor/editor-model-owner.tssrc/renderer/src/components/editor/editor-same-file-peers.test.tssrc/renderer/src/components/editor/editor-same-file-peers.tssrc/renderer/src/components/editor/editor-save-queue.tssrc/renderer/src/components/editor/useEditorPanelExternalContentEvents.saved-peers.test.tsxsrc/renderer/src/components/editor/useEditorPanelExternalContentEvents.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.
| for (const sourceId of [detail.fileId, ...cleanPeerIds]) { | ||
| updateSavedPreviewTabs(openFilesRef.current, sourceId, detail.content, setFileContents) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Refresh diff content for clean peers.
If a clean peer is showing its changes view, this loop updates its file content and previews but leaves its cached diff content unchanged. The diff update below targets only the saving file. Refresh the active peer’s diff and invalidate inactive peers’ diff caches, as the external-change handler does.
Tabs in different workspaces that show the same file on the same host already share one Monaco model, but their draft, dirty flag and disk state were tracked per tab. Mirror those across every tab of the buffer, let a newly opened tab adopt the buffer's unsaved state, and serialize saves per buffer. The model URI keeps the real file name last, with any host owner in a leading path folder so the TypeScript worker still resolves imports and recognises .tsx.
925ce41 to
49550d4
Compare
nwparker
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
994c4329-7385-40e8-945b-9a8a143cf213
📥 Commits
Reviewing files that changed from the base of the PR and between 925ce418ae94105d07b42eced6bfade825177cdf and 49550d4.
📒 Files selected for processing: 8
src/renderer/src/components/editor/editor-model-uri-language-service.test.tssrc/renderer/src/components/editor/editor-save-queue.tssrc/renderer/src/components/editor/useEditorPanelExternalContentEvents.tssrc/renderer/src/store/slices/editor/actions/editor-draft-state.tssrc/renderer/src/store/slices/editor/actions/open-file-action.tssrc/renderer/src/store/slices/editor/actions/open-file-mutations.tssrc/renderer/src/store/slices/editor/file-ids/editor-file-buffer.test.tssrc/renderer/src/store/slices/editor/file-ids/editor-file-buffer.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.
| const bumpSaveGeneration = (fileId: string): void => { | ||
| saveGeneration.set(fileId, (saveGeneration.get(fileId) ?? 0) + 1) | ||
| const key = bufferKeyOf(fileId) | ||
| saveGeneration.set(key, (saveGeneration.get(key) ?? 0) + 1) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
# Find quiesceFileSave / bumpSaveGeneration callers and whether peers can still be open at that time.
rg -nP --type=ts -C5 '\b(quiesceFileSave|bumpSaveGeneration)\s*\(' -g '!**/*.test.ts' src/rendererRepository: stablyai/orca
Length of output: 4935
Check whether quiescing one tab cancels peer save bookkeeping.
quiesceFileSave uses the shared buffer key to read the pending save, then clears the shared autosave timer and increments the shared save generation before awaiting that save. A peer save can therefore complete successfully but skip its post-save bookkeeping, leaving the peer dirty with a stale disk baseline.
Quiescing one tab must not invalidate a save that remains active for another tab sharing the buffer. Only cancel the shared generation when the whole buffer is being quiesced or no peer remains.
| // Every tab of this file shares one buffer, so all of them now match disk. | ||
| const ids = getSameBufferFileIds(useAppStore.getState(), file.id) | ||
| setFileContents((prev) => { | ||
| const next = { ...prev } | ||
| for (const id of ids) { | ||
| next[id] = { content: detail.content, isBinary: false } | ||
| } | ||
| return next | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Refresh markdown preview tabs of peer tabs after a save.
This block updates fileContents for every ID in ids. Line 186 still calls updateSavedPreviewTabs with detail only. That function matches only previews where markdownPreviewSourceFileId === detail.fileId.
Example: tab A saves, and peer tab B has a markdown preview. The save clears the shared draft. B's preview stops showing the live draft and falls back to its own cached fileContents entry. That entry still holds the content from before the save, so the preview shows old text.
Pass the same-buffer IDs to updateSavedPreviewTabs.
Proposed fix
--- "a/src/renderer/src/components/editor/useEditorPanelExternalContentEvents.ts"
+++ "b/src/renderer/src/components/editor/useEditorPanelExternalContentEvents.ts"
@@ -172,10 +172,13 @@
if (!file) {
return
}
- if (file.mode === 'edit' || file.mode === 'markdown-preview') {
- // Every tab of this file shares one buffer, so all of them now match disk.
- const ids = getSameBufferFileIds(useAppStore.getState(), file.id)
+ // Every tab of this file shares one buffer, so all of them now match disk.
+ const ids =
+ file.mode === 'edit' || file.mode === 'markdown-preview'
+ ? getSameBufferFileIds(useAppStore.getState(), file.id)
+ : [file.id]
+ if (file.mode === 'edit' || file.mode === 'markdown-preview') {
setFileContents((prev) => {
const next = { ...prev }
for (const id of ids) {
next[id] = { content: detail.content, isBinary: false }// Line 186 and updateSavedPreviewTabs:
updateSavedPreviewTabs(openFilesRef.current, new Set(ids), detail.content, setFileContents)
function updateSavedPreviewTabs(
openFiles: OpenFile[],
sourceIds: ReadonlySet<string>,
content: string,
setFileContents: Dispatch<SetStateAction<Record<string, FileContent>>>
): void {
const previewTabs = openFiles.filter(
(openFile) =>
openFile.mode === 'markdown-preview' &&
openFile.markdownPreviewSourceFileId !== undefined &&
sourceIds.has(openFile.markdownPreviewSourceFileId)
)
// ...unchanged, using `content`
}- Each panel's cached disk text follows any panel that loads, reloads or saves the same buffer, so a mounting tab never pushes stale text into the shared model. - Closing a tab whose buffer another tab still shows neither prompts nor discards; only the last tab's close asks to save. - Disk-verification pauses are mirrored across the buffer and copied to a joining tab; any tab's pause (incl. owner migration) pauses autosave.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟠 Major · Do not write a stale fallback after a shared-buffer save. · editor-save-queue.ts:127
src/renderer/src/components/editor/editor-save-queue.ts:127
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDo not write a stale fallback after a shared-buffer save.
If two tabs queue saves for one buffer, the first save can clear both drafts. The second save then uses its captured
fallbackContent. If that content is older, the second write replaces the newer disk text. The shared queue serializes these writes but does not make the second request current. Resolve content from the live shared buffer before writing, or discard a queued request superseded by a completed save. The test atsrc/renderer/src/store/slices/editor/file-ids/editor-file-buffer.test.ts:94-111supplies identical content to both requests, so it does not cover this case.
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
647dbbfc-cf5c-4edb-9075-906b00c7ab55
📒 Files selected for processing: 9
src/renderer/src/components/editor/editor-save-queue.tssrc/renderer/src/components/editor/use-editor-buffer-content-sync.test.tsxsrc/renderer/src/components/editor/use-editor-buffer-content-sync.tssrc/renderer/src/components/editor/useEditorPanelContentState.tssrc/renderer/src/components/terminal/useTerminalSaveDialog.tssrc/renderer/src/components/use-terminal-editor-close-queue.tssrc/renderer/src/store/slices/editor/actions/open-file-mutations.tssrc/renderer/src/store/slices/editor/file-ids/editor-file-buffer.test.tssrc/renderer/src/store/slices/editor/file-ids/editor-file-buffer.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.
| for (const [fileId, content] of Object.entries(fileContents)) { | ||
| if ( | ||
| content === previous[fileId] || | ||
| !isFreshText(content) || | ||
| receivedRef.current.has(content) || | ||
| openFilesRef.current.find((file) => file.id === fileId)?.mode !== 'edit' | ||
| ) { | ||
| continue | ||
| } | ||
| for (const listener of listeners) { | ||
| listener(fileId, content) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
# Inspect the loader's read-generation and setFileContents paths.
fd 'use-editor-panel-file-content-loader|useEditorPanelFileContentLoader' src/renderer/src/components/editor
rg -n -C 5 'fileReadGenerationRef|setFileContents|loadFileContent' src/renderer/src/components/editorRepository: stablyai/orca
Length of output: 41761
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- loader outline and source ---'
ast-grep outline src/renderer/src/components/editor/useEditorPanelFileContentLoader.ts --view expanded || true
nl -ba src/renderer/src/components/editor/useEditorPanelFileContentLoader.ts
printf '%s\n' '--- sync hook ---'
nl -ba src/renderer/src/components/editor/use-editor-buffer-content-sync.ts
printf '%s\n' '--- content state and file identity references ---'
nl -ba src/renderer/src/components/editor/useEditorPanelContentState.ts | sed -n '1,245p'
rg -n -F --glob '*.ts' --glob '*.tsx' -e 'bufferId' -e 'modelUri' -e 'fileId' src/renderer/src/components/editor | head -200Repository: stablyai/orca
Length of output: 41929
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- shared-buffer identity ---'
rg -n -F --glob '*.ts' --glob '*.tsx' 'function getSameBufferFileIds' src
rg -n -C 8 -F --glob '*.ts' --glob '*.tsx' 'getSameBufferFileIds' src/renderer/src/store src/renderer/src/components/editor src/renderer/src/lib || true
printf '%s\n' '--- file-content reconciliation consumers ---'
rg -n -C 8 -F --glob '*.ts' --glob '*.tsx' -e 'fileContents[activeFile.id]' -e 'fileContents[file.id]' -e 'content={fileContents' -e 'cached disk text' -e 'setValue' src/renderer/src/components/editor || trueRepository: stablyai/orca
Length of output: 41928
Guard shared-buffer reads with a buffer-level generation.
useEditorPanelFileContentLoader rejects results only when the generation for the same tab ID changes. A forced reload for tab A can start a newer read while tab B’s older read remains in flight. If B’s read resolves later, the loader accepts it and useEditorBufferContentSync copies it to every tab sharing the buffer. This can replace A’s newer cache with older disk text and allow stale content to reach the shared Monaco model.
Track the latest read generation per shared buffer, and reject superseded results before setFileContents publishes them.
|
Pausing as draft: reviews of this stack keep tracing back to one root cause — an open file's content and unsaved state live in several places (per-tab store fields, per-panel caches, the draft store, the editor model, rich editors). A short design for a single editor buffer per file is being written first; this PR will be re-scoped against it. |
Phase 1b part one: edit tabs of one physical file (host + normalized, exact-case path; retired runtime pairings quarantined) map to one record with one save queue and autosave timer. Only the last tab that can save prompts on close. Dirty+dirty joins stay separate with their own model. Reuses the URI shape, close rule and cache sync from #30714.
Phase 1b part one: edit tabs of one physical file (host + normalized, exact-case path; retired runtime pairings quarantined) map to one record with one save queue and autosave timer. Only the last tab that can save prompts on close. Dirty+dirty joins stay separate with their own model. Reuses the URI shape, close rule and cache sync from #30714.
ELI5
When the same file is open in two workspaces (same machine, same path), Orca now treats it as one document. Both tabs show the same text, the same "unsaved" dot, and saving from either one saves both. Before, the two tabs shared the visible text but each kept its own idea of what was unsaved, so one tab could quietly write old text over the other's save.
What Changed
Before: two tabs for one physical file shared the editor's text buffer, but the unsaved draft, the dirty flag, the "last known disk version" and the "changed on disk" warning were tracked per tab. Typing in workspace A changed the text B displayed while B still thought it was clean. B could then autosave stale text, show no warning, or reset A's edits when it was shown again.
After: every edit tab for the same file on the same host shares one buffer and one unsaved state. Editing in A marks B dirty with the same draft. Saving from either tab clears both. An external change warns both. A tab opened later picks up the unsaved edits already in progress. The same path on a different host (another SSH machine or runtime) stays separate. Split panes of one tab work as before.
Mechanism:
store/slices/editor/file-ids/editor-file-buffer.ts: the buffer key is the Monaco model URI (host owner + normalized path, no workspace), so model sharing and state sharing can never disagree.getSameBufferFileIdsfinds the tabs that share one buffer. It only resolves hosts for tabs whose path already matches, because it runs on every keystroke.setEditorDraft,clearEditorDraft(s),markFileDirty,setLastKnownDiskSignature,setExternalMutationand the disk-verification setters apply to every tab of the buffer in one update. Callers don't change. Closing a tab still removes only that tab's state.openFile: a newly added tab adopts the open buffer's draft, dirty flag and disk state.use-editor-buffer-content-sync.ts: each panel caches the last disk text per tab. When any panel loads, reloads or saves a buffer's disk text, every other panel's cache for that buffer follows. Otherwise a tab mounting later would push stale text into the shared model.needsSavePromptOnClose, used by the close queue and the save dialog)./.orca-owner/<hash>/folder, so the real file name stays last. The TypeScript worker then recognises.tsxand resolves imports between open files on the same host. Local files keep plainfile://URIs.Why
The underlying bug was a mismatch: one shared text buffer, but several independent sets of unsaved state. Giving each tab its own buffer (this PR's first version) removed the mismatch but created two editable copies of one file, which can race each other's saves and drift apart. Keeping one buffer per physical file is how mature editors behave: the file is the document, and tabs are views onto it. A single mapping layer in the store keeps the diff small and leaves every caller unchanged.
Linked Issue
N/A (maintainer)
Visual Proof
N/A: no visual change; this fixes which unsaved state the tabs of one file share.
Testing
I manually tested these changes locally
Automated tests added/updated, or explained why not below
editor-file-buffer.test.ts:use-editor-buffer-content-sync.test.tsx: a reload in one panel refreshes another panel's cache for the same file.editor-model-uri-language-service.test.ts: the TypeScript service reads.tsxas TSX (local and remote) and resolves an import to the file on the same host, not another host's copy.editor-model-uri.test.ts: Windows drive letters, UNC paths, spaces,%20and#round-trip.The existing model-lifetime, closed-tab undo and host-isolation suites pass.
The renderer
components,store,libandruntimesuites pass, as dopnpm tcandcheck:code-quality:changed.AI Disclosure
Review
Agent skill upstream boundary
Notes
Ensure no issues in: Security, Cross-platoform support (Linux, Windows, Mac), Remote SSH, Mobile, general backwards compatibility, performance
Buffers are scoped by host, so SSH and runtime files never mix with local ones. Model keys and buffer keys are renderer-only and never go over the wire.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)