From 65fe1d89f9fa38ccac300e28cbce0a9b0288f999 Mon Sep 17 00:00:00 2001 From: alpha Date: Thu, 27 Aug 2026 22:04:20 -0400 Subject: [PATCH 1/2] feat: ask to save the interview before anything discards it Clear, Start and closing the app all empty the transcript and the suggestions, which exist only in main-process memory - nothing reaches disk until an export. All three now ask first, through one dialog. The question is only worth asking about a real interview, and the array lengths cannot tell you that: the panels are seeded with placeholder copy on launch and again after every Clear, so `transcripts.length === 0` is never true. `AppState.hasHistory` is derived in main instead and follows the writes, since only the transcript and suggestion services write those keys and they only ever write real content. That also fixes the export path, where the same length check was already live: Export on a machine that had never run an interview passed it and billed a summarize call on "Transcripts will be here". Closing cannot ask on its own behalf - the decision is taken in main - so the close is vetoed, the renderer is asked, and it closes the window by replying. Vetoing a close cancels an in-flight quit, so a confirmed close restarts the quit rather than closing the window. Closes #114 Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 48 +++++ src/main/index.ts | 6 + src/main/preload.cts | 10 ++ src/main/services/app-state.service.ts | 58 +++++- src/main/services/tools.service.ts | 7 +- src/main/types/app-state.ts | 9 + src/main/window-close-guard.ts | 74 ++++++++ .../components/custom/control-panel/index.tsx | 8 + .../custom/control-panel/tools-group.tsx | 95 ++-------- .../custom/export-success-toast.tsx | 82 +++++++++ src/renderer/components/custom/main-frame.tsx | 4 + .../components/custom/save-history-dialog.tsx | 169 ++++++++++++++++++ src/renderer/hooks/use-app-state.tsx | 3 + src/renderer/hooks/use-save-history-guard.ts | 61 +++++++ src/renderer/types/app-state.ts | 5 + src/renderer/types/electron-api.d.ts | 6 + test/helpers.mjs | 10 +- test/run.mjs | 3 + test/save-history.test.mjs | 93 ++++++++++ 19 files changed, 663 insertions(+), 88 deletions(-) create mode 100644 src/main/window-close-guard.ts create mode 100644 src/renderer/components/custom/export-success-toast.tsx create mode 100644 src/renderer/components/custom/save-history-dialog.tsx create mode 100644 src/renderer/hooks/use-save-history-guard.ts create mode 100644 test/save-history.test.mjs diff --git a/CLAUDE.md b/CLAUDE.md index fa512b07..ccd88bf0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -118,6 +118,54 @@ the default at that moment, in an effect - never in the render body, where the s re-enters React mid-commit and a failed IPC call rolls the value back into the same condition that triggered it, one write per frame. +### Saving the interview before it is lost + +The transcript and the suggestions live only in main-process memory. Nothing is written to disk +until an export, so the three actions that empty them - Clear, Start (which opens with +`clearAll()`), and closing the app - are the only paths in the app that destroy work with no way +back. All three now ask first, through one dialog: +[save-history-dialog.tsx](src/renderer/components/custom/save-history-dialog.tsx), mounted once in +`MainFrame` because the three do not share a screen - the control panel is not rendered in stealth +mode, and the close prompt arrives from main with no component of its own. + +**The question is only worth asking about a real interview, and length cannot tell you that.** +`setPlaceholderState()` seeds the panels with one transcript and two suggestions so an empty app +has something to show, and it runs on launch and again after every Clear - so +`transcripts.length === 0` is never true and the old guard let the placeholder through. That was +already a live defect on the export path: Export on a machine that had never run an interview +passed the length check and billed a summarize call on "Transcripts will be here", then wrote the +model's answer into a document titled as a record of the candidate's interview. + +`AppState.hasHistory` is the replacement, derived in `withHistory()` and never set by a caller. +Only the transcript and suggestion services write the three history keys and they only ever write +real content, so a write to any of them retires the placeholder and the flag is recomputed from +what the write leaves behind. The untouched arrays are emptied in the same write: `clearAll()` +runs before every session so mixed state is not reachable today, but a real transcript sitting +beside two lines of sample suggestion copy is the one shape that would put placeholder text into +an exported report. The export guard and `nothingToExport` both read the flag now, and +`test/save-history.test.mjs` pins it. + +**Closing is the one that cannot ask on its own behalf.** Clear and Start are renderer-initiated +and confirm before they act; a close is decided in main - the window button, Cmd+Q, `app.quit()` - +and the renderer would hear about it too late to matter. So +[window-close-guard.ts](src/main/window-close-guard.ts) vetoes the close, sends +`app:save-history-prompt`, and the renderer closes the window itself by replying. Exactly one of +`window:close-confirmed` / `window:close-cancelled` has to come back or the app cannot be closed +at all, which is why the guard gives up on a renderer that is destroyed or crashed rather than +holding the window open with nobody to ask. + +Three pieces of state, each for a failure the others do not cover. `closeConfirmed` lets the +answered close through instead of re-prompting on it. `prompting` stops a second close - the +window button pressed while the dialog is up - stacking another prompt. And `quitting`, set from +`before-quit`, is what makes Cmd+Q work: vetoing the close *cancels the quit*, so confirming has +to call `app.quit()` again rather than `win.close()`, or the app would sit there with one window +fewer. Cancelling resets it, or the next Cmd+Q would take that branch for a session the user just +chose to keep. + +A save that the user cancels at the system save dialog leaves the prompt open rather than reading +as a decision to discard, and so does a failed export - going ahead there would destroy the +interview on the one path where keeping it did not work. + ### Interview language One setting decides three things: which speech model transcribes the call, what language suggestions come back in, and the language of the exported report. `Language` is mirrored across the processes the way `SuggestionMode` is - [src/main/types/language.ts](src/main/types/language.ts) for the request bodies, [src/renderer/types/language.ts](src/renderer/types/language.ts) for the same enum plus the display metadata the picker needs. 28 languages, which is exactly what the backend's Deepgram Nova-3 ASR streams: offering one the ASR cannot hear would not degrade, it would answer a question that was never asked. diff --git a/src/main/index.ts b/src/main/index.ts index 7775ee4b..5f5a1bec 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -30,6 +30,7 @@ import { restoreWindow, setWindowReference } from './services/window-control.ser import { setWindowReference as setZoomWindowReference } from './services/zoom.service.js'; import { configStore } from './store/config.store.js'; import { EnvUtil } from './utils/env.js'; +import { installCloseGuard, registerCloseGuardHandlers } from './window-close-guard.js'; let win: BrowserWindow | null = null; @@ -170,6 +171,10 @@ async function createWindow() { } }); + // After the bounds listener, so a close the guard vetoes has still recorded where the window + // was - the user is about to be asked a question, not to have their layout forgotten. + installCloseGuard(win); + // Clear cache before loading await win.webContents.session.clearCache(); @@ -209,6 +214,7 @@ app.whenReady().then(async () => { registerToolsHandlers(); registerAutoUpdaterHandlers(); registerExternalHandlers(); + registerCloseGuardHandlers(); // Create window await createWindow(); diff --git a/src/main/preload.cts b/src/main/preload.cts index f9b2da32..b9b320f1 100644 --- a/src/main/preload.cts +++ b/src/main/preload.cts @@ -163,6 +163,16 @@ const electronApi = { minimize: () => ipcRenderer.send('window:minimize'), maximize: () => ipcRenderer.send('window:maximize'), + // Main vetoes a close that would drop an unsaved interview and asks here instead. The reply + // is what actually closes the window, so exactly one of these two has to be sent back. + onSaveHistoryPrompt: (callback: () => void) => { + const handler = () => callback(); + ipcRenderer.on('app:save-history-prompt', handler); + return () => ipcRenderer.removeListener('app:save-history-prompt', handler); + }, + confirmClose: () => ipcRenderer.send('window:close-confirmed'), + cancelClose: () => ipcRenderer.send('window:close-cancelled'), + zoom: { increase: () => ipcRenderer.send('zoom:in'), decrease: () => ipcRenderer.send('zoom:out'), diff --git a/src/main/services/app-state.service.ts b/src/main/services/app-state.service.ts index 9f86f012..47f6fca4 100644 --- a/src/main/services/app-state.service.ts +++ b/src/main/services/app-state.service.ts @@ -29,8 +29,13 @@ const DEFAULT_STATE: AppState = { providedLLMModel: undefined, interviewConfig: { fullName: '', profileData: '', context: '' }, interviewConfigLoaded: false, + hasHistory: false, }; +/** The three arrays that together make up an interview's history. */ +const HISTORY_KEYS = ['transcripts', 'liveSuggestions', 'actionSuggestions'] as const; +type HistoryKey = (typeof HISTORY_KEYS)[number]; + // Every broadcast structured-clones the whole renderer state, and they fire on each streamed // token and each ASR partial - roughly 20/second across two channels, against a transcript array // that grows for the whole interview. Coalescing bounds that cost per unit time instead of per @@ -41,12 +46,21 @@ export class AppStateService { private state: AppState; private broadcastTimer: ReturnType | null = null; + /** + * Whether the history arrays currently hold the placeholder copy. + * + * Tracked here rather than inferred from the contents, because the placeholder is + * indistinguishable from a one-line interview by shape and only this class ever writes it. + */ + private placeholderActive = false; + constructor() { this.state = { ...DEFAULT_STATE }; this.setPlaceholderState(); } setPlaceholderState() { + this.placeholderActive = true; const tstampNow = Date.now(); this.state = { ...this.state, @@ -82,6 +96,9 @@ export class AppStateService { error: '', }, ], + // Sample copy is not an interview. Everything that destroys history asks to save it + // first, and this is the state a freshly launched app sits in. + hasHistory: false, }; // Clear reaches main and resets the state here, but the renderer only ever learns about // state through this broadcast - it does not poll while the push API exists. Without this @@ -112,7 +129,40 @@ export class AppStateService { }; } - updateState(updates: Partial): AppState { + /** + * Fold a write into `updates` so that `hasHistory` follows it. + * + * Only the transcript and suggestion services write the history keys, and they only ever + * write real interview content - the placeholder comes from here alone. So a write to any + * one of them retires the placeholder, and the flag is recomputed from what the write leaves + * behind rather than tracked by each caller. + * + * The untouched arrays are emptied along with it. `clearAll` runs before every session so + * that mixed state is not reachable in practice, but a real transcript sitting beside two + * lines of sample suggestion copy is the one shape that would put placeholder text into an + * exported report. + */ + private withHistory(updates: Partial): Partial { + const touched = HISTORY_KEYS.filter((key) => updates[key] !== undefined); + if (touched.length === 0) return updates; + + const next: Partial = { ...updates }; + if (this.placeholderActive) { + for (const key of HISTORY_KEYS) { + if (!touched.includes(key)) next[key] = [] as never; + } + this.placeholderActive = false; + } + + next.hasHistory = HISTORY_KEYS.some( + (key: HistoryKey) => (next[key] ?? this.state[key]).length > 0 + ); + return next; + } + + updateState(updatesIn: Partial): AppState { + const updates = this.withHistory(updatesIn); + // The health-check loops re-report identical values every 1-5s. Broadcasting those would // re-render every subscriber for nothing, so only notify when something actually moved. const changed = (Object.keys(updates) as (keyof AppState)[]).some( @@ -184,13 +234,11 @@ export class AppStateService { } addLiveSuggestion(s: LiveSuggestion): void { - this.state = { ...this.state, liveSuggestions: [...this.state.liveSuggestions, s] }; - this.notifyRenderer(); + this.updateState({ liveSuggestions: [...this.state.liveSuggestions, s] }); } addActionSuggestion(s: ActionSuggestion): void { - this.state = { ...this.state, actionSuggestions: [...this.state.actionSuggestions, s] }; - this.notifyRenderer(); + this.updateState({ actionSuggestions: [...this.state.actionSuggestions, s] }); } } diff --git a/src/main/services/tools.service.ts b/src/main/services/tools.service.ts index 9fde508e..42d9ca67 100644 --- a/src/main/services/tools.service.ts +++ b/src/main/services/tools.service.ts @@ -25,7 +25,12 @@ class ToolsService { // call whose only possible output is invented, and it lands in a document the candidate is // told is a record of their interview. The export button is live whenever the assistant is // idle, which includes every launch before the first session. - if (transcripts.length === 0 && suggestions.length === 0) { + // + // On `hasHistory` rather than on the array lengths: those are never zero, because the panels + // are seeded with placeholder copy on launch and again after every Clear. So the length + // check passed on a machine that had never run an interview, and the document it produced + // was a model's summary of "Transcripts will be here". + if (!appStateService.getState().hasHistory) { throw new Error('There is nothing to export yet. Run an interview first.'); } diff --git a/src/main/types/app-state.ts b/src/main/types/app-state.ts index cfceaacf..32b893a7 100644 --- a/src/main/types/app-state.ts +++ b/src/main/types/app-state.ts @@ -101,6 +101,15 @@ export interface AppState { interviewConfig: InterviewConfig; /** False until the account's config has been read this session; editing is unsafe before then. */ interviewConfigLoaded: boolean; + /** + * Whether the three arrays above hold a real interview rather than the placeholder copy. + * + * Derived, never set by a caller. The panels are seeded with one transcript and two + * suggestions so an empty app has something to show, so a length check cannot tell an + * interview worth saving from the sample text - and everything that destroys history asks + * before it does, which is a question nobody should be asked about placeholder data. + */ + hasHistory: boolean; } /** The app state as sent to the renderer, with the interview config reduced to a summary. */ diff --git a/src/main/window-close-guard.ts b/src/main/window-close-guard.ts new file mode 100644 index 00000000..1cc0ee97 --- /dev/null +++ b/src/main/window-close-guard.ts @@ -0,0 +1,74 @@ +import { app, BrowserWindow, ipcMain } from 'electron'; + +import { appStateService } from './services/app-state.service.js'; +import { getWindowReference } from './services/window-control.service.js'; + +/** + * Hold the window open long enough to ask whether the interview should be saved. + * + * Clear and Start are renderer-initiated and can ask before they act. Closing cannot: the + * decision is taken in main, by the OS close button, Cmd+Q or `app.quit()`, and by the time the + * renderer hears about it the answer would arrive too late to matter. So the close is vetoed, + * the renderer is asked, and it closes the window itself once the user has answered. + * + * The transcript and the suggestions live only in main-process memory - nothing is written to + * disk until an export - so a close taken at face value is the one path in this app that + * destroys an interview with no way back. + */ + +// The user has answered and the next close is theirs. Set immediately before we ask for it. +let closeConfirmed = false; + +// A prompt is already on screen. A second close - the window button while the dialog is up, or +// a quit arriving behind it - must not stack another one on top of it. +let prompting = false; + +// A quit is in flight. Vetoing the close aborts it, so confirming has to restart it rather than +// close the window, or Cmd+Q would leave a quitting app sitting there with one window less. +let quitting = false; + +export function installCloseGuard(win: BrowserWindow): void { + win.on('close', (event) => { + if (closeConfirmed || !appStateService.getState().hasHistory) return; + + // Nobody to ask. A renderer that has crashed or is already torn down would swallow the + // prompt, and a window that cannot be closed is worse than one that closes unasked. + const wc = win.webContents; + if (wc.isDestroyed() || wc.isCrashed()) return; + + event.preventDefault(); + if (prompting) return; + + prompting = true; + wc.send('app:save-history-prompt'); + }); +} + +export function registerCloseGuardHandlers(): void { + app.on('before-quit', () => { + quitting = true; + }); + + ipcMain.on('window:close-confirmed', () => { + prompting = false; + closeConfirmed = true; + + // `app.quit()` rather than `win.close()` when a quit was already under way: the veto + // cancelled it, and closing the window alone would leave `will-quit` unrun on macOS, where + // the app can outlive its windows. + if (quitting) { + app.quit(); + return; + } + + const win = getWindowReference(); + if (win && !win.isDestroyed()) win.close(); + }); + + ipcMain.on('window:close-cancelled', () => { + prompting = false; + // Reset, or the next Cmd+Q would take the quit branch above and quit a session the user + // has just chosen to keep. + quitting = false; + }); +} diff --git a/src/renderer/components/custom/control-panel/index.tsx b/src/renderer/components/custom/control-panel/index.tsx index f70b7e60..67ea216a 100644 --- a/src/renderer/components/custom/control-panel/index.tsx +++ b/src/renderer/components/custom/control-panel/index.tsx @@ -8,6 +8,7 @@ import { useAudioInputDevices } from '@/hooks/use-audio-devices'; import { useConfigStore } from '@/hooks/use-config-store'; import { useConfigurationDialog } from '@/hooks/use-configuration-dialog'; import useIsStealthMode from '@/hooks/use-is-stealth-mode'; +import { useSaveHistoryGuard } from '@/hooks/use-save-history-guard'; import { isMac } from '@/lib/consts'; import { getElectron } from '@/lib/utils'; import { RunningState } from '@/types/app-state'; @@ -34,6 +35,7 @@ export default function ControlPanel() { const { runningState, appState } = useAppState(); const { config } = useConfigStore(); const { openConfigurationDialog } = useConfigurationDialog(); + const { confirmDiscard } = useSaveHistoryGuard(); const [permGateOpen, setPermGateOpen] = useState(false); const { devices: audioInputDevices, ready: audioDevicesReady } = useAudioInputDevices(); @@ -111,6 +113,12 @@ export default function ControlPanel() { const handleStartClick = async () => { if (!checkCanStart()) return; + // `startAssistant` opens with `clearAll()`, so the previous interview is gone the moment + // this goes ahead. Asked before the permission gate rather than after: a user who is about + // to be sent into System Settings should not have answered a question first that the trip + // makes moot. + if (!(await confirmDiscard('start'))) return; + if (isMac) { const electron = getElectron(); if (electron) { diff --git a/src/renderer/components/custom/control-panel/tools-group.tsx b/src/renderer/components/custom/control-panel/tools-group.tsx index 36f71ba0..33e2c5ad 100644 --- a/src/renderer/components/custom/control-panel/tools-group.tsx +++ b/src/renderer/components/custom/control-panel/tools-group.tsx @@ -1,19 +1,8 @@ -import { - Captions, - CaptionsOff, - CircleCheck, - FileIcon, - FileText, - FolderOpenIcon, - Hash, - Loader, - Save, - Trash2, - XIcon, -} from 'lucide-react'; +import { Captions, CaptionsOff, FileText, Hash, Loader, Save, Trash2 } from 'lucide-react'; import { useState } from 'react'; import { toast } from 'sonner'; +import { showExportSuccessToast } from '@/components/custom/export-success-toast'; import { Button } from '@/components/ui/button'; import { DropdownMenu, @@ -23,10 +12,11 @@ import { } from '@/components/ui/dropdown-menu'; import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip'; import { useAppState } from '@/hooks/use-app-state'; +import { useSaveHistoryGuard } from '@/hooks/use-save-history-guard'; import useTools from '@/hooks/use-tools'; import { useTranscriptPanel } from '@/hooks/use-transcript-panel'; import { Hotkey, HOTKEYS } from '@/lib/hotkeys'; -import { cn, getElectron } from '@/lib/utils'; +import { cn } from '@/lib/utils'; import { RunningState } from '@/types/app-state'; import type { ExportFormat } from '@/types/export'; @@ -40,9 +30,14 @@ export function ToolsGroup({ getDisabled }: ToolsGroupProps) { const { runningState, appState } = useAppState(); const { exporting, exportTranscript, clearAll, setPlaceholderData } = useTools(); const { visible: transcriptVisible, toggle: onToggleTranscript } = useTranscriptPanel(); + const { confirmDiscard } = useSaveHistoryGuard(); const [clearing, setClearing] = useState(false); const onClear = async () => { + // Asked before the spinner goes up, and a no-op when there is nothing but placeholder copy + // to lose. The transcript and the suggestions exist only in main-process memory. + if (!(await confirmDiscard('clear'))) return; + setClearing(true); try { // Placeholder state only rewrites what the renderer sees. The service buffers keep the @@ -62,8 +57,11 @@ export function ToolsGroup({ getDisabled }: ToolsGroupProps) { // Electron's "Error invoking remote method 'tools:export-transcript'" prefix, which is not a // sentence to put in front of someone. The service keeps its own guard because it is what // stops the billed summarize call, and this state can be stale by a broadcast. - const nothingToExport = - (appState?.transcripts?.length ?? 0) === 0 && (appState?.liveSuggestions?.length ?? 0) === 0; + // + // On `hasHistory` rather than on the array lengths, which are never zero: the panels carry + // placeholder copy on launch and again after every Clear, so the old check let a summarize + // request be billed for a document about "Transcripts will be here". + const nothingToExport = !appState?.hasHistory; const onExportTranscript = async (format: ExportFormat) => { if (nothingToExport) { @@ -76,70 +74,7 @@ export function ToolsGroup({ getDisabled }: ToolsGroupProps) { try { const filePath = await exportTranscript(format); if (!filePath) return; - const electron = getElectron(); - const toastId = `export-${Date.now()}`; - toast.custom( - () => ( -
- - - Interview exported as {format === 'md' ? 'Markdown' : 'Word'} - -
- - - - - Open file - - - - - - Show in folder - - - - - - Dismiss - -
-
- ), - { id: toastId, duration: 10_000, style: { width: 'var(--width, 356px)' } } - ); + showExportSuccessToast(filePath, format); } catch (error) { console.error(error); // The message when there is nothing to export names the reason, and a generic "failed" diff --git a/src/renderer/components/custom/export-success-toast.tsx b/src/renderer/components/custom/export-success-toast.tsx new file mode 100644 index 00000000..15f99f86 --- /dev/null +++ b/src/renderer/components/custom/export-success-toast.tsx @@ -0,0 +1,82 @@ +import { CircleCheck, FileIcon, FolderOpenIcon, XIcon } from 'lucide-react'; +import { toast } from 'sonner'; + +import { Button } from '@/components/ui/button'; +import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip'; +import { getElectron } from '@/lib/utils'; +import type { ExportFormat } from '@/types/export'; + +/** + * Confirm an export and offer the file. + * + * A save dialog's own path is gone the moment it closes, so "where did that go" is the next + * question every time. Shared between the export menu and the save-before-clearing prompt so + * the answer does not depend on which one the user reached for. + */ +export function showExportSuccessToast(filePath: string, format: ExportFormat): void { + const electron = getElectron(); + const toastId = `export-${Date.now()}`; + + toast.custom( + () => ( +
+ + + Interview exported as {format === 'md' ? 'Markdown' : 'Word'} + +
+ + + + + Open file + + + + + + Show in folder + + + + + + Dismiss + +
+
+ ), + { id: toastId, duration: 10_000, style: { width: 'var(--width, 356px)' } } + ); +} diff --git a/src/renderer/components/custom/main-frame.tsx b/src/renderer/components/custom/main-frame.tsx index b9b9fc73..3ce86ba4 100644 --- a/src/renderer/components/custom/main-frame.tsx +++ b/src/renderer/components/custom/main-frame.tsx @@ -7,6 +7,7 @@ import usePointerLockGuard from '@/hooks/use-pointer-lock-guard'; import type { PushNotification } from '@/types/push-notification'; import ConfigurationDialog from './configuration-dialog'; +import SaveHistoryDialog from './save-history-dialog'; import Titlebar from './titlebar'; import { UpdateNotification } from './update-notification'; @@ -60,6 +61,9 @@ export default function MainFrame({ children }: { children: React.ReactNode }) { + {/* Mounted here rather than on the interview page: it also answers a close prompt from + main, which can arrive while the user is on the login or payment route. */} + ); diff --git a/src/renderer/components/custom/save-history-dialog.tsx b/src/renderer/components/custom/save-history-dialog.tsx new file mode 100644 index 00000000..d9143592 --- /dev/null +++ b/src/renderer/components/custom/save-history-dialog.tsx @@ -0,0 +1,169 @@ +import { FileText, Hash, Loader } from 'lucide-react'; +import { useEffect, useState } from 'react'; +import { toast } from 'sonner'; + +import { Button } from '@/components/ui/button'; +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from '@/components/ui/dialog'; +import { type SaveHistoryReason, useSaveHistoryPrompt } from '@/hooks/use-save-history-guard'; +import useTools from '@/hooks/use-tools'; +import { getElectron } from '@/lib/utils'; +import type { ExportFormat } from '@/types/export'; + +import { showExportSuccessToast } from './export-success-toast'; + +/** + * The action is named in the title and again on the button that goes through with it. "Discard" + * on its own is the same word for three different losses, and this dialog can appear on a close + * the user asked for seconds ago and on a Clear they pressed by accident. + */ +const COPY: Record = { + clear: { + title: 'Save this interview before clearing?', + body: 'Clearing drops the transcript and the suggestions from this session.', + discard: 'Clear without saving', + }, + start: { + title: 'Save this interview before starting a new one?', + body: 'Starting a session drops the transcript and the suggestions from the last one.', + discard: 'Start without saving', + }, + close: { + title: 'Save this interview before closing?', + body: 'Closing drops the transcript and the suggestions from this session.', + discard: 'Close without saving', + }, +}; + +/** + * Asks whether to export before something destroys the interview. + * + * Mounted once, near the root, because the three things it guards do not share a screen: Clear + * and Start are on the control panel, which stealth mode does not render, and the close prompt + * arrives from main with no component of its own at all. + */ +export default function SaveHistoryDialog() { + const { reason, settle, prompt } = useSaveHistoryPrompt(); + const { exportTranscript } = useTools(); + const [saving, setSaving] = useState(null); + + // Main vetoes a close that would lose the interview and asks here instead, so the window is + // held open until one of these two replies is sent. Registered once, for the lifetime of the + // app: the prompt can arrive at any moment and there is no component tied to closing. + useEffect(() => { + const electron = getElectron(); + if (!electron?.onSaveHistoryPrompt) return; + + return electron.onSaveHistoryPrompt(() => { + void prompt('close').then((proceed) => { + if (proceed) electron.confirmClose(); + else electron.cancelClose(); + }); + }); + }, [prompt]); + + const save = async (format: ExportFormat) => { + setSaving(format); + try { + const filePath = await exportTranscript(format); + // Cancelled at the system save dialog. That is backing out of the file, not out of the + // question, so the prompt stays up rather than reading as a decision to discard. + if (!filePath) return; + + showExportSuccessToast(filePath, format); + settle(true); + } catch (error) { + console.error(error); + // The prompt stays open on a failure. Going ahead with the action here would destroy the + // interview the user has just asked to keep, on the one path where saving did not work. + toast.error(error instanceof Error ? error.message : 'Failed to export interview'); + } finally { + setSaving(null); + } + }; + + const copy = reason ? COPY[reason] : null; + const busy = saving !== null; + + return ( + { + if (next || busy) return; + settle(false); + }} + > + + + {copy?.title} + + {copy?.body} Nothing is written to disk until you export, so this is the only chance to + keep it. + + + + +
+ + +
+
+ + +
+ + + + ); +} diff --git a/src/renderer/hooks/use-app-state.tsx b/src/renderer/hooks/use-app-state.tsx index 723a406c..afcdb5d8 100644 --- a/src/renderer/hooks/use-app-state.tsx +++ b/src/renderer/hooks/use-app-state.tsx @@ -43,6 +43,9 @@ class AppStateManager { providedLLMModel: raw.providedLLMModel, interviewConfig: raw.interviewConfig ?? { fullName: '', hasProfileData: false }, interviewConfigLoaded: raw.interviewConfigLoaded ?? false, + // Defaults false, which is the safe direction: an older main that does not send it makes + // the save prompt silent rather than making it fire on every Clear with nothing to save. + hasHistory: raw.hasHistory ?? false, }; } diff --git a/src/renderer/hooks/use-save-history-guard.ts b/src/renderer/hooks/use-save-history-guard.ts new file mode 100644 index 00000000..630f0d41 --- /dev/null +++ b/src/renderer/hooks/use-save-history-guard.ts @@ -0,0 +1,61 @@ +import { create } from 'zustand'; + +import { useAppState } from './use-app-state'; + +/** + * What is about to destroy the interview. Only the copy differs - the choice is the same one + * every time, and phrasing it in terms of the action is what makes it answerable. + */ +export type SaveHistoryReason = 'clear' | 'start' | 'close'; + +interface SaveHistoryPromptStore { + /** The action awaiting an answer, or null when nothing is being asked. */ + reason: SaveHistoryReason | null; + resolve: ((proceed: boolean) => void) | null; + + /** Ask, unconditionally. Resolves true to go ahead with the action, false to abandon it. */ + prompt: (reason: SaveHistoryReason) => Promise; + /** Answer the open prompt and close it. */ + settle: (proceed: boolean) => void; +} + +export const useSaveHistoryPrompt = create((set, get) => ({ + reason: null, + resolve: null, + + prompt: (reason) => { + // A second question arriving over the first abandons it rather than stacking. Leaving the + // earlier promise unresolved would strand whichever caller is awaiting it - and for the + // close prompt that caller is the one holding the window open. + get().resolve?.(false); + + return new Promise((resolve) => { + set({ reason, resolve }); + }); + }, + + settle: (proceed) => { + const { resolve } = get(); + set({ reason: null, resolve: null }); + resolve?.(proceed); + }, +})); + +/** + * Ask before an action that drops the interview, but only when there is one to drop. + * + * `hasHistory` is derived in main and is false for the placeholder copy the panels are seeded + * with, so a freshly launched app starts and clears without a question - which is the state + * most Start presses happen in, and a confirmation there would be pure friction. + */ +export function useSaveHistoryGuard() { + const { appState } = useAppState(); + const hasHistory = appState?.hasHistory ?? false; + + const confirmDiscard = async (reason: SaveHistoryReason): Promise => { + if (!hasHistory) return true; + return useSaveHistoryPrompt.getState().prompt(reason); + }; + + return { hasHistory, confirmDiscard }; +} diff --git a/src/renderer/types/app-state.ts b/src/renderer/types/app-state.ts index 4d3e0178..a1cd23a8 100644 --- a/src/renderer/types/app-state.ts +++ b/src/renderer/types/app-state.ts @@ -42,4 +42,9 @@ export interface AppState { providedLLMModel?: string; interviewConfig: InterviewConfigSummary; interviewConfigLoaded: boolean; + /** + * Whether the arrays above hold a real interview rather than the placeholder copy the panels + * are seeded with. Derived in main; the renderer only reads it. + */ + hasHistory: boolean; } diff --git a/src/renderer/types/electron-api.d.ts b/src/renderer/types/electron-api.d.ts index 6fd2e1b1..07240c02 100644 --- a/src/renderer/types/electron-api.d.ts +++ b/src/renderer/types/electron-api.d.ts @@ -191,6 +191,12 @@ declare global { minimize: () => void; maximize: () => void; + // Main vetoes a close that would drop an unsaved interview and asks here instead. Exactly + // one of confirmClose/cancelClose has to be sent back, or the window never closes. + onSaveHistoryPrompt: (callback: () => void) => () => void; + confirmClose: () => void; + cancelClose: () => void; + // Zoom controls zoom: { increase: () => void; diff --git a/test/helpers.mjs b/test/helpers.mjs index 90a470f9..edb61b12 100644 --- a/test/helpers.mjs +++ b/test/helpers.mjs @@ -55,6 +55,12 @@ const app = { dockCalls, }; const ipcMain = { on: () => {}, handle: () => {} }; +// Named imports are resolved at link time, so a module under test that merely *mentions* one of +// these fails to load even on paths that never call it. tools.service reaches the export guard +// before it touches the save dialog, and pulls in the capture service on the way. +const BrowserWindow = { getAllWindows: () => [], fromWebContents: () => null }; +const desktopCapturer = { getSources: async () => [] }; +const dialog = { showSaveDialog: async () => ({ canceled: true, filePath: undefined }) }; const screen = { getPrimaryDisplay: () => ({ workAreaSize: { width: 1920, height: 1080 } }), getAllDisplays: () => [] }; // Records what was handed to the OS, so a test can assert that a blocked scheme never reaches it. const openExternalCalls = []; @@ -64,8 +70,8 @@ const shell = { openExternalCalls, showItemInFolder: () => {}, }; -export { app, ipcMain, screen, shell }; -export default { app, ipcMain, screen, shell };`, +export { app, BrowserWindow, desktopCapturer, dialog, ipcMain, screen, shell }; +export default { app, BrowserWindow, desktopCapturer, dialog, ipcMain, screen, shell };`, }; }, }); diff --git a/test/run.mjs b/test/run.mjs index b7fd0be2..15700da6 100644 --- a/test/run.mjs +++ b/test/run.mjs @@ -27,6 +27,9 @@ for (const module of [ // reads the same running state through its own copy of window-control. './running-surface.test.mjs', './tools-export.test.mjs', + // After tools-export: it drives the shared appStateService singleton through the placeholder + // and back, which the export helpers above do not read. + './save-history.test.mjs', './audio-device-switch.test.mjs', './language-switch.test.mjs', './rtl-rendering.test.mjs', diff --git a/test/save-history.test.mjs b/test/save-history.test.mjs new file mode 100644 index 00000000..8d813d34 --- /dev/null +++ b/test/save-history.test.mjs @@ -0,0 +1,93 @@ +/** + * Everything that destroys the interview asks to save it first, and the question is only worth + * asking about a real one. The panels are seeded with placeholder copy on launch and again + * after every Clear, so `transcripts.length` is never zero and cannot be the test. + * + * `hasHistory` is what the prompt and the export guard both read. These pin that it follows the + * writes rather than the array lengths, and that an export refuses the placeholder - which the + * length check let through, producing a billed summary of "Transcripts will be here". + */ +import { createChecker, loadMain, readSource } from './helpers.mjs'; + +export async function run() { + const { check, failures } = createChecker('save-history'); + + const { appStateService } = await loadMain('services/app-state.service.js'); + const { toolsService } = await loadMain('services/tools.service.js'); + + appStateService.setPlaceholderState(); + const placeholder = appStateService.getState(); + check( + 'the placeholder fills the panels', + placeholder.transcripts.length > 0 && placeholder.liveSuggestions.length > 0 + ); + check('the placeholder is not history', placeholder.hasHistory === false); + check( + 'the renderer is told the same', + appStateService.getRendererState().hasHistory === false + ); + + let exportError = null; + try { + await toolsService.exportTranscript('md'); + } catch (e) { + exportError = e; + } + check('exporting the placeholder is refused', exportError !== null); + check( + 'and says why rather than failing on the request', + /nothing to export/i.test(exportError?.message ?? '') + ); + + // A real ingest writes one array. The other two still hold the placeholder at that moment, and + // an interview carrying two lines of sample suggestion copy into an exported report is the + // failure that shape produces. + appStateService.updateState({ + transcripts: [ + { timestamp: 1, text: 'Tell me about a hard bug.', speaker: 'other', isFinal: true }, + ], + }); + const real = appStateService.getState(); + check('a real transcript is history', real.hasHistory === true); + check('the placeholder suggestions go with it', real.liveSuggestions.length === 0); + check('so do the placeholder action suggestions', real.actionSuggestions.length === 0); + + // Clear empties all three; the flag has to come back down or the next Clear asks about + // nothing, which is exactly the prompt fatigue that makes people stop reading it. + appStateService.updateState({ transcripts: [] }); + appStateService.updateState({ liveSuggestions: [] }); + appStateService.updateState({ actionSuggestions: [] }); + check('clearing drops the flag', appStateService.getState().hasHistory === false); + + appStateService.updateState({ + liveSuggestions: [{ timestamp: 2, last_question: 'q', answer: 'a' }], + }); + check('a suggestion alone is history too', appStateService.getState().hasHistory === true); + + appStateService.setPlaceholderState(); + check( + 're-seeding the placeholder is not history again', + appStateService.getState().hasHistory === false + ); + + // Source-level, like the renderer checks in this directory: the close is vetoed in main and + // the window is only closed by the renderer's answer, so dropping either reply leaves an app + // that cannot be closed at all. There is no runtime harness that would catch that. + const guard = readSource(new URL('../src/main/window-close-guard.ts', import.meta.url)); + check('the close is vetoed', guard.includes('event.preventDefault()')); + check('the renderer is asked', guard.includes("wc.send('app:save-history-prompt')")); + check( + 'a confirmed close is not vetoed twice', + guard.includes('if (closeConfirmed ||') && guard.includes('closeConfirmed = true') + ); + check( + 'a quit that was vetoed is restarted rather than left half-done', + guard.includes('if (quitting) {') && guard.includes('app.quit()') + ); + check( + 'a crashed renderer cannot hold the window open', + guard.includes('wc.isDestroyed() || wc.isCrashed()') + ); + + return failures; +} From 2975536b5865a4979e94f153fade7e8dcf9fbc36 Mon Sep 17 00:00:00 2001 From: alpha Date: Thu, 27 Aug 2026 22:29:25 -0400 Subject: [PATCH 2/2] fix: two defects found reviewing the save prompt `hasHistory` counted action suggestions, which the export does not write - `exportTranscript` builds the report from transcripts and live suggestions alone. A session whose only content was a screenshot therefore offered to save what the save could not contain, and passed the export guard onto a billed summarize call over an empty transcript. That is the failure the guard exists to stop, reached through another door. And installing an update is a quit the guard must not veto. `quitAndInstall` launches the installer and requests the quit after it, so vetoing leaves an installer running against an app that will not exit; on Windows it ends with the installer killing it. The updater handler now disarms the guard before installing and re-arms it if nothing was launched, and `update-notification.tsx` asks in front of the install instead. Also: `updateState` strips an incoming `hasHistory` so no caller can set what the close guard trusts, the guard resets per window, and a renderer reload clears a pending prompt that died with it - otherwise the veto stood with nobody left to answer and the window could not be closed. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 34 +++++++-- src/main/ipc/auto-updater.ts | 13 +++- src/main/services/app-state.service.ts | 34 ++++++--- src/main/services/auto-updater.service.ts | 11 ++- src/main/types/app-state.ts | 15 ++-- src/main/window-close-guard.ts | 32 +++++++++ .../components/custom/save-history-dialog.tsx | 5 ++ .../components/custom/update-notification.tsx | 20 +++++- src/renderer/hooks/use-save-history-guard.ts | 2 +- test/save-history.test.mjs | 70 +++++++++++++++++-- 10 files changed, 205 insertions(+), 31 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index ccd88bf0..92612379 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -136,13 +136,21 @@ already a live defect on the export path: Export on a machine that had never run passed the length check and billed a summarize call on "Transcripts will be here", then wrote the model's answer into a document titled as a record of the candidate's interview. -`AppState.hasHistory` is the replacement, derived in `withHistory()` and never set by a caller. -Only the transcript and suggestion services write the three history keys and they only ever write -real content, so a write to any of them retires the placeholder and the flag is recomputed from -what the write leaves behind. The untouched arrays are emptied in the same write: `clearAll()` -runs before every session so mixed state is not reachable today, but a real transcript sitting -beside two lines of sample suggestion copy is the one shape that would put placeholder text into -an exported report. The export guard and `nothingToExport` both read the flag now, and +`AppState.hasHistory` is the replacement, derived in `withHistory()` and never set by a caller - +`updateState` strips it off incoming updates, because it arrives inside a `Partial` the +renderer composes and the close guard trusts it. Only the transcript and suggestion services +write the three history keys and they only ever write real content, so a write to any of them +retires the placeholder and the flag is recomputed from what the write leaves behind. The +untouched arrays are emptied in the same write: `clearAll()` runs before every session so mixed +state is not reachable today, but a real transcript sitting beside two lines of sample suggestion +copy is the one shape that would put placeholder text into an exported report. + +**The flag is read from the transcripts and live suggestions only, not from all three.** Those are +what `exportTranscript` builds the report out of; action suggestions have never been in it. So a +session whose only content is a screenshot has nothing a save could capture, and counting it would +both offer to save what the save cannot contain and let the export guard through on an empty +transcript - the billed summarize call over nothing that the guard exists to stop, reached through +a different door. The export guard and `nothingToExport` both read the flag now, and `test/save-history.test.mjs` pins it. **Closing is the one that cannot ask on its own behalf.** Clear and Start are renderer-initiated @@ -166,6 +174,18 @@ A save that the user cancels at the system save dialog leaves the prompt open ra as a decision to discard, and so does a failed export - going ahead there would destroy the interview on the one path where keeping it did not work. +**Installing an update is a quit the guard must not veto.** `quitAndInstall()` launches the +installer - on macOS `shell.openPath` has already opened the .dmg - and requests the quit +*afterwards*, so a veto there does not cancel the update. It leaves an installer running against +an app that refuses to exit, which on Windows ends with the installer killing it: the interview is +lost anyway, and the prompt asking about it was on screen for a second. So the updater IPC handler +calls `allowNextClose()` before it installs, and `rearmCloseGuard()` if nothing was launched +(`quitAndInstall` returns whether it committed to quitting, which the macOS "no downloaded file" +path does not). The question is asked one layer up instead, in `update-notification.tsx`, in front +of the install rather than behind it. `confirmDiscard` is held in a ref there: it closes over the +app state, so it is a new function on every broadcast from main, and naming it as a dependency +would re-run the update-status effect several times a second during an interview. + ### Interview language One setting decides three things: which speech model transcribes the call, what language suggestions come back in, and the language of the exported report. `Language` is mirrored across the processes the way `SuggestionMode` is - [src/main/types/language.ts](src/main/types/language.ts) for the request bodies, [src/renderer/types/language.ts](src/renderer/types/language.ts) for the same enum plus the display metadata the picker needs. 28 languages, which is exactly what the backend's Deepgram Nova-3 ASR streams: offering one the ASR cannot hear would not degrade, it would answer a question that was never asked. diff --git a/src/main/ipc/auto-updater.ts b/src/main/ipc/auto-updater.ts index 523c469a..71d427a0 100644 --- a/src/main/ipc/auto-updater.ts +++ b/src/main/ipc/auto-updater.ts @@ -1,6 +1,7 @@ import { ipcMain } from 'electron'; import { autoUpdaterService } from '../services/auto-updater.service.js'; +import { allowNextClose, rearmCloseGuard } from '../window-close-guard.js'; export function registerAutoUpdaterHandlers(): void { ipcMain.handle('auto-updater:check-for-updates', async () => { @@ -17,11 +18,19 @@ export function registerAutoUpdaterHandlers(): void { }); ipcMain.handle('auto-updater:quit-and-install', async () => { + // Armed before the call rather than after it: the installer is launched and the quit + // requested inside quitAndInstall, so a guard still active at that moment vetoes a quit the + // update has already committed to. The renderer asks about an unsaved interview before it + // invokes this. + allowNextClose(); try { - await autoUpdaterService.quitAndInstall(); - return { success: true }; + const quitting = await autoUpdaterService.quitAndInstall(); + // Nothing was launched, so the app is staying and the guard goes back on. + if (!quitting) rearmCloseGuard(); + return { success: quitting }; } catch (error) { console.error('[IPC] Failed to quit and install:', error); + rearmCloseGuard(); return { success: false, error: error instanceof Error ? error.message : 'Unknown error', diff --git a/src/main/services/app-state.service.ts b/src/main/services/app-state.service.ts index 47f6fca4..f81859ab 100644 --- a/src/main/services/app-state.service.ts +++ b/src/main/services/app-state.service.ts @@ -32,9 +32,22 @@ const DEFAULT_STATE: AppState = { hasHistory: false, }; -/** The three arrays that together make up an interview's history. */ +/** The three arrays a session fills, and that the placeholder seeds. */ const HISTORY_KEYS = ['transcripts', 'liveSuggestions', 'actionSuggestions'] as const; -type HistoryKey = (typeof HISTORY_KEYS)[number]; + +/** + * The two an export actually reads. + * + * `hasHistory` drives the save prompt and the export guard, and for both it has to mean "there + * is an interview that saving would capture". `exportTranscript` builds the report from + * transcripts and live suggestions alone - action suggestions have never been in it - so + * counting them would offer to save something the save cannot contain, and would let the export + * guard through on a session whose only content is a screenshot: a billed summarize call over + * an empty transcript, writing a report out of nothing. That is the exact failure the guard + * exists to stop, reached through a different door. + */ +const EXPORTABLE_KEYS = ['transcripts', 'liveSuggestions'] as const; +type ExportableKey = (typeof EXPORTABLE_KEYS)[number]; // Every broadcast structured-clones the whole renderer state, and they fire on each streamed // token and each ASR partial - roughly 20/second across two channels, against a transcript array @@ -142,11 +155,16 @@ export class AppStateService { * lines of sample suggestion copy is the one shape that would put placeholder text into an * exported report. */ - private withHistory(updates: Partial): Partial { - const touched = HISTORY_KEYS.filter((key) => updates[key] !== undefined); - if (touched.length === 0) return updates; + private withHistory(updatesIn: Partial): Partial { + // Derived here and nowhere else. It crosses IPC inside a `Partial` the renderer + // composes, and the close guard trusts it, so a caller that set it - by mistake, or by + // echoing back state it was sent - would switch the save prompt off with no symptom. + const next: Partial = { ...updatesIn }; + delete next.hasHistory; + + const touched = HISTORY_KEYS.filter((key) => next[key] !== undefined); + if (touched.length === 0) return next; - const next: Partial = { ...updates }; if (this.placeholderActive) { for (const key of HISTORY_KEYS) { if (!touched.includes(key)) next[key] = [] as never; @@ -154,8 +172,8 @@ export class AppStateService { this.placeholderActive = false; } - next.hasHistory = HISTORY_KEYS.some( - (key: HistoryKey) => (next[key] ?? this.state[key]).length > 0 + next.hasHistory = EXPORTABLE_KEYS.some( + (key: ExportableKey) => (next[key] ?? this.state[key]).length > 0 ); return next; } diff --git a/src/main/services/auto-updater.service.ts b/src/main/services/auto-updater.service.ts index 1edf15a6..f8c3fbbf 100644 --- a/src/main/services/auto-updater.service.ts +++ b/src/main/services/auto-updater.service.ts @@ -277,17 +277,22 @@ class AutoUpdaterService { } } - async quitAndInstall(): Promise { + /** + * @returns whether the app is now on its way out. False means nothing was launched and + * nothing was quit, which the close guard has to know so it can re-arm. + */ + async quitAndInstall(): Promise { if (process.platform === 'darwin') { if (!this.macDownloadedFilePath) { - return; + return false; } await shell.openPath(this.macDownloadedFilePath); app.quit(); - return; + return true; } autoUpdater.quitAndInstall(false, true); + return true; } getCurrentVersion(): string { diff --git a/src/main/types/app-state.ts b/src/main/types/app-state.ts index 32b893a7..7b565564 100644 --- a/src/main/types/app-state.ts +++ b/src/main/types/app-state.ts @@ -102,12 +102,17 @@ export interface AppState { /** False until the account's config has been read this session; editing is unsafe before then. */ interviewConfigLoaded: boolean; /** - * Whether the three arrays above hold a real interview rather than the placeholder copy. + * Whether there is an interview that saving would actually capture. * - * Derived, never set by a caller. The panels are seeded with one transcript and two - * suggestions so an empty app has something to show, so a length check cannot tell an - * interview worth saving from the sample text - and everything that destroys history asks - * before it does, which is a question nobody should be asked about placeholder data. + * Derived, never set by a caller - `updateState` strips it off incoming updates. The panels + * are seeded with one transcript and two suggestions so an empty app has something to show, + * so a length check cannot tell an interview worth saving from the sample text, and + * everything that destroys history asks before it does - a question nobody should be asked + * about placeholder copy. + * + * Read from the transcripts and live suggestions only, because those are what the export + * writes. Action suggestions are not in the report, so a session holding nothing else has + * nothing to save. */ hasHistory: boolean; } diff --git a/src/main/window-close-guard.ts b/src/main/window-close-guard.ts index 1cc0ee97..21e7758e 100644 --- a/src/main/window-close-guard.ts +++ b/src/main/window-close-guard.ts @@ -28,6 +28,18 @@ let prompting = false; let quitting = false; export function installCloseGuard(win: BrowserWindow): void { + // This module's state outlives the window - the single-instance lock rebuilds one that was + // destroyed - and a stale `closeConfirmed` would let the replacement close unasked. + closeConfirmed = false; + prompting = false; + + // A load replaces the renderer that was going to answer, so the question dies with it. + // Without this the flag stays set and every later close is vetoed without a prompt being + // sent: a window that cannot be closed at all. + win.webContents.on('did-finish-load', () => { + prompting = false; + }); + win.on('close', (event) => { if (closeConfirmed || !appStateService.getState().hasHistory) return; @@ -44,6 +56,26 @@ export function installCloseGuard(win: BrowserWindow): void { }); } +/** + * Let the next close through without asking, and put the guard back. + * + * For a quit that commits to something irreversible *before* it calls `app.quit()`. + * `autoUpdater.quitAndInstall()` spawns the installer - and on macOS `shell.openPath` has + * already opened the .dmg - and only then quits, so vetoing that quit does not cancel the + * update. It leaves an installer running against an app that refuses to exit, which on Windows + * ends with the installer killing it: the interview is lost anyway, and the prompt asking about + * it was on screen for a second. The question belongs before the install starts, and the + * renderer asks it there. + */ +export function allowNextClose(): void { + closeConfirmed = true; +} + +/** Re-arm after an `allowNextClose()` whose quit never happened. */ +export function rearmCloseGuard(): void { + closeConfirmed = false; +} + export function registerCloseGuardHandlers(): void { app.on('before-quit', () => { quitting = true; diff --git a/src/renderer/components/custom/save-history-dialog.tsx b/src/renderer/components/custom/save-history-dialog.tsx index d9143592..c97045bb 100644 --- a/src/renderer/components/custom/save-history-dialog.tsx +++ b/src/renderer/components/custom/save-history-dialog.tsx @@ -39,6 +39,11 @@ const COPY: Record(null); const downloadToastIdRef = useRef(null); + // Held in a ref rather than listed as a dependency. `confirmDiscard` closes over the current + // app state, so it is a new function on every broadcast from main - several a second during + // an interview - and naming it in the effect below would re-run the whole status machine + // that often. + const confirmDiscardRef = useRef(confirmDiscard); + useEffect(() => { + confirmDiscardRef.current = confirmDiscard; + }); + useEffect(() => { if (!updateStatus) return; @@ -65,7 +76,14 @@ export function UpdateNotification() { duration: Infinity, action: { label: isMac ? 'Open Installer' : 'Restart Now', - onClick: () => quitAndInstall(), + // Installing takes the app down, so it loses the interview exactly as closing + // does - and the install cannot be vetoed once started, because the installer is + // launched before the quit. So the question is asked here, in front of it. + onClick: () => { + void confirmDiscardRef.current('update').then((proceed) => { + if (proceed) void quitAndInstall(); + }); + }, }, }); } diff --git a/src/renderer/hooks/use-save-history-guard.ts b/src/renderer/hooks/use-save-history-guard.ts index 630f0d41..940d1127 100644 --- a/src/renderer/hooks/use-save-history-guard.ts +++ b/src/renderer/hooks/use-save-history-guard.ts @@ -6,7 +6,7 @@ import { useAppState } from './use-app-state'; * What is about to destroy the interview. Only the copy differs - the choice is the same one * every time, and phrasing it in terms of the action is what makes it answerable. */ -export type SaveHistoryReason = 'clear' | 'start' | 'close'; +export type SaveHistoryReason = 'clear' | 'start' | 'close' | 'update'; interface SaveHistoryPromptStore { /** The action awaiting an answer, or null when nothing is being asked. */ diff --git a/test/save-history.test.mjs b/test/save-history.test.mjs index 8d813d34..8e41dfb2 100644 --- a/test/save-history.test.mjs +++ b/test/save-history.test.mjs @@ -22,10 +22,7 @@ export async function run() { placeholder.transcripts.length > 0 && placeholder.liveSuggestions.length > 0 ); check('the placeholder is not history', placeholder.hasHistory === false); - check( - 'the renderer is told the same', - appStateService.getRendererState().hasHistory === false - ); + check('the renderer is told the same', appStateService.getRendererState().hasHistory === false); let exportError = null; try { @@ -64,6 +61,39 @@ export async function run() { }); check('a suggestion alone is history too', appStateService.getState().hasHistory === true); + // Action suggestions are not in the exported report - `exportTranscript` builds from + // transcripts and live suggestions alone - so a session holding only a screenshot has nothing + // a save could capture. Counting it would offer to save what the save cannot contain, and + // would put a billed summarize call over an empty transcript back on the table. + appStateService.updateState({ liveSuggestions: [] }); + appStateService.updateState({ + actionSuggestions: [{ timestamp: 3, last_question: 'q', answer: 'a', image_urls: [] }], + }); + check( + 'a screenshot alone is not something a save could capture', + appStateService.getState().hasHistory === false + ); + + let screenshotExportError = null; + try { + await toolsService.exportTranscript('md'); + } catch (e) { + screenshotExportError = e; + } + check('and exporting it is refused rather than billed', screenshotExportError !== null); + appStateService.updateState({ actionSuggestions: [] }); + + // Derived in main and trusted by the close guard, so a caller must not be able to set it. + // It reaches `updateState` inside a Partial the renderer composes. + appStateService.updateState({ + transcripts: [{ timestamp: 4, text: 'real', speaker: 'other', isFinal: true }], + }); + appStateService.updateState({ hasHistory: false }); + check('an incoming hasHistory is ignored', appStateService.getState().hasHistory === true); + appStateService.updateState({ transcripts: [] }); + appStateService.updateState({ hasHistory: true }); + check('in both directions', appStateService.getState().hasHistory === false); + appStateService.setPlaceholderState(); check( 're-seeding the placeholder is not history again', @@ -88,6 +118,38 @@ export async function run() { 'a crashed renderer cannot hold the window open', guard.includes('wc.isDestroyed() || wc.isCrashed()') ); + check( + 'a reload cannot strand the veto with nobody left to answer', + guard.includes("win.webContents.on('did-finish-load'") + ); + + // The installer is launched *inside* quitAndInstall and the quit requested after it, so a + // guard still armed at that point vetoes a quit the update has already committed to - leaving + // an installer running against an app that will not exit. Source-level because driving + // electron-updater in this harness is not something the stub can do. + const updaterIpc = readSource(new URL('../src/main/ipc/auto-updater.ts', import.meta.url)); + check( + 'the updater disarms the close guard before installing', + updaterIpc.indexOf('allowNextClose()') < updaterIpc.indexOf('quitAndInstall()') + ); + check( + 'and re-arms it when nothing was launched', + updaterIpc.includes('if (!quitting) rearmCloseGuard();') && + updaterIpc.includes('catch') && + updaterIpc.split('rearmCloseGuard()').length === 3 + ); + + const updateToast = readSource( + new URL('../src/renderer/components/custom/update-notification.tsx', import.meta.url) + ); + check( + 'the install asks about an unsaved interview first', + updateToast.includes("confirmDiscardRef.current('update')") + ); + check( + 'and only installs when the answer is yes', + updateToast.includes('if (proceed) void quitAndInstall();') + ); return failures; }