From cf30f1f7b8e32d5ed548d8b583b3978de882e1a6 Mon Sep 17 00:00:00 2001 From: Linghui Gong Date: Thu, 27 Aug 2026 12:46:23 +0800 Subject: [PATCH 1/2] test: raise coverage gates and add CI coverage thresholds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the hardening pass — closes the coverage gaps the review surfaced, test-first throughout. Server (30 → 84 tests; line coverage 79% → 96%): - fs.ts (10%→96%): listDir/searchDir/searchPaths pickers and deleteOmpSession — the only function that destroys user files - omp.ts (8%→97%): CLI bridge via a generated fake omp binary (arg passing, stdout parsing, failure + timeout kill) and listOmpSessions header scanning with an injectable sessions root - sessions/manager.ts (88%→99%): delete data-safety (real file removal, resumedFromHistory protection), non-mock spawn identity handshake run against the bundled mock host as a fake omp binary, spawn-failure surfacing and start() retry semantics - rpc/process.ts (80%→87%): readiness timeout, close() SIGTERM→SIGKILL escalation, kill(), frame delivery - rpc/frame.ts (100% lines): chunk metadata/base64/sequence validation - app.ts (75%→88%): cross-site WS upgrade refusal, fs picker endpoints, WS refresh_session/stop_session control messages, non-mock omp CLI error surfacing; replay-pruning test de-flaked with a sentinel Web (19 → 57 tests; measured files now include api.ts at 80%): - SessionStore state machine driven by a fake transport (new injectable socket options on the constructor): hello/hydration, message upsert + history dedupe, immutable tool executions with late-joiner synthesis, extension dialog queue/cancel/answer, widget/status/editor surfaces, open_url http(s)-only gating, notice/output caps, registry refcounting - markdown unescape/tag-strip pipeline; format/content helpers Also fixes two real bugs the new tests exposed: - spawnOmpProcess threw synchronously on spawn failure, leaving its rejected `ready` promise unconsumed; it now reports failures through `ready` - start() could never retry a session whose spawn had failed (dead process handle blocked respawning) CI: test step now runs under `--coverage` with ratcheted thresholds (server ≥93%, web ≥90%) and a README badge. --- .github/workflows/ci.yml | 8 +- README.md | 2 + server/scripts/mock-rpc-host.ts | 7 + server/src/omp.ts | 14 +- server/src/rpc/process.ts | 11 +- server/src/sessions/manager.ts | 18 +- server/test/fixtures/spawn-test-agent.sh | 14 + server/test/frame.test.ts | 127 ++++++++ server/test/fs.test.ts | 129 ++++++++ server/test/hardening.test.ts | 157 +++++++++- server/test/manager.test.ts | 273 ++++++++++++++++ server/test/omp.test.ts | 173 +++++++++++ server/test/process.test.ts | 85 +++++ web/src/api.ts | 40 ++- web/src/components/chat/ExtensionUi.tsx | 4 +- web/test/formatContent.test.ts | 132 ++++++++ web/test/markdownPipeline.test.ts | 77 +++++ web/test/sessionStore.test.ts | 377 +++++++++++++++++++++++ 18 files changed, 1626 insertions(+), 22 deletions(-) create mode 100755 server/test/fixtures/spawn-test-agent.sh create mode 100644 server/test/frame.test.ts create mode 100644 server/test/fs.test.ts create mode 100644 server/test/manager.test.ts create mode 100644 server/test/omp.test.ts create mode 100644 server/test/process.test.ts create mode 100644 web/test/formatContent.test.ts create mode 100644 web/test/markdownPipeline.test.ts create mode 100644 web/test/sessionStore.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 46598cf..f8009eb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -28,5 +28,9 @@ 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. + - name: Tests with coverage gates + run: | + set -e + cd server && bun test --coverage --coverage-threshold 93 + cd ../web && bun test --coverage --coverage-threshold 90 diff --git a/README.md b/README.md index bfcb56a..cad7773 100644 --- a/README.md +++ b/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, diff --git a/server/scripts/mock-rpc-host.ts b/server/scripts/mock-rpc-host.ts index 9b1a421..5ff919c 100644 --- a/server/scripts/mock-rpc-host.ts +++ b/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": diff --git a/server/src/omp.ts b/server/src/omp.ts index f52f684..95461ad 100644 --- a/server/src/omp.ts +++ b/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 { +export function runOmpCli(bin: string, args: string[], timeoutMs: number = CLI_TIMEOUT_MS): Promise { 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 { 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 { 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 }); diff --git a/server/src/rpc/process.ts b/server/src/rpc/process.ts index af7467b..0948edc 100644 --- a/server/src/rpc/process.ts +++ b/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 = ""; diff --git a/server/src/sessions/manager.ts b/server/src/sessions/manager.ts index 8302181..3a28e52 100644 --- a/server/src/sessions/manager.ts +++ b/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); diff --git a/server/test/fixtures/spawn-test-agent.sh b/server/test/fixtures/spawn-test-agent.sh new file mode 100755 index 0000000..717de10 --- /dev/null +++ b/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 …`. 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" diff --git a/server/test/frame.test.ts b/server/test/frame.test.ts new file mode 100644 index 0000000..2eccb7d --- /dev/null +++ b/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): Record { + 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"); + }); +}); diff --git a/server/test/fs.test.ts b/server/test/fs.test.ts new file mode 100644 index 0000000..bda5ff0 --- /dev/null +++ b/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(); + }); +}); diff --git a/server/test/hardening.test.ts b/server/test/hardening.test.ts index 5f440f8..2a06917 100644 --- a/server/test/hardening.test.ts +++ b/server/test/hardening.test.ts @@ -302,7 +302,10 @@ describe("reconnect dialog replay", () => { await sleep(300); const second = await openSocket(sessionId); - await sleep(1_500); // let the replay burst arrive + // Sentinel: the server's private auto-hydration response is queued + // AFTER every buffered replay frame on this socket, so once it + // arrives the replay burst is provably complete — no fixed sleep. + await agentFrame(second, f => f.type === "response" && String(f.id ?? "").startsWith("auto:state:")); const resurrected = second.frames .map(unwrap) .some(f => f.type === "extension_ui_request" && f.method === "select" && f.id === requestId); @@ -338,3 +341,155 @@ describe("reconnect dialog replay", () => { } }, 30_000); }); + +// ── filesystem picker endpoints ────────────────────────────────────────────── + +describe("non-mock omp CLI bridge errors surface to clients", () => { + let bridgeBase: string; + + beforeAll(async () => { + // A fake omp binary that always fails: the bridge must surface its + // stderr as a client-visible error instead of swallowing it. + const fakeBin = path.join(tempDir, "fake-omp-fail"); + await Bun.write(fakeBin, '#!/usr/bin/env bun\nprocess.stderr.write("boom\\n");\nprocess.exit(3);\n'); + fs.chmodSync(fakeBin, 0o755); + const config = loadConfig({ + dataDir: path.join(tempDir, "data-cli"), + port: 0, + host: "127.0.0.1", + mockMode: false, + webDistDir: tempDir, + ompBin: fakeBin, + }); + const bridgeManager = new SessionManager(config); + await bridgeManager.load(); + bridgeBase = `http://127.0.0.1:${createApp({ config, manager: bridgeManager }).server.port}`; + }); + + test("config/models endpoints surface omp CLI errors", async () => { + const get = await fetch(`${bridgeBase}/api/config`); + expect(get.status).toBe(500); + expect(((await get.json()) as { error: string }).error).toContain("boom"); + + const put = await fetch(`${bridgeBase}/api/config`, { + method: "PUT", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ key: "a", value: "b" }), + }); + expect(put.status).toBe(400); + expect(((await put.json()) as { error: string }).error).toContain("boom"); + + const del = await fetch(`${bridgeBase}/api/config?key=a`, { method: "DELETE" }); + expect(del.status).toBe(400); + expect(((await del.json()) as { error: string }).error).toContain("boom"); + + const models = await fetch(`${bridgeBase}/api/models`); + expect(models.status).toBe(500); + expect(((await models.json()) as { error: string }).error).toContain("boom"); + }); +}); + +describe("fs picker endpoints", () => { + let pickerCwd: string; + + beforeAll(async () => { + pickerCwd = path.join(tempDir, "fs-picker-cwd"); + fs.mkdirSync(pickerCwd, { recursive: true }); + fs.writeFileSync(path.join(pickerCwd, "page.html"), "

picker

"); + fs.mkdirSync(path.join(pickerCwd, "subdir")); + }); + + test("list resolves path/parent and lists subdirectories", async () => { + const res = await fetch(`${baseUrl}/api/fs/list?path=${encodeURIComponent(pickerCwd)}`); + expect(res.status).toBe(200); + const body = (await res.json()) as { path: string; parent: string; dirs: string[] }; + expect(body.path).toBe(pickerCwd); + expect(body.parent).toBe(path.dirname(pickerCwd)); + expect(body.dirs).toEqual(["subdir"]); + }); + + test("list on a file or missing path yields 400", async () => { + expect( + (await fetch(`${baseUrl}/api/fs/list?path=${encodeURIComponent(path.join(pickerCwd, "page.html"))}`)).status, + ).toBe(400); + expect((await fetch(`${baseUrl}/api/fs/list?path=/definitely/missing`)).status).toBe(400); + }); + + test("search matches directory names by basename prefix", async () => { + const res = await fetch(`${baseUrl}/api/fs/search?prefix=${encodeURIComponent(path.join(tempDir, "fs-pic"))}`); + expect(res.status).toBe(200); + const body = (await res.json()) as { matches: string[] }; + expect(body.matches).toEqual([path.join(tempDir, "fs-picker-cwd")]); + }); + + test("paths lists files and dirs under a cwd", async () => { + const res = await fetch(`${baseUrl}/api/fs/paths?prefix=./&cwd=${encodeURIComponent(pickerCwd)}`); + expect(res.status).toBe(200); + const body = (await res.json()) as { matches: string[] }; + expect(body.matches).toEqual([path.join(pickerCwd, "page.html"), `${path.join(pickerCwd, "subdir")}/`]); + }); +}); + +// ── WS control messages ────────────────────────────────────────────────────── + +describe("ws control messages", () => { + let sessionId: string; + + beforeAll(async () => { + sessionId = await createSession("ws-control"); + const deadline = Date.now() + 20_000; + while (Date.now() < deadline) { + const detail = (await fetch(`${baseUrl}/api/sessions/${sessionId}`).then(r => r.json())) as { + session: { status: string }; + }; + if (detail.session.status === "running" || detail.session.status === "error") return; + await new Promise(resolve => setTimeout(resolve, 200)); + } + throw new Error("ws-control session did not start"); + }); + + afterAll(async () => { + if (sessionId) await jsonDelete(sessionId); + }); + + function openSocket(): Promise { + return new Promise((resolve, reject) => { + const ws = new WebSocket(`${baseUrl.replace("http", "ws")}/ws/sessions/${sessionId}`); + ws.onopen = () => resolve(ws); + ws.onerror = () => reject(new Error("ws open failed")); + setTimeout(() => reject(new Error("ws open timeout")), 10_000); + }); + } + + test("refresh_session replies with a session snapshot", async () => { + const ws = await openSocket(); + const reply = await new Promise>(resolve => { + ws.onmessage = event => { + const parsed = JSON.parse(String(event.data)) as Record; + if (parsed.type === "session") resolve(parsed); + }; + ws.send(JSON.stringify({ type: "refresh_session" })); + }); + expect((reply.session as { id?: string }).id).toBe(sessionId); + ws.close(); + }); + + test("stop_session stops the agent and broadcasts the stopped status", async () => { + const ws = await openSocket(); + const stopped = await new Promise>(resolve => { + ws.onmessage = event => { + const parsed = JSON.parse(String(event.data)) as Record; + if (parsed.type === "session" && (parsed.session as { status?: string }).status === "stopped") { + resolve(parsed); + } + }; + ws.send(JSON.stringify({ type: "stop_session" })); + }); + expect((stopped.session as { id?: string }).id).toBe(sessionId); + const detail = (await fetch(`${baseUrl}/api/sessions/${sessionId}`).then(r => r.json())) as { + session: { status: string }; + }; + expect(detail.session.status).toBe("stopped"); + ws.close(); + }); +}); diff --git a/server/test/manager.test.ts b/server/test/manager.test.ts new file mode 100644 index 0000000..723fee5 --- /dev/null +++ b/server/test/manager.test.ts @@ -0,0 +1,273 @@ +/** + * Unit tests for SessionManager data safety around deletion. The session + * registry is seeded through `load()` (the same path a real restart takes), so + * no agent process is spawned and the tests exercise exactly the rule that + * matters: deleting a web session also deletes the underlying omp session + * files — UNLESS the session was opened from the history browser + * (resumedFromHistory), in which case pre-existing user data must survive. + */ +import { afterAll, beforeEach, 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 { loadConfig } from "../src/config"; +import { SessionManager } from "../src/sessions/manager"; + +let tempDir: string; + +beforeEach(() => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "omp-manager-")); +}); + +afterAll(() => { + try { + fs.rmSync(tempDir, { recursive: true, force: true }); + } catch { + // best effort + } +}); + +interface SeedOptions { + sessionFile?: string; + resumedFromHistory?: boolean; + mockMode?: boolean; +} + +async function seededManager(options: SeedOptions = {}): Promise { + const dataDir = path.join(tempDir, `data-${Math.random().toString(36).slice(2)}`); + fs.mkdirSync(dataDir, { recursive: true }); + const config = loadConfig({ + dataDir, + port: 0, + host: "127.0.0.1", + webDistDir: tempDir, + mockMode: options.mockMode ?? false, + }); + const registry = { + sessions: [ + { + id: "s1", + name: "seeded", + cwd: tempDir, + createdAt: 0, + updatedAt: 0, + messageCount: 0, + ...(options.sessionFile ? { sessionFile: options.sessionFile } : {}), + ...(options.resumedFromHistory ? { resumedFromHistory: true } : {}), + }, + ], + }; + fs.writeFileSync(path.join(dataDir, "sessions.json"), JSON.stringify(registry)); + const manager = new SessionManager(config); + await manager.load(); + return manager; +} + +describe("session delete data safety", () => { + test("delete removes the underlying omp session file and its artifacts", async () => { + const sessionFile = path.join(tempDir, "session-abc.jsonl"); + const artifacts = path.join(tempDir, "session-abc"); + fs.writeFileSync(sessionFile, "{}"); + fs.mkdirSync(artifacts); + fs.writeFileSync(path.join(artifacts, "shot.png"), "x"); + + const manager = await seededManager({ sessionFile }); + await manager.delete("s1"); + + expect(fs.existsSync(sessionFile)).toBe(false); + expect(fs.existsSync(artifacts)).toBe(false); + expect(manager.get("s1")).toBeUndefined(); + }); + + test("a session resumed from history never deletes the underlying file", async () => { + const sessionFile = path.join(tempDir, "history-session.jsonl"); + const artifacts = path.join(tempDir, "history-session"); + fs.writeFileSync(sessionFile, "user data"); + fs.mkdirSync(artifacts); + fs.writeFileSync(path.join(artifacts, "keep.png"), "x"); + + const manager = await seededManager({ sessionFile, resumedFromHistory: true }); + await manager.delete("s1"); + + expect(fs.readFileSync(sessionFile, "utf8")).toBe("user data"); + expect(fs.existsSync(artifacts)).toBe(true); + expect(manager.get("s1")).toBeUndefined(); + }); + + test("mock mode never touches the filesystem", async () => { + const sessionFile = path.join(tempDir, "mock-session.jsonl"); + fs.writeFileSync(sessionFile, "{}"); + + const manager = await seededManager({ sessionFile, mockMode: true }); + await manager.delete("s1"); + + expect(fs.existsSync(sessionFile)).toBe(true); + }); + + test("a session without a recorded session file just leaves the registry", async () => { + const manager = await seededManager(); + await manager.delete("s1"); + expect(manager.get("s1")).toBeUndefined(); + }); + + test("create() with an initial prompt sends it to the agent once ready", async () => { + const config = loadConfig({ + dataDir: path.join(tempDir, "data-prompt"), + port: 0, + host: "127.0.0.1", + mockMode: true, + webDistDir: tempDir, + }); + const manager = new SessionManager(config); + await manager.load(); + const session = await manager.create({ cwd: tempDir, prompt: "hello from test" }); + + // The mock host echoes the prompt as a live user message_start frame. + const deadline = Date.now() + 10_000; + let echoed = false; + while (Date.now() < deadline && !echoed) { + echoed = manager + .bufferedFrames(session.id) + .some( + frame => + (frame as { type?: string; message?: { role?: string; content?: unknown } }).type === + "message_start" && + (frame as { message?: { role?: string; content?: unknown } }).message?.role === "user" && + (frame as { message?: { content?: string } }).message?.content === "hello from test", + ); + await new Promise(resolve => setTimeout(resolve, 150)); + } + expect(echoed).toBe(true); + await manager.shutdown(); + }); +}); + +// ── non-mock spawn identity flow ───────────────────────────────────────────── +// +// With mockMode off the spawn handshake mints (or re-captures) the omp session +// identity: fresh sessions run new_session → set_session_name → get_state and +// record id + file; resumed sessions re-capture the session file. The bundled +// mock RPC host plays the agent so the full handshake runs for real. + +describe("non-mock spawn identity flow", () => { + const TEST_AGENT = path.resolve(import.meta.dir, "fixtures/spawn-test-agent.sh"); + let tempDir2: string; + + beforeEach(() => { + fs.chmodSync(TEST_AGENT, 0o755); + tempDir2 = fs.mkdtempSync(path.join(os.tmpdir(), "omp-spawn-")); + }); + + afterAll(async () => { + // shutdown of the last manager happens inside each test + try { + fs.rmSync(tempDir2, { recursive: true, force: true }); + } catch { + // best effort + } + }); + + function spawnConfig(): ReturnType { + return loadConfig({ + dataDir: path.join(tempDir2, "data"), + port: 0, + host: "127.0.0.1", + mockMode: false, + webDistDir: tempDir2, + // The wrapper IS the omp binary: it receives the exact CLI args the + // server would pass to a real agent (--mode rpc, --session …). + ompBin: TEST_AGENT, + }); + } + + async function waitStatus(manager: SessionManager, id: string): Promise<"running" | "error"> { + const deadline = Date.now() + 20_000; + for (;;) { + const info = manager.get(id); + if (info?.status === "running" || info?.status === "error") return info.status; + if (Date.now() > deadline) throw new Error(`session did not settle: ${info?.status}`); + await new Promise(resolve => setTimeout(resolve, 150)); + } + } + + async function waitFor(predicate: () => boolean, what: string): Promise { + const deadline = Date.now() + 10_000; + while (!predicate()) { + if (Date.now() > deadline) throw new Error(`timed out waiting for ${what}`); + await new Promise(resolve => setTimeout(resolve, 100)); + } + } + + test("a fresh session records the minted omp identity", async () => { + const manager = new SessionManager(spawnConfig()); + await manager.load(); + const session = await manager.create({ cwd: tempDir2, name: "identity-flow" }); + expect(await waitStatus(manager, session.id)).toBe("running"); + + // The identity handshake runs after the running transition — poll for it. + await waitFor(() => manager.get(session.id)?.ompSessionId !== undefined, "ompSessionId"); + const info = manager.get(session.id); + // The agent mints the id; the registry must record it plus its file. + expect(info?.ompSessionId?.startsWith("mock-")).toBe(true); + expect(info?.sessionFile).toBe(`/mock/sessions/${info?.ompSessionId}`); + await manager.shutdown(); + }, 30_000); + + test("a resumed session keeps its ompSessionId and captures the session file", async () => { + const manager = new SessionManager(spawnConfig()); + await manager.load(); + const session = await manager.create({ cwd: tempDir2, resumeOmpSessionId: "resume-1", name: "resumed" }); + expect(await waitStatus(manager, session.id)).toBe("running"); + + await waitFor(() => manager.get(session.id)?.sessionFile !== undefined, "sessionFile"); + const info = manager.get(session.id); + expect(info?.ompSessionId).toBe("resume-1"); + expect(info?.sessionFile).toBe("/mock/sessions/resume-1"); + await manager.shutdown(); + }, 30_000); + + test("a broken omp binary surfaces an error status", async () => { + const config = loadConfig({ + dataDir: path.join(tempDir2, "data-broken"), + port: 0, + host: "127.0.0.1", + mockMode: false, + webDistDir: tempDir2, + ompBin: "/definitely/not/a/real/omp", + }); + const manager = new SessionManager(config); + await manager.load(); + const session = await manager.create({ cwd: tempDir2 }); + expect(await waitStatus(manager, session.id)).toBe("error"); + expect(manager.get(session.id)?.error).toBeTruthy(); + await manager.shutdown(); + }, 30_000); + + test("start() on an already-running session is a no-op and a broken spawn throws", async () => { + const manager = new SessionManager(spawnConfig()); + await manager.load(); + const session = await manager.create({ cwd: tempDir2, name: "start-idempotent" }); + expect(await waitStatus(manager, session.id)).toBe("running"); + + const before = manager.get(session.id); + const again = await manager.start(session.id); + expect(again.id).toBe(before?.id); + expect(again.status).toBe("running"); + + const broken = loadConfig({ + dataDir: path.join(tempDir2, "data-broken-start"), + port: 0, + host: "127.0.0.1", + mockMode: false, + webDistDir: tempDir2, + ompBin: "/definitely/not/a/real/omp", + }); + const manager2 = new SessionManager(broken); + await manager2.load(); + const failed = await manager2.create({ cwd: tempDir2 }); + await waitStatus(manager2, failed.id); + await expect(manager2.start(failed.id)).rejects.toThrow(); + await manager.shutdown(); + await manager2.shutdown(); + }, 30_000); +}); diff --git a/server/test/omp.test.ts b/server/test/omp.test.ts new file mode 100644 index 0000000..4533162 --- /dev/null +++ b/server/test/omp.test.ts @@ -0,0 +1,173 @@ +/** + * Unit tests for the omp CLI bridge. A fake executable stands in for the real + * `omp` binary — each test generates a dedicated script with the mode inlined + * (Bun children do NOT inherit runtime `process.env` mutations, so env-driven + * fakes are unreliable) — letting us pin down argument passing, stdout + * parsing, failure and timeout handling, plus the on-disk session-history + * scanner with an injectable root. + */ +import { afterAll, beforeEach, 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 { + listOmpSessions, + ompConfigList, + ompConfigPath, + ompConfigReset, + ompConfigSet, + ompModelList, + runOmpCli, +} from "../src/omp"; + +let scratch: string; +let counter = 0; + +beforeEach(() => { + scratch = fs.mkdtempSync(path.join(os.tmpdir(), "omp-cli-")); +}); + +afterAll(() => { + try { + fs.rmSync(path.join(os.tmpdir(), "omp-cli-"), { recursive: true, force: true, maxRetries: 3 }); + } catch { + // best effort — leftover temp dirs are harmless + } +}); + +/** + * Generate an executable that behaves like a canned `omp` invocation. + * Modes: "json" prints `stdout`; "argv" records its argv to `argvFile`; + * "fail" exits 3 with stderr "boom"; "slow" idles until killed. + */ +async function makeFakeBin(mode: "json" | "argv" | "fail" | "slow", opts: { stdout?: string; argvFile?: string } = {}) { + const script = `#!/usr/bin/env bun +const mode = ${JSON.stringify(mode)}; +const stdout = ${JSON.stringify(opts.stdout ?? "")}; +const argvFile = ${JSON.stringify(opts.argvFile ?? "")}; +if (mode === "argv") { + if (argvFile) await Bun.write(argvFile, JSON.stringify(process.argv.slice(2))); + process.exit(0); +} +if (mode === "fail") { + process.stderr.write("boom\\n"); + process.exit(3); +} +if (mode === "slow") { + await new Promise(() => {}); +} +process.stdout.write(stdout); +`; + const file = path.join(scratch, `fake-omp-${counter++}`); + await Bun.write(file, script); + fs.chmodSync(file, 0o755); + return file; +} + +describe("runOmpCli", () => { + test("resolves with stdout on success", async () => { + const bin = await makeFakeBin("json", { stdout: "hello omp" }); + expect(await runOmpCli(bin, ["--version"], 5_000)).toBe("hello omp"); + }); + + test("rejects with stderr on non-zero exit", async () => { + const bin = await makeFakeBin("fail"); + await expect(runOmpCli(bin, ["config", "list"], 5_000)).rejects.toThrow("boom"); + }); + + test("kills the process and rejects when the timeout fires", async () => { + const bin = await makeFakeBin("slow"); + const started = Date.now(); + await expect(runOmpCli(bin, ["config", "list"], 150)).rejects.toThrow("timed out"); + expect(Date.now() - started).toBeLessThan(5_000); + }); +}); + +describe("config + model bridges", () => { + test("config list parses the JSON catalog", async () => { + const bin = await makeFakeBin("json", { + stdout: JSON.stringify({ modelRoles: { value: {}, type: "record", description: "roles" } }), + }); + expect(await ompConfigList(bin)).toEqual([ + { key: "modelRoles", value: {}, type: "record", description: "roles" }, + ]); + }); + + test("config path is trimmed", async () => { + const bin = await makeFakeBin("json", { stdout: "/mock/.omp/agent/config.yml\n" }); + expect(await ompConfigPath(bin)).toBe("/mock/.omp/agent/config.yml"); + }); + + test("config set/reset pass key and value as single argv entries", async () => { + const argvFile = path.join(scratch, "argv.json"); + const bin = await makeFakeBin("argv", { argvFile }); + + await ompConfigSet(bin, "modelRoles", '{"default":"a/b"}'); + let argv = JSON.parse(await fs.promises.readFile(argvFile, "utf8")) as string[]; + expect(argv).toEqual(["config", "set", "modelRoles", '{"default":"a/b"}']); + + await ompConfigSet(bin, "note", "spaces are kept"); + argv = JSON.parse(await fs.promises.readFile(argvFile, "utf8")) as string[]; + expect(argv[3]).toBe("spaces are kept"); + + await ompConfigReset(bin, "modelRoles"); + argv = JSON.parse(await fs.promises.readFile(argvFile, "utf8")) as string[]; + expect(argv).toEqual(["config", "reset", "modelRoles"]); + }); + + test("models ls parses the catalog payload", async () => { + const bin = await makeFakeBin("json", { + stdout: JSON.stringify({ models: [{ provider: "p", id: "m", name: "Model" }] }), + }); + expect(await ompModelList(bin)).toEqual([{ provider: "p", id: "m", name: "Model" }]); + }); +}); + +describe("listOmpSessions", () => { + let sessionsRoot: string; + + beforeEach(() => { + sessionsRoot = path.join(scratch, "sessions"); + const write = (rel: string, lines: object[], mtime: number) => { + const file = path.join(sessionsRoot, rel); + fs.mkdirSync(path.dirname(file), { recursive: true }); + fs.writeFileSync(file, `${lines.map(line => JSON.stringify(line)).join("\n")}\n`); + fs.utimesSync(file, new Date(mtime), new Date(mtime)); + }; + const day = 86_400_000; + write( + "a/old.jsonl", + [ + { type: "title", title: "Old chat" }, + { type: "session", id: "s-old", cwd: "/work/proj", timestamp: "2026-01-01T00:00:00Z" }, + ], + Date.now() - 3 * day, + ); + write("b/new.jsonl", [{ type: "session", id: "s-new", cwd: "/work/proj" }], Date.now() - day); + write("b/other.jsonl", [{ type: "session", id: "s-other", cwd: "/somewhere/else" }], Date.now()); + write("a/broken.jsonl", [{ type: "title", title: "no session header here" }], Date.now()); + fs.writeFileSync(path.join(sessionsRoot, "a", "notes.txt"), "not a session"); + }); + + test("matches sessions by header cwd, newest first", () => { + const sessions = listOmpSessions("/work/proj", 10, sessionsRoot); + expect(sessions.map(s => s.id)).toEqual(["s-new", "s-old"]); + expect(sessions[1]?.title).toBe("Old chat"); + expect(sessions[1]?.file).toBe(path.join(sessionsRoot, "a", "old.jsonl")); + expect(sessions[1]?.createdAt).toBe(Date.parse("2026-01-01T00:00:00Z")); + expect(sessions[1]?.sizeBytes).toBeGreaterThan(0); + }); + + test("other working directories, broken headers and non-jsonl files are skipped", () => { + expect(listOmpSessions("/somewhere/else", 10, sessionsRoot).map(s => s.id)).toEqual(["s-other"]); + expect(listOmpSessions("/no/such/cwd", 10, sessionsRoot)).toEqual([]); + }); + + test("limit caps the result", () => { + expect(listOmpSessions("/work/proj", 1, sessionsRoot).map(s => s.id)).toEqual(["s-new"]); + }); + + test("a missing sessions root yields an empty list", () => { + expect(listOmpSessions("/work/proj", 10, path.join(sessionsRoot, "missing"))).toEqual([]); + }); +}); diff --git a/server/test/process.test.ts b/server/test/process.test.ts new file mode 100644 index 0000000..5370b56 --- /dev/null +++ b/server/test/process.test.ts @@ -0,0 +1,85 @@ +/** + * Unit tests for OmpProcess lifecycle: readiness timeout, graceful close with + * SIGTERM escalation to SIGKILL, and the exit callback. A real child process + * (bun -e) is used so signal handling behaves exactly like production. + */ +import { describe, expect, test } from "bun:test"; +import { spawnOmpProcess } from "../src/rpc/process"; + +const IDLER = "setInterval(() => {}, 1e6)"; + +describe("spawnOmpProcess lifecycle", () => { + test("readiness timeout rejects and close terminates the child", async () => { + const exits: Array<{ code: number | null; signal: string | null }> = []; + const proc = spawnOmpProcess({ + bin: process.execPath, + args: ["-e", IDLER], + cwd: import.meta.dir, + onFrame: () => {}, + onExit: info => exits.push(info), + readyTimeoutMs: 150, + }); + await expect(proc.ready).rejects.toThrow(/timed out waiting for omp RPC ready/); + + await proc.close(150); + expect(exits.length).toBe(1); + expect(exits[0]?.code === 0 || exits[0]?.signal !== null).toBe(true); + }); + + test("a child that ignores SIGTERM is escalated to SIGKILL", async () => { + const exits: Array<{ code: number | null; signal: string | null }> = []; + const proc = spawnOmpProcess({ + bin: process.execPath, + // Trap SIGTERM and keep running — only SIGKILL can end us. + args: ["-e", "process.on('SIGTERM', () => {}); setInterval(() => {}, 1e6)"], + cwd: import.meta.dir, + onFrame: () => {}, + onExit: info => exits.push(info), + readyTimeoutMs: 150, + }); + await proc.ready.catch(() => {}); + + const started = Date.now(); + await proc.close(150); + // close() returns right after sending SIGKILL; wait for the death to land. + const deadline = Date.now() + 2_000; + while (exits.length === 0 && Date.now() < deadline) await new Promise(r => setTimeout(r, 50)); + expect(exits.length).toBe(1); + expect(exits[0]?.signal).toBe("SIGKILL"); + expect(Date.now() - started).toBeLessThan(10_000); + }); + + test("kill() terminates the child directly", async () => { + const exits: Array<{ code: number | null; signal: string | null }> = []; + const proc = spawnOmpProcess({ + bin: process.execPath, + args: ["-e", IDLER], + cwd: import.meta.dir, + onFrame: () => {}, + onExit: info => exits.push(info), + readyTimeoutMs: 150, + }); + await proc.ready.catch(() => {}); + + proc.kill(); + await new Promise(resolve => setTimeout(resolve, 300)); + expect(exits.length).toBe(1); + expect(exits[0]?.signal).toBe("SIGTERM"); + }); + + test("frames from the child reach onFrame after decoder reassembly", async () => { + const frames: unknown[] = []; + const proc = spawnOmpProcess({ + bin: process.execPath, + args: ["-e", "console.log(JSON.stringify({ type: 'ready', protocolVersion: 1 }))"], + cwd: import.meta.dir, + onFrame: frame => frames.push(frame), + onExit: () => {}, + readyTimeoutMs: 2_000, + }); + const deadline = Date.now() + 3_000; + while (frames.length === 0 && Date.now() < deadline) await new Promise(r => setTimeout(r, 50)); + await proc.close(150); + expect(frames.some(f => (f as { type?: string }).type === "ready")).toBe(true); + }); +}); diff --git a/web/src/api.ts b/web/src/api.ts index 0f03fce..65137d9 100644 --- a/web/src/api.ts +++ b/web/src/api.ts @@ -207,8 +207,9 @@ export interface SessionSnapshot { widgets: Record; /** Persistent status-line entries (setStatus), keyed by statusKey. */ statusEntries: Record; - /** open_url requests (login flows) not yet dismissed. */ - openUrls: Array<{ at: number; url: string; launchUrl?: string; instructions?: string }>; + /** open_url requests (login flows) not yet dismissed. Identified by a + * monotonic id — millisecond timestamps can collide. */ + openUrls: Array<{ id: number; at: number; url: string; launchUrl?: string; instructions?: string }>; /** Text output from local slash commands (command_output frames). */ commandOutputs: Array<{ at: number; text: string }>; /** Composer prefill pushed by the agent (set_editor_text); seq bumps on change. */ @@ -229,6 +230,9 @@ function sameMessage(a: WireMessage, b: WireMessage): boolean { return true; } +/** Monotonic id for agent-pushed open_url cards (timestamps can collide). */ +let openUrlSeq = 0; + export const emptySnapshot = (): SessionSnapshot => ({ phase: "connecting", messages: [], @@ -245,6 +249,18 @@ export const emptySnapshot = (): SessionSnapshot => ({ loginProviders: [], }); +export interface SessionStoreOptions { + /** Endpoint override (tests inject this instead of touching `location`). */ + socketUrl?: (sessionId: string) => string; + /** Socket construction override (tests inject a fake transport). */ + socketFactory?: (url: string) => WebSocket; +} + +function defaultSocketUrl(sessionId: string): string { + const protocol = location.protocol === "https:" ? "wss:" : "ws:"; + return `${protocol}//${location.host}/ws/sessions/${sessionId}`; +} + export class SessionStore { readonly sessionId: string; #listeners = new Set<() => void>(); @@ -258,9 +274,13 @@ export class SessionStore { /** In-flight streaming slots (from message_start/update/end). */ #liveMessages: RenderedMessage[] = []; #historyApplied = false; + readonly #socketUrl: (sessionId: string) => string; + readonly #socketFactory: (url: string) => WebSocket; - constructor(sessionId: string) { + constructor(sessionId: string, options: SessionStoreOptions = {}) { this.sessionId = sessionId; + this.#socketUrl = options.socketUrl ?? defaultSocketUrl; + this.#socketFactory = options.socketFactory ?? (url => new WebSocket(url)); this.#snapshot = emptySnapshot(); this.connect(); } @@ -313,8 +333,7 @@ export class SessionStore { s.phase = "connecting"; s.error = undefined; }); - const protocol = location.protocol === "https:" ? "wss:" : "ws:"; - const ws = new WebSocket(`${protocol}//${location.host}/ws/sessions/${this.sessionId}`); + const ws = this.#socketFactory(this.#socketUrl(this.sessionId)); this.#ws = ws; ws.onopen = () => { @@ -708,9 +727,10 @@ export class SessionStore { break; case "open_url": { this.#mutate(s => { + const id = ++openUrlSeq; s.openUrls = [ ...s.openUrls.slice(-2), - { at: Date.now(), url: frame.url, launchUrl: frame.launchUrl, instructions: frame.instructions }, + { id, at: Date.now(), url: frame.url, launchUrl: frame.launchUrl, instructions: frame.instructions }, ]; }); // Agent-supplied URL: only absolute http(s) may auto-open. Anything @@ -739,9 +759,9 @@ export class SessionStore { } /** Dismiss a rendered open_url link (login flow). */ - dismissOpenUrl(at: number) { + dismissOpenUrl(id: number) { this.#mutate(s => { - s.openUrls = s.openUrls.filter(link => link.at !== at); + s.openUrls = s.openUrls.filter(link => link.id !== id); }); } @@ -802,11 +822,11 @@ const stores = new Map(); const storeRefs = new Map(); /** Take ownership of the session's store, creating it on first use. */ -export function acquireSessionStore(sessionId: string): SessionStore { +export function acquireSessionStore(sessionId: string, options?: SessionStoreOptions): SessionStore { storeRefs.set(sessionId, (storeRefs.get(sessionId) ?? 0) + 1); let store = stores.get(sessionId); if (!store) { - store = new SessionStore(sessionId); + store = new SessionStore(sessionId, options); stores.set(sessionId, store); } return store; diff --git a/web/src/components/chat/ExtensionUi.tsx b/web/src/components/chat/ExtensionUi.tsx index fdad61c..a1b80cc 100644 --- a/web/src/components/chat/ExtensionUi.tsx +++ b/web/src/components/chat/ExtensionUi.tsx @@ -33,7 +33,7 @@ export function ExtensionSurfaces({ store, snapshot }: { store: SessionStore; sn const hrefOk = isHttpUrl(link.url); return (
@@ -61,7 +61,7 @@ export function ExtensionSurfaces({ store, snapshot }: { store: SessionStore; sn ) : null}
- store.dismissOpenUrl(link.at)} /> + store.dismissOpenUrl(link.id)} /> ); })} diff --git a/web/test/formatContent.test.ts b/web/test/formatContent.test.ts new file mode 100644 index 0000000..bc10fe9 --- /dev/null +++ b/web/test/formatContent.test.ts @@ -0,0 +1,132 @@ +/** + * Unit tests for the pure formatting/content helpers used across the UI. + */ +import { describe, expect, test } from "bun:test"; +import type { WireMessage } from "@omp-web/shared"; +import { assistantText, messagePreview, toolResultText, userText } from "../src/lib/content"; +import { + formatCost, + formatDuration, + formatRelativeTime, + formatTime, + formatTokens, + shortId, + truncate, +} from "../src/lib/format"; + +describe("formatRelativeTime", () => { + test("buckets seconds, minutes, hours and days", () => { + const now = Date.now(); + expect(formatRelativeTime(now - 5_000)).toBe("5s ago"); + expect(formatRelativeTime(now - 120_000)).toBe("2m ago"); + expect(formatRelativeTime(now - 7_200_000)).toBe("2h ago"); + expect(formatRelativeTime(now - 172_800_000)).toBe("2d ago"); + }); + + test("future timestamps use the 'ahead' suffix and invalid input renders a dash", () => { + expect(formatRelativeTime(Date.now() + 60_000)).toBe("1m ahead"); + expect(formatRelativeTime(undefined)).toBe("—"); + expect(formatRelativeTime(Number.NaN)).toBe("—"); + expect(formatRelativeTime("not-a-date")).toBe("—"); + }); +}); + +describe("formatTime / formatDuration / formatTokens / formatCost", () => { + test("formatTime renders HH:MM:SS locally", () => { + const date = new Date(2026, 0, 1, 9, 5, 3); + expect(formatTime(date.getTime())).toBe("09:05:03"); + expect(formatTime(undefined)).toBe(""); + }); + + test("formatDuration covers ms, seconds and minutes", () => { + expect(formatDuration(400)).toBe("400ms"); + expect(formatDuration(1_500)).toBe("1.5s"); + expect(formatDuration(65_000)).toBe("1m 5s"); + expect(formatDuration(-1)).toBe("—"); + expect(formatDuration(Number.NaN)).toBe("—"); + }); + + test("formatTokens buckets k/M", () => { + expect(formatTokens(500)).toBe("500"); + expect(formatTokens(4_200)).toBe("4.2k"); + expect(formatTokens(3_100_000)).toBe("3.10M"); + expect(formatTokens(null)).toBe("—"); + }); + + test("formatCost picks precision by magnitude", () => { + expect(formatCost({ total: 0.0042 })).toBe("$0.0042"); + expect(formatCost({ total: 1.5 })).toBe("$1.50"); + expect(formatCost({ total: 0 })).toBe(""); + expect(formatCost(undefined)).toBe(""); + }); +}); + +describe("shortId / truncate", () => { + test("shortId truncates long ids only", () => { + expect(shortId("abcdefghij", 4)).toBe("abcd"); + expect(shortId("ab", 4)).toBe("ab"); + }); + + test("truncate appends an ellipsis past the limit", () => { + expect(truncate("abcdef", 4)).toBe("abcd…"); + expect(truncate("abc", 4)).toBe("abc"); + }); +}); + +// ── content helpers ────────────────────────────────────────────────────────── + +const assistantMessage: WireMessage = { + role: "assistant", + content: [ + { type: "thinking", thinking: "secret plan" }, + { type: "text", text: "visible answer" }, + ], + model: "m", + usage: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, totalTokens: 0, cost: { total: 0 } }, + stopReason: "stop", + timestamp: 1, +}; + +describe("content helpers", () => { + test("userText joins text blocks and marks images", () => { + const message = { + role: "user", + content: [ + { type: "text", text: "look at" }, + { type: "image", data: "x", mimeType: "image/png" }, + { type: "text", text: "this" }, + ], + timestamp: 1, + } as unknown as WireMessage; + expect(userText(message)).toBe("look at\n(image)\nthis"); + expect(userText({ ...message, content: "plain" } as WireMessage)).toBe("plain"); + expect(userText(assistantMessage)).toBe(""); + }); + + test("toolResultText extracts result text", () => { + const message = { + role: "toolResult", + toolCallId: "t", + toolName: "read", + content: [{ type: "text", text: "file body" }], + isError: false, + timestamp: 1, + }; + expect(toolResultText(message as unknown as WireMessage)).toBe("file body"); + }); + + test("assistantText drops thinking blocks", () => { + expect(assistantText(assistantMessage)).toBe("visible answer"); + }); + + test("messagePreview falls back to tool names then a placeholder", () => { + expect(messagePreview(assistantMessage)).toBe("visible answer"); + const toolOnly = { + ...assistantMessage, + content: [{ type: "toolCall", id: "1", name: "bash", arguments: {} }], + } as unknown as WireMessage; + expect(messagePreview(toolOnly)).toBe("⚙ bash"); + const empty = { ...assistantMessage, content: [] } as unknown as WireMessage; + expect(messagePreview(empty)).toBe("(assistant)"); + }); +}); diff --git a/web/test/markdownPipeline.test.ts b/web/test/markdownPipeline.test.ts new file mode 100644 index 0000000..9341ce0 --- /dev/null +++ b/web/test/markdownPipeline.test.ts @@ -0,0 +1,77 @@ +/** + * Unit tests for the markdown rendering pipeline's less-traveled halves: the + * entity unescape → re-escape round trip and the omp-extension tag stripping + * inside raw HTML tokens. The link/image safety gates have their own suite in + * markdown.test.ts. + */ +import { describe, expect, test } from "bun:test"; +import { renderMarkdown } from "../src/lib/markdown"; + +describe("raw html handling", () => { + test("omp extension tags are unwrapped: tags stripped, inner text kept", () => { + expect(renderMarkdown("temp notekept")).toContain("temp notekept"); + expect(renderMarkdown('droppedafter')).toContain("droppedafter"); + expect(renderMarkdown("yz")).toContain("yz"); + }); + + test("self-closing extension tags disappear entirely", () => { + expect(renderMarkdown("beforeafter")).toContain("beforeafter"); + }); + + test("entities inside raw html survive as escaped text, never live markup", () => { + const out = renderMarkdown("<img src=x onerror=alert(1)>"); + expect(out).not.toContain(" { + const out = renderMarkdown("a & b<b>bold</b>"); + expect(out).not.toContain("bold"); + expect(out).toContain("<b>bold</b>"); + }); + + test("entity decoding covers named, decimal and hex references; unknowns survive verbatim", () => { + // Block-level html arrives as one raw token, exercising the full + // unescape → re-escape pipeline on its content. + const out = renderMarkdown("
a b&cAB&unknown;�
"); + expect(out).not.toContain(" "); + // decoded nbsp/A/B, while decoded '&' is re-escaped + expect(out).toContain("a b&cAB"); + expect(out).toContain("&unknown;"); + expect(out).not.toContain("�"); + }); + + test("entity decoding covers named, decimal and hex references; unknowns survive verbatim", () => { + const out = renderMarkdown("
a b&cAB&unknown;�
"); + expect(out).not.toContain(" "); + // decoded nbsp/A/B, while decoded '&' is re-escaped + expect(out).toContain("a b&cAB"); + expect(out).toContain("&unknown;"); + expect(out).not.toContain("�"); + }); + + test("the remaining named entities decode inside block-level html", () => { + // decode → re-escape is a round trip for lt/gt/quot/apos + const out = renderMarkdown("
< > " '
"); + expect(out).toContain("< > ""); + expect(out).not.toContain("
"); + }); +}); + +describe("basic rendering", () => { + test("code blocks keep their content escaped", () => { + const out = renderMarkdown("```\n\n```"); + expect(out).not.toContain("