test: raise coverage and add CI coverage gates

.github/workflows/ci.yml

@@ -28,5 +28,10 @@ jobs:
       - name: Lint + typecheck (biome + tsc)
         run: bun run check
 
-      - name: Server E2E + web unit tests
-        run: bun run test
+      # Coverage gates: thresholds are a ratchet — only ever raised.
+      # (`=` syntax is required: space-separated numbers parse as test filters.)
+      - name: Tests with coverage gates
+        run: |
+          set -e
+          cd server && bun test --coverage --coverage-threshold=93
+          cd ../web && bun test --coverage --coverage-threshold=90

README.md

@@ -1,5 +1,7 @@
 # omp-web
 
+[![CI](https://github.com/danielglh/omp-web/actions/workflows/ci.yml/badge.svg)](https://github.com/danielglh/omp-web/actions/workflows/ci.yml)
+
 Run and operate [omp](https://omp.sh/) coding agents on a remote server through a
 browser UI. omp runs on the server; the web UI drives it — start sessions, prompt
 the agent, watch streaming transcripts with thinking blocks and tool-call cards,

server/scripts/mock-rpc-host.ts

@@ -236,6 +236,13 @@ function handleFrame(frame: unknown) {
 				],
 			});
 			return;
+		case "new_session": {
+			// Like a real agent: mint a fresh identity and report it back.
+			state.sessionId = `mock-${Date.now().toString(36)}-${Math.random().toString(36).slice(2, 6)}`;
+			state.sessionFile = `/mock/sessions/${state.sessionId}`;
+			respond("new_session", { sessionId: state.sessionId, sessionFile: state.sessionFile });
+			return;
+		}
 		case "prompt":
 		case "steer":
 		case "follow_up":

server/src/omp.ts

@@ -42,7 +42,7 @@ export interface OmpSessionSummary {
 const CLI_TIMEOUT_MS = 20_000;
 
 /** Run the omp CLI and capture stdout; non-zero exit rejects with stderr. */
-export function runOmpCli(bin: string, args: string[]): Promise<string> {
+export function runOmpCli(bin: string, args: string[], timeoutMs: number = CLI_TIMEOUT_MS): Promise<string> {
 	return new Promise((resolve, reject) => {
 		const proc = Bun.spawn([bin, ...args], {
 			cwd: os.homedir(),
@@ -53,7 +53,7 @@ export function runOmpCli(bin: string, args: string[]): Promise<string> {
 		const timer = setTimeout(() => {
 			proc.kill("SIGKILL");
 			reject(new Error(`omp ${args[0]} timed out`));
-		}, CLI_TIMEOUT_MS);
+		}, timeoutMs);
 		timer.unref?.();
 		proc.exited.then(async code => {
 			clearTimeout(timer);
@@ -109,13 +109,19 @@ export async function ompModelList(bin: string): Promise<OmpModelSummary[]> {
 
 const MAX_SESSION_SCAN = 1000;
 
+/** Where omp keeps session history; injectable so tests can point at a temp dir. */
+const DEFAULT_SESSIONS_ROOT = path.join(os.homedir(), ".omp", "agent", "sessions");
+
 /**
  * List resumable omp sessions whose header cwd matches. The encoded directory
  * name is not decoded — instead every file's header line (first 2 KiB) is read
  * and matched by its recorded `cwd`, which is robust across encodings.
  */
-export function listOmpSessions(cwd: string, limit = 100): OmpSessionSummary[] {
-	const sessionsRoot = path.join(os.homedir(), ".omp", "agent", "sessions");
+export function listOmpSessions(
+	cwd: string,
+	limit = 100,
+	sessionsRoot: string = DEFAULT_SESSIONS_ROOT,
+): OmpSessionSummary[] {
 	let dirs: fs.Dirent[];
 	try {
 		dirs = fs.readdirSync(sessionsRoot, { withFileTypes: true });

server/src/rpc/process.ts

@@ -170,8 +170,17 @@ export function spawnOmpProcess(options: OmpProcessOptions): OmpProcess {
 		proc = spawnTyped(bin, args, cwd, { ...process.env, ...env });
 	} catch (error) {
 		const message = error instanceof Error ? error.message : String(error);
+		// Surface the failure through `ready` instead of throwing: callers await
+		// `ready`, and a synchronous throw would leave that rejected promise
+		// unconsumed while their flow moves on.
 		settleReady(new Error(`failed to spawn ${bin}: ${message}`));
-		throw error;
+		return {
+			pid: undefined,
+			ready,
+			send() {},
+			async close() {},
+			kill() {},
+		};
 	}
 	const decoder = new RpcFrameDecoder();
 	let stderrBuffer = "";

server/src/sessions/manager.ts

@@ -222,7 +222,13 @@ export class SessionManager {
 		const record = this.#records.get(id);
 		if (!record) throw new Error(`session ${id} not found`);
 		const runtime = this.#runtimeFor(id);
-		if (runtime.process) return this.#info(record);
+		// Already alive (or spawning): idempotent. An errored session retries
+		// from scratch — that's the UI's start button recovering a failure.
+		if (runtime.process && (runtime.status === "running" || runtime.status === "starting")) {
+			return this.#info(record);
+		}
+		runtime.process = undefined;
+		runtime.pid = undefined;
 		runtime.status = "starting";
 		runtime.error = undefined;
 		this.#emit(record);
@@ -296,7 +302,15 @@ export class SessionManager {
 		runtime.process = ompProc;
 		runtime.pid = ompProc.pid;
 
-		await ompProc.ready;
+		try {
+			await ompProc.ready;
+		} catch (error) {
+			// Spawn (or readiness) failed: drop the dead process handle so a
+			// later start() can retry from scratch.
+			runtime.process = undefined;
+			runtime.pid = undefined;
+			throw error;
+		}
 		runtime.status = "running";
 		this.#emit(record);
 

server/test/fixtures/spawn-test-agent.sh

@@ -0,0 +1,14 @@
+#!/bin/sh
+# Test double for a real `omp` binary. The server invokes it exactly like the
+# real agent: `--mode rpc [--approval-mode …] --session <id> …`. bun cannot
+# parse those flags, so this wrapper extracts just what the bundled mock RPC
+# host understands (--session) and execs it cleanly.
+session=""
+prev=""
+for arg in "$@"; do
+	if [ "$prev" = "--session" ]; then
+		session="$arg"
+	fi
+	prev="$arg"
+done
+exec bun "$(dirname "$0")/../../scripts/mock-rpc-host.ts" --session "$session"

server/test/frame.test.ts

@@ -0,0 +1,127 @@
+/**
+ * Unit tests for the RPC frame decoder's error handling: malformed chunk
+ * metadata, broken base64, interrupted sequences — the validation layer that
+ * keeps untrusted agent output from blowing up the bridge.
+ *
+ * Note on scale: the protocol only chunks frames ABOVE 1 MB, so valid chunk
+ * metadata always declares a byteLength ≥ MAX_RPC_FRAME_BYTES.
+ */
+import { describe, expect, test } from "bun:test";
+import {
+	MAX_RPC_FRAME_BYTES,
+	MAX_RPC_REASSEMBLED_BYTES,
+	RPC_CHUNK_PAYLOAD_BYTES,
+	RpcFrameDecoder,
+} from "../src/rpc/frame";
+
+function chunk(overrides: Record<string, unknown>): Record<string, unknown> {
+	return {
+		type: "rpc_chunk",
+		chunkId: "c1",
+		index: 0,
+		count: 2,
+		byteLength: MAX_RPC_FRAME_BYTES + 1,
+		data: Buffer.from("{}", "utf8").toString("base64"),
+		...overrides,
+	};
+}
+
+function bigPayload(): Buffer {
+	// Exactly 4 × 256 KB bytes: every quarter-chunk fits the transport limit
+	// while the declared byteLength still clears the 1 MB floor.
+	const head = Buffer.from('{"type":"response","ok":true,"blob":"', "utf8");
+	const tail = Buffer.from('"}', "utf8");
+	return Buffer.concat([
+		head,
+		Buffer.alloc(4 * RPC_CHUNK_PAYLOAD_BYTES - head.byteLength - tail.byteLength, 97),
+		tail,
+	]);
+}
+
+describe("RpcFrameDecoder", () => {
+	test("passes plain frames straight through", () => {
+		const decoder = new RpcFrameDecoder();
+		const frame = { type: "ready", protocolVersion: 1 };
+		expect(decoder.push(frame)).toBe(frame);
+	});
+
+	test("reassembles a valid chunk sequence", () => {
+		const decoder = new RpcFrameDecoder();
+		const payload = bigPayload();
+		const quarter = Math.ceil(payload.byteLength / 4);
+		const shared = { chunkId: "c1", count: 4, byteLength: payload.byteLength };
+		for (let index = 0; index < 3; index++) {
+			const data = payload.subarray(index * quarter, (index + 1) * quarter).toString("base64");
+			expect(decoder.push(chunk({ ...shared, index, data }))).toBeUndefined();
+			expect(decoder.hasPending).toBe(true);
+		}
+		const data = payload.subarray(3 * quarter).toString("base64");
+		const assembled = decoder.push(chunk({ ...shared, index: 3, data }));
+		expect(assembled).toEqual(JSON.parse(payload.toString("utf8")));
+		expect(decoder.hasPending).toBe(false);
+	});
+
+	test("rejects invalid chunk metadata", () => {
+		const decoder = new RpcFrameDecoder();
+		for (const bad of [
+			chunk({ count: 1 }), // a single-chunk message is never chunked
+			chunk({ index: 2, count: 2 }), // index out of range
+			chunk({ index: -1, count: 2 }),
+			chunk({ byteLength: 10 }), // below the 1 MB transport floor
+			chunk({ byteLength: MAX_RPC_REASSEMBLED_BYTES + 1 }), // above the reassembly ceiling
+			chunk({ byteLength: 1.5 }), // non-integer
+			chunk({ chunkId: "" }),
+			chunk({ chunkId: "x".repeat(129) }),
+		]) {
+			expect(() => decoder.push(bad)).toThrow("invalid rpc chunk metadata");
+		}
+	});
+
+	test("rejects bad base64 payloads", () => {
+		const decoder = new RpcFrameDecoder();
+		expect(() => decoder.push(chunk({ data: "!!!not-base64!!!" }))).toThrow("invalid rpc chunk data");
+	});
+
+	test("rejects oversized single chunks", () => {
+		const decoder = new RpcFrameDecoder();
+		const big = Buffer.alloc(RPC_CHUNK_PAYLOAD_BYTES + 1, 65).toString("base64");
+		expect(() => decoder.push(chunk({ data: big }))).toThrow("rpc chunk payload exceeds transport limit");
+	});
+
+	test("rejects sequences whose received bytes exceed the declared length", () => {
+		const decoder = new RpcFrameDecoder();
+		const declared = MAX_RPC_FRAME_BYTES; // the metadata floor; 4 full chunks fit exactly
+		const fullChunk = Buffer.alloc(RPC_CHUNK_PAYLOAD_BYTES, 97).toString("base64");
+		for (let index = 0; index < 4; index++) {
+			expect(
+				decoder.push(chunk({ chunkId: "c", count: 6, byteLength: declared, index, data: fullChunk })),
+			).toBeUndefined();
+		}
+		expect(() =>
+			decoder.push(chunk({ chunkId: "c", count: 6, byteLength: declared, index: 4, data: "ZQ==" })),
+		).toThrow("rpc chunk sequence exceeds declared length");
+	});
+
+	test("declared length must match the reassembled bytes exactly", () => {
+		const decoder = new RpcFrameDecoder();
+		const declared = MAX_RPC_FRAME_BYTES + 4;
+		const part = Buffer.alloc(RPC_CHUNK_PAYLOAD_BYTES, 97).toString("base64");
+		decoder.push(chunk({ chunkId: "c", count: 2, byteLength: declared, index: 0, data: part }));
+		expect(() => decoder.push(chunk({ chunkId: "c", count: 2, byteLength: declared, index: 1, data: part }))).toThrow(
+			"rpc chunk sequence length mismatch",
+		); // 2×262 144 ≠ declared
+	});
+
+	test("an interleaved plain frame aborts a pending chunk sequence", () => {
+		const decoder = new RpcFrameDecoder();
+		decoder.push(chunk({ index: 0 }));
+		expect(() => decoder.push({ type: "notice" })).toThrow("rpc chunk sequence interrupted");
+	});
+
+	test("sequence mismatch on id or order aborts", () => {
+		const decoder = new RpcFrameDecoder();
+		decoder.push(chunk({ chunkId: "c1", index: 0 }));
+		expect(() => decoder.push(chunk({ chunkId: "c2", index: 1 }))).toThrow("rpc chunk sequence mismatch");
+		expect(() => decoder.push(chunk({ chunkId: "c1", index: 0 }))).toThrow("rpc chunk sequence mismatch");
+	});
+});

server/test/fs.test.ts

@@ -0,0 +1,129 @@
+/**
+ * Unit tests for the filesystem helpers behind the directory/file pickers and
+ * — most importantly — `deleteOmpSession`, the only function in the codebase
+ * that destroys user files.
+ */
+import { afterAll, beforeAll, describe, expect, test } from "bun:test";
+import * as fs from "node:fs";
+import * as os from "node:os";
+import * as path from "node:path";
+import { deleteOmpSession, listDir, searchDir, searchPaths } from "../src/fs";
+
+let root: string;
+let scratch: string;
+
+beforeAll(() => {
+	root = fs.mkdtempSync(path.join(os.tmpdir(), "omp-fs-root-"));
+	fs.mkdirSync(path.join(root, "alpha"));
+	fs.mkdirSync(path.join(root, "alpha", "nested"));
+	fs.mkdirSync(path.join(root, "alphabet"));
+	fs.mkdirSync(path.join(root, ".hidden"));
+	fs.writeFileSync(path.join(root, "file.txt"), "x");
+	scratch = fs.mkdtempSync(path.join(os.tmpdir(), "omp-fs-scratch-"));
+});
+
+afterAll(() => {
+	for (const dir of [root, scratch]) {
+		try {
+			fs.rmSync(dir, { recursive: true, force: true });
+		} catch {
+			// best effort
+		}
+	}
+});
+
+describe("listDir", () => {
+	test("lists subdirectories only, resolves parent, hides hidden by default", () => {
+		const listing = listDir(root);
+		expect(listing.path).toBe(root);
+		expect(listing.parent).toBe(path.dirname(root));
+		expect(listing.dirs.sort()).toEqual(["alpha", "alphabet"]);
+	});
+
+	test("showHidden reveals dot directories", () => {
+		expect(listDir(root, true).dirs.sort()).toEqual([".hidden", "alpha", "alphabet"]);
+	});
+
+	test("defaults to the home directory", () => {
+		expect(listDir().path).toBe(os.homedir());
+	});
+
+	test("filesystem root has no parent", () => {
+		expect(listDir("/").parent).toBe(null);
+	});
+
+	test("files and missing paths are rejected", () => {
+		expect(() => listDir(path.join(root, "file.txt"))).toThrow();
+		expect(() => listDir(path.join(root, "missing"))).toThrow();
+	});
+});
+
+describe("searchDir", () => {
+	test("an existing directory offers its subdirectories", () => {
+		const res = searchDir(root);
+		expect(res.matches.sort()).toEqual([path.join(root, "alpha"), path.join(root, "alphabet")].sort());
+	});
+
+	test("basename prefix matches subdirectories case-insensitively", () => {
+		expect(searchDir(path.join(root, "ALPH")).matches.sort()).toEqual([
+			path.join(root, "alpha"),
+			path.join(root, "alphabet"),
+		]);
+	});
+
+	test("no basename match, missing parent, or empty prefix yields no matches", () => {
+		expect(searchDir(path.join(root, "zzz")).matches).toEqual([]);
+		expect(searchDir("/definitely/not/here/xx").matches).toEqual([]);
+		expect(searchDir("").matches).toEqual([]);
+	});
+});
+
+describe("searchPaths", () => {
+	test("an existing directory lists files and dirs (dirs with trailing slash)", () => {
+		const res = searchPaths("./", root);
+		expect(res.matches).toEqual([
+			`${path.join(root, "alpha")}/`,
+			`${path.join(root, "alphabet")}/`,
+			path.join(root, "file.txt"),
+		]);
+	});
+
+	test("prefix narrows entries under the parent, hidden excluded", () => {
+		expect(searchPaths("alph", root).matches).toEqual([
+			`${path.join(root, "alpha")}/`,
+			`${path.join(root, "alphabet")}/`,
+		]);
+		expect(searchPaths("file", root).matches).toEqual([path.join(root, "file.txt")]);
+		expect(searchPaths(".hidden", root).matches).toEqual([]);
+	});
+
+	test("empty prefix yields no matches", () => {
+		expect(searchPaths("", root).matches).toEqual([]);
+	});
+});
+
+describe("deleteOmpSession", () => {
+	test("removes the session file and its artifacts directory", () => {
+		const file = path.join(scratch, "session-1.jsonl");
+		fs.writeFileSync(file, "{}");
+		const artifacts = path.join(scratch, "session-1");
+		fs.mkdirSync(artifacts);
+		fs.writeFileSync(path.join(artifacts, "img.png"), "x");
+
+		deleteOmpSession(file);
+
+		expect(fs.existsSync(file)).toBe(false);
+		expect(fs.existsSync(artifacts)).toBe(false);
+	});
+
+	test("refuses paths that do not end in .jsonl", () => {
+		const file = path.join(scratch, "keep-me.txt");
+		fs.writeFileSync(file, "precious");
+		deleteOmpSession(file);
+		expect(fs.existsSync(file)).toBe(true);
+	});
+
+	test("a missing session file does not throw", () => {
+		expect(() => deleteOmpSession(path.join(scratch, "nope.jsonl"))).not.toThrow();
+	});
+});