Skip to content

Commit 0f602a2

Browse files
committed
fix(browser): fair time budget, remove mutable state, fix processNames conflict
- Restore per-browser time budget to fairly distribute timeout across candidates - Replace _lastDetectedBrowsers/_lastTriedBrowsers with LaunchResult return value - Remove 'chrome.exe' from Chromium processNames to avoid Windows conflict with Chrome - Remove redundant env spread in spawn options
1 parent ea269b1 commit 0f602a2

File tree

2 files changed

+29
-18
lines changed

2 files changed

+29
-18
lines changed

src/browser/bridge.ts

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,16 @@ export type BrowserBridgeState = 'idle' | 'connecting' | 'connected' | 'closing'
2222
/**
2323
* Browser factory: manages daemon lifecycle and provides IPage instances.
2424
*/
25+
interface LaunchResult {
26+
connected: boolean;
27+
detected: string[];
28+
tried: string[];
29+
}
30+
2531
export class BrowserBridge implements IBrowserFactory {
2632
private _state: BrowserBridgeState = 'idle';
2733
private _page: Page | null = null;
2834
private _daemonProc: ChildProcess | null = null;
29-
private _lastDetectedBrowsers: string[] = [];
30-
private _lastTriedBrowsers: string[] = [];
3135
private _inferredBrowserName: string | null = null;
3236

3337
get state(): BrowserBridgeState {
@@ -82,8 +86,9 @@ export class BrowserBridge implements IBrowserFactory {
8286
if (process.env.OPENCLI_VERBOSE || process.stderr.isTTY) {
8387
process.stderr.write('⏳ Waiting for Browser Bridge extension to connect...\n');
8488
}
85-
if (await this._tryLaunchBrowsers(timeoutMs)) return;
86-
throw new Error(this._buildExtensionError(this._lastDetectedBrowsers, this._lastTriedBrowsers));
89+
const result = await this._tryLaunchBrowsers(timeoutMs);
90+
if (result.connected) return;
91+
throw new Error(this._buildExtensionError(result.detected, result.tried));
8792
}
8893

8994
// No daemon — spawn one
@@ -110,10 +115,11 @@ export class BrowserBridge implements IBrowserFactory {
110115
this._daemonProc.unref();
111116

112117
// Wait for daemon + extension with faster polling
113-
if (await this._tryLaunchBrowsers(timeoutMs)) return;
118+
const result = await this._tryLaunchBrowsers(timeoutMs);
119+
if (result.connected) return;
114120

115121
if ((await fetchDaemonStatus()) !== null) {
116-
throw new Error(this._buildExtensionError(this._lastDetectedBrowsers, this._lastTriedBrowsers));
122+
throw new Error(this._buildExtensionError(result.detected, result.tried));
117123
}
118124

119125
throw new Error(
@@ -123,19 +129,22 @@ export class BrowserBridge implements IBrowserFactory {
123129
);
124130
}
125131

126-
private async _tryLaunchBrowsers(timeoutMs: number): Promise<boolean> {
132+
private async _tryLaunchBrowsers(timeoutMs: number): Promise<LaunchResult> {
127133
const candidates = getBrowserCandidates();
128-
this._lastDetectedBrowsers = candidates.map(c => c.name);
129-
this._lastTriedBrowsers = [];
134+
const detected = candidates.map(c => c.name);
135+
const tried: string[] = [];
130136

131-
if (await isExtensionConnected()) return true;
137+
if (await isExtensionConnected()) return { connected: true, detected, tried };
132138

133139
const deadline = Date.now() + timeoutMs;
140+
const perBrowserWaitMs = candidates.length > 0
141+
? Math.min(MAX_PER_BROWSER_WAIT_MS, Math.max(EXTENSION_POLL_INTERVAL_MS, Math.floor(timeoutMs / candidates.length)))
142+
: timeoutMs;
134143

135144
for (const candidate of candidates) {
136145
if (Date.now() >= deadline) break;
137146

138-
this._lastTriedBrowsers.push(candidate.name);
147+
tried.push(candidate.name);
139148
if (process.env.OPENCLI_VERBOSE || process.stderr.isTTY) {
140149
process.stderr.write(` Trying browser: ${candidate.name}\n`);
141150
}
@@ -144,21 +153,23 @@ export class BrowserBridge implements IBrowserFactory {
144153
await launchBrowserCandidate(candidate);
145154
if (await isExtensionConnected()) {
146155
this._inferredBrowserName = candidate.name;
147-
return true;
156+
return { connected: true, detected, tried };
148157
}
149158
}
150159

151-
const waitMs = Math.min(MAX_PER_BROWSER_WAIT_MS, Math.max(0, deadline - Date.now()));
160+
const waitMs = Math.min(perBrowserWaitMs, Math.max(0, deadline - Date.now()));
152161
if (waitMs > 0 && await this._waitForExtensionConnection(waitMs)) {
153162
this._inferredBrowserName = candidate.name;
154-
return true;
163+
return { connected: true, detected, tried };
155164
}
156165
}
157166

158167
// Use any remaining time for a final wait
159168
const remaining = Math.max(0, deadline - Date.now());
160-
if (remaining > 0 && await this._waitForExtensionConnection(remaining)) return true;
161-
return false;
169+
if (remaining > 0 && await this._waitForExtensionConnection(remaining)) {
170+
return { connected: true, detected, tried };
171+
}
172+
return { connected: false, detected, tried };
162173
}
163174

164175
private async _waitForExtensionConnection(timeoutMs: number): Promise<boolean> {

src/browser/candidates.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ const BROWSER_DEFS: BrowserDef[] = [
5050
macAppName: 'Chromium',
5151
linuxBins: ['chromium', 'chromium-browser'],
5252
winExeParts: [['Chromium', 'Application', 'chrome.exe']],
53-
processNames: ['Chromium', 'chromium', 'chromium-browser', 'chrome.exe'],
53+
processNames: ['Chromium', 'chromium', 'chromium-browser'],
5454
},
5555
];
5656

@@ -135,7 +135,7 @@ export function getBrowserCandidates(): BrowserCandidate[] {
135135
}
136136

137137
export async function launchBrowserCandidate(candidate: BrowserCandidate): Promise<void> {
138-
const opts = { detached: true as const, stdio: 'ignore' as const, env: { ...process.env } };
138+
const opts = { detached: true as const, stdio: 'ignore' as const };
139139

140140
const cmd = process.platform === 'darwin'
141141
? { bin: 'open', args: [candidate.executable] }

0 commit comments

Comments
 (0)