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
6 changes: 6 additions & 0 deletions scripts/cli-args.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const valueOptions = new Set([
'--batch-size',
'--jobs',
'--port',
'--host',
]);
const flagOptions = new Set([
'--help',
Expand All @@ -26,6 +27,7 @@ const flagOptions = new Set([
'--no-agent',
'--force',
'--worktree',
'--no-browser',
]);
const pathOptions = new Set([
'--summaries',
Expand Down Expand Up @@ -61,6 +63,8 @@ Options:
--force Regenerate all agent notes
--remote NAME|URL Git remote (default: origin)
--port NUMBER Local page port (default: 2299)
--host ADDRESS Page bind address (default: localhost)
--no-browser Do not open the page in a browser
-h, --help Show this help
-v, --version Show the installed version

Expand Down Expand Up @@ -322,6 +326,8 @@ export function parseCliArgs(
agentArgs,
port: Number(portValue),
portWasPassed: options.has('--port'),
host: options.get('--host') || 'localhost',
browserEnabled: !options.has('--no-browser'),
forceSummaryRegeneration: options.has('--force'),
};
}
6 changes: 4 additions & 2 deletions scripts/present.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ if (cli.doctor) {
process.exit(report.ready ? 0 : 1);
}

const { agentEnabled, port } = cli;
const { agentEnabled, browserEnabled, host, port } = cli;
const feedArgs = [...cli.feedArgs];
const agentArgs = [...cli.agentArgs];
if (agentEnabled) {
Expand Down Expand Up @@ -168,6 +168,8 @@ function startSite() {
outputPath,
'--port',
String(port),
'--host',
host,
'--project',
projectKey,
...(!cli.portWasPassed ? ['--increment-port'] : []),
Expand All @@ -190,7 +192,7 @@ function startSite() {
}
console.log(line);
const match = line.match(/^Diffsplain: (http:\/\/\S+)$/);
if (!browserOpened && !browserOpenTimer && match) {
if (browserEnabled && !browserOpened && !browserOpenTimer && match) {
browserOpenTimer = setTimeout(() => {
browserOpenTimer = undefined;
browserOpened = true;
Expand Down
71 changes: 59 additions & 12 deletions scripts/serve-built.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ function option(name, fallback) {
const output = resolve(option('--output', resolve(root, '.cache/diff-data.json')));
const project = option('--project', '');
const portValue = option('--port', '2299');
const host = option('--host', 'localhost');
if (!/^\d+$/.test(portValue) || Number(portValue) > 65_535) {
throw new Error('--port must be a number from 0 to 65535');
}
Expand Down Expand Up @@ -86,6 +87,15 @@ async function fetchAsset(request) {
return fileResponse(file);
}

function jsonResponse(value) {
return new Response(JSON.stringify(value), {
headers: {
'cache-control': 'no-store',
'content-type': 'application/json; charset=utf-8',
},
});
}

function nodeRequest(request) {
const host = request.headers.host || 'localhost';
const init = {
Expand All @@ -111,15 +121,20 @@ async function send(nodeResponse, response) {
const server = createServer(async (request, response) => {
try {
const webRequest = nodeRequest(request);
if (new URL(webRequest.url).pathname === '/events') {
const url = new URL(webRequest.url);
if (url.pathname === '/health') {
await send(response, jsonResponse(readyState));
return;
}
if (url.pathname === '/events') {
response.writeHead(200, {
'content-type': 'text/event-stream',
'cache-control': 'no-store',
connection: 'keep-alive',
});
response.write('retry: 250\nevent: ready\ndata: {}\n\n');
eventClients.add(response);
const requestProject = new URL(webRequest.url).searchParams.get('project');
const requestProject = url.searchParams.get('project');
if (project && requestProject === project) {
console.log('Diffsplain tab: connected');
}
Expand Down Expand Up @@ -148,19 +163,53 @@ watchFile(output, { interval: 100 }, (current, previous) => {
});

let selectedPort = Number(portValue);
let readyState;
let closing = false;

function urlFor(address, port) {
const formattedAddress = address.includes(':') ? `[${address}]` : address;
return `http://${formattedAddress}:${port}`;
}

function isLoopback(address) {
return (
address === 'localhost' ||
address === '::1' ||
address === '::ffff:127.0.0.1' ||
/^127(?:\.\d{1,3}){3}$/.test(address)
);
}

function listen() {
server.listen(selectedPort, 'localhost');
server.listen(selectedPort, host);
Comment thread
itsjling marked this conversation as resolved.
}

server.on('listening', () => {
if (closing) {
server.close();
return;
}
const address = server.address();
const readyAddress =
address && typeof address === 'object' ? address.address : host;
const readyPort =
address && typeof address === 'object' ? address.port : selectedPort;
const url = urlFor(host, readyPort);
const projectHash = project
? `#project=${encodeURIComponent(project)}`
: '';
console.log(`Diffsplain: http://localhost:${readyPort}${projectHash}`);
readyState = {
status: 'ok',
address: readyAddress,
port: readyPort,
};
if (!isLoopback(readyAddress)) {
console.warn(
`Warning: Diffsplain is listening on ${readyAddress}. Anyone who can reach this address can view this review.`,
);
}
console.log(`Diffsplain: ${url}${projectHash}`);
console.log(JSON.stringify({ event: 'ready', ...readyState, url }));
});

server.on('error', (error) => {
Expand All @@ -175,22 +224,20 @@ server.on('error', (error) => {
return;
}
console.error(`Could not start Diffsplain: ${error.message}`);
process.exitCode = 1;
close(1);
});

listen();

let closing = false;
function close() {
function close(exitCode = 0) {
if (closing) return;
closing = true;
process.exitCode = exitCode;
unwatchFile(output);
for (const client of eventClients) client.end();
eventClients.clear();
server.close(() => {
process.exitCode = 0;
});
if (server.listening) server.close();
Comment thread
itsjling marked this conversation as resolved.
}

process.on('SIGINT', close);
process.on('SIGTERM', close);
process.on('SIGINT', () => close());
process.on('SIGTERM', () => close());
12 changes: 12 additions & 0 deletions tests/cli-args.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,22 @@ test('leaves agent selection open when no agent is passed', () => {
assert.equal(parsed.agent, undefined);
assert.equal(parsed.port, 2299);
assert.equal(parsed.portWasPassed, false);
assert.equal(parsed.host, 'localhost');
assert.equal(parsed.browserEnabled, true);
assert.deepEqual(parsed.feedArgs, ['--repo', cwd, '--checkout']);
assert.deepEqual(parsed.agentArgs, ['--repo', cwd, '--checkout']);
});

test('accepts headless browser and explicit bind options', () => {
const parsed = parseCliArgs(['--no-browser', '--host', '0.0.0.0'], {
callerDirectory: cwd,
pathExists: missing,
});

assert.equal(parsed.browserEnabled, false);
assert.equal(parsed.host, '0.0.0.0');
});

test('accepts a GitHub owner/name repo and a branch', () => {
const parsed = parseCliArgs(['acme/widgets', '--branch', 'feature/search'], {
callerDirectory: cwd,
Expand Down
86 changes: 86 additions & 0 deletions tests/present-instances.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,22 @@ function waitForUrl(child) {
});
}

function waitForText(stream, pattern) {
return new Promise((resolve, reject) => {
let output = '';
const timer = setTimeout(() => {
reject(new Error(`Did not find ${pattern}: ${output}`));
}, 12_000);
stream.on('data', (chunk) => {
output += chunk;
if (pattern.test(output)) {
clearTimeout(timer);
resolve(output);
}
});
});
}

async function waitFor(read, timeout = 8_000) {
const deadline = Date.now() + timeout;
let lastError;
Expand Down Expand Up @@ -308,3 +324,73 @@ test('reuses a matching project tab when it reconnects', async () => {
await rm(root, { recursive: true, force: true });
}
});

test('stays available when browser launch fails and skips it when asked', async () => {
const root = await mkdtemp(join(tmpdir(), 'diffsplain-headless-'));
const browserLog = join(root, 'browser.log');
const browser = join(root, 'browser');
let failingPresenter;
let headlessPresenter;

try {
const repo = await makeRepo(root, 'repo', 'file.txt');
failingPresenter = spawn(
process.execPath,
[
script,
'--repo',
repo,
'--worktree',
'--no-agent',
'--port',
'0',
],
{
cwd: root,
env: { ...process.env, BROWSER: join(root, 'missing-browser') },
stdio: ['ignore', 'pipe', 'pipe'],
},
);
const browserFailure = waitForText(
failingPresenter.stderr,
/Could not open the browser:/,
);
const failingUrl = await waitForUrl(failingPresenter);
await browserFailure;
assert.equal((await fetch(new URL('health', failingUrl))).status, 200);
assert.equal(failingPresenter.exitCode, null);

await writeFile(browser, '#!/bin/sh\nprintf opened > "$BROWSER_LOG"\n');
await chmod(browser, 0o755);
headlessPresenter = spawn(
process.execPath,
[
script,
'--repo',
repo,
'--worktree',
'--no-agent',
'--no-browser',
'--port',
'0',
],
{
cwd: root,
env: { ...process.env, BROWSER: browser, BROWSER_LOG: browserLog },
stdio: ['ignore', 'pipe', 'pipe'],
},
);
const headlessUrl = await waitForUrl(headlessPresenter);
assert.equal((await fetch(new URL('health', headlessUrl))).status, 200);
await new Promise((resolve) => setTimeout(resolve, 900));
await assert.rejects(readFile(browserLog, 'utf8'));
} finally {
if (failingPresenter && failingPresenter.exitCode === null) {
await stop(failingPresenter);
}
if (headlessPresenter && headlessPresenter.exitCode === null) {
await stop(headlessPresenter);
}
await rm(root, { recursive: true, force: true });
}
});
Loading
Loading