fix(session): stop startup from throwing away tabs it could not read - #401
Merged
PathGao merged 1 commit intoAug 2, 2026
Merged
Conversation
Three ways a restore lost documents, all of them permanent because the trimmed result was written straight back to the snapshot. - A single failed read evicted the tab. A network share not yet mounted, an external drive not plugged in, a file briefly locked - the tab was gone, and plugging the drive back in did not bring it back. The tab now stays, with its buffer marked through the existing `isTruncated` flag: every writer already refuses such a buffer, and `ensureFullContent` already re-reads and clears it, so the tab heals itself the next time it is opened. A dirty buffer is never marked - unsaved text outranks a failed read of its file. - An interrupted restore deleted the whole snapshot. The sftwrdotdev#260 breadcrumb recorded that a restore was running, not what it was running, so the only available response was collective punishment. It now records the document it was on, so the next launch defers that one and restores everything else. After three interruptions startup restores the tab list without reading any file, which is a stable end state that loses nothing. A deferred path is released once Markpad has read it successfully - a quarantine with no exit is a permanent loss on a longer timescale. - The `'HOME'` sentinel was written into the snapshot, because the filter tested `path !== ''` while `hasRealFilePath()` - used everywhere else - tests for both. Reading it back invoked `read_file_content('HOME')`, which threw, which took the first path above. Both sides now use `hasRealFilePath`; the read side is required because snapshots already on disk contain it. `restore()` no longer deletes the snapshot anywhere, including its outer catch. `discardPersistedState` survives for explicit exit only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Three ways a restore lost documents. All three were permanent, because the trimmed result was written straight back to the snapshot — the failure of one launch became the state of every launch after it.
1. One failed read evicted the tab
The per-tab
catchcalleddropRestoredTab(wired tocloseTab), and the loop then re-serialised. A network share not yet mounted, an external drive not plugged in, a file briefly locked by another program — the tab was gone, and plugging the drive back in did not bring it back.The tab now stays, with its path, and its empty buffer is marked through the existing
isTruncatedflag rather than a new state. That choice is the point:isTruncatedalready means "this buffer is not the whole document", and every writer already refuses it —saveContent/saveContentAsbail out,canTransfer/canDetachrefuse. A new flag would need each of those to learn about it, which is one chance per call site to forget, and the cost of forgetting is an empty buffer written over the user's file.ensureFullContentalready re-reads the file and clears the flag when the tab is opened for editing, so the tab heals itself once the drive is back. No new recovery machinery.documentSession.loadMarkdown: a read failure keeps the buffer rather than closing the tab.A dirty buffer is never marked — unsaved text outranks a failed read of its file.
2. An interrupted restore deleted the whole snapshot
Any truthy
restoreInProgressKeytriggereddiscardPersistedState(), which clears both localStorage keys and invokesclear_window_state. The whole session record, gone.The #260 breadcrumb was right in intent and too coarse in shape: it recorded that a restore was running, not what it was running, so the only response available was collective punishment. It now records:
pendingis written before each document's read and cleared after it, so a crash leaves the breadcrumb sitting on the culprit. The next launch defers that one path and restores everything else.persistState()drops any deferred path whose tab now holds real content — Markpad read it successfully at some point, so it is not the problem any more. Without this, one bad startup would cost that file automatic restore forever; a quarantine with no exit is a permanent loss on a longer timescale.'true'value fails JSON parsing and reads as "interrupted, no suspect" — it costs one retry instead of the session.restore()no longer deletes the snapshot anywhere, including its outer catch, which now only reports.discardPersistedStatesurvives for explicit exit — the user's own choice.3. The
'HOME'sentinel was written into the snapshotserializeStatefilteredt.path !== '', whileaddHomeTabsetspath: 'HOME'.tabFileActions.hasRealFilePath()tests for both, and is used everywhere except here — two spellings of "is this a real file", one of them wrong.Reading it back invoked
read_file_content('HOME'), which threw, which took path 1 above. A window whose only tab was HOME restored empty.Both sides now use
hasRealFilePath. The read side is not optional: snapshots already on users' disks contain the sentinel. A third guard in the restore loop means'HOME'cannot reach the backend by any route.These three ship together deliberately. Fix 1 without fix 3 would turn a legacy HOME entry from something silently dropped into a permanently unreadable phantom tab that re-persists every launch — a regression. They are the same failure: startup losing tabs it should have kept.
Tests
16 new tests across two files, executing the real code — both
tabs.svelte.tsandwindowSession.svelte.tsimport cleanly undernode --test --import tsxwith the rune stubs the repo already uses intruncatedBufferGuard.test.ts, plus a stubbed__TAURI_INTERNALS__andlocalStorage.sessionRestoreResilience.test.tsdrivescreateWindowSession().restore()against a fake disk and a fake snapshot store, then asserts on the tab list, the persisted snapshot, and the breadcrumb.Counter-proof, source files stashed and tests kept:
master+ these testsNamed failures include: a file that cannot be read keeps its tab · the failed read is not written back into the snapshot · an interrupted restore keeps the snapshot instead of deleting it · an interrupted restore names the document it was on · a deferred document is released once Markpad has read it · a breadcrumb from an older build no longer wipes the session · a session snapshot never carries the home tab · a HOME entry in an older snapshot is never read as a file.
Two existing test files asserted the old behaviour verbatim (
filter((t) => t.path !== ''),dropRestoredTab(tab.id),restoreInProgressKey … discardPersistedState()). They could not both stay and the bugs be fixed; only the assertions naming the old behaviour were rewritten, every unrelated assertion kept, each pointed at the behavioural test that now covers it.Not covered
ensureFullContentand saving is refused — but the tab looks like an empty document. A "couldn't read this — retry" affordance needsMarkdownViewer.svelteand a new i18n key; deliberately out of scope here.MAX_INTERRUPTIONS = 3andMAX_DEFERRED = 8are judgement calls, not measurements. Chrome and Firefox handle the same situation by asking the user after a crashed session, which is the better answer and needs UI./notes/A.mdand/notes/a.mdare distinct. Same limitationrefuseIfLossilyDecodedalready documents; closing it needs backend canonicalisation.🤖 Generated with Claude Code