Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 39 additions & 7 deletions app/e2e-mock-ipc.js
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,21 @@ function install({ ipcMain }) {
? 'mlx-community/parakeet-tdt-0.6b-v3'
: 'istupakov/parakeet-tdt-0.6b-v3-onnx';

// Mirrors main.js: a successful check that finds no update settles an earlier
// FAILED check, and it does so in the state the About tab rehydrates from —
// so the T1 spec asserts the contract (check clears it for good), not just a
// local setState. Seeded by STENOAI_E2E_SEED_UPDATE_ERROR.
let seededUpdateError =
process.env.STENOAI_E2E_SEED_UPDATE_ERROR === '1'
? "Steno couldn't reach the update server. Check your connection — it will try again later."
: null;

// Only the FIRST get-update-status is delayed, so the About tab's mount-time
// request is still in flight when the check that settles the error completes,
// and its stale reply lands last. Delaying every call would let the check's
// own re-read arrive after it and paper over the bug being guarded against.
let slowUpdateStatusCallsLeft = process.env.STENOAI_E2E_SLOW_UPDATE_STATUS === '1' ? 1 : 0;

const DEFAULTS = {
'get-app-version': { success: true, version: '0.0.0-e2e', name: 'Steno' },
// Read-only display poll for the About tab's "Check for Updates" button
Expand All @@ -605,6 +620,10 @@ function install({ ipcMain }) {
osUpdateEligible: false,
};
}
// Up to date, and that settles a previously failed check — main.js
// clears the persisted error in exactly this branch, so the mock does
// too and the spec can assert it stays gone across a remount.
seededUpdateError = null;
return {
success: true,
updateAvailable: false,
Expand All @@ -624,13 +643,26 @@ function install({ ipcMain }) {
// so the About tab's mount-time rehydration (settings-about.t1) can assert
// the failure is restored on navigation, not just from the live one-shot
// 'update-error' event.
'get-update-status': () => ({
success: true,
downloadedVersion: null,
downloadPercent: null,
downloadError:
process.env.STENOAI_E2E_SEED_UPDATE_ERROR === '1' ? 'network unreachable' : null,
}),
'get-update-status': async () => {
// Answer with the state as of the REQUEST, not as of the reply. That is
// what main.js does (it reads pendingUpdateError when the handler runs),
// and it is what makes STENOAI_E2E_SLOW_UPDATE_STATUS a real race rather
// than a sleep.
const snapshot = seededUpdateError;
if (slowUpdateStatusCallsLeft > 0) {
slowUpdateStatusCallsLeft -= 1;
await new Promise((resolve) => setTimeout(resolve, 1000));
}
return {
success: true,
downloadedVersion: null,
downloadPercent: null,
// main.js sends the About tab a finished sentence, not the raw
// electron-updater text (update-error-copy.js), so the seed mirrors that
// shape and the spec asserts what a user would actually read.
downloadError: snapshot,
};
},
// Fires on first paint once signed in (Sidebar + RouteView gate the
// Shared notes feature on it). Default to feature-enabled to match the
// adapter's default and keep the org-lock spec's UI unchanged. A spec can
Expand Down
101 changes: 97 additions & 4 deletions app/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ const { createTeardownRegistry } = require('./teardown');
const { registerFoldersIpc } = require('./folders-ipc');
const { registerSettingsIpc } = require('./settings-ipc');
const { isSafeToAutoInstall } = require('./update-idle-gate');
const { describeUpdateError, updateErrorPhase } = require('./update-error-copy');
const { isOSUpdateEligible, MIN_MACOS_FOR_AUTOUPDATE } = require('./update-os-gate');
const processingLog = require('./processing-log');
const { isMeetingApp, allowsDeviceLevelFallback, isMacos14Plus } = require('./meeting-detect');
Expand Down Expand Up @@ -6133,6 +6134,15 @@ let pendingDownloadPercent = null;
// a failed background update would show nothing. Cleared when a new cycle
// starts (check / available / progress) or a download completes.
let pendingUpdateError = null;
// Whether that error survives a later successful check. A dropped connection
// is disproved by one; a full disk, a missing update feed or a permission
// problem is not, and clearing those on an unrelated success would hide a
// condition that is still true. See update-error-copy.js.
let pendingUpdateErrorSticky = false;
// True between calling quitAndInstall and the app actually going away, so an
// error that fires in that window is reported as a failed install rather than
// as a failed check.
let installingUpdate = false;

// ── Idle auto-install ──
// True once an update has finished downloading and is staged for install.
Expand Down Expand Up @@ -6219,6 +6229,8 @@ async function maybeAutoInstallWhenIdle() {
// Bypass the mainWindow 'close' handler's preventDefault+hide (same reason as
// the manual install-update path) so quitAndInstall actually quits + applies.
isQuitting = true;
// From here on, an updater error is an INSTALL failure, not a failed check.
installingUpdate = true;
// isSilent=true, isForceRunAfter=true — install without the wizard and
// relaunch the app afterwards.
autoUpdater.quitAndInstall(true, true);
Expand Down Expand Up @@ -6278,32 +6290,75 @@ function setupAutoUpdater() {
autoUpdater.autoDownload = true;
autoUpdater.autoInstallOnAppQuit = true;

// Drop a stale non-sticky failure AND tell a mounted About tab, so the banner
// never outlives the state it describes. Sticky ones are left alone: a full
// disk, a permission problem or a missing feed is not disproved by starting or
// finding an update, only by bytes actually arriving (download-progress).
// Both callers need exactly this, and having it in one place is what stops the
// two from drifting apart — the earlier version cleared here but only emitted
// there, so the event never fired and About kept showing a settled failure
// next to a running download.
const clearNonStickyUpdateError = () => {
if (!pendingUpdateError || pendingUpdateErrorSticky) return;
pendingUpdateError = null;
if (mainWindow) mainWindow.webContents.send('update-error-cleared');
};

autoUpdater.on('checking-for-update', () => {
sendDebugLog('Auto-updater: checking for updates...');
// Fresh cycle — clear any stale error from a previous failed check so a
// rehydrating About tab doesn't show an error that's now being retried.
pendingUpdateError = null;
// Sticky ones stay: starting a check does not free disk space, grant write
// permission, or give this build an update feed, and those conditions are
// still true until something actually succeeds. They are cleared where that
// happens: a version found and fetched (update-available / download-progress
// / update-downloaded below), or a poll that comes back clean in the
// check-for-updates handler.
clearNonStickyUpdateError();
});

autoUpdater.on('update-available', (info) => {
sendDebugLog(`Auto-updater: update available (v${info.version})`);
// Matches the renderer's own `setDownloadPercent((p) => p ?? 0)` — marks
// a download as started before the first real progress tick arrives.
if (pendingDownloadPercent === null) pendingDownloadPercent = 0;
pendingUpdateError = null;
// Only the non-sticky ones. This event says a version was FOUND, i.e. the
// feed was readable — it does not say a single byte was written, so it
// cannot disprove a full disk or a permission problem. Those clear one
// event later, on the first download-progress tick, which does prove it.
// (Clearing them here made the banner vanish and come straight back when
// the unchanged condition failed the download again.)
//
// Usually a no-op, since checking-for-update already ran for this cycle —
// it covers a failure that arrived between the two events.
clearNonStickyUpdateError();
if (mainWindow) {
mainWindow.webContents.send('update-available', { version: info.version });
}
});

autoUpdater.on('update-not-available', () => {
sendDebugLog('Auto-updater: up to date');
// A completed cycle with nothing pending — clear even a sticky failure.
// This is the updater itself reporting success, unlike the GitHub poll in
// the check-for-updates handler (our own request, which proves nothing
// about whether the updater can read its feed or write to disk). Nothing is
// waiting to download or install any more, so a banner about a past attempt
// is describing a state that no longer exists — and a condition that really
// is still broken (no update feed) errors before ever reaching this event.
pendingUpdateError = null;
pendingUpdateErrorSticky = false;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: A successful download can still leave a previous failure banner visible. update-downloaded clears main state but does not notify the mounted About tab's error state, so emit update-error-cleared when this transition clears an error (or clear/invalidate it in offDownloaded).

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/main.js, line 6350:

<comment>A successful download can still leave a previous failure banner visible. `update-downloaded` clears main state but does not notify the mounted About tab's error state, so emit `update-error-cleared` when this transition clears an error (or clear/invalidate it in `offDownloaded`).</comment>

<file context>
@@ -6278,32 +6290,75 @@ function setupAutoUpdater() {
+    // is describing a state that no longer exists — and a condition that really
+    // is still broken (no update feed) errors before ever reaching this event.
+    pendingUpdateError = null;
+    pendingUpdateErrorSticky = false;
+    // Tell a mounted About tab too — it keeps its own copy, and the events it
+    // already listens to (available/progress/downloaded) don't fire on a clean
</file context>

// Tell a mounted About tab too — it keeps its own copy, and the events it
// already listens to (available/progress/downloaded) don't fire on a clean
// cycle, so without this the banner would sit there until a remount.
if (mainWindow) mainWindow.webContents.send('update-error-cleared');
});

autoUpdater.on('download-progress', (progress) => {
sendDebugLog(`Auto-updater: downloading ${Math.round(progress.percent)}%`);
pendingDownloadPercent = Math.round(progress.percent);
pendingUpdateError = null;
pendingUpdateErrorSticky = false;
if (mainWindow) {
mainWindow.webContents.send('update-download-progress', { percent: Math.round(progress.percent) });
}
Expand All @@ -6313,6 +6368,7 @@ function setupAutoUpdater() {
sendDebugLog(`Auto-updater: v${info.version} ready to install`);
pendingDownloadPercent = null;
pendingUpdateError = null;
pendingUpdateErrorSticky = false;
pendingUpdateVersion = info.version;
if (mainWindow) {
mainWindow.webContents.send('update-downloaded', { version: info.version });
Expand All @@ -6328,6 +6384,21 @@ function setupAutoUpdater() {

autoUpdater.on('error', (err) => {
const msg = (err && err.message) || String(err);
// Which phase failed, captured BEFORE the state is cleared below. Note it
// does NOT consider a staged update: a periodic check can fail long after a
// download succeeded, and calling that a failed download is the lie this
// whole path exists to stop telling.
//
// Known limit: electron-updater's error event does not say which attempt it
// belongs to, so a periodic check that fails WHILE a download is genuinely
// running is attributed to the download. That case was already reported as
// a download failure before this module existed, so the heuristic is not a
// regression — closing it needs per-attempt correlation the library does
// not expose.
const phase = updateErrorPhase({
downloadInFlight: pendingDownloadPercent !== null,
installing: installingUpdate,
});
// Clear any in-flight download state regardless of which branch below
// fires — otherwise a failure after 'update-available' (which seeds
// pendingDownloadPercent to 0) leaves get-update-status reporting a
Expand All @@ -6343,12 +6414,19 @@ function setupAutoUpdater() {
sendDebugLog('Auto-updater: no update feed published for this release yet — skipping.');
return;
}
// The raw text stays here, where it's useful for diagnosis. What reaches
// the About tab is one sentence naming the phase that failed and whether
// the user has to do anything — see update-error-copy.js.
sendDebugLog(`Auto-updater error: ${msg}`);
const { message: userMessage, sticky } = describeUpdateError(msg, { phase });
// The install attempt (if any) is over — a later error is a fresh cycle.
installingUpdate = false;
// Persist so a later About-tab mount can rehydrate it (the event below is
// one-shot and only reaches an already-mounted listener).
pendingUpdateError = msg;
pendingUpdateError = userMessage;
pendingUpdateErrorSticky = sticky;
if (mainWindow) {
mainWindow.webContents.send('update-error', { message: msg });
mainWindow.webContents.send('update-error', { message: userMessage });
}
});

Expand Down Expand Up @@ -6377,6 +6455,8 @@ ipcMain.on('install-update', () => {
// quitAndInstall's window-close step actually quits the app. Without this
// the app just minimises and Squirrel never gets to apply the update.
isQuitting = true;
// From here on, an updater error is an INSTALL failure, not a failed check.
installingUpdate = true;
autoUpdater.quitAndInstall(false, true);
});

Expand Down Expand Up @@ -9438,6 +9518,19 @@ ipcMain.handle('check-for-updates', async () => {
if (!IS_E2E && app.isPackaged && osEligible) {
autoUpdater.checkForUpdates().catch(() => {});
}
// A check that just succeeded and found nothing settles an earlier FAILED
// check — otherwise a stale banner sits under a fresh "You're on the latest
// version", two contradictory answers to one question. Cleared here, in the
// state the About tab rehydrates from, so it stays cleared across a remount
// rather than only until the user switches tabs.
//
// Only non-sticky errors: this poll is our own GitHub request, so it says
// nothing about whether the updater can write to /Applications or whether
// this build has an update feed at all. Those conditions are still true and
// stay on screen (see update-error-copy.js).
if (result.success && !result.updateAvailable && !pendingUpdateErrorSticky) {
pendingUpdateError = null;
}
// Surface eligibility so the About tab can explain why an "update available"
// won't auto-install on an under-floor Mac, rather than offering a broken
// Restart. (Display-only; the safety is the gated kick above.)
Expand Down
2 changes: 1 addition & 1 deletion app/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
"typecheck:renderer": "tsc -p renderer/tsconfig.json --noEmit",
"lint:renderer": "eslint --config renderer/eslint.config.mjs renderer/src",
"format:renderer": "prettier --write renderer/src",
"test:unit": "node --test processing-log.test.js meeting-detect.test.js notes-file.test.js backend-stream.test.js ipc-contract.test.js shortcut-url.test.js setup-check-parse.test.js diagnostics-forward.test.js analytics-helpers.test.js live-snapshot-sweep.test.js backend-cli.test.js debug-log.test.js teardown.test.js folders-ipc.test.js settings-ipc.test.js regen-title-busy-guard.test.js update-idle-gate.test.js update-os-gate.test.js && vitest run",
"test:unit": "node --test processing-log.test.js meeting-detect.test.js notes-file.test.js backend-stream.test.js ipc-contract.test.js shortcut-url.test.js setup-check-parse.test.js diagnostics-forward.test.js analytics-helpers.test.js live-snapshot-sweep.test.js backend-cli.test.js debug-log.test.js teardown.test.js folders-ipc.test.js settings-ipc.test.js regen-title-busy-guard.test.js update-idle-gate.test.js update-os-gate.test.js update-error-copy.test.js && vitest run",
"build": "npm run build:renderer && electron-builder",
"pack:unsigned": "npm run build:renderer && electron-builder --dir --config electron-builder.ci.yml",
"build-mac": "npm run build:renderer && electron-builder --mac",
Expand Down
4 changes: 4 additions & 0 deletions app/preload.js
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,10 @@ const stenoai = {
updateDownloadProgress: (cb) => subscribe('update-download-progress', cb),
updateDownloaded: (cb) => subscribe('update-downloaded', cb),
updateError: (cb) => subscribe('update-error', cb),
// Main cleared a failure the About tab may still be showing (a cycle that
// came back clean). Without this the banner would linger on a mounted tab
// until the user navigated away and back.
updateErrorCleared: (cb) => subscribe('update-error-cleared', cb),
googleAuthChanged: (cb) => subscribe('google-auth-changed', cb),
outlookAuthChanged: (cb) => subscribe('outlook-auth-changed', cb),
shortcutStartRecording: (cb) => subscribe('shortcut-start-recording', cb),
Expand Down
1 change: 1 addition & 0 deletions app/renderer/src/lib/ipc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1107,6 +1107,7 @@ export interface StenoaiBridge {
updateDownloadProgress: Subscribe<UpdateProgressEvent>;
updateDownloaded: Subscribe<UpdateDownloadedEvent>;
updateError: Subscribe<UpdateErrorEvent>;
updateErrorCleared: Subscribe<void>;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: About now subscribes to this new event, but the E2E IPC mock does not expose update-error-cleared; mounting the About tab under the mock can therefore fail before the updater scenario runs, and the mock cannot model the new clear behavior. Adding the channel to the mock (including a no-op/emit implementation consistent with the other subscriptions) keeps the test bridge in sync with the preload contract.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/renderer/src/lib/ipc.ts, line 1110:

<comment>About now subscribes to this new event, but the E2E IPC mock does not expose `update-error-cleared`; mounting the About tab under the mock can therefore fail before the updater scenario runs, and the mock cannot model the new clear behavior. Adding the channel to the mock (including a no-op/emit implementation consistent with the other subscriptions) keeps the test bridge in sync with the preload contract.</comment>

<file context>
@@ -1107,6 +1107,7 @@ export interface StenoaiBridge {
     updateDownloadProgress: Subscribe<UpdateProgressEvent>;
     updateDownloaded: Subscribe<UpdateDownloadedEvent>;
     updateError: Subscribe<UpdateErrorEvent>;
+    updateErrorCleared: Subscribe<void>;
     googleAuthChanged: Subscribe<{ connected: boolean }>;
     outlookAuthChanged: Subscribe<{ connected: boolean }>;
</file context>

googleAuthChanged: Subscribe<{ connected: boolean }>;
outlookAuthChanged: Subscribe<{ connected: boolean }>;
shortcutStartRecording: Subscribe<ShortcutStartRecordingEvent>;
Expand Down
Loading
Loading