pastilhasandClaude Opus 5
a3fced25c4
The code editor held file contents per panel while Monaco keyed the model by
file URI globally, so two panels on one file had two buffers and one model.
@monaco-editor/react resolves a model as getModel(uri) || createModel(value),
so the second panel adopted the first one's model and its own value was
ignored; the value-sync effect then overwrote the shared model with
executeEdits, and the flag suppressing onChange around that write is
per-instance — so the OTHER panel's onChange fired, and the editor answers that
with setContent. One panel's buffer silently became the other's and the tab
went dirty with no keystroke.
useEditorState now keys buffers by path in one shared store and keeps only the
tab set and active path per panel. openFile discards the content it is handed
when the file is already open elsewhere, mirroring what Monaco does with the
model — that is what removes the divergence, because there is then no value for
the sync effect to reconcile. Buffers are refcounted by holders and dropped
when the last panel closes, so closing a tab cannot discard another panel's
unsaved work; closing the last one still discards, as before.
keepCurrentModel is the other half and was needed under any fix: the default
disposes the shared model on unmount, and key={activeFile.path} makes that
every tab switch rather than only a panel close.
Writes go through queryClient.setQueryData rather than useGlobal's setter,
which applies a functional update to the data captured in render — invisible
with one writer, wrong with one per panel.
Nine tests, and the editor had none before. The known-bugs card stays OPEN: this
removes a mechanism that produces the reported shape, not the mystery of where
the log text came from. The trap stays armed, and it can now fire legitimately
when two panels share a file, so the stack matters more than the count.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Officer-Session: f7a62308-0977-4cf4-aa29-c020853fd420
138 lines
10 KiB
Markdown
138 lines
10 KiB
Markdown
# Background-job output appears inside an open file
|
|
|
|
An open bug, not reproducible on demand, with a trap already set for it. **If someone has just reported
|
|
this, skip to "When it is reported".**
|
|
|
|
Status as of 2026-08-16: cause unknown, two candidates eliminated, instrumentation shipped.
|
|
|
|
## What it looks like
|
|
|
|
A file is open in the code editor while a background job is producing output — a dev server, a watcher.
|
|
The output appears **inside the file's buffer**, spliced into the middle of it. Observed once with a
|
|
`bun dev` job: the JSX of `SigninScreen.tsx` and eight lines of dotenv/POSTGRES_URL output shared the
|
|
buffer, with Monaco's line numbers continuing straight through (…180, the log lines, then 189 `</Link>`).
|
|
|
|
Two details from that report do most of the diagnostic work:
|
|
|
|
- **The tab was marked dirty.** So the model really was mutated; this is not a rendering artefact.
|
|
- **The file on disk was untouched.** The corruption is in the browser and was never saved. Reopening the
|
|
tab fixes it.
|
|
|
|
## What has been ruled out, and why
|
|
|
|
Do not re-investigate these without new evidence.
|
|
|
|
| Candidate | Why it is out |
|
|
| ------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
|
|
| The end-of-turn refresh re-reading the file | `reloadFile` sets `originalContent` and `isDirty: false`. It cannot produce a dirty tab. |
|
|
| A shared component between the task panel and the editor | `CodeSurface` (the task log) renders with shiki. It is not Monaco and holds no editor model. |
|
|
| ~~Two Monaco editors sharing a model~~ **— WRONG, see below** | This asked which OTHER component mounts Monaco and answered `EditorRenderer` (still true: no `path`, so `inmemory://model/N`, no collision). It never asked whether the **code editor collides with ITSELF**. It does — the panel is addable to a dashboard twice. |
|
|
| Something typing into the page | No `execCommand('insertText')`, no synthesised `KeyboardEvent`, no clipboard writes anywhere near the editor. |
|
|
|
|
That leaves **Monaco's own `onChange`** — the model was edited the way a paste or a programmatic
|
|
`executeEdits` edits it. Which is why this needs to be caught in the act rather than reasoned about.
|
|
|
|
## Two code-editor panels share one model and two buffers (2026-08-25)
|
|
|
|
The suspicion recorded here — `2b70a570` giving `<Editor>` a real `path`, so models are keyed by file URI
|
|
**globally** rather than per mount — is correct, and it is worse than "should be an improvement". Verified
|
|
by reading `@monaco-editor/react@4.7.0`'s built source, not inferred:
|
|
|
|
- **Model lookup is `getModel(Uri.parse(path)) || createModel(value, …)`.** An existing model for that path
|
|
wins and the second mount's `value` is **ignored at creation**. So the second panel to open a file adopts
|
|
the first panel's buffer.
|
|
- **The value-sync effect then overwrites it**, because `value !== editor.getValue()`:
|
|
`executeEdits('', [{ range: getFullModelRange(), text: value }])`.
|
|
- **The guard that suppresses `onChange` during that write is per-INSTANCE.** The package sets an internal
|
|
flag around its own `executeEdits` and checks it inside `onDidChangeModelContent`. Panel B's flag is not
|
|
panel A's. So B's programmatic write **fires A's `onChange`** with B's content.
|
|
- **Unmount disposes the shared model.** `keepCurrentModel` defaults to `false` and we never set it, so the
|
|
cleanup calls `editor.getModel()?.dispose()`. And `CodeEditor` mounts `<Editor key={activeFile.path}>`,
|
|
so this runs on every **tab switch**, not only on panel close — switching tabs in one panel disposes the
|
|
model another panel is still displaying.
|
|
|
|
The other half is ours: `useEditorState` holds `files` in a plain `useState`, so it is **per panel
|
|
instance**. Its own comment says so — "this app also mounts as a dashboard panel, and a dashboard can hold
|
|
two of them". One shared model, two independent `content` values, and an `onChange` that crosses between
|
|
them. `CodeEditor`'s handler does `setContent(activeFile.path, value)`, so panel A's buffer silently becomes
|
|
panel B's content and `isDirty` flips with no keystroke — dirty tab, content the user never typed, disk
|
|
untouched, fixed by reopening. That is the reported symptom's exact shape.
|
|
|
|
**What this does NOT yet explain is how JOB OUTPUT becomes one of those `value`s.** Nothing in the app
|
|
writes job output into `useEditorState` — `openFile`/`setContent`/`reloadFile` are called only from
|
|
`CodeEditor.tsx`, and the task log renders through shiki. So this is a real defect that produces the right
|
|
shape, and it is not proven to be the reported instance. **The trap is still the thing that names the
|
|
text's source.** Do not delete it on the strength of this section.
|
|
|
|
## Fixed 2026-08-25: the buffer is shared, because the model already was
|
|
|
|
`useEditorState` now keys buffers by path in one store shared across panels, and keeps only the tab set and
|
|
the active path per panel. The rule that matters: **`openFile` discards the content it is handed when the
|
|
file is already open somewhere else.** That mirrors what Monaco does with the model, and it is what removes
|
|
the divergence — with both panels reading one buffer there is never a `value` for the sync effect to
|
|
reconcile, so the `executeEdits` never runs and the cross-panel `onChange` never fires.
|
|
|
|
A buffer outlives the tab that opened it. `holders` is the set of panels with the file open, and the entry
|
|
is dropped when the last one closes, so closing a tab in one panel cannot take unsaved work out from under
|
|
another. Closing the last one still discards unsaved edits, exactly as before.
|
|
|
|
`<Editor keepCurrentModel>` is the other half, and it was needed under any fix: the default disposes the
|
|
shared model on unmount, and `key={activeFile.path}` makes that every **tab switch**, not just a panel
|
|
close. The cost is one retained model per file opened in a session.
|
|
|
|
Two things worth knowing:
|
|
|
|
- **Writes go through `queryClient.setQueryData`, not `useGlobal`'s setter.** `useGlobal` applies a
|
|
functional update to the `data` captured in render rather than to what is in the cache now. With one
|
|
writer that is invisible; this slot has one writer per panel, which is the whole scenario.
|
|
- **The trap can now fire legitimately.** Two panels on one file share a model, so a large paste in one
|
|
fires the other's `onChange` while it is unfocused — which is what `reportSuspiciousEdit` looks for. A
|
|
report whose `stack` runs through the editor's own `onChange` with another panel open is that, not the
|
|
bug. This is the reason to keep reading the `stack` rather than the count.
|
|
|
|
Pinned by `useEditorState.test.ts` (9 tests): adoption, cross-panel edit, close-with-holders, drop on last
|
|
close, release on unmount, and that the tab set stays per panel.
|
|
|
|
**Still not closed.** This removes the mechanism, not the mystery: where the log text came from is still
|
|
unknown, and nothing found so far writes job output into an editor buffer. Keep the trap.
|
|
|
|
## When it is reported
|
|
|
|
Ask for one thing. In the browser's console, on the machine where it happened:
|
|
|
|
```js
|
|
JSON.parse(localStorage.getItem('officer:editor-buffer-trap'));
|
|
```
|
|
|
|
That returns up to the last five occurrences, each with:
|
|
|
|
| Field | What it tells you |
|
|
| ------------------ | ----------------------------------------------------------------------------------------------------- |
|
|
| `stack` | **The answer.** The call stack at the moment of the edit — whatever inserted the text is named in it. |
|
|
| `inserted` | The first 200 characters that arrived. Confirms it is job output rather than a real paste. |
|
|
| `charactersAdded` | How much arrived at once. |
|
|
| `path` | Which file was open. |
|
|
| `at` | Lines it up against the job's own logs. |
|
|
| `editorWasFocused` | Always `false` — the trap only fires when it is. |
|
|
|
|
It is written by `reportSuspiciousEdit` in `apps/CodeEditor/CodeEditor.tsx`, which fires on any insertion
|
|
over 200 characters arriving while the editor does **not** have focus. Nobody types 200 characters into an
|
|
editor they are not focused on.
|
|
|
|
If the key is empty but the corruption happened, that is itself informative: either the editor **was**
|
|
focused (so it plausibly is a paste, and the question becomes what pasted it), or the insertion arrived in
|
|
chunks under 200 characters — in which case lower `SUSPICIOUS_INSERT` and wait for it again.
|
|
|
|
## Why a trap rather than a fix
|
|
|
|
Because every fix available without knowing the cause is a guess that would look like a solution. A guard
|
|
that refuses suspicious writes would hide the symptom on the one screen where it is visible, and the same
|
|
mechanism would go on doing whatever else it does. The corruption is never saved and reopening the tab
|
|
clears it, so the cost of waiting is low and the cost of a wrong fix is a bug that stops being reportable.
|
|
|
|
## When it is solved
|
|
|
|
Delete all of it: `reportSuspiciousEdit`, `EDITOR_TRAP_KEY`, its call in the `onChange` handler, the entry
|
|
in `TODO.md` → Known bugs, and this file. It is instrumentation for one bug, and instrumentation nobody
|
|
remembers the reason for is worse than none.
|