Skip to content

Harden KclManager against multiple writers#10918

Open
franknoirot wants to merge 4 commits intomainfrom
franknoirot/adhoc/editor-hardening
Open

Harden KclManager against multiple writers#10918
franknoirot wants to merge 4 commits intomainfrom
franknoirot/adhoc/editor-hardening

Conversation

@franknoirot
Copy link
Copy Markdown
Contributor

Codex-assisted audit and hardening meant to close #10809 and other bugs reported in Discord and Slack around code loss when the app is open with multiple instances. Stacked on top of #10750 because that PR augments the editor history and change flows already, so I wanted to build upon that. I'm gonna do a bit of an info dump here from my Codex session.

Prompt

We have a number of issues with shaky editor state in KclManager.ts. #10809 is a related bug, but we've also got reports that typing in the code editor, which triggers a number of effects that asynchronously execute the code, can result in a jittery typing and selecting experience. When users run multiple instances of the app, which they sometimes do to run multiple projects or multiple AI agents with it, they have reported suddenly losing their work with no way to recover it.

I suspect that a lot of this is due to the many ways that we can update the canonical CodeMirror editor state, which drives so much of our other systems. We have updateCodeEditor, and updateEditorWithAstAndWriteToFile, and probably more ways I'm not aware of.

Could you please analyze KclManager.ts, write integration tests that exercise it for these problematic "multiple competing updates" code paths within KclManager.spec.ts, and write me a report on how we might harden this system to make unrecoverable loss of code impossible?

Initial response and report

Added integration coverage in KclManager.spec.ts#L203 for the main competing-update paths:

  • debounced direct editor typing only executes the latest buffer
  • repeated programmatic updates only write the latest buffer
  • clean file-watcher reloads still apply
  • dirty in-memory edits are not overwritten by a later async disk reload
  • updateEditorWithAstAndWriteToFile still refuses to replace code with an empty AST

I also made a narrow hardening change in KclManager.ts#L673, KclManager.ts#L1118, KclManager.ts#L1185, and KclManager.ts#L2328: KclManager now tracks the last disk-synced buffer and skips automatic watcher reloads when the editor has unsaved local changes, which closes the most direct “second app instance overwrote my work” path.

Verified with:

  • npx vitest run --mode=development src/lang/KclManager.spec.ts
  • npx eslint src/lang/KclManager.ts src/lang/KclManager.spec.ts

Hardening Report
The current architecture still has one structural weakness: CodeMirror document updates, execution, and file persistence are coupled by side-effect listeners rather than one serialized mutation pipeline. That means the canonical buffer can still be affected by multiple producers with different clocks: user typing, AST recasts, watcher reloads, undo/redo effects, and delayed writes/executions.

To make unrecoverable loss impossible, I’d harden it in this order:

  • Introduce a monotonic documentVersion and require every async path to carry it. Execution results, watcher reads, AST recasts, and writes should only commit if their version still matches the live buffer.
  • Persist an append-only local recovery journal for every doc change before any async work. Losing the latest text should then become a recovery problem, not a data-loss problem.
  • Treat disk writes as compare-and-swap against the last known disk revision, not blind writes. If disk changed externally while local buffer is dirty, surface a conflict instead of overwriting or auto-reloading.
  • Collapse code replacement to one reducer-style entry point. updateCodeEditor, updateEditorWithAstAndWriteToFile, file-watch reloads, and undo/redo should all funnel through the same provenance-aware transaction API.
  • Separate “editor text changed” from “execute this text” more explicitly. The jitter reports are consistent with heavy async execution work being too tightly coupled to keystrokes.
  • Add explicit conflict UX: “disk changed externally, keep mine / load theirs / diff”. Silent reload is the real failure mode.

The related GitHub issue, #10809, opened April 3, 2026, describes sketch-edit flicker followed by code-pane deletion. That matches the broader pattern here: multiple async systems are allowed to treat the current editor buffer as writable state instead of treating it as an owned versioned document.

Final response after some additional prompts to stack this work on top of #10750

Implemented the three hardening steps in KclManager.ts.

KclManager now maintains a monotonic document revision at KclManager.ts#L689 and uses it to drop stale async completions. That now guards:

  • external file-watch reloads
  • sketch checkpoint restores at KclManager.ts#L2728
  • AST-driven rewrites from updateEditorWithAstAndWriteToFile() at KclManager.ts#L3019

Disk writes are now CAS-style at KclManager.ts#L2927: before writing, KclManager re-reads the file, skips the write if disk changed since the last sync, and avoids clobbering newer external contents.

There is now a local recovery journal keyed by file path. Unsynced edits are persisted as the user types at KclManager.ts#L1578, and fromFile() restores that recovery buffer on reopen at KclManager.ts#L1474, while still treating the actual disk contents as the sync base.

I added focused regressions in KclManager.spec.ts#L400, KclManager.spec.ts#L428, and KclManager.spec.ts#L454 for:

  • stale checkpoint restore vs newer local edit
  • stale AST rewrite vs newer local edit
  • CAS write conflict
  • recovery snapshot restore on reopen

Verified with:

  • npx vitest run --mode=development src/lang/KclManager.spec.ts
  • npx eslint src/lang/KclManager.ts src/lang/KclManager.spec.ts

This materially changes the risk profile: late async work can no longer blindly win, external disk changes won’t be overwritten silently, and unsaved text now survives process loss.

@franknoirot franknoirot requested review from a team as code owners April 9, 2026 03:58
@vercel
Copy link
Copy Markdown

vercel bot commented Apr 9, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
modeling-app Ready Ready Preview, Comment Apr 9, 2026 4:05pm

Request Review

@franknoirot franknoirot changed the base branch from main to kurt-undo-rust-checkpoint April 9, 2026 03:58
@franknoirot franknoirot force-pushed the franknoirot/adhoc/editor-hardening branch from 991afaf to 38c74c6 Compare April 9, 2026 03:58
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Apr 9, 2026

Merging this PR will not alter performance

✅ 167 untouched benchmarks
⏩ 93 skipped benchmarks1


Comparing franknoirot/adhoc/editor-hardening (991afaf) with kurt-undo-rust-checkpoint (1a14f37)

Open in CodSpeed

Footnotes

  1. 93 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Base automatically changed from kurt-undo-rust-checkpoint to main April 9, 2026 06:32
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.

[BUG] Edit Sketch Flicker+ Code Pane Delete

1 participant