test: end-to-end smoke over the real server

[djimit]

Summary

The P1 item that guards the whole vertical: previously no test booted the actual server — 1,213 unit tests could pass while the composed system failed to start or stream. This adds one test that fails if the platform's spine breaks.

It spawns src/index.ts as a real subprocess on a temp SQLite database and drives everything over the wire:

1. Bootstrap-admin login (POST /api/auth/login)
2. Authenticated WebSocket handshake (/ws?token=…) — asserts the health frame streams
3. Task creation (POST /api/tasks)
4. Execution on the mock executor — and if the policy engine gates it, approves through POST /approvals/:id/approve, so the approval path is exercised too
5. Polls to status: completed
6. Asserts execution events were persisted (GET /api/tasks/:id/events)

Ponytail note: deliberately server-level rather than Playwright — the ws client and fetch are already available, so this costs zero new dependencies and ~9s of CI time while covering login, auth middleware, WS auth, task routes, the execution engine, policy gating, and event persistence in one pass. A browser-level test can layer on later if the dashboard UI needs its own guard.

Validation

• Runs green in 9.4s standalone; full server suite green with it included (1,214 tests)
• Random port derived from PID (no collisions with parallel workers), temp DB cleaned up in afterAll

🤖 Generated with Claude Code

[kilo-code-bot]

Kilo Code Review could not run — your account is out of credits.

Add credits or switch to a free model to enable reviews on this change.

[gemini-code-assist]

Code Review

This pull request introduces a comprehensive end-to-end smoke test (e2e-smoke.test.ts) that boots the server as a subprocess and verifies the full application flow, including authentication, WebSocket communication, task execution, and event persistence. The review feedback focuses on improving test robustness and preventing resource leaks. Key recommendations include: refactoring the until helper to avoid swallowing legitimate errors, making the afterAll hook asynchronous to prevent file-locking race conditions when deleting the database, properly scoping and closing the WebSocket connection, clearing the handshake timeout timer, and failing fast if a task enters a terminal failure state.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

The until helper catches all errors thrown by the polled function and treats them as transient failures, returning null and retrying. This swallows legitimate programming errors, assertion failures, or API errors, making debugging extremely difficult because the test will always hang for the full timeout (e.g., 60 seconds) before throwing a generic timeout error.

We should remove the catch-all .catch(() => null) from the until helper so that legitimate errors propagate immediately, and instead explicitly catch connection errors in the health check poll where they are expected.

Suggested change

	async function until<T>(fn: () => Promise<T | null>, timeoutMs: number, intervalMs = 500): Promise<T> {
	const deadline = Date.now() + timeoutMs;
	for (;;) {
	const value = await fn().catch(() => null);
	if (value !== null) return value;
	if (Date.now() > deadline) throw new Error(`Timed out after ${timeoutMs}ms`);
	await new Promise((resolve) => setTimeout(resolve, intervalMs));
	}
	}
	async function until<T>(fn: () => Promise<T | null>, timeoutMs: number, intervalMs = 500): Promise<T> {
	const deadline = Date.now() + timeoutMs;
	for (;;) {
	const value = await fn();
	if (value !== null) return value;
	if (Date.now() > deadline) throw new Error(`Timed out after ${timeoutMs}ms`);
	await new Promise((resolve) => setTimeout(resolve, intervalMs));
	}
	}

[gemini-code-assist]

Since we are removing the catch-all error swallowing from until, we should explicitly catch connection errors during the initial server startup health check, as the server may not be fully bound and listening yet.

Suggested change

	await until(async () => {
	const response = await fetch(`${BASE}/health`);
	return response.ok ? true : null;
	}, 60_000);
	await until(async () => {
	const response = await fetch(`${BASE}/health`).catch(() => null);
	return response?.ok ? true : null;
	}, 60_000);

[gemini-code-assist]

In afterAll, rmSync is called synchronously immediately after sending SIGTERM to the server process. Since the subprocess terminates asynchronously, it may still be running and holding file locks on the SQLite database when rmSync attempts to delete the directory. This can cause flaky test failures (especially on Windows) or leave orphaned files/processes.

We should make afterAll asynchronous, wait for the server process to fully exit, and also ensure the WebSocket connection is closed.

  afterAll(async () => {
    if (ws) {
      try {
        ws.close();
      } catch {}
    }
    if (server && server.exitCode === null && server.signalCode === null) {
      const exitPromise = new Promise<void>((resolve) => {
        server!.once('exit', () => resolve());
      });
      server.kill('SIGTERM');
      const killTimeout = setTimeout(() => server?.kill('SIGKILL'), 3000);
      killTimeout.unref();
      await exitPromise;
      clearTimeout(killTimeout);
    }
    rmSync(dataDir, { recursive: true, force: true });
  });

[gemini-code-assist]

Check for terminal failure or cancellation states in the task status poll. If the task fails or is cancelled, we should throw an error immediately to fail the test fast, rather than waiting for the 60-second timeout.

Suggested change

	const finalStatus = await until(async () => {
	const task = await api<{ status: string }>(token, `/tasks/${taskId}`, {});
	return task.body.status === 'completed' ? task.body.status : null;
	}, 60_000, 1000);
	const finalStatus = await until(async () => {
	const task = await api<{ status: string }>(token, `/tasks/${taskId}`, {});
	const status = task.body.status;
	if (status === 'failed' || status === 'cancelled') {
	throw new Error(`Task execution ended with terminal status: ${status}`);
	}
	return status === 'completed' ? status : null;
	}, 60_000, 1000);

[gemini-code-assist]

Declare ws in the outer scope so that it can be safely closed in the afterAll hook, ensuring no hanging WebSocket connections are left open if the test fails midway.

Suggested change

	let server: ChildProcess | null = null;
	let dataDir: string;
	let server: ChildProcess | null = null;
	let dataDir: string;
	let ws: WebSocket | null = null;

[gemini-code-assist]

Assign the WebSocket instance to the outer-scope ws variable instead of declaring a local constant.

Suggested change

	const ws = new WebSocket(`ws://127.0.0.1:${PORT}/ws?token=${token}`);
	ws = new WebSocket(`ws://127.0.0.1:${PORT}/ws?token=${token}`);

[gemini-code-assist]

The setTimeout used for the WebSocket handshake timeout is never cleared when the promise resolves or rejects. This leaves a dangling timer in the Node.js event loop for up to 10 seconds, which can delay test completion or cause resource leaks.

We should clear the timeout when the promise resolves or rejects.

    await new Promise<void>((resolve, reject) => {
      const timer = setTimeout(() => reject(new Error('WS handshake timeout')), 10_000);
      ws!.on('message', (data) => {
        wsFrames.push(String(data));
        clearTimeout(timer);
        resolve();
      });
      ws!.on('error', (err) => {
        clearTimeout(timer);
        reject(err);
      });
      ws!.on('close', (code) => {
        clearTimeout(timer);
        reject(new Error(`WS closed: ${code}`));
      });
    });

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8dbf512fbf

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Kill the actual server process tree

When this test runs through npx, server is the npm wrapper process (the local npx shim rewrites to npm exec), not necessarily the Node process running src/index.ts. Sending SIGTERM/SIGKILL only to that PID can leave the real server descendant orphaned, so local or CI runs leak a process that keeps the port open after the test finishes. Spawn the actual tsx binary directly, or start a detached process group and kill the group in teardown.

Useful? React with 👍 / 👎.