fix(documents): read through the checked command everywhere, and stop the auto-save toast repeating

scripts/checkedReadMigration.test.ts

@@ -0,0 +1,223 @@
+import assert from 'node:assert/strict';
+import { readFileSync } from 'node:fs';
+import test from 'node:test';
+
+// Runes and the Tauri bridge, shimmed the way truncatedBufferGuard.test.ts
+// shims them: the stores are runes modules, and Node's test runner gives every
+// file its own process, so this cannot leak into another suite.
+const g = globalThis as any;
+const runeEffect = (fn: () => void) => {
+	void fn;
+};
+runeEffect.root = (fn: () => unknown) => fn();
+g.$state = (value: unknown) => value;
+g.$state.raw = (value: unknown) => value;
+g.$state.snapshot = (value: unknown) => value;
+g.$derived = (value: unknown) => value;
+g.$derived.by = (fn: () => unknown) => fn();
+g.$effect = runeEffect;
+g.window = g.window ?? {};
+Object.defineProperty(g, 'navigator', { value: { language: 'en-US' }, configurable: true });
+Object.defineProperty(g, 'localStorage', {
+	value: { getItem: () => null, setItem: () => {}, removeItem: () => {}, clear: () => {} },
+	configurable: true,
+});
+
+/*
+ * #379 made every read that fills a WRITABLE buffer report whether the decode
+ * was lossy, so the tab can refuse to write U+FFFD back over a file that was
+ * merely in another encoding. Three reads kept the bare command:
+ * `ensureFullContent`, and the two in MarkdownViewer that fill the editor and
+ * the split pane. They were safe — but only because each re-read a file whose
+ * tab `loadMarkdown` had already flagged. That is an invariant held by call
+ * sites agreeing with each other, and the failure mode when one stops agreeing
+ * is a destroyed document. It is now held by the code: every one of them reads
+ * the fidelity itself.
+ *
+ * `ensureFullContent` is exercised for real below. The two MarkdownViewer sites
+ * are in a Svelte component and are asserted against its source: those tests
+ * establish that the checked command is called and its verdict stored, not that
+ * the store then behaves — `lossyDecodeSaveGuard.test.ts` covers that.
+ *
+ * The second half of this file is the toast the guard's refusal used to
+ * trigger every 1.5 seconds.
+ */
+
+const PARTIAL = 'first half';
+const FULL = 'first half and the rest';
+
+/** Set per test. `get_os_type` is the settings store booting on import. */
+let handleInvoke: (cmd: string, args: Record<string, unknown>) => unknown = (cmd) => {
+	if (cmd === 'get_os_type') return 'macos';
+	throw new Error(`unexpected invoke: ${cmd}`);
+};
+const errors: string[] = [];
+
+g.window.__TAURI_INTERNALS__ = {
+	invoke: (command: string, args: Record<string, unknown>) =>
+		Promise.resolve(handleInvoke(command.replace(/^plugin:[^|]*\|/, ''), args ?? {})),
+	transformCallback: (fn: unknown) => fn,
+};
+
+const { tabManager } = await import('../src/lib/stores/tabs.svelte.js');
+const { createDocumentSession } = await import('../src/lib/sessions/documentSession.svelte.js');
+
+function makeSession() {
+	return createDocumentSession({
+		setShowHome: () => {},
+		currentFile: () => tabManager.activeTab?.path ?? '',
+		resetScrollHistory: () => {},
+		renderMarkdown: async () => '<p>x</p>',
+		afterLoad: async () => {},
+		saveRecentFile: () => {},
+		deleteRecentFile: () => {},
+		setLoadingTabs: () => {},
+		measureInitialViewport: () => {},
+		isScrolling: () => false,
+		renderRichContent: () => {},
+		onError: (message) => errors.push(message),
+		selfWriteGraceMs: 400,
+		cancelPendingAutoSave: () => {},
+		askClose: async () => 'discard' as const,
+		onCloseSaveNewerEdits: () => {},
+		onCloseAutoSaveFailed: () => {},
+	});
+}
+
+/** Open a >50KB file and leave the background full read pending forever. */
+async function openPartial() {
+	tabManager.closeAll();
+	errors.length = 0;
+	handleInvoke = (cmd) => {
+		if (cmd === 'open_markdown_preview') return ['<p>preview</p>', PARTIAL, false, false];
+		if (cmd === 'read_file_content_checked') return new Promise(() => {});
+		throw new Error(`unexpected invoke: ${cmd}`);
+	};
+	const session = makeSession();
+	await session.loadMarkdown('/docs/big.md');
+	const tab = tabManager.activeTab!;
+	assert.equal(tab.isTruncated, true, 'precondition: the buffer is partial');
+	assert.equal(tab.hasReplacementChars, false, 'precondition: the preview decoded cleanly');
+	return { session, tab };
+}
+
+test('completing a partial buffer carries the tail\'s own verdict', async () => {
+	// The case the old comment called safe: the preview covered the first 50KB
+	// and decoded cleanly, but a file can be valid UTF-8 up to there and not
+	// after. With the bare command the tab kept the preview's verdict and the
+	// next auto-save wrote U+FFFD over the file.
+	const { session, tab } = await openPartial();
+
+	handleInvoke = (cmd) => {
+		if (cmd === 'read_file_content_checked') return [FULL, true];
+		throw new Error(`unexpected invoke: ${cmd}`);
+	};
+
+	assert.equal(await session.ensureFullContent(tab.id), true);
+	assert.equal(tab.rawContent, FULL);
+	assert.equal(tab.hasReplacementChars, true, 'the completed buffer must carry its own fidelity');
+});
+
+test('completing a clean tail also clears a stale flag', async () => {
+	// The same mechanism in the other direction: a verdict that is decided on
+	// every read cannot go stale.
+	const { session, tab } = await openPartial();
+	tabManager.setTabDecodedLossy(tab.id, true);
+
+	handleInvoke = (cmd) => {
+		if (cmd === 'read_file_content_checked') return [FULL, false];
+		throw new Error(`unexpected invoke: ${cmd}`);
+	};
+
+	assert.equal(await session.ensureFullContent(tab.id), true);
+	assert.equal(tab.hasReplacementChars, false);
+});
+
+test('the completed buffer is refused or accepted according to that verdict', async () => {
+	// End to end: the flag is not decoration, it decides the write.
+	const { session, tab } = await openPartial();
+	handleInvoke = (cmd) => {
+		if (cmd === 'read_file_content_checked') return [FULL, true];
+		if (cmd === 'save_file_content') return null;
+		throw new Error(`unexpected invoke: ${cmd}`);
+	};
+	await session.ensureFullContent(tab.id);
+
+	assert.equal(await session.saveContent(tab.id), false, 'a lossy buffer must not overwrite its file');
+	assert.equal(session.isLossySaveRefused(tab.id), true);
+	assert.ok(errors.length > 0, 'and the user is told once');
+});
+
+// --- the two component call sites -------------------------------------------
+
+const viewer = readFileSync('src/lib/MarkdownViewer.svelte', 'utf8');
+
+test('no writable buffer in the app is filled by the unchecked command', () => {
+	for (const [name, source] of [
+		['MarkdownViewer.svelte', viewer],
+		['documentSession.svelte.ts', readFileSync('src/lib/sessions/documentSession.svelte.ts', 'utf8')],
+		['windowSession.svelte.ts', readFileSync('src/lib/sessions/windowSession.svelte.ts', 'utf8')],
+	] as const) {
+		assert.doesNotMatch(source, /invoke\('read_file_content'/, `${name} still uses the unchecked command`);
+	}
+});
+
+test('entering the editor reads the fidelity and stores it', () => {
+	const toggle = viewer.slice(viewer.indexOf('async function toggleEdit'), viewer.indexOf('async function saveContent'));
+	assert.match(toggle, /\[content, lossy\] = \(await invoke\('read_file_content_checked', \{ path: tab\.path \}\)\)/);
+	const read = toggle.indexOf('read_file_content_checked');
+	const flag = toggle.indexOf('setTabDecodedLossy(tab.id, lossy)');
+	const store = toggle.indexOf('setTabRawContent(tab.id, content)');
+	assert.notEqual(flag, -1, 'the verdict must reach the tab');
+	assert.ok(read < flag && flag < store, 'flag the tab before the buffer is published');
+});
+
+test('entering split view reads the fidelity and stores it', () => {
+	const split = viewer.slice(viewer.indexOf('async function toggleSplitView'));
+	const enter = split.slice(0, split.indexOf('} else {'));
+	assert.match(enter, /\[content, lossy\] = \(await invoke\('read_file_content_checked', \{ path: tab\.path \}\)\)/);
+	const flag = enter.indexOf('setTabDecodedLossy(tab.id, lossy)');
+	const store = enter.indexOf('setTabRawContent(tab.id, content)');
+	assert.notEqual(flag, -1, 'the verdict must reach the tab');
+	assert.ok(flag < store, 'flag the tab before the buffer is published');
+});
+
+// --- the repeating toast ------------------------------------------------------
+
+test('the session can tell a refusal from a failure', () => {
+	// `saveContent` returns false for both, which is why the auto-save timer
+	// could not tell them apart.
+	assert.match(
+		readFileSync('src/lib/sessions/documentSession.svelte.ts', 'utf8'),
+		/function isLossySaveRefused\(tabId: string\): boolean \{\s*return lossySaveWarnedTabs\.has\(tabId\);/,
+	);
+});
+
+test('a refused save does not add a generic toast to its own explanation', () => {
+	const effect = viewer.slice(viewer.indexOf('Auto-save effect.'));
+	const body = effect.slice(0, effect.indexOf('for (const id of ['));
+	const check = body.indexOf('if (documentSession.isLossySaveRefused(s.id)) return;');
+	const toast = body.indexOf("t('toast.autoSaveFailed'");
+	assert.notEqual(check, -1, 'the refusal must be recognised');
+	assert.notEqual(toast, -1);
+	assert.ok(check < toast, 'and recognised before the generic toast is raised');
+});
+
+test('a tab that can only be refused stops re-arming the timer', () => {
+	// Auto-save re-arms on every keystroke. Without this the guard was reached
+	// again every 1.5s for as long as the user kept typing — each time
+	// producing the console warning, the wasted round trip, and (before the
+	// test above) the toast.
+	const effect = viewer.slice(viewer.indexOf('Auto-save effect.'));
+	const body = effect.slice(0, effect.indexOf('for (const id of ['));
+	assert.match(body, /decodedLossily: tab\.hasReplacementChars/);
+	assert.match(body, /const eligible = [^;]*!\(s\.decodedLossily && documentSession\.isLossySaveRefused\(s\.id\)\)/);
+	// The FIRST attempt must still happen: it is what produces the explanation.
+	// `isLossySaveRefused` is false until the guard has spoken, so eligibility
+	// only drops afterwards — and "Save As" to a new file clears
+	// `hasReplacementChars`, which restores it.
+	assert.match(
+		readFileSync('src/lib/sessions/documentSession.svelte.ts', 'utf8'),
+		/lossySaveWarnedTabs\.delete\(tab\.id\)/,
+	);
+});

scripts/lossyDecodeSaveGuard.test.ts

@@ -93,13 +93,20 @@ test('the editable-pane shortcut reports fidelity too', () => {
 		body,
 		/if \(initialIsEditing \|\| initialIsSplit\) \{\s*\[content, lossy\] = \(await invoke\('read_file_content_checked'/,
 	);
-	// `ensureFullContent` is the one place the bare command survives, and only
-	// because it re-reads a file whose tab loadMarkdown already flagged.
+	// `ensureFullContent` was the one place the bare command survived, and only
+	// because it re-read a file whose tab loadMarkdown had already flagged —
+	// an invariant enforced by two call sites agreeing rather than by the code.
+	// It now reads the fidelity itself, so no writable buffer in this file is
+	// filled by a command that cannot report one.
 	const bare = session.match(/invoke\('read_file_content'/g)?.length ?? 0;
-	assert.equal(bare, 1, 'only ensureFullContent may use the unchecked command');
+	assert.equal(bare, 0, 'no writable buffer may be filled by the unchecked command');
 	assert.match(
 		slice(session, 'async function ensureFullContent', 'const lossySaveWarnedTabs'),
-		/invoke\('read_file_content'/,
+		/invoke\('read_file_content_checked'/,
+	);
+	assert.match(
+		slice(session, 'async function ensureFullContent', 'const lossySaveWarnedTabs'),
+		/setTabDecodedLossy\(tabId, lossy\)/,
 	);
 });
 

scripts/truncatedBufferGuard.test.ts

@@ -134,7 +134,7 @@ test('completing a partial buffer replaces it with the whole file and unblocks s
 	const { session, tab } = await openPartial();
 
 	handleInvoke = (cmd) => {
-		if (cmd === 'read_file_content') return FULL;
+		if (cmd === 'read_file_content_checked') return [FULL, false];
 		if (cmd === 'save_file_content') return null;
 		throw new Error(`unexpected invoke: ${cmd}`);
 	};
@@ -171,7 +171,7 @@ test('a partial buffer that already carries edits is never silently discarded',
 	tabManager.updateTabRawContent(tab.id, `${PARTIAL}edited`);
 
 	handleInvoke = (cmd) => {
-		if (cmd === 'read_file_content') return FULL;
+		if (cmd === 'read_file_content_checked') return [FULL, false];
 		throw new Error(`unexpected invoke: ${cmd}`);
 	};
 
@@ -220,7 +220,7 @@ test('toggling a task checkbox completes the buffer before writing it', async ()
 	assert.equal(tab.rawContent, partial);
 
 	handleInvoke = (cmd) => {
-		if (cmd === 'read_file_content') return doc;
+		if (cmd === 'read_file_content_checked') return [doc, false];
 		if (cmd === 'save_file_content') return null;
 		throw new Error(`unexpected invoke: ${cmd}`);
 	};

src/lib/MarkdownViewer.svelte

@@ -1748,7 +1748,14 @@ import { createDocumentSession, type LoadMarkdownOptions } from './sessions/docu
 					tab.isEditing = true;
 				} else {
 					try {
-						const content = (await invoke('read_file_content', { path: tab.path })) as string;
+						// This buffer is about to be handed to the editor, and
+						// the editor arms auto-save. The checked command is what
+						// says whether the decode was lossy, so the tab carries
+						// its own verdict instead of relying on the one
+						// `loadMarkdown` left behind — a file can be converted
+						// to UTF-8 (or away from it) between the two reads.
+						const [content, lossy] = (await invoke('read_file_content_checked', { path: tab.path })) as [string, boolean];
+						tabManager.setTabDecodedLossy(tab.id, lossy);
 						// Goes through the store so a tab that held only the
 						// large-file preview slice stops being flagged partial.
 						tabManager.setTabRawContent(tab.id, content);
@@ -1862,6 +1869,10 @@ import { createDocumentSession, type LoadMarkdownOptions } from './sessions/docu
 			// Reactive too: clearing a conflict re-arms the timer on the next
 			// keystroke, so answering the bar resumes normal auto-save.
 			hasPendingConflict: externalChangeConflicts[tab.id] === true,
+			// Reactive as well, and that is the point: "Save As" to a new file
+			// clears the flag, so the tab becomes eligible again on the next
+			// pass without anything having to remember to re-arm it.
+			decodedLossily: tab.hasReplacementChars,
 		}));
 
 		untrack(() => {
@@ -1878,7 +1889,13 @@ import { createDocumentSession, type LoadMarkdownOptions } from './sessions/docu
 				// through: pressing Save IS the answer "keep mine", and the
 				// `saveContent` wrapper clears the conflict so the bar comes
 				// down instead of re-asking.
-				const eligible = s.isDirty && s.path !== '' && s.editable && !s.hasPendingConflict;
+				// A tab whose buffer was decoded lossily is dropped once the
+				// guard has refused it and said why. The first attempt is what
+				// produces that explanation, so it is deliberately allowed
+				// through; re-arming after it would only reach the same refusal
+				// every 1.5s for as long as the user keeps typing.
+				const eligible = s.isDirty && s.path !== '' && s.editable && !s.hasPendingConflict
+					&& !(s.decodedLossily && documentSession.isLossySaveRefused(s.id));
 				const prevRef = lastContentRefByTab.get(s.id);
 				const refChanged = prevRef !== s.contentRef;
 
@@ -1915,6 +1932,10 @@ import { createDocumentSession, type LoadMarkdownOptions } from './sessions/docu
 						(ok) => {
 							if (!ok) {
 								console.error('Auto-save failed for tab', s.id);
+								// A refusal already explained itself, naming the
+								// file and the way out. "Auto-save failed" on top
+								// of that says nothing new.
+								if (documentSession.isLossySaveRefused(s.id)) return;
 								addToast(
 									t('toast.autoSaveFailed', settings.language),
 									'error',
@@ -2368,7 +2389,11 @@ import { createDocumentSession, type LoadMarkdownOptions } from './sessions/docu
 			// why the same bug never reached the full editor.
 			if (tab.path && !tab.isEditing && !tab.rawContent) {
 				try {
-					const content = (await invoke('read_file_content', { path: tab.path })) as string;
+					// Checked, like every other read that fills an editable
+					// buffer: split view is an editor, so this tab must carry
+					// the fidelity of its own decode rather than inherit one.
+					const [content, lossy] = (await invoke('read_file_content_checked', { path: tab.path })) as [string, boolean];
+					tabManager.setTabDecodedLossy(tab.id, lossy);
 					tabManager.setTabRawContent(tab.id, content);
 				} catch (e) {
 					console.error('Failed to load raw content for split view', e);

src/lib/sessions/documentSession.svelte.ts

@@ -116,12 +116,15 @@ export function createDocumentSession(options: DocumentSessionOptions) {
 		if (!tab.path) return true;
 		if (tab.isDirty) return false;
 		try {
-			// Safe on the bare command: this only re-reads a file whose tab was
-			// already flagged by `loadMarkdown`, and `setTabRawContent` does not
-			// clear that flag. Migrating it to `read_file_content_checked` is
-			// still worth doing — see the note on the two MarkdownViewer call
-			// sites — but nothing depends on it today.
-			const full = (await invoke('read_file_content', { path: tab.path })) as string;
+			// Checked, like every other read that ends in a writable buffer.
+			// The bare command was safe here only because this re-reads a file
+			// whose tab `loadMarkdown` had already flagged — an invariant held
+			// by two call sites agreeing, not by anything in the code. Reading
+			// the fidelity again costs nothing and makes the flag a property of
+			// the buffer instead of a memory of how it was obtained; it also
+			// CLEARS the flag for a file converted to UTF-8 since the load.
+			const [full, lossy] = (await invoke('read_file_content_checked', { path: tab.path })) as [string, boolean];
+			tabManager.setTabDecodedLossy(tabId, lossy);
 			tabManager.setTabRawContent(tabId, full);
 			return true;
 		} catch (error) {
@@ -168,6 +171,22 @@ export function createDocumentSession(options: DocumentSessionOptions) {
 		return true;
 	}
 
+	/**
+	 * True once this tab has been told, in words, that its buffer cannot be
+	 * written back over its own file.
+	 *
+	 * The explanation is deduplicated per tab above, but the callers' own
+	 * failure reporting was not: `saveContent` returns `false` for a refusal
+	 * exactly as it does for a failed write, so the auto-save timer added its
+	 * generic "auto-save failed" on top — and, because auto-save re-arms on
+	 * every keystroke, repeated it every 1.5s for as long as the user kept
+	 * typing. A refusal is not a failure to report again; it is a standing
+	 * condition the user has already been told about and given an exit from.
+	 */
+	function isLossySaveRefused(tabId: string): boolean {
+		return lossySaveWarnedTabs.has(tabId);
+	}
+
 	function updateLoading(tabId: string, loading: boolean) {
 		if (loading) loadingTabs.add(tabId);
 		else loadingTabs.delete(tabId);
@@ -418,5 +437,5 @@ export function createDocumentSession(options: DocumentSessionOptions) {
 		return true;
 	}
 
-	return { loadMarkdown, saveContent, saveContentAs, toggleTaskCheckbox, shouldReloadExternalChange, resolveExternalChange, ensureFullContent, canCloseTab };
+	return { loadMarkdown, saveContent, saveContentAs, toggleTaskCheckbox, shouldReloadExternalChange, resolveExternalChange, ensureFullContent, canCloseTab, isLossySaveRefused };
 }