Skip to content

Commit 65bd5b2

Browse files
OrKoNOrKoN
committed
chore: fix arg forwarding
1 parent 8dbf862 commit 65bd5b2

6 files changed

Lines changed: 225 additions & 80 deletions

File tree

‎src/bin/chrome-devtools.ts‎

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,15 @@ import process from 'node:process';
1111
import yargs, {type Options, type PositionalOptions} from 'yargs';
1212
import {hideBin} from 'yargs/helpers';
1313

14+
import {parseArguments} from '../cli.js';
1415
import {
1516
startDaemon,
1617
stopDaemon,
1718
sendCommand,
1819
handleResponse,
1920
} from '../daemon/client.js';
2021
import {isDaemonRunning} from '../daemon/utils.js';
22+
import {logDisclaimers} from '../server.js';
2123
import type {CallToolResult} from '../third_party/index.js';
2224
import {VERSION} from '../version.js';
2325

@@ -31,6 +33,12 @@ if (argv.length === 0 || argv[0] === '--custom-help') {
3133
process.exit(0);
3234
}
3335

36+
async function start(args: string[]) {
37+
const combinedArgs = [...args, ...defaultArgs];
38+
await startDaemon([...args, ...defaultArgs]);
39+
logDisclaimers(parseArguments(VERSION, combinedArgs));
40+
}
41+
3442
const defaultArgs = ['--viaCli', '--experimentalStructuredContent'];
3543

3644
const y = yargs(argv)
@@ -44,28 +52,42 @@ const y = yargs(argv)
4452
y.command(
4553
'start',
4654
'Start or restart chrome-devtools-mcp',
47-
y => y.help(false), // Disable help for start command to avoid parsing issues with passed args
55+
y =>
56+
y
57+
.help(false) // Disable help for start command to avoid parsing issues with passed args.
58+
.example(
59+
'$0 start --port 8080 --url http://localhost:8080',
60+
'Start the server on port 8080 with a specific URL',
61+
)
62+
.strict(false), // Don't validate arguments for start, as they are passed through to the daemon.
4863
async () => {
4964
if (isDaemonRunning()) {
5065
await stopDaemon();
5166
}
5267
// Extract args after 'start'
5368
const startIndex = process.argv.indexOf('start');
5469
const args = startIndex !== -1 ? process.argv.slice(startIndex + 1) : [];
55-
await startDaemon([...args, ...defaultArgs]);
70+
await start(args);
71+
process.exit(0);
5672
},
57-
);
73+
).strict(); // Re-enable strict validation for other commands; this is applied to the yargs instance itself
5874

5975
y.command('status', 'Checks if chrome-devtools-mcp is running', async () => {
6076
if (isDaemonRunning()) {
6177
console.log('chrome-devtools-mcp daemon is running.');
6278
} else {
63-
console.log('chrome-devtools-mcp daemon is not running');
79+
console.log('chrome-devtools-mcp daemon is not running.');
6480
}
81+
process.exit(0);
6582
});
6683

6784
y.command('stop', 'Stop chrome-devtools-mcp if any', async () => {
85+
if (!isDaemonRunning()) {
86+
process.exit(0);
87+
return;
88+
}
6889
await stopDaemon();
90+
process.exit(0);
6991
});
7092

7193
for (const [commandName, commandDef] of Object.entries(commands)) {
@@ -127,7 +149,7 @@ for (const [commandName, commandDef] of Object.entries(commands)) {
127149
async argv => {
128150
try {
129151
if (!isDaemonRunning()) {
130-
await startDaemon(defaultArgs);
152+
await start([]);
131153
}
132154

133155
const commandArgs: Record<string, unknown> = {};

‎src/daemon/client.ts‎

Lines changed: 41 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import fs from 'node:fs';
99
import net from 'node:net';
1010

1111
import {logger} from '../logger.js';
12-
import {START_INDICATOR} from '../server.js';
1312
import type {CallToolResult} from '../third_party/index.js';
1413
import {PipeTransport} from '../third_party/index.js';
1514

@@ -21,27 +20,46 @@ import {
2120
isDaemonRunning,
2221
} from './utils.js';
2322

23+
const FILE_TIMEOUT = 10_000;
24+
2425
/**
25-
* Waits for a file to be created and populated.
26+
* Waits for a file to be created and populated (removed = false) or removed (removed = true).
2627
*/
27-
function waitForFile(filePath: string, timeout = 5000) {
28+
function waitForFile(filePath: string, removed = false) {
2829
return new Promise<void>((resolve, reject) => {
29-
if (fs.existsSync(filePath) && fs.statSync(filePath).size > 0) {
30+
const check = () => {
31+
const exists = fs.existsSync(filePath);
32+
if (removed) {
33+
return !exists;
34+
}
35+
if (!exists) {
36+
return false;
37+
}
38+
try {
39+
return fs.statSync(filePath).size > 0;
40+
} catch {
41+
return false;
42+
}
43+
};
44+
45+
if (check()) {
3046
resolve();
3147
return;
3248
}
3349

3450
const timer = setTimeout(() => {
3551
fs.unwatchFile(filePath);
3652
reject(
37-
new Error(`Timeout: file ${filePath} not found within ${timeout}ms`),
53+
new Error(
54+
`Timeout: file ${filePath} ${removed ? 'not removed' : 'not found'} within ${FILE_TIMEOUT}ms`,
55+
),
3856
);
39-
}, timeout);
57+
}, FILE_TIMEOUT);
4058

41-
fs.watchFile(filePath, {interval: 500}, curr => {
42-
if (curr.size > 0) {
59+
fs.watchFile(filePath, {interval: 500}, () => {
60+
if (check()) {
4361
clearTimeout(timer);
44-
fs.unwatchFile(filePath); // Always clean up your listeners!
62+
fs.unwatchFile(filePath);
4563
resolve();
4664
}
4765
});
@@ -54,38 +72,22 @@ export async function startDaemon(mcpArgs: string[] = []) {
5472
return;
5573
}
5674

75+
const pidFilePath = getPidFilePath();
76+
77+
if (fs.existsSync(pidFilePath)) {
78+
fs.unlinkSync(pidFilePath);
79+
}
80+
5781
logger('Starting daemon...');
5882
const child = spawn(process.execPath, [DAEMON_SCRIPT_PATH, ...mcpArgs], {
5983
detached: true,
60-
stdio: ['ignore', 'ignore', 'pipe'],
84+
stdio: 'ignore',
85+
env: process.env,
6186
cwd: process.cwd(),
6287
});
88+
child.unref();
6389

64-
await new Promise<void>((resolve, reject) => {
65-
child.on('error', err => {
66-
reject(err);
67-
});
68-
child.on('exit', code => {
69-
logger(`Child exited with code ${code}`);
70-
reject(new Error(`Daemon process exited prematurely with code ${code}`));
71-
});
72-
73-
waitForFile(getPidFilePath()).then(resolve).catch(reject);
74-
});
75-
76-
logger(`Pid file found ${getPidFilePath()}`);
77-
78-
child.stderr.pipe(process.stderr);
79-
await new Promise<void>(resolve => {
80-
child.stderr.on('data', data => {
81-
if (data.toString().includes(START_INDICATOR)) {
82-
child.stderr.unpipe(process.stderr);
83-
child.stderr.destroy();
84-
child.unref();
85-
resolve();
86-
}
87-
});
88-
});
90+
await waitForFile(pidFilePath);
8991
}
9092

9193
const SEND_COMMAND_TIMEOUT = 60_000; // ms
@@ -135,7 +137,11 @@ export async function stopDaemon() {
135137
return;
136138
}
137139

140+
const pidFilePath = getPidFilePath();
141+
138142
await sendCommand({method: 'stop'});
143+
144+
await waitForFile(pidFilePath, /*removed=*/ true);
139145
}
140146

141147
export function handleResponse(

‎src/main.ts‎

Lines changed: 2 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import process from 'node:process';
1010

1111
import {cliOptions, parseArguments} from './cli.js';
1212
import {logger, saveLogsToFile} from './logger.js';
13-
import {createMcpServer, START_INDICATOR} from './server.js';
13+
import {createMcpServer, logDisclaimers} from './server.js';
1414
import {computeFlagUsage} from './telemetry/flagUtils.js';
1515
import {StdioServerTransport} from './third_party/index.js';
1616
import {VERSION} from './version.js';
@@ -35,37 +35,12 @@ if (process.env['CHROME_DEVTOOLS_MCP_CRASH_ON_UNCAUGHT'] !== 'true') {
3535
}
3636

3737
logger(`Starting Chrome DevTools MCP Server v${VERSION}`);
38-
39-
const logDisclaimers = () => {
40-
console.error(
41-
`chrome-devtools-mcp exposes content of the browser instance to the MCP clients allowing them to inspect,
42-
debug, and modify any data in the browser or DevTools.
43-
Avoid sharing sensitive or personal information that you do not want to share with MCP clients.`,
44-
);
45-
46-
if (!args.slim && args.performanceCrux) {
47-
console.error(
48-
`Performance tools may send trace URLs to the Google CrUX API to fetch real-user experience data. To disable, run with --no-performance-crux.`,
49-
);
50-
}
51-
52-
if (!args.slim && args.usageStatistics) {
53-
console.error(
54-
`
55-
Google collects usage statistics to improve Chrome DevTools MCP. To opt-out, run with --no-usage-statistics.
56-
For more details, visit: https://github.com/ChromeDevTools/chrome-devtools-mcp#usage-statistics`,
57-
);
58-
}
59-
60-
console.error(START_INDICATOR);
61-
};
62-
6338
const {server, clearcutLogger} = await createMcpServer(args, {
6439
logFile,
6540
});
6641
const transport = new StdioServerTransport();
6742
await server.connect(transport);
6843
logger('Chrome DevTools MCP Server connected');
69-
logDisclaimers();
44+
logDisclaimers(args);
7045
void clearcutLogger?.logDailyActiveIfNeeded();
7146
void clearcutLogger?.logServerStart(computeFlagUsage(args, cliOptions));

‎src/server.ts‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -257,4 +257,24 @@ export async function createMcpServer(
257257
return {server, clearcutLogger};
258258
}
259259

260-
export const START_INDICATOR = 'Server started.';
260+
export const logDisclaimers = (args: ReturnType<typeof parseArguments>) => {
261+
console.error(
262+
`chrome-devtools-mcp exposes content of the browser instance to the MCP clients allowing them to inspect,
263+
debug, and modify any data in the browser or DevTools.
264+
Avoid sharing sensitive or personal information that you do not want to share with MCP clients.`,
265+
);
266+
267+
if (!args.slim && args.performanceCrux) {
268+
console.error(
269+
`Performance tools may send trace URLs to the Google CrUX API to fetch real-user experience data. To disable, run with --no-performance-crux.`,
270+
);
271+
}
272+
273+
if (!args.slim && args.usageStatistics) {
274+
console.error(
275+
`
276+
Google collects usage statistics to improve Chrome DevTools MCP. To opt-out, run with --no-usage-statistics.
277+
For more details, visit: https://github.com/ChromeDevTools/chrome-devtools-mcp#usage-statistics`,
278+
);
279+
}
280+
};

‎tests/daemon/client.test.ts‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
*/
66

77
import assert from 'node:assert';
8-
import {describe, it, afterEach} from 'node:test';
8+
import {describe, it, afterEach, beforeEach} from 'node:test';
99

1010
import {
1111
handleResponse,
@@ -16,12 +16,12 @@ import {isDaemonRunning} from '../../src/daemon/utils.js';
1616

1717
describe('daemon client', () => {
1818
describe('start/stop', () => {
19+
beforeEach(async () => {
20+
await stopDaemon();
21+
});
22+
1923
afterEach(async () => {
20-
if (isDaemonRunning()) {
21-
await stopDaemon();
22-
// Wait a bit for the daemon to fully terminate and clean up its files.
23-
await new Promise(resolve => setTimeout(resolve, 1000));
24-
}
24+
await stopDaemon();
2525
});
2626

2727
it('should start and stop daemon', async () => {
@@ -31,7 +31,6 @@ describe('daemon client', () => {
3131
assert.ok(isDaemonRunning(), 'Daemon should be running after start');
3232

3333
await stopDaemon();
34-
await new Promise(resolve => setTimeout(resolve, 1000));
3534
assert.ok(!isDaemonRunning(), 'Daemon should not be running after stop');
3635
});
3736

@@ -54,12 +53,12 @@ describe('daemon client', () => {
5453
});
5554

5655
describe('parsing', () => {
57-
it('handles MCP response with text format', () => {
56+
it('handles MCP response with text format', async () => {
5857
const textResponse = {content: [{type: 'text' as const, text: 'test'}]};
5958
assert.strictEqual(handleResponse(textResponse, 'text'), 'test');
6059
});
6160

62-
it('handles JSON response', () => {
61+
it('handles JSON response', async () => {
6362
const jsonResponse = {
6463
content: [],
6564
structuredContent: {
@@ -73,7 +72,7 @@ describe('daemon client', () => {
7372
);
7473
});
7574

76-
it('handles error response when isError is true', () => {
75+
it('handles error response when isError is true', async () => {
7776
const errorResponse = {
7877
isError: true,
7978
content: [{type: 'text' as const, text: 'Something went wrong'}],
@@ -84,7 +83,7 @@ describe('daemon client', () => {
8483
);
8584
});
8685

87-
it('handles text response when json format is requested but no structured content', () => {
86+
it('handles text response when json format is requested but no structured content', async () => {
8887
const textResponse = {
8988
content: [{type: 'text' as const, text: 'Fall through text'}],
9089
};
@@ -94,7 +93,7 @@ describe('daemon client', () => {
9493
);
9594
});
9695

97-
it('throws error for unsupported content type', () => {
96+
it('throws error for unsupported content type', async () => {
9897
const unsupportedContentResponse = {
9998
content: [
10099
{

0 commit comments

Comments
 (0)