Repository navigation
Editor tab state is written to its buffer record, not copied into it afterwards (phase 1a) - #31251
Merged
Merged
Conversation
…or tab buffer state Writers of the six buffer fields read and write the record, with the OpenFile fields written as a mirror in the same store update. The store middleware no longer copies tab values into the record; it only keeps records keyed by open tabs (seeded from the tab on create, dropped on close) and, in dev and test builds, asserts every mirror field equals its record. Host-mirrored web tabs adopt the host's published values explicitly in the session sync patch. Co-Authored-By: Claude <noreply@anthropic.com>
…he record helpers Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 6 minutes. View limit details
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 |
Co-Authored-By: Claude <noreply@anthropic.com>
7 of 9 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ELI5
Each open editor tab carries a few flags: "has unsaved edits", "which version of the file on disk it came from", "the file changed on disk", and a few "still checking the disk" markers. Since #30885 those flags also live in a per-tab record, which is the groundwork for later sharing one record between every tab showing the same file. But the record was only a copy: code still wrote the flags on the tab, and a piece of store code copied them into the record after every change. This PR makes the record the place the flags are written, with the tab kept as a mirror updated in the same step.
What Changed
updateEditorBuffer(state, fileId, patch)checks the change against the record, then writes the record and the tab's mirror in one store update. The setters inopen-file-mutations.tsandsetRestoredEditorOwnerMigrationPendinguse it;markFileDirtyreads its current values from the record.adoptMirroredEditorBuffers), only for tabs the host owns. Wire format unchanged.editorBuffersguard in remote session sync is removed (its type already requires the field).User-visible behaviour is unchanged. Readers still read the tab's fields; drafts, text models and the content-compare dirty check are untouched.
Why
This follows the "1a" step of the editor-buffer design discussed in #30851: writers update the record, the tab fields stay a synchronized mirror until a later phase removes them. Copying tab → record after every write hid writers that skipped the record and cost a reconcile pass on every
openFilesupdate. Writing the record directly, with a dev/e2e check that fails loudly on drift, is what the next phase (one record shared across workspaces) needs.Linked Issue
Follow-up to #30885.
Visual Proof
N/A: no visible behaviour change.
Testing
pnpm tc,pnpm run check:code-quality:changed,oxfmt --checkpass.Renderer unit tests: runtime, store, editor, floating-terminal and tab-group: 1,169 files / 10,232 tests pass; an earlier full
src/rendererrun passed apart from terminal-pane timeouts that also occur without this change.Reviewed by GPT and Opus independently. Both audited every renderer write of the six fields (open, preview, hydrate, layout frames, re-keys, teardown, owner moves, web session sync, mobile bridge) and found no production writer that skips the record. Their one shared finding, an e2e fixture that set
isDirtydirectly, now usesmarkFileDirty.The e2e suite was not run locally; CI covers it.
I manually tested these changes locally
Automated tests added/updated, or explained why not below
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
Mixed-version remote/web hosts: host-published tabs are adopted as before (a missing host
isDirtystill reads as clean). No persisted-state change.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)