Skip to content

Commit 8f2e057

Browse files
committed
refactor(chrome-extension): improve code quality per review feedback
- Extract ~300 lines of CDP/browser helpers from test file into chrome-extension-helpers.ts with proper type definitions - Define CdpTarget and ExtensionSettings interfaces to eliminate `any` - Replace globalThis.__extId with describe-scope variable - Replace numeric state machine with NavigateInjectStep type union - Extract magic numbers into named constants - Fix event listener leak: store xvfbCleanup ref, removeListener in destroy() - Delete unused openBrowserWithExtension from test-utils.ts
1 parent c084457 commit 8f2e057

4 files changed

Lines changed: 357 additions & 347 deletions

File tree

‎packages/computer/src/device.ts‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,7 @@ export class ComputerDevice implements AbstractInterface {
298298
private description?: string;
299299
private destroyed = false;
300300
private xvfbInstance?: XvfbInstance;
301+
private xvfbCleanup?: () => void;
301302
/**
302303
* On macOS, use AppleScript for keyboard operations by default
303304
* to avoid focus issues with system overlays (e.g. Spotlight).
@@ -350,16 +351,16 @@ export class ComputerDevice implements AbstractInterface {
350351
process.env.DISPLAY = this.xvfbInstance.display;
351352
debugDevice(`Xvfb started on display ${this.xvfbInstance.display}`);
352353

353-
// Clean up Xvfb on process exit
354-
const cleanup = () => {
354+
// Clean up Xvfb on process exit (stored for removal in destroy())
355+
this.xvfbCleanup = () => {
355356
if (this.xvfbInstance) {
356357
this.xvfbInstance.stop();
357358
this.xvfbInstance = undefined;
358359
}
359360
};
360-
process.on('exit', cleanup);
361-
process.on('SIGINT', cleanup);
362-
process.on('SIGTERM', cleanup);
361+
process.on('exit', this.xvfbCleanup);
362+
process.on('SIGINT', this.xvfbCleanup);
363+
process.on('SIGTERM', this.xvfbCleanup);
363364
}
364365

365366
// Load libnut on first connect
@@ -816,6 +817,12 @@ Available Displays: ${displays.length > 0 ? displays.map((d) => d.name).join(',
816817
this.xvfbInstance.stop();
817818
this.xvfbInstance = undefined;
818819
}
820+
if (this.xvfbCleanup) {
821+
process.removeListener('exit', this.xvfbCleanup);
822+
process.removeListener('SIGINT', this.xvfbCleanup);
823+
process.removeListener('SIGTERM', this.xvfbCleanup);
824+
this.xvfbCleanup = undefined;
825+
}
819826

820827
this.destroyed = true;
821828
debugDevice('Computer device destroyed');
Lines changed: 334 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,334 @@
1+
import { execSync, spawn } from 'node:child_process';
2+
import fs from 'node:fs';
3+
import path from 'node:path';
4+
import { sleep } from '@midscene/core/utils';
5+
import { isHeadlessLinux } from './test-utils';
6+
7+
// ─── Constants ──────────────────────────────────────────────────────────────
8+
9+
export const CDP_PORT = 9222;
10+
const USER_DATA_DIR = '/tmp/midscene-chrome-ext-test';
11+
const BROWSER_STARTUP_DELAY = 10_000;
12+
const EXTENSION_POLL_INTERVAL = 2_000;
13+
const CDP_INJECTION_TIMEOUT = 10_000;
14+
const NAVIGATE_INJECT_TIMEOUT = 15_000;
15+
const RELOAD_TIMEOUT = 5_000;
16+
17+
// ─── Types ──────────────────────────────────────────────────────────────────
18+
19+
export interface CdpTarget {
20+
type: string;
21+
url: string;
22+
webSocketDebuggerUrl?: string;
23+
}
24+
25+
interface ExtensionSettings {
26+
manifest?: { name?: string };
27+
path?: string;
28+
}
29+
30+
// ─── Environment Config Keys ────────────────────────────────────────────────
31+
32+
const EXTENSION_ENV_KEYS = [
33+
'MIDSCENE_OPENAI_INIT_CONFIG_JSON',
34+
'MIDSCENE_MODEL_INIT_CONFIG_JSON',
35+
'MIDSCENE_MODEL_NAME',
36+
'MIDSCENE_MODEL_API_KEY',
37+
'MIDSCENE_MODEL_BASE_URL',
38+
'MIDSCENE_MODEL_FAMILY',
39+
'MIDSCENE_USE_QWEN3_VL',
40+
'OPENAI_API_KEY',
41+
'OPENAI_BASE_URL',
42+
] as const;
43+
44+
// ─── Browser Helpers ────────────────────────────────────────────────────────
45+
46+
function findExtensionCapableBrowser(): string {
47+
// Check puppeteer cache first (Chrome for Testing supports --load-extension)
48+
const puppeteerBase = path.join(
49+
process.env.HOME ?? '~',
50+
'.cache/puppeteer/chrome',
51+
);
52+
if (fs.existsSync(puppeteerBase)) {
53+
const versions = fs
54+
.readdirSync(puppeteerBase)
55+
.filter((d) => d.startsWith('linux-'));
56+
if (versions.length > 0) {
57+
const chromeBin = path.join(
58+
puppeteerBase,
59+
versions[0],
60+
'chrome-linux64',
61+
'chrome',
62+
);
63+
if (fs.existsSync(chromeBin)) {
64+
return chromeBin;
65+
}
66+
}
67+
}
68+
69+
for (const bin of ['chromium-browser', 'chromium']) {
70+
try {
71+
execSync(`which ${bin}`, { stdio: 'ignore' });
72+
return bin;
73+
} catch {
74+
// try next
75+
}
76+
}
77+
78+
throw new Error(
79+
'No extension-capable browser found. Need Chrome for Testing or Chromium.',
80+
);
81+
}
82+
83+
export async function launchChromeWithExtension(
84+
extensionPath: string,
85+
url: string,
86+
): Promise<void> {
87+
if (!isHeadlessLinux()) {
88+
throw new Error('Only supports headless Linux CI');
89+
}
90+
execSync(`rm -rf '${USER_DATA_DIR}'`, { stdio: 'ignore' });
91+
92+
const browser = findExtensionCapableBrowser();
93+
const args = [
94+
'--no-sandbox',
95+
'--disable-gpu',
96+
'--disable-dev-shm-usage',
97+
'--no-first-run',
98+
'--no-default-browser-check',
99+
`--load-extension=${extensionPath}`,
100+
`--disable-extensions-except=${extensionPath}`,
101+
`--user-data-dir=${USER_DATA_DIR}`,
102+
`--remote-debugging-port=${CDP_PORT}`,
103+
'--window-size=1920,1080',
104+
'--start-maximized',
105+
url,
106+
];
107+
108+
console.log(`DISPLAY=${process.env.DISPLAY}`);
109+
console.log('Launching browser...');
110+
111+
const child = spawn(browser, args, {
112+
stdio: ['ignore', 'pipe', 'pipe'],
113+
detached: true,
114+
env: process.env,
115+
});
116+
117+
child.stderr?.on('data', (data: Buffer) => {
118+
const line = data.toString().trim();
119+
if (line && !line.includes('dbus')) console.log(`[Chrome stderr] ${line}`);
120+
});
121+
122+
child.unref();
123+
await sleep(BROWSER_STARTUP_DELAY);
124+
}
125+
126+
// ─── Extension ID Reader ────────────────────────────────────────────────────
127+
128+
export async function readExtensionId(maxAttempts = 15): Promise<string> {
129+
const prefsPath = path.join(USER_DATA_DIR, 'Default', 'Preferences');
130+
131+
for (let i = 0; i < maxAttempts; i++) {
132+
if (fs.existsSync(prefsPath)) {
133+
try {
134+
const prefs = JSON.parse(fs.readFileSync(prefsPath, 'utf-8'));
135+
const extensions: Record<string, ExtensionSettings> | undefined =
136+
prefs?.extensions?.settings;
137+
if (extensions) {
138+
for (const [id, ext] of Object.entries(extensions)) {
139+
if (
140+
ext.manifest?.name === 'Midscene.js' ||
141+
ext.path?.includes('chrome-extension/dist')
142+
) {
143+
return id;
144+
}
145+
}
146+
}
147+
} catch {
148+
// retry
149+
}
150+
}
151+
console.log(
152+
`Waiting for extension in Preferences (${i + 1}/${maxAttempts})...`,
153+
);
154+
await sleep(EXTENSION_POLL_INTERVAL);
155+
}
156+
throw new Error(
157+
`Midscene.js extension not found after ${maxAttempts} attempts`,
158+
);
159+
}
160+
161+
// ─── CDP Helpers ────────────────────────────────────────────────────────────
162+
163+
function cdpSend(ws: WebSocket, id: number, method: string, params = {}) {
164+
ws.send(JSON.stringify({ id, method, params }));
165+
}
166+
167+
function cdpParse(event: MessageEvent): {
168+
id?: number;
169+
[key: string]: unknown;
170+
} {
171+
return JSON.parse(
172+
typeof event.data === 'string' ? event.data : String(event.data),
173+
);
174+
}
175+
176+
export async function findExtensionPageTarget(
177+
extensionId: string,
178+
): Promise<CdpTarget | null> {
179+
const res = await fetch(`http://127.0.0.1:${CDP_PORT}/json`);
180+
const targets: CdpTarget[] = await res.json();
181+
console.log(
182+
'CDP targets:',
183+
targets.map((t) => `${t.type}: ${t.url?.substring(0, 80)}`),
184+
);
185+
const extPrefix = `chrome-extension://${extensionId}`;
186+
return (
187+
targets.find((t) => t.url?.startsWith(extPrefix) && t.type === 'page') ??
188+
null
189+
);
190+
}
191+
192+
// ─── Config Injection ───────────────────────────────────────────────────────
193+
194+
function buildExtensionEnvConfig(): string {
195+
const lines: string[] = [];
196+
for (const key of EXTENSION_ENV_KEYS) {
197+
const value = process.env[key];
198+
if (value) {
199+
lines.push(`${key}=${value}`);
200+
}
201+
}
202+
return lines.join('\n');
203+
}
204+
205+
async function injectViaWebSocket(
206+
wsUrl: string,
207+
configString: string,
208+
): Promise<void> {
209+
return new Promise((resolve, reject) => {
210+
const ws = new WebSocket(wsUrl);
211+
ws.onopen = () => {
212+
const escaped = JSON.stringify(configString);
213+
cdpSend(ws, 1, 'Runtime.evaluate', {
214+
expression: `localStorage.setItem('midscene-env-config', ${escaped})`,
215+
});
216+
};
217+
ws.onmessage = (event) => {
218+
const msg = cdpParse(event);
219+
if (msg.id === 1) {
220+
console.log('Config injected successfully via CDP');
221+
ws.close();
222+
resolve();
223+
}
224+
};
225+
ws.onerror = (e) => reject(e);
226+
setTimeout(() => {
227+
ws.close();
228+
reject(new Error('CDP injection timed out'));
229+
}, CDP_INJECTION_TIMEOUT);
230+
});
231+
}
232+
233+
type NavigateInjectStep = 'navigating' | 'injecting' | 'restoring';
234+
235+
async function navigateAndInject(
236+
wsUrl: string,
237+
extUrl: string,
238+
configString: string,
239+
originalUrl: string,
240+
): Promise<void> {
241+
return new Promise((resolve, reject) => {
242+
const ws = new WebSocket(wsUrl);
243+
let step: NavigateInjectStep = 'navigating';
244+
ws.onopen = () => cdpSend(ws, 1, 'Page.navigate', { url: extUrl });
245+
ws.onmessage = (event) => {
246+
const msg = cdpParse(event);
247+
if (msg.id === 1 && step === 'navigating') {
248+
step = 'injecting';
249+
setTimeout(() => {
250+
const escaped = JSON.stringify(configString);
251+
cdpSend(ws, 2, 'Runtime.evaluate', {
252+
expression: `localStorage.setItem('midscene-env-config', ${escaped})`,
253+
});
254+
}, EXTENSION_POLL_INTERVAL);
255+
}
256+
if (msg.id === 2 && step === 'injecting') {
257+
step = 'restoring';
258+
console.log('Config injected via navigate fallback');
259+
cdpSend(ws, 3, 'Page.navigate', { url: originalUrl });
260+
}
261+
if (msg.id === 3 && step === 'restoring') {
262+
ws.close();
263+
resolve();
264+
}
265+
};
266+
ws.onerror = (e) => reject(e);
267+
setTimeout(() => {
268+
ws.close();
269+
reject(new Error('Navigate-and-inject timed out'));
270+
}, NAVIGATE_INJECT_TIMEOUT);
271+
});
272+
}
273+
274+
export async function injectExtensionConfig(
275+
extensionId: string,
276+
): Promise<void> {
277+
const configString = buildExtensionEnvConfig();
278+
if (!configString) {
279+
console.log('No env config to inject, skipping');
280+
return;
281+
}
282+
console.log(
283+
'Injecting env config keys:',
284+
configString
285+
.split('\n')
286+
.map((l) => l.split('=')[0])
287+
.join(', '),
288+
);
289+
290+
const target = await findExtensionPageTarget(extensionId);
291+
292+
if (!target) {
293+
console.log(
294+
'No extension page target, navigating existing tab to inject...',
295+
);
296+
const res = await fetch(`http://127.0.0.1:${CDP_PORT}/json`);
297+
const allTargets: CdpTarget[] = await res.json();
298+
const anyPage = allTargets.find(
299+
(t) => t.type === 'page' && t.webSocketDebuggerUrl,
300+
);
301+
if (!anyPage) {
302+
throw new Error('No CDP page targets available for config injection');
303+
}
304+
await navigateAndInject(
305+
anyPage.webSocketDebuggerUrl!,
306+
`chrome-extension://${extensionId}/index.html`,
307+
configString,
308+
anyPage.url,
309+
);
310+
return;
311+
}
312+
313+
await injectViaWebSocket(target.webSocketDebuggerUrl!, configString);
314+
}
315+
316+
export async function reloadViaWebSocket(wsUrl: string): Promise<void> {
317+
return new Promise((resolve, reject) => {
318+
const ws = new WebSocket(wsUrl);
319+
ws.onopen = () => cdpSend(ws, 1, 'Page.reload');
320+
ws.onmessage = (event) => {
321+
const msg = cdpParse(event);
322+
if (msg.id === 1) {
323+
console.log('Side panel reloaded to apply config');
324+
ws.close();
325+
resolve();
326+
}
327+
};
328+
ws.onerror = (e) => reject(e);
329+
setTimeout(() => {
330+
ws.close();
331+
resolve();
332+
}, RELOAD_TIMEOUT);
333+
});
334+
}

0 commit comments

Comments
 (0)