Repository navigation
docs(editor): design for one editor buffer per file - #30851
Conversation
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: 🔵 Low · up to
|
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:
94f99a32-3e9a-4d88-b6d2-e60e8491fb70
📒 Files selected for processing: 1
docs/editor-buffer-per-file-design.md
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.
| - Capture `{text, version, baseline, consent}` in one tick, then do a guarded write that expects `consent ?? baseline`. | ||
| - On success, mark the file clean only if its version hasn't changed since the capture. Otherwise only move `baseline` forward, and not if a reload changed it while the save was in flight. | ||
| - A refusal sets `conflict.observed` to `currentSignature`. | ||
| - Autosave never recreates a deleted file. An explicit save may. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '55,75p' docs/editor-buffer-per-file-design.md
rg -n 'allowMissing|expectedSignature|file-write-disk-precondition' srcRepository: stablyai/orca
Length of output: 14867
🏁 Script executed:
nl -ba src/shared/file-write-disk-precondition.ts | sed -n '1,115p'
printf '\n--- local precondition ---\n'
nl -ba src/main/ipc/filesystem/local-disk-write-precondition.ts | sed -n '1,95p'
printf '\n--- relevant tests ---\n'
nl -ba src/main/ipc/filesystem/local-disk-write-precondition.test.ts | sed -n '135,175p;400,465p'
printf '\n--- design lines ---\n'
nl -ba docs/editor-buffer-per-file-design.md | sed -n '45,72p'Repository: stablyai/orca
Length of output: 17981
🏁 Script executed:
nl -ba src/shared/disk-baseline-signature.ts | sed -n '1,150p'
rg -n -F -- '"deleted"' docs src/renderer src/shared src/main | head -80Repository: stablyai/orca
Length of output: 4775
Specify allowMissing and the deleted-file reappearance outcome.
The guarded write requires allowMissing: true to recreate a missing file. The design must state that explicit saves set it and autosaves keep it false.
The design already selects consent ?? baseline for a reappearing file. However, deleted-file consent is the literal "deleted", not a disk signature, so the guard treats a reappearing file as changed and refuses the write. Document that behavior.
Suggested design clarification
--- "a/docs/editor-buffer-per-file-design.md"
+++ "b/docs/editor-buffer-per-file-design.md"
@@ -65,7 +65,7 @@
- Capture `{text, version, baseline, consent}` in one tick, then do a guarded write that expects `consent ?? baseline`.
- On success, mark the file clean only if its version hasn't changed since the capture. Otherwise only move `baseline` forward, and not if a reload changed it while the save was in flight.
- A refusal sets `conflict.observed` to `currentSignature`.
- - Autosave never recreates a deleted file. An explicit save may.
+ - An explicit save after consent to a deleted file sets `allowMissing: true`. If the file reappears before the write, compare its disk signature with `consent ?? baseline` and refuse the write when it differs; deleted-file consent is not a disk signature and therefore refuses a reappearing file. Autosaves keep `allowMissing: false`.
- **Restore**: hot exit saves one draft per record, together with its baseline. A restored record starts dirty, with autosave paused until the disk has been checked. Sessions saved in the old per-tab format take the newest draft for each key.
- **Rename or move**: the record takes the new key, its views follow it, and a dirty file stays dirty.
- **Remote hosts**: the guarded write runs on the host that executes it: the main process for local and WSL, the runtime for SSH and remote hosts. It is pinned to the connection that answered the capability check. If contact with the host is lost, the record and its draft stay. Nothing is thrown away or reloaded until the host answers again.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - Autosave never recreates a deleted file. An explicit save may. | |
| - An explicit save after consent to a deleted file sets `allowMissing: true`. If the file reappears before the write, compare its disk signature with `consent ?? baseline` and refuse the write when it differs; deleted-file consent is not a disk signature and therefore refuses a reappearing file. Autosaves keep `allowMissing: false`. |
…, interim adapter, phase 1 split)
…ogress, identity rules
…irty, pending flush, exact-case identity
…s, fenced adapter commits
…ed-model records, keep toast for unconverted views
…-installed fence base, mobile rollback, host continuity
ELI5
When the same file is open in more than one place (split panes, two workspaces, rich markdown, a diff), Orca keeps several copies of its text and of "which version of the file on disk this text came from". Bugs happen when a save takes the text from one copy and the disk version from another. This doc proposes keeping one record per file, so that every place that shows the file reads from and writes to that one record.
What Changed
Docs only. Adds
docs/editor-buffer-per-file-design.md, which covers:Before: three draft PRs each patched one copy of the text or the disk version, and each review round found another copy they missed. After: one design that the follow-up PRs implement phase by phase.
Why
About 20 review rounds on #30714, #30681 and #30417 kept finding the same kind of bug: text and its disk version living in different places. Each extra guard added another copy to keep in sync. The doc proposes removing the copies instead: one buffer per file, which is how mature editors work. The host-side guarded write (#30605, merged) already gives the check at write time. What is missing is a single owner on the client side.
Linked Issue
N/A (maintainer)
Visual Proof
N/A: documentation only.
Testing
Docs-only change; no code or tests. Prettier-formatted.
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
The design covers SSH, WSL and runtime hosts (one record per host and path, and nothing is discarded when contact is lost), keeping the mobile wire format the same, and reading sessions saved by older versions.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)