Skip to content

Commit 201c2b6

Browse files
committed
Bug fixes: stable sentinel, quoted .env parser, Python version gate, ffmpeg check, dedupe first-run
1 parent 31106f0 commit 201c2b6

3 files changed

Lines changed: 139 additions & 40 deletions

File tree

main.js

Lines changed: 39 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
require("dotenv").config();
22

3+
const path = require("path");
34
const { app, BrowserWindow, globalShortcut, session, ipcMain } = require("electron");
45

56
// ── Linux GPU process crash workaround ──
@@ -58,7 +59,10 @@ class ApplicationController {
5859
// a settings-window prompt on first launch so users don't have to
5960
// dig through docs to figure out they need a Gemini API key.
6061
this.firstRunManager = new FirstRunManager({
61-
logger: logger
62+
logger: logger,
63+
// Sentinel lives in userData so it survives cwd changes
64+
// (the app may be launched from any directory).
65+
sentinelPath: path.join(app.getPath("userData"), ".opencluely-firstrun-completed"),
6266
});
6367
// Lazily-initialised in getWhisperInstaller() so tests can mock
6468
// the constructor without polluting main-process startup.
@@ -166,10 +170,21 @@ class ApplicationController {
166170
// Small delay to ensure desktop/space detection is accurate
167171
await new Promise((resolve) => setTimeout(resolve, 200));
168172

169-
// During first-run onboarding, defer showing the main overlay
170-
// window until the wizard finishes (it needs API keys to function).
171-
const status = this.firstRunManager.getStatus();
173+
// First-run onboarding: ensure .env exists and read status once
174+
// so we can decide whether to defer showing the main overlay.
175+
let status;
176+
try {
177+
this.firstRunManager.ensureEnv();
178+
status = this.firstRunManager.getStatus();
179+
this.isFirstRun = status.needsOnboarding;
180+
logger.info("First-run status", status);
181+
} catch (e) {
182+
logger.warn("First-run check failed", { error: e.message });
183+
status = { needsOnboarding: false };
184+
this.isFirstRun = false;
185+
}
172186
const isFirstRun = status.needsOnboarding;
187+
173188
await windowManager.initializeWindows({ showMainWindow: !isFirstRun });
174189
this.setupGlobalShortcuts();
175190

@@ -179,36 +194,26 @@ class ApplicationController {
179194
this.starting = false;
180195
this.isReady = true;
181196

182-
// First-run onboarding: ensure .env exists and launch the
183-
// multi-step onboarding wizard if the user hasn't completed it.
184-
// Non-blocking — failure here just logs.
185-
try {
186-
this.firstRunManager.ensureEnv();
187-
const status = this.firstRunManager.getStatus();
188-
this.isFirstRun = status.needsOnboarding;
189-
logger.info("First-run status", status);
190-
if (this.isFirstRun) {
191-
// Defer slightly so all windows finish loading before we pop
192-
// the wizard on top of them.
193-
setTimeout(() => {
194-
try {
195-
windowManager.showOnboarding();
196-
windowManager.broadcastToAllWindows("first-run", status);
197-
logger.info("First-run onboarding: wizard opened");
198-
} catch (e) {
199-
logger.warn("Could not open first-run onboarding window", {
200-
error: e.message
201-
});
202-
// Fallback to legacy settings prompt
203-
try { this.showSettings(); } catch (_) { /* ignore */ }
204-
}
205-
}, 800);
206-
} else {
207-
// Already configured — mark completed so we never nag again.
208-
this.firstRunManager.markCompleted();
209-
}
210-
} catch (e) {
211-
logger.warn("First-run check failed", { error: e.message });
197+
// Launch the onboarding wizard if this is the first run.
198+
if (this.isFirstRun) {
199+
// Defer slightly so all windows finish loading before we pop
200+
// the wizard on top of them.
201+
setTimeout(() => {
202+
try {
203+
windowManager.showOnboarding();
204+
windowManager.broadcastToAllWindows("first-run", status);
205+
logger.info("First-run onboarding: wizard opened");
206+
} catch (e) {
207+
logger.warn("Could not open first-run onboarding window", {
208+
error: e.message
209+
});
210+
// Fallback to legacy settings prompt
211+
try { this.showSettings(); } catch (_) { /* ignore */ }
212+
}
213+
}, 800);
214+
} else {
215+
// Already configured — mark completed so we never nag again.
216+
this.firstRunManager.markCompleted();
212217
}
213218

214219
logger.info("Application initialized successfully", {

src/core/first-run.js

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -94,13 +94,29 @@ class FirstRunManager {
9494
try {
9595
const content = fs.readFileSync(this.envPath, 'utf8');
9696
const result = {};
97-
for (const line of content.split(/\r?\n/)) {
98-
const trimmed = line.trim();
99-
if (!trimmed || trimmed.startsWith('#')) continue;
100-
const eq = trimmed.indexOf('=');
97+
for (const rawLine of content.split(/\r?\n/)) {
98+
const line = rawLine.trim();
99+
if (!line || line.startsWith('#')) continue;
100+
const eq = line.indexOf('=');
101101
if (eq === -1) continue;
102-
const key = trimmed.slice(0, eq).trim();
103-
const value = trimmed.slice(eq + 1).trim();
102+
const key = line.slice(0, eq).trim();
103+
let value = line.slice(eq + 1).trim();
104+
105+
// If the value is quoted, find the matching closing quote and
106+
// take everything between. Anything after the closing quote is
107+
// treated as trailing whitespace/comment.
108+
if (value.startsWith('"') || value.startsWith("'")) {
109+
const quote = value[0];
110+
const closeIdx = value.indexOf(quote, 1);
111+
if (closeIdx !== -1) {
112+
value = value.slice(1, closeIdx);
113+
}
114+
} else {
115+
// Unquoted: strip trailing inline comment (a " #" sequence).
116+
const hashIdx = value.indexOf(' #');
117+
if (hashIdx !== -1) value = value.slice(0, hashIdx).trim();
118+
}
119+
104120
result[key] = value;
105121
}
106122
return result;

src/core/whisper-installer.js

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,22 @@ class WhisperInstaller {
230230
}
231231
log(`✓ Found Python: ${python}`);
232232

233+
// openai-whisper requires Python 3.9+ (3.10+ recommended for best
234+
// performance). Catch version mismatch BEFORE attempting pip
235+
// install — the pip error is cryptic and confusing.
236+
const version = await this._getPythonVersion(python);
237+
if (!version) {
238+
log('! Could not determine Python version');
239+
return { ok: false, command: null, message: 'Could not determine Python version', logs: '' };
240+
}
241+
log(`→ Python version: ${version}`);
242+
if (!this._isPythonVersionOk(version)) {
243+
const msg = `Python ${version} is too old. openai-whisper requires Python 3.9 or newer. Please upgrade Python and retry.`;
244+
log(`! ${msg}`);
245+
return { ok: false, command: null, message: msg, logs: msg };
246+
}
247+
log('✓ Python version OK');
248+
233249
const vp = this.venvPaths;
234250
const venvExists = fs.existsSync(vp.python);
235251

@@ -286,6 +302,21 @@ class WhisperInstaller {
286302
return { ok: false, command: null, message: msg, logs: msg };
287303
}
288304

305+
// Check for ffmpeg — whisper needs it for any non-WAV audio.
306+
// We log a warning but don't fail; user can install it later.
307+
const ffmpeg = await this._probeFfmpeg();
308+
if (ffmpeg.found) {
309+
log(`✓ ffmpeg detected (${ffmpeg.path})`);
310+
} else {
311+
const ffmpegMsg = this.platform === 'win32'
312+
? 'ffmpeg not found — install with `winget install ffmpeg` or download from gyan.dev. Required for non-WAV audio.'
313+
: this.platform === 'darwin'
314+
? 'ffmpeg not found — install with `brew install ffmpeg`. Required for non-WAV audio.'
315+
: 'ffmpeg not found — install with `sudo apt install ffmpeg` (Debian/Ubuntu). Required for non-WAV audio.';
316+
log(`! ${ffmpegMsg}`);
317+
log(' (Whisper will work for WAV files; install ffmpeg later for other formats)');
318+
}
319+
289320
const commandStr = `${vp.python} -m whisper`;
290321
log(`✓ Whisper CLI ready: ${commandStr} (v${verify.version || '?'})`);
291322

@@ -294,9 +325,29 @@ class WhisperInstaller {
294325
command: commandStr,
295326
message: `Installed Whisper v${verify.version || '?'} into ${this.venvPath}`,
296327
logs: pipResult.stdout,
328+
ffmpegDetected: ffmpeg.found,
297329
};
298330
}
299331

332+
/**
333+
* Probe for ffmpeg on PATH. Returns { found, path }.
334+
*/
335+
async _probeFfmpeg() {
336+
try {
337+
const { spawnSync } = require('child_process');
338+
const r = spawnSync(
339+
this.platform === 'win32' ? 'where' : 'which',
340+
['ffmpeg'],
341+
{ windowsHide: true },
342+
);
343+
if (r.status === 0) {
344+
const path = (r.stdout || '').toString().split(/\r?\n/)[0].trim();
345+
return { found: true, path };
346+
}
347+
} catch (_) { /* ignore */ }
348+
return { found: false, path: null };
349+
}
350+
300351
/**
301352
* Short platform-tailored hints to show the user in the wizard.
302353
*/
@@ -427,6 +478,33 @@ class WhisperInstaller {
427478
}
428479
return null;
429480
}
481+
482+
/**
483+
* Resolve Python's `--version` string into a `major.minor` tuple
484+
* (e.g. '3.11'). Returns null if it can't be determined.
485+
*/
486+
async _getPythonVersion(pythonCmd) {
487+
const r = await this.runExec(pythonCmd, ['-c', 'import sys; print(f"{sys.version_info.major}.{sys.version_info.minor}")'], {
488+
timeout: 10000,
489+
});
490+
if (!r.ok) return null;
491+
const m = (r.stdout || '').trim().match(/^(\d+)\.(\d+)/);
492+
return m ? `${m[1]}.${m[2]}` : null;
493+
}
494+
495+
/**
496+
* openai-whisper requires Python 3.9+. We warn below 3.10 and refuse
497+
* below 3.9. Returns false if the version is too old.
498+
*/
499+
_isPythonVersionOk(version) {
500+
const m = (version || '').match(/^(\d+)\.(\d+)/);
501+
if (!m) return false;
502+
const major = parseInt(m[1], 10);
503+
const minor = parseInt(m[2], 10);
504+
if (major > 3) return true;
505+
if (major === 3 && minor >= 9) return true;
506+
return false;
507+
}
430508
}
431509

432510
module.exports = WhisperInstaller;

0 commit comments

Comments
 (0)