From a871c055bf76293ed66a004ca75204e41b4f1eba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Sat, 15 Aug 2026 23:00:18 +0000 Subject: [PATCH] fix: one empty write and the editor could never save its tabs again MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported not working, and the database said exactly why: {"editor":{"p-1786832482923-77":{"open":[],"active":null}}} The write worked. It wrote EMPTY, on the first mount before any file was open, and from then on the panel was locked: - an empty saved value is truthy - the restore effect bailed on `open.length === 0` WITHOUT marking itself done - the write guard was `if (restored && !didRestore.current) return` so every later save was blocked by a restore that had already decided not to run. Self-inflicted, and the guard was mine from two commits ago. Underneath it was a second ordering bug that would have bitten even with the first fixed: the restore is ASYNC — it reads each file — while the write effect runs on the same mount, sees an empty `files`, and persists over the list it is in the middle of reopening. Marking the restore done synchronously would just have moved the failure. Both are now one flag. `restoreDone` is state, set when the restore COMPLETES — including when it completes by having nothing to do — and the write waits on it. Nothing is persisted until the restore is genuinely finished. Cleared the poisoned `editor` bag from the live row so the next test starts clean rather than against state the old code had already wedged. Worth naming the pattern: the guard conflated "there is saved state" with "a restore is pending". Those differ exactly when the saved state is empty, which is the state every panel starts in — so the bug was guaranteed rather than unlucky. tsgo clean, frontend builds, 846 pass / 7 fail unchanged. Co-Authored-By: Claude Opus 5 --- .../src/apps/CodeEditor/CodeEditor.tsx | 47 +++++++++++-------- 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/src/workspaces/officerdev/src/apps/CodeEditor/CodeEditor.tsx b/src/workspaces/officerdev/src/apps/CodeEditor/CodeEditor.tsx index 079dfcc4..9ca79226 100644 --- a/src/workspaces/officerdev/src/apps/CodeEditor/CodeEditor.tsx +++ b/src/workspaces/officerdev/src/apps/CodeEditor/CodeEditor.tsx @@ -98,49 +98,56 @@ export const CodeEditorView = ({ }, [activePath, files]); /** - * Reopen what this panel had open before the reload. + * Reopen what this panel had open before the reload, then start remembering. * - * Sequential rather than parallel, because the order of the tabs is the order you left them in and - * `openFile` appends. Each path is read fresh; one that no longer reads is skipped, so a file deleted - * while the page was closed drops out of the strip instead of coming back as an empty buffer. + * `restoreDone` is a state flag rather than a ref, and both halves depend on it, because the two ways + * this goes wrong are both about ORDER: * - * `active` is applied at the end and only if it actually opened — otherwise the last surviving tab wins, - * which is what `closeFile` does when the active tab goes. + * 1. Writing before the restore finishes clobbers the thing being restored. The restore is async — it + * reads each file — while the write effect runs on the same mount, sees an empty `files`, and + * persists `{open: [], active: null}` over the list it was about to reopen. + * + * 2. Treating "nothing to restore" as "restore still pending" locks the panel forever. That is the bug + * this replaces: an empty saved value is TRUTHY, the restore bailed on `open.length === 0` without + * marking itself done, and the write guard then blocked every subsequent save. One empty write and + * the panel could never record anything again — which is exactly what the database showed. + * + * So the flag is set when the restore COMPLETES, including when it completes by having nothing to do. + * Sequential opens, because tab order is the order you left them in and `openFile` appends. */ - const didRestore = useRef(false); + const [restoreDone, setRestoreDone] = useState(false); + const startedRestore = useRef(false); + useEffect(() => { - if (didRestore.current || !restored || restored.open.length === 0) return; - didRestore.current = true; + if (startedRestore.current) return; + startedRestore.current = true; let alive = true; void (async () => { + const wanted = restored?.open ?? []; const opened: string[] = []; - for (const path of restored.open) { + for (const path of wanted) { try { const res = await readFileRef.current(path); if (!alive) return; openFile(path, path.split('/').pop() ?? path, res.content); opened.push(path); } catch { - // Gone or unreadable — leave it out rather than open an empty tab named after it. + // Gone or unreadable — leave it out rather than open an empty tab named after a missing file. } } - if (alive && restored.active && opened.includes(restored.active)) setActivePath(restored.active); + if (!alive) return; + if (restored?.active && opened.includes(restored.active)) setActivePath(restored.active); + setRestoreDone(true); })(); return () => { alive = false; }; }, [restored]); - /** - * Write the strip back whenever it changes. - * - * Guarded on the restore having run, or the first render — with no tabs open yet — would persist an - * empty list over the very state it is about to read. - */ useEffect(() => { - if (restored && !didRestore.current) return; + if (!restoreDone) return; remember({ open: files.map((f) => f.path), active: activePath }); - }, [files, activePath]); + }, [files, activePath, restoreDone]); /** * Re-read the disk: the tree, and every open file that has no unsaved edits.