fix(close): acknowledge document save before session snapshot

scripts/close-dialog-smoke-runner.mjs

@@ -54,6 +54,16 @@ async function replaceEditorText(win, content) {
   }, 60_000);
 }
 
+async function appendEditorText(win, marker) {
+  await waitFor('editor', () => win.webContents.executeJavaScript(`Boolean(document.querySelector('.cm-content'))`));
+  await win.webContents.executeJavaScript(`document.querySelector('.cm-content')?.focus(); true`);
+  win.focus();
+  win.webContents.focus();
+  await win.webContents.executeJavaScript(`document.querySelector('.cm-content')?.focus(); true`);
+  win.webContents.insertText(`\n${marker}\n`);
+  await waitFor('CodeMirror append', async () => (await editorText(win)).includes(marker), 60_000);
+}
+
 function createFixture(userData) {
   const doc = join(userData, 'close-smoke.md');
   const secondDoc = join(userData, 'close-smoke-second.md');
@@ -68,6 +78,17 @@ function createFixture(userData) {
   return { doc, secondDoc, largeDoc };
 }
 
+function assertPersistedDocument(filePath, expected) {
+  const actual = readFileSync(filePath, 'utf8');
+  if (expected.kind === 'large') {
+    if (!actual.includes(expected.marker) || actual.length <= expected.originalLength) {
+      throw new Error(`large save did not persist the edited marker for ${filePath}`);
+    }
+    return;
+  }
+  if (actual !== expected.content) throw new Error(`save did not persist ${filePath}`);
+}
+
 async function worker() {
   const { app, BrowserWindow, Menu } = electron;
   require(resolve(REPO, 'dist/main/main.js'));
@@ -79,18 +100,27 @@ async function worker() {
   const dirtyWindows = base.startsWith('quit')
     ? await waitFor('two windows', () => BrowserWindow.getAllWindows().filter((candidate) => !candidate.isDestroyed()).length === 2 ? BrowserWindow.getAllWindows() : null)
     : [win];
+  const expectedByPath = new Map();
   await Promise.all(dirtyWindows.map((dirtyWindow) => waitFor('editor', () =>
     dirtyWindow.webContents.executeJavaScript(`Boolean(document.querySelector('.cm-content'))`),
   )));
-  for (const dirtyWindow of dirtyWindows) {
+  if (scenario.endsWith('-large')) {
+    await Promise.all(dirtyWindows.map((dirtyWindow) => waitFor('large document content', async () =>
+      (await editorText(dirtyWindow)).includes('# Close smoke large'),
+    )));
+  }
+  for (const [index, dirtyWindow] of dirtyWindows.entries()) {
+    const marker = `close smoke ${scenario} ${dirtyWindow.id}`;
+    const path = index === 0 ? documentPath : secondDocumentPath;
+    const originalLength = scenario.endsWith('-large') ? readFileSync(path, 'utf8').length : null;
     if (scenario.endsWith('-large')) {
-      dirtyWindow.webContents.insertText(`\nclose smoke ${scenario} ${dirtyWindow.id}\n`);
+      await appendEditorText(dirtyWindow, marker);
+      expectedByPath.set(path, { kind: 'large', marker, originalLength });
     } else {
-      dirtyWindow.webContents.sendInputEvent({ type: 'keyDown', keyCode: 'A', modifiers: ['meta'] });
-      dirtyWindow.webContents.sendInputEvent({ type: 'keyUp', keyCode: 'A', modifiers: ['meta'] });
-      await delay(30);
-      dirtyWindow.webContents.insertText(`close smoke ${scenario} ${dirtyWindow.id}`);
+      await replaceEditorText(dirtyWindow, marker);
+      expectedByPath.set(path, { kind: 'exact', content: marker });
     }
+    if (!(await editorText(dirtyWindow)).includes(marker)) throw new Error(`${scenario} edit marker was not applied`);
   }
   await delay(100);
 
@@ -120,12 +150,27 @@ async function worker() {
     console.log('[close-dialog-smoke] quit-discard-closed-two-dirty-windows');
     return;
   }
+  if (base === 'quit-save') {
+    const closed = new Promise((resolveClosed) => app.once('window-all-closed', resolveClosed));
+    app.quit();
+    await Promise.race([
+      closed,
+      delay(20_000).then(() => { throw new Error('quit save did not close both dirty windows'); }),
+    ]);
+    for (const [path, expected] of expectedByPath) assertPersistedDocument(path, expected);
+    console.log('[close-dialog-smoke] quit-save-closed-two-dirty-windows');
+    return;
+  }
   const closed = new Promise((resolveClosed) => win.once('closed', resolveClosed));
   win.close();
   await Promise.race([
     closed,
     delay(20_000).then(() => { throw new Error(`${scenario} did not close the window`); }),
   ]);
+  if (base === 'save') {
+    const expected = expectedByPath.get(documentPath);
+    if (expected != null) assertPersistedDocument(documentPath, expected);
+  }
   console.log(`[close-dialog-smoke] ${scenario}-closed-window`);
   app.exit(0);
 }
@@ -282,21 +327,21 @@ if (scenario) {
 } else {
   void (async () => {
     if (!existsSync(resolve(REPO, 'dist/main/main.js'))) throw new Error('dist/main/main.js missing; build the app before smoke execution');
-    // The legacy seven scenarios intentionally share ONE userData in their
+    // The legacy scenarios intentionally share ONE userData in their
     // historical order, exactly as before the shutdown pairs were added: a
     // per-scenario cold userData exposes a pre-existing cold-start save-close
     // delay (>20s on some machines) that made `save`/`save-large` flaky.
     // Isolation is only required between the legacy block and the shutdown
     // pairs below, which each use their own pair-local userData.
     const userData = mkdtempSync(join(tmpdir(), 'notepad-ai-close-smoke-'));
     const { doc, secondDoc, largeDoc } = createFixture(userData);
-    for (const choice of ['discard', 'save', 'cancel', 'quit-cancel', 'quit-discard', 'discard-large', 'save-large']) {
+    for (const choice of ['discard', 'save', 'cancel', 'quit-cancel', 'quit-discard', 'quit-save', 'discard-large', 'save-large']) {
       const base = choice.replace(/-large$/, '');
       await waitForWorker(choice, {
         NOTEPAD_AI_CLOSE_SMOKE_SCENARIO: choice,
         NOTEPAD_AI_CLOSE_SMOKE_DOCUMENT: choice.endsWith('-large') ? largeDoc : doc,
         NOTEPAD_AI_CLOSE_SMOKE_SECOND_DOCUMENT: secondDoc,
-        NOTEPAD_AI_CLOSE_DIALOG_CHOICE: base === 'quit-discard' ? 'discard' : base === 'quit-cancel' ? 'cancel' : base,
+        NOTEPAD_AI_CLOSE_DIALOG_CHOICE: base === 'quit-discard' ? 'discard' : base === 'quit-cancel' ? 'cancel' : base === 'quit-save' ? 'save' : base,
         NOTEPAD_AI_USERDATA: userData,
       });
       console.log(`[close-dialog-smoke] ${choice}=PASS`);

src/__tests__/app-windows-discard.test.ts

@@ -91,7 +91,12 @@ const electron = vi.hoisted(() => {
   };
 });
 
+const appLog = vi.hoisted(() => ({
+  logWarn: vi.fn(),
+}));
+
 vi.mock('electron', () => electron);
+vi.mock('../main/app-log', () => appLog);
 
 import type { WindowRecord } from '../main/window-registry';
 
@@ -203,7 +208,10 @@ async function setup(
 }
 
 describe('discard close IPC waiters', () => {
-  beforeEach(() => electron.reset());
+  beforeEach(() => {
+    electron.reset();
+    appLog.logWarn.mockReset();
+  });
   it('approves a save only after a fresh matching post-save revision', async () => {
     let stateQueries = 0;
     const commitQuitSession = vi.fn(async () => {});
@@ -508,6 +516,23 @@ describe('discard close IPC waiters', () => {
       vi.useRealTimers();
     }
   });
+  it('logs a renderer state timeout while keeping close approval denied', async () => {
+    vi.useFakeTimers();
+    try {
+      const { appWindows } = await setup(() => {}, async () => 'cancel', 1, () => true, async () => {}, async () => {}, async () => {}, () => {});
+      const approval = appWindows.approveAllForQuit('quit');
+
+      await vi.advanceTimersByTimeAsync(400);
+      await expect(approval).resolves.toBe(false);
+      expect(appLog.logWarn).toHaveBeenCalledWith(
+        'close',
+        'renderer state response timed out',
+        expect.objectContaining({ webContentsId: 1001, timeoutMs: 400 }),
+      );
+    } finally {
+      vi.useRealTimers();
+    }
+  });
   it('times out a missing rollback ACK, releases the session fence, and denies teardown', async () => {
     vi.useFakeTimers();
     try {

src/main/app-windows.ts

@@ -143,10 +143,12 @@ export function createAppWindows({
     webContentsId: number,
     send: () => void,
   ) => new Promise<CloseGuardState | null>((resolve) => {
+    const timeoutMs = 400;
     const timer = setTimeout(() => {
       pendingState.delete(id);
+      void logWarn('close', 'renderer state response timed out', { requestId: id, webContentsId, timeoutMs });
       resolve(null);
-    }, 400);
+    }, timeoutMs);
     pendingState.set(id, {
       webContentsId,
       resolve: (value) => {
@@ -315,7 +317,11 @@ export function createAppWindows({
       };
       pendingSave.set(id, pending);
       win.once('closed', onDestroyed);
-      timer = setTimeout(() => pending.resolve({ saved: false, committedRevision: null }), Math.max(0, deadline - Date.now()));
+      const timeoutMs = Math.max(0, deadline - Date.now());
+      timer = setTimeout(() => {
+        void logWarn('close', 'renderer save response timed out', { requestId: id, webContentsId: pending.webContentsId, timeoutMs });
+        pending.resolve({ saved: false, committedRevision: null });
+      }, timeoutMs);
       win.webContents.send('close:save', { requestId: id, revision });
     });
   };
@@ -350,10 +356,12 @@ export function createAppWindows({
     if (!activeLease(win, leaseId)) return Promise.resolve(false);
     if (leaseId && mainOwnedShutdownLeases.has(leaseId)) return Promise.resolve(true);
     return new Promise((resolve) => {
+      const timeoutMs = 400;
       const timer = setTimeout(() => {
         pendingAuthorize.delete(leaseId!);
+        void logWarn('close', 'renderer authorize response timed out', { requestId: leaseId, webContentsId: win.webContents.id, timeoutMs });
         resolve(false);
-      }, 400);
+      }, timeoutMs);
       pendingAuthorize.set(leaseId!, {
         webContentsId: win.webContents.id,
         resolve: (valid) => {
@@ -369,10 +377,12 @@ export function createAppWindows({
     if (!activeLease(win, leaseId)) return Promise.resolve(false);
     if (leaseId && mainOwnedShutdownLeases.has(leaseId)) return Promise.resolve(true);
     return new Promise((resolve) => {
+      const timeoutMs = 400;
       const timer = setTimeout(() => {
         pendingConsume.delete(leaseId!);
+        void logWarn('close', 'renderer consume response timed out', { requestId: leaseId, webContentsId: win.webContents.id, timeoutMs });
         resolve(false);
-      }, 400);
+      }, timeoutMs);
       pendingConsume.set(leaseId!, {
         webContentsId: win.webContents.id,
         resolve: (consumed) => {

src/renderer/close-save-handler.test.ts

@@ -0,0 +1,41 @@
+import { describe, expect, it, vi } from 'vitest';
+import { handleCloseSaveRequest } from './close-save-handler';
+
+describe('handleCloseSaveRequest', () => {
+  it('reports document save before waiting for session snapshot persistence', async () => {
+    let releaseSnapshot!: () => void;
+    const snapshotFlush = new Promise<void>((resolve) => { releaseSnapshot = resolve; });
+    const send = vi.fn();
+    const request = handleCloseSaveRequest({
+      requestId: 'save:1',
+      requestedRevision: 4,
+      save: async () => 4,
+      isDirty: () => false,
+      flushSessionSnapshot: () => snapshotFlush,
+      onSnapshotFlushError: vi.fn(),
+      send,
+    });
+
+    await vi.waitFor(() => expect(send).toHaveBeenCalledWith('save:1', { saved: true, committedRevision: 4 }));
+    releaseSnapshot();
+    await request;
+  });
+
+  it('keeps a snapshot flush failure after a successful document ACK', async () => {
+    const send = vi.fn();
+    const onSnapshotFlushError = vi.fn();
+    await handleCloseSaveRequest({
+      requestId: 'save:2',
+      requestedRevision: 4,
+      save: async () => 4,
+      isDirty: () => false,
+      flushSessionSnapshot: async () => { throw new Error('session write failed'); },
+      onSnapshotFlushError,
+      send,
+    });
+
+    expect(send).toHaveBeenCalledTimes(1);
+    expect(send).toHaveBeenCalledWith('save:2', { saved: true, committedRevision: 4 });
+    expect(onSnapshotFlushError).toHaveBeenCalledOnce();
+  });
+});

src/renderer/close-save-handler.ts

@@ -0,0 +1,50 @@
+type CloseSaveResult = {
+  saved: boolean;
+  committedRevision: number | null;
+};
+
+export type CloseSaveRequest = {
+  requestId: string;
+  requestedRevision: number;
+  save: () => Promise<number | null>;
+  isDirty: () => boolean;
+  flushSessionSnapshot?: () => Promise<void>;
+  onSnapshotFlushError?: (error: unknown) => void;
+  send: (requestId: string, result: CloseSaveResult) => void;
+};
+
+/**
+ * Complete the authoritative document save handshake before doing best-effort
+ * session persistence. The main process can then finish the close transaction
+ * without waiting on the recovery snapshot write.
+ */
+export async function handleCloseSaveRequest({
+  requestId,
+  requestedRevision,
+  save,
+  isDirty,
+  flushSessionSnapshot,
+  onSnapshotFlushError,
+  send,
+}: CloseSaveRequest): Promise<void> {
+  let committedRevision: number | null;
+  try {
+    committedRevision = await save();
+  } catch {
+    send(requestId, { saved: false, committedRevision: null });
+    return;
+  }
+
+  const saved = committedRevision !== null && committedRevision >= requestedRevision;
+  send(requestId, {
+    saved,
+    committedRevision: saved ? committedRevision : null,
+  });
+
+  if (!saved || isDirty() || !flushSessionSnapshot) return;
+  try {
+    await flushSessionSnapshot();
+  } catch (error) {
+    onSnapshotFlushError?.(error);
+  }
+}

src/renderer/main.ts

@@ -30,6 +30,7 @@ import { folderFromFilePath, initProjectWizardFlow } from './project-wizard-flow
 import { initSessionSnapshot } from './session-snapshot';
 import { initUpdateBanner } from './update-banner';
 import { handleCloseQueryState } from './close-query-state';
+import { handleCloseSaveRequest } from './close-save-handler';
 
 const workspace = document.querySelector('.workspace') as HTMLElement;
 const editorHost = document.getElementById('editor-host') as HTMLDivElement;
@@ -308,17 +309,14 @@ window.api.onCloseQuiesceRollback(({ requestId }) => {
 });
 window.api.sendCloseQuiesceReady();
 window.api.onCloseSave((requestId, requestedRevision) => {
-  void (async () => {
-    const committedRevision = await docLifecycle.save();
-    const saved = committedRevision !== null && committedRevision >= requestedRevision;
-    if (saved && !ctx.dirty && sessionSnapshot) await sessionSnapshot.flushSessionSnapshot();
-    window.api.sendCloseSaveResult(requestId, {
-      saved,
-      // This is the actual document revision written, not merely the request.
-      committedRevision: saved ? committedRevision : null,
-    });
-  })().catch(() => {
-    window.api.sendCloseSaveResult(requestId, { saved: false, committedRevision: null });
+  void handleCloseSaveRequest({
+    requestId,
+    requestedRevision,
+    save: () => docLifecycle.save(),
+    isDirty: () => ctx.dirty,
+    flushSessionSnapshot: () => sessionSnapshot.flushSessionSnapshot(),
+    onSnapshotFlushError: (error) => console.warn('[session] close snapshot flush failed:', error),
+    send: window.api.sendCloseSaveResult,
   });
 });
 window.api.onCloseAuthorize((requestId) => {