diff --git a/CLAUDE.md b/CLAUDE.md index fa512b07..92612379 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -118,6 +118,74 @@ 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 - +`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 +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. + +**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/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/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/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..f81859ab 100644 --- a/src/main/services/app-state.service.ts +++ b/src/main/services/app-state.service.ts @@ -29,8 +29,26 @@ const DEFAULT_STATE: AppState = { providedLLMModel: undefined, interviewConfig: { fullName: '', profileData: '', context: '' }, interviewConfigLoaded: false, + hasHistory: false, }; +/** The three arrays a session fills, and that the placeholder seeds. */ +const HISTORY_KEYS = ['transcripts', 'liveSuggestions', 'actionSuggestions'] as const; + +/** + * 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 // that grows for the whole interview. Coalescing bounds that cost per unit time instead of per @@ -41,12 +59,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 +109,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 +142,45 @@ 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(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; + + if (this.placeholderActive) { + for (const key of HISTORY_KEYS) { + if (!touched.includes(key)) next[key] = [] as never; + } + this.placeholderActive = false; + } + + next.hasHistory = EXPORTABLE_KEYS.some( + (key: ExportableKey) => (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 +252,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/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/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..7b565564 100644 --- a/src/main/types/app-state.ts +++ b/src/main/types/app-state.ts @@ -101,6 +101,20 @@ export interface AppState { interviewConfig: InterviewConfig; /** False until the account's config has been read this session; editing is unsafe before then. */ interviewConfigLoaded: boolean; + /** + * Whether there is an interview that saving would actually capture. + * + * 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; } /** 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..21e7758e --- /dev/null +++ b/src/main/window-close-guard.ts @@ -0,0 +1,106 @@ +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 { + // 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; + + // 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'); + }); +} + +/** + * 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; + }); + + 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..c97045bb --- /dev/null +++ b/src/renderer/components/custom/save-history-dialog.tsx @@ -0,0 +1,174 @@ +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', + }, + update: { + title: 'Save this interview before installing the update?', + body: 'Installing restarts the app and drops the transcript and the suggestions from this session.', + discard: 'Install 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/components/custom/update-notification.tsx b/src/renderer/components/custom/update-notification.tsx index b82a3dc6..fdaaf26e 100644 --- a/src/renderer/components/custom/update-notification.tsx +++ b/src/renderer/components/custom/update-notification.tsx @@ -2,12 +2,23 @@ import { useEffect, useRef } from 'react'; import { toast } from 'sonner'; import { UpdateStatus, useAutoUpdater } from '@/hooks/use-auto-updater'; +import { useSaveHistoryGuard } from '@/hooks/use-save-history-guard'; export function UpdateNotification() { const { updateStatus, quitAndInstall } = useAutoUpdater(); + const { confirmDiscard } = useSaveHistoryGuard(); const lastStatusRef = useRef(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-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..940d1127 --- /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' | 'update'; + +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..8e41dfb2 --- /dev/null +++ b/test/save-history.test.mjs @@ -0,0 +1,155 @@ +/** + * 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); + + // 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', + 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()') + ); + 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; +}