From e1ea87e8a01be4871282b5e66444818debd985eb Mon Sep 17 00:00:00 2001 From: Victor Garcia Date: Tue, 28 Jul 2026 00:45:06 -0600 Subject: [PATCH 01/19] style(tests): normalize launcher test formatting Preserve the pre-existing launcher-test formatting pass as an isolated change before Browser integration work. Test names, timeout behavior, subprocess commands, assertions, and lock-race semantics remain unchanged. --- tests/launchers.test.mjs | 79 ++++++++++++++++++++-------------------- 1 file changed, 40 insertions(+), 39 deletions(-) diff --git a/tests/launchers.test.mjs b/tests/launchers.test.mjs index b439686..7110184 100644 --- a/tests/launchers.test.mjs +++ b/tests/launchers.test.mjs @@ -552,46 +552,47 @@ test("record start/stop drive the workspace session and name the file", () => { assert.equal(runScript("record.sh", ["bogus"]).status, 2); }); -test('with_timeout kills a hung command and returns 137', () => { - const r = spawnSync('bash', ['-c', - `. "${repoRoot}/scripts/lib.sh" && with_timeout 1 sleep 30; echo "rc=$?"`], - { env: freshEnv(), encoding: 'utf8', timeout: 10_000 }); - assert.match(r.stdout, /rc=137/); +test("with_timeout kills a hung command and returns 137", () => { + const r = spawnSync( + "bash", + [ + "-c", + `. "${repoRoot}/scripts/lib.sh" && with_timeout 1 sleep 30; echo "rc=$?"`, + ], + { env: freshEnv(), encoding: "utf8", timeout: 10_000 }, + ); + assert.match(r.stdout, /rc=137/); }); -test( - "a live holder is never stolen from, even after the wait budget", - { timeout: 20_000 }, - async () => { - fs.rmSync(path.join(stateDir, "pane-id-w9"), { force: true }); - fs.rmSync(path.join(stateDir, "open-lock-w9"), { - recursive: true, - force: true, +test("a live holder is never stolen from, even after the wait budget", { + timeout: 20_000, +}, async () => { + fs.rmSync(path.join(stateDir, "pane-id-w9"), { force: true }); + fs.rmSync(path.join(stateDir, "open-lock-w9"), { + recursive: true, + force: true, + }); + const lock = path.join(stateDir, "open-lock-w9"); + fs.mkdirSync(lock, { recursive: true }); + const holder = spawn("sleep", ["3"], { stdio: "ignore" }); + fs.writeFileSync(path.join(lock, "pid"), `${holder.pid}\n`); + const started = Date.now(); + const env = freshEnv({ HERDR_BROWSER_LOCK_TRIES: "5" }); + const result = await new Promise((resolve) => { + const child = spawn("bash", [path.join(repoRoot, "scripts", "open.sh")], { + env, }); - const lock = path.join(stateDir, "open-lock-w9"); - fs.mkdirSync(lock, { recursive: true }); - const holder = spawn("sleep", ["3"], { stdio: "ignore" }); - fs.writeFileSync(path.join(lock, "pid"), `${holder.pid}\n`); - const started = Date.now(); - const env = freshEnv({ HERDR_BROWSER_LOCK_TRIES: "5" }); - const result = await new Promise((resolve) => { - const child = spawn( - "bash", - [path.join(repoRoot, "scripts", "open.sh")], - { env }, - ); - let stderr = ""; - child.stderr.on("data", (chunk) => { - stderr += chunk; - }); - child.on("close", (status) => resolve({ status, stderr })); + let stderr = ""; + child.stderr.on("data", (chunk) => { + stderr += chunk; }); - holder.kill(); - assert.equal(result.status, 0, result.stderr); - assert.ok( - Date.now() - started > 2_500, - "waiter waited for the live holder instead of stealing", - ); - assert.equal(fs.existsSync(lock), false, "lock released after holder exit"); - }, -); + child.on("close", (status) => resolve({ status, stderr })); + }); + holder.kill(); + assert.equal(result.status, 0, result.stderr); + assert.ok( + Date.now() - started > 2_500, + "waiter waited for the live holder instead of stealing", + ); + assert.equal(fs.existsSync(lock), false, "lock released after holder exit"); +}); From e9e9faeaccc3e45dc01521a85c00c13263825835 Mon Sep 17 00:00:00 2001 From: Victor Garcia Date: Mon, 27 Jul 2026 17:14:39 -0600 Subject: [PATCH 02/19] ci(browser): add minimal validation workflow Run the existing Node test suite and shell syntax checks on Ubuntu with Node 20 using immutable official action pins. Add focused manifest validation for TOML parsing, version parity, entrypoint existence and containment, and executable script modes. --- .github/workflows/ci.yml | 28 +++++++ scripts/check-manifest.mjs | 157 +++++++++++++++++++++++++++++++++++++ scripts/lib.sh | 0 scripts/record.sh | 0 tests/manifest.test.mjs | 87 ++++++++++++++++++++ 5 files changed, 272 insertions(+) create mode 100644 .github/workflows/ci.yml create mode 100755 scripts/check-manifest.mjs mode change 100644 => 100755 scripts/lib.sh mode change 100644 => 100755 scripts/record.sh create mode 100644 tests/manifest.test.mjs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..2167fe5 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,28 @@ +name: CI + +on: + pull_request: + push: + +permissions: + contents: read + +jobs: + validate: + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - name: Check out repository + # v4.2.2 + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 + - name: Set up Node.js + # v4.0.3 + uses: actions/setup-node@1e60f620b9541d16bece96c5465dc8ee9832be0b + with: + node-version: 20 + - name: Run tests + run: npm test + - name: Validate shell syntax + run: bash -n scripts/*.sh + - name: Validate plugin manifest + run: node scripts/check-manifest.mjs diff --git a/scripts/check-manifest.mjs b/scripts/check-manifest.mjs new file mode 100755 index 0000000..ad014e6 --- /dev/null +++ b/scripts/check-manifest.mjs @@ -0,0 +1,157 @@ +#!/usr/bin/env node +import { spawnSync } from "node:child_process"; +import fs from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +const TOML_TO_JSON = ` +import json +import sys +import tomllib + +with open(sys.argv[1], "rb") as manifest: + json.dump(tomllib.load(manifest), sys.stdout) +`; + +function parseManifest(manifestPath) { + const result = spawnSync("python3", ["-c", TOML_TO_JSON, manifestPath], { + encoding: "utf8", + }); + if (result.error) { + throw new Error( + `could not run python3 to parse the manifest: ${result.error.message}`, + ); + } + if (result.status !== 0) { + throw new Error(`manifest is not valid TOML: ${result.stderr.trim()}`); + } + return JSON.parse(result.stdout); +} + +function manifestCommands(manifest, errors) { + const commands = []; + for (const section of ["build", "startup", "actions", "panes", "events"]) { + const entries = manifest[section] ?? []; + if (!Array.isArray(entries)) { + errors.push(`manifest section ${section} must be an array`); + continue; + } + for (const [index, entry] of entries.entries()) { + if (!Array.isArray(entry.command) || entry.command.length === 0) { + errors.push( + `${section}[${index}] must declare a non-empty command array`, + ); + continue; + } + commands.push({ label: `${section}[${index}]`, command: entry.command }); + } + } + return commands; +} + +function commandEntrypoint(command) { + if (["bash", "node", "sh"].includes(command[0])) return command[1]; + if (typeof command[0] === "string" && command[0].includes("/")) { + return command[0]; + } + return null; +} + +function isContained(root, target) { + const relative = path.relative(root, target); + return ( + relative !== ".." && + !relative.startsWith(`..${path.sep}`) && + !path.isAbsolute(relative) + ); +} + +export function validateRepository(root) { + const errors = []; + let packageJson; + let manifest; + + try { + packageJson = JSON.parse( + fs.readFileSync(path.join(root, "package.json"), "utf8"), + ); + } catch (error) { + errors.push(`package.json could not be parsed: ${error.message}`); + } + + try { + manifest = parseManifest(path.join(root, "herdr-plugin.toml")); + } catch (error) { + errors.push(error.message); + } + + if (!packageJson || !manifest) { + return { errors, entrypointCount: 0, scriptCount: 0 }; + } + + if (typeof manifest.version !== "string") { + errors.push("manifest version must be a string"); + } else if (packageJson.version !== manifest.version) { + errors.push( + `version mismatch: package.json=${packageJson.version} herdr-plugin.toml=${manifest.version}`, + ); + } + + const commands = manifestCommands(manifest, errors); + const resolvedRoot = fs.realpathSync(root); + let entrypointCount = 0; + for (const { label, command } of commands) { + const entrypoint = commandEntrypoint(command); + if (!entrypoint) continue; + entrypointCount += 1; + if (typeof entrypoint !== "string") { + errors.push(`${label} entrypoint must be a string`); + continue; + } + const target = path.resolve(resolvedRoot, entrypoint); + if (!isContained(resolvedRoot, target)) { + errors.push(`${label} entrypoint escapes the repository: ${entrypoint}`); + continue; + } + if (!fs.existsSync(target) || !fs.statSync(target).isFile()) { + errors.push(`${label} entrypoint does not exist: ${entrypoint}`); + continue; + } + if (!isContained(resolvedRoot, fs.realpathSync(target))) { + errors.push(`${label} entrypoint escapes the repository: ${entrypoint}`); + } + } + + const scriptsDir = path.join(resolvedRoot, "scripts"); + let scripts = []; + try { + scripts = fs + .readdirSync(scriptsDir) + .filter((name) => fs.statSync(path.join(scriptsDir, name)).isFile()); + } catch (error) { + errors.push(`scripts directory could not be read: ${error.message}`); + } + if (scripts.length === 0) errors.push("no scripts found in scripts/"); + for (const script of scripts) { + const mode = fs.statSync(path.join(scriptsDir, script)).mode; + if ((mode & 0o111) === 0) { + errors.push(`script is not executable: scripts/${script}`); + } + } + + return { errors, entrypointCount, scriptCount: scripts.length }; +} + +const sourcePath = fileURLToPath(import.meta.url); +if (process.argv[1] && path.resolve(process.argv[1]) === sourcePath) { + const root = path.resolve(path.dirname(sourcePath), ".."); + const result = validateRepository(root); + if (result.errors.length > 0) { + for (const error of result.errors) console.error(`error: ${error}`); + process.exitCode = 1; + } else { + console.log( + `Manifest valid: versions match, ${result.entrypointCount} entrypoints exist, ${result.scriptCount} scripts are executable.`, + ); + } +} diff --git a/scripts/lib.sh b/scripts/lib.sh old mode 100644 new mode 100755 diff --git a/scripts/record.sh b/scripts/record.sh old mode 100644 new mode 100755 diff --git a/tests/manifest.test.mjs b/tests/manifest.test.mjs new file mode 100644 index 0000000..ecc3096 --- /dev/null +++ b/tests/manifest.test.mjs @@ -0,0 +1,87 @@ +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import test from "node:test"; +import { fileURLToPath } from "node:url"; +import { validateRepository } from "../scripts/check-manifest.mjs"; + +const root = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); + +function fixture(t) { + const base = fs.mkdtempSync(path.join(os.tmpdir(), "herdr-browser-manifest-")); + const dir = path.join(base, "repository"); + fs.mkdirSync(path.join(dir, "scripts"), { recursive: true }); + fs.writeFileSync( + path.join(dir, "package.json"), + JSON.stringify({ version: "1.2.3" }), + ); + fs.writeFileSync( + path.join(dir, "herdr-plugin.toml"), + 'version = "1.2.3"\n[[actions]]\nid = "open"\ncommand = ["bash", "scripts/open.sh"]\n', + ); + fs.writeFileSync(path.join(dir, "scripts", "open.sh"), "#!/bin/sh\n", { + mode: 0o755, + }); + t.after(() => fs.rmSync(base, { recursive: true, force: true })); + return { base, dir }; +} + +test("repository manifest passes CI validation", () => { + assert.deepEqual(validateRepository(root).errors, []); +}); + +test("manifest validation reports version, entrypoint, and executable-bit failures", (t) => { + const { dir } = fixture(t); + fs.writeFileSync( + path.join(dir, "package.json"), + JSON.stringify({ version: "9.9.9" }), + ); + fs.rmSync(path.join(dir, "scripts", "open.sh")); + fs.writeFileSync(path.join(dir, "scripts", "other.sh"), "#!/bin/sh\n", { + mode: 0o644, + }); + + const { errors } = validateRepository(dir); + assert.ok(errors.some((error) => error.startsWith("version mismatch:"))); + assert.ok( + errors.some((error) => error.includes("entrypoint does not exist")), + ); + assert.ok(errors.some((error) => error.includes("script is not executable"))); +}); + +test("manifest validation rejects lexical and symlink repository escapes", (t) => { + const { base, dir } = fixture(t); + const outside = path.join(base, "outside.sh"); + fs.writeFileSync(outside, "#!/bin/sh\n", { mode: 0o755 }); + fs.symlinkSync(outside, path.join(dir, "scripts", "linked.sh")); + fs.writeFileSync( + path.join(dir, "herdr-plugin.toml"), + [ + 'version = "1.2.3"', + "[[actions]]", + 'id = "lexical-escape"', + 'command = ["bash", "../outside.sh"]', + "[[actions]]", + 'id = "symlink-escape"', + 'command = ["bash", "scripts/linked.sh"]', + "", + ].join("\n"), + ); + + const escapes = validateRepository(dir).errors.filter((error) => + error.includes("entrypoint escapes the repository"), + ); + assert.equal(escapes.length, 2); +}); + +test("manifest validation reports invalid TOML", (t) => { + const { dir } = fixture(t); + fs.writeFileSync(path.join(dir, "herdr-plugin.toml"), 'version = "unterminated\n'); + + assert.ok( + validateRepository(dir).errors.some((error) => + error.startsWith("manifest is not valid TOML:"), + ), + ); +}); From fad75b4a746bc72c25640e5344adda75f155a71a Mon Sep 17 00:00:00 2001 From: Victor Garcia Date: Mon, 27 Jul 2026 17:16:09 -0600 Subject: [PATCH 03/19] [pi] Implemented and committed Browser CI. --- tests/manifest.test.mjs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/manifest.test.mjs b/tests/manifest.test.mjs index ecc3096..a61f382 100644 --- a/tests/manifest.test.mjs +++ b/tests/manifest.test.mjs @@ -9,7 +9,9 @@ import { validateRepository } from "../scripts/check-manifest.mjs"; const root = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); function fixture(t) { - const base = fs.mkdtempSync(path.join(os.tmpdir(), "herdr-browser-manifest-")); + const base = fs.mkdtempSync( + path.join(os.tmpdir(), "herdr-browser-manifest-"), + ); const dir = path.join(base, "repository"); fs.mkdirSync(path.join(dir, "scripts"), { recursive: true }); fs.writeFileSync( @@ -77,7 +79,10 @@ test("manifest validation rejects lexical and symlink repository escapes", (t) = test("manifest validation reports invalid TOML", (t) => { const { dir } = fixture(t); - fs.writeFileSync(path.join(dir, "herdr-plugin.toml"), 'version = "unterminated\n'); + fs.writeFileSync( + path.join(dir, "herdr-plugin.toml"), + 'version = "unterminated\n', + ); assert.ok( validateRepository(dir).errors.some((error) => From d78c2b061960151fc028894c99c626c79bd5710e Mon Sep 17 00:00:00 2001 From: Victor Garcia Date: Tue, 28 Jul 2026 01:59:19 -0600 Subject: [PATCH 04/19] feat(recording): add run-scoped observation bundles Replace session-derived recording paths with a private, run-keyed state machine that pins the start session through stop. Validate artifacts before atomically completing metadata with byte size and SHA-256, while retaining failed stops for retry and leaving legacy recordings untouched.\n\nAdd focused containment, traversal, duplicate-start, environment drift, special-file, oversized-file, failure, and integrity tests. Document the observational non-attestation boundary and release Browser 0.6.0 without changing action IDs or adding dependencies. --- README.md | 29 ++++- bin/record.mjs | 242 +++++++++++++++++++++++++++++++++++++++ herdr-plugin.toml | 2 +- package.json | 2 +- scripts/record.sh | 41 +++---- tests/launchers.test.mjs | 16 --- tests/manifest.test.mjs | 11 ++ tests/record.test.mjs | 223 ++++++++++++++++++++++++++++++++++++ 8 files changed, 519 insertions(+), 47 deletions(-) create mode 100644 bin/record.mjs create mode 100644 tests/record.test.mjs diff --git a/README.md b/README.md index b2b890b..5252213 100644 --- a/README.md +++ b/README.md @@ -229,16 +229,34 @@ echo "my-agent-session" \ ## Recording -Start and stop recording through the two recording actions. Files are written -to: +Start and stop recording through the existing recording actions. Each new +capture is a run-scoped observation bundle: ```text -/recordings/herdr-ws--YYYYMMDD-HHMMSS.webm +/runs/run-/browser/ + evidence.json + recording.webm ``` +Set `HERDR_BROWSER_RUN_ID`, or put a run ID on the first line of +`/run-id`. IDs must be 1–128 ASCII letters, digits, dots, +underscores, or hyphens and must start with a letter or digit. When neither is +set, Browser generates an ID. One active recording is allowed per workspace; +Stop always uses the run and browser session pinned by Start, even if the +current environment changed. + +On successful Stop, `evidence.json` records the WebM byte count and SHA-256. +The bundle is an **unreviewed, operator-reviewable observation**, not a test +result, acceptance decision, provenance claim, or cryptographic attestation. +Its digest detects later content changes but does not identify who recorded or +reviewed it. A missing, empty, non-regular, symlinked, or oversized WebM is not +marked complete. Failed stops retain the active pointer so Stop can be retried. + Starting a recording creates a fresh browser context: the page reloads, while cookies and localStorage are preserved. Start recording before the flow you -want to capture. Recordings persist until you delete them. +want to capture. Bundles persist until you delete them. Files created by 0.5 +under `recordings/*.webm` remain untouched as legacy, unscoped recordings and +are not relabeled or migrated. ## Configuration @@ -247,6 +265,7 @@ Plugin config files contain one value on their first line: | File | Values | Default | Purpose | | --- | --- | --- | --- | | `session` | Session name | `herdr-ws-` | Watch a different agent-browser session | +| `run-id` | Valid run ID | Generated | Correlate a recording bundle with an external run | | `render` | `kitty`, `symbols`, `text` | Automatic probe | Force a rendering mode | Equivalent environment controls: @@ -254,6 +273,7 @@ Equivalent environment controls: | Variable | Default | Purpose | | --- | --- | --- | | `HERDR_BROWSER_SESSION` | Workspace session | Override the watched session | +| `HERDR_BROWSER_RUN_ID` | Config or generated ID | Select the recording run ID | | `HERDR_BROWSER_RENDER` | Automatic probe | Override the rendering mode | | `HERDR_BROWSER_INTERVAL_MS` | `1000` | Polling interval; clamped to safe bounds | | `AGENT_BROWSER_IDLE_TIMEOUT_MS` | `1800000` | Idle timeout for plugin-created browser daemons | @@ -267,6 +287,7 @@ Environment variables take precedence over config files. - Workspace identifiers are sanitized before they are used in state paths. - Polling frames are cached as PNG; streamed frames are cached as JPEG. Frame files are mode `0600` and removed when the pane exits. +- Recording bundles use private directories (`0700`) and files (`0600`), and session names are metadata only—never path components. - WebM recordings are intentionally retained under the plugin state directory. - Browser sessions are a trusted local boundary: any local process that knows a session name can drive it, including authenticated pages. diff --git a/bin/record.mjs b/bin/record.mjs new file mode 100644 index 0000000..3bd841a --- /dev/null +++ b/bin/record.mjs @@ -0,0 +1,242 @@ +#!/usr/bin/env node +import { spawnSync } from "node:child_process"; +import crypto from "node:crypto"; +import fs from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +const PLUGIN_VERSION = "0.6.0"; +const RUN_ID_RE = /^[A-Za-z0-9][A-Za-z0-9._-]{0,127}$/; +const JSON_LIMIT = 64 * 1024; +export const MAX_RECORDING_BYTES = 10 * 1024 * 1024 * 1024; + +export function validateRunId(value) { + if (typeof value !== "string" || !RUN_ID_RE.test(value)) { + throw new Error("run ID must be 1-128 ASCII letters, digits, dots, underscores, or hyphens and start with a letter or digit"); + } + return value; +} + +export function isContained(root, target) { + const relative = path.relative(root, target); + return relative !== ".." && !relative.startsWith(`..${path.sep}`) && !path.isAbsolute(relative); +} + +function safeError(error) { + return String(error?.message ?? error).replace(/[\u0000-\u001f\u007f]/g, " ").slice(0, 512); +} + +function ensureDirectory(dir) { + fs.mkdirSync(dir, { recursive: true, mode: 0o700 }); + const stat = fs.lstatSync(dir); + if (!stat.isDirectory() || stat.isSymbolicLink()) throw new Error(`unsafe state directory: ${dir}`); + fs.chmodSync(dir, 0o700); +} + +function canonicalState(rawState) { + if (!path.isAbsolute(rawState)) throw new Error("plugin state directory must be absolute"); + ensureDirectory(rawState); + const root = fs.realpathSync(rawState); + if (!path.isAbsolute(root)) throw new Error("could not canonicalize plugin state directory"); + return root; +} + +function assertSafeTarget(root, target, kind = "file") { + if (!isContained(root, target)) throw new Error(`${kind} escapes plugin state`); + const parent = fs.realpathSync(path.dirname(target)); + if (!isContained(root, parent) && parent !== root) throw new Error(`${kind} parent escapes plugin state`); + if (fs.existsSync(target)) { + const stat = fs.lstatSync(target); + if (stat.isSymbolicLink()) throw new Error(`${kind} must not be a symlink`); + if (kind.includes("JSON") && !stat.isFile()) throw new Error(`${kind} must be a regular file`); + } +} + +export function atomicWriteJson(root, target, value) { + assertSafeTarget(root, target, "JSON file"); + const data = `${JSON.stringify(value, null, 2)}\n`; + if (Buffer.byteLength(data) > JSON_LIMIT) throw new Error("JSON document is too large"); + const temp = path.join(path.dirname(target), `.${path.basename(target)}.${process.pid}.${crypto.randomBytes(6).toString("hex")}.tmp`); + assertSafeTarget(root, temp, "temporary JSON file"); + let fd; + try { + fd = fs.openSync(temp, "wx", 0o600); + fs.writeFileSync(fd, data, "utf8"); + fs.fsyncSync(fd); + fs.closeSync(fd); + fd = undefined; + fs.renameSync(temp, target); + fs.chmodSync(target, 0o600); + const dirFd = fs.openSync(path.dirname(target), "r"); + try { fs.fsyncSync(dirFd); } finally { fs.closeSync(dirFd); } + } finally { + if (fd !== undefined) fs.closeSync(fd); + try { fs.unlinkSync(temp); } catch (error) { if (error.code !== "ENOENT") throw error; } + } +} + +function readJsonFile(root, target, label) { + assertSafeTarget(root, target, label); + const stat = fs.lstatSync(target); + if (!stat.isFile() || stat.size > JSON_LIMIT) throw new Error(`${label} must be a bounded regular file`); + return JSON.parse(fs.readFileSync(target, "utf8")); +} + +function runIdFromEnvironment() { + if (process.env.HERDR_BROWSER_RUN_ID !== undefined && process.env.HERDR_BROWSER_RUN_ID !== "") { + return validateRunId(process.env.HERDR_BROWSER_RUN_ID); + } + const configDir = process.env.HERDR_PLUGIN_CONFIG_DIR; + if (configDir) { + const source = path.join(configDir, "run-id"); + if (fs.existsSync(source)) { + const stat = fs.lstatSync(source); + if (!stat.isFile() || stat.isSymbolicLink() || stat.size > 1024) throw new Error("run-id config must be a bounded regular file"); + const firstLine = fs.readFileSync(source, "utf8").split(/\r?\n/, 1)[0]; + if (firstLine) return validateRunId(firstLine); + } + } + const stamp = new Date().toISOString().replace(/[-:]/g, "").replace(/\.\d{3}Z$/, "Z"); + return `browser-${stamp}-${crypto.randomBytes(4).toString("hex")}`; +} + +export function buildManifest({ runId, workspaceId, session, startedAt = new Date().toISOString() }) { + return { + schema_version: 1, + evidence_type: "browser-recording", + run_id: runId, + plugin: { id: "structupath.browser", version: PLUGIN_VERSION }, + workspace_id: workspaceId, + browser_session: session, + status: "recording", + started_at: startedAt, + completed_at: null, + recording_context_reset: true, + artifact: { path: "recording.webm", media_type: "video/webm", bytes: null, sha256: null }, + review: { status: "unreviewed", required: true, attestation: "none" }, + error: null, + }; +} + +function engine(session, args) { + const result = spawnSync("agent-browser", ["--session", session, "record", ...args], { + stdio: ["ignore", "ignore", "pipe"], encoding: "utf8", timeout: 15_000, killSignal: "SIGKILL", + }); + if (result.error || result.status !== 0) { + throw new Error(result.error?.code === "ETIMEDOUT" ? "agent-browser timed out" : result.stderr?.trim() || `agent-browser exited ${result.status}`); + } +} + +function pathsFor(root, workspaceId, runId) { + const runs = path.join(root, "runs"); + ensureDirectory(runs); + const bundle = path.join(runs, `run-${runId}`, "browser"); + const pointer = path.join(runs, `active-${workspaceId}.json`); + return { runs, bundle, manifest: path.join(bundle, "evidence.json"), artifact: path.join(bundle, "recording.webm"), pointer }; +} + +function withWorkspaceLock(root, workspaceId, callback) { + const lock = path.join(root, "runs", `.record-${workspaceId}.lock`); + assertSafeTarget(root, lock, "recording lock"); + try { fs.mkdirSync(lock, { mode: 0o700 }); } catch (error) { + if (error.code === "EEXIST") throw new Error("another recording action is in progress for this workspace"); + throw error; + } + try { return callback(); } finally { fs.rmdirSync(lock); } +} + +export function sha256File(file) { + const hash = crypto.createHash("sha256"); + const fd = fs.openSync(file, "r"); + const buffer = Buffer.allocUnsafe(1024 * 1024); + try { + for (;;) { const count = fs.readSync(fd, buffer, 0, buffer.length, null); if (count === 0) break; hash.update(buffer.subarray(0, count)); } + } finally { fs.closeSync(fd); } + return hash.digest("hex"); +} + +function context() { + const root = canonicalState(process.env.HERDR_BROWSER_STATE_DIR ?? ""); + const workspaceId = process.env.HERDR_BROWSER_WORKSPACE_ID ?? ""; + if (!/^[A-Za-z0-9_-]+$/.test(workspaceId)) throw new Error("invalid workspace ID"); + const session = process.env.HERDR_BROWSER_SESSION_PINNED ?? ""; + if (!session) throw new Error("browser session is required"); + return { root, workspaceId, session }; +} + +function start() { + const { root, workspaceId, session } = context(); + const runId = runIdFromEnvironment(); + const paths = pathsFor(root, workspaceId, runId); + return withWorkspaceLock(root, workspaceId, () => { + if (fs.existsSync(paths.pointer)) throw new Error("a recording is already active for this workspace"); + if (fs.existsSync(path.join(paths.runs, `run-${runId}`))) throw new Error(`recording bundle already exists for run ${runId}`); + ensureDirectory(path.dirname(paths.bundle)); + ensureDirectory(paths.bundle); + const manifest = buildManifest({ runId, workspaceId, session }); + try { + engine(session, ["start", paths.artifact]); + atomicWriteJson(root, paths.manifest, manifest); + atomicWriteJson(root, paths.pointer, { schema_version: 1, run_id: runId, workspace_id: workspaceId, browser_session: session, engine_stopped: false }); + } catch (error) { + manifest.status = "failed"; + manifest.error = safeError(error); + atomicWriteJson(root, paths.manifest, manifest); + throw error; + } + console.log(`herdr-browser: recording run_id=${runId} manifest=${paths.manifest}`); + }); +} + +function stop() { + const { root, workspaceId } = context(); + return withWorkspaceLock(root, workspaceId, () => { + const pointerPath = path.join(root, "runs", `active-${workspaceId}.json`); + if (!fs.existsSync(pointerPath)) throw new Error("no recording is active for this workspace"); + const pointer = readJsonFile(root, pointerPath, "active recording pointer"); + validateRunId(pointer.run_id); + if (pointer.workspace_id !== workspaceId || typeof pointer.browser_session !== "string" || !pointer.browser_session || typeof pointer.engine_stopped !== "boolean") throw new Error("active recording pointer is invalid"); + const paths = pathsFor(root, workspaceId, pointer.run_id); + const manifest = readJsonFile(root, paths.manifest, "evidence manifest"); + if (manifest.status !== "recording" || manifest.run_id !== pointer.run_id || manifest.browser_session !== pointer.browser_session) throw new Error("active recording state is inconsistent"); + try { + if (!pointer.engine_stopped) { + engine(pointer.browser_session, ["stop"]); + pointer.engine_stopped = true; + atomicWriteJson(root, paths.pointer, pointer); + } + assertSafeTarget(root, paths.artifact, "recording artifact"); + const stat = fs.lstatSync(paths.artifact); + if (!stat.isFile() || stat.isSymbolicLink()) throw new Error("recording artifact must be a regular file"); + if (stat.size === 0) throw new Error("recording artifact is empty"); + if (stat.size > MAX_RECORDING_BYTES) throw new Error("recording artifact exceeds the 10 GiB limit"); + fs.chmodSync(paths.artifact, 0o600); + manifest.status = "complete"; + manifest.completed_at = new Date().toISOString(); + manifest.artifact.bytes = stat.size; + manifest.artifact.sha256 = sha256File(paths.artifact); + manifest.error = null; + atomicWriteJson(root, paths.manifest, manifest); + fs.unlinkSync(paths.pointer); + } catch (error) { + manifest.status = "recording"; + manifest.completed_at = null; + manifest.artifact.bytes = null; + manifest.artifact.sha256 = null; + manifest.error = safeError(error); + atomicWriteJson(root, paths.manifest, manifest); + throw error; + } + console.log(`herdr-browser: completed run_id=${pointer.run_id} manifest=${paths.manifest}`); + }); +} + +export function main(mode = process.argv[2]) { + if (mode === "start") return start(); + if (mode === "stop") return stop(); + throw new Error("usage: record.mjs start|stop"); +} + +if (process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)) { + try { main(); } catch (error) { console.error(`herdr-browser: ${safeError(error)}`); process.exitCode = 3; } +} diff --git a/herdr-plugin.toml b/herdr-plugin.toml index 14518ab..a6b30ec 100644 --- a/herdr-plugin.toml +++ b/herdr-plugin.toml @@ -1,6 +1,6 @@ id = "structupath.browser" name = "Browser" -version = "0.5.0" +version = "0.6.0" min_herdr_version = "0.7.0" description = "Driveable browser pane: live screenshots, console output, and localhost link handling via agent-browser" platforms = ["macos", "linux"] diff --git a/package.json b/package.json index 1fec3ea..fdd1261 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "herdr-browser", - "version": "0.5.0", + "version": "0.6.0", "private": true, "type": "module", "engines": { "node": ">=20" }, diff --git a/scripts/record.sh b/scripts/record.sh index 63f32c4..fb4b840 100755 --- a/scripts/record.sh +++ b/scripts/record.sh @@ -1,36 +1,27 @@ #!/usr/bin/env bash -# Record the workspace browser session to a WebM video (agent-browser record). -# Note: record start creates a fresh browser context (the page reloads; -# cookies and localStorage are preserved) — that is the engine's trade-off -# for capturing video, so start recording before the flow you want to show. +# Create or complete a run-scoped WebM observation bundle. +# Recording resets the browser context (the page reloads; cookies and +# localStorage are preserved), so start before the flow to capture. set -uo pipefail cd "${HERDR_PLUGIN_ROOT:-$(dirname "$0")/..}" || exit 1 . scripts/lib.sh -require_agent_browser - -session="$(session_name)" -dir="$(state_dir)/recordings" -mkdir -p "$dir" - case "${1:-}" in -start) - f="$dir/$session-$(date +%Y%m%d-%H%M%S).webm" - if ! with_timeout 15 agent-browser --session "$session" record start "$f" >/dev/null; then - echo "herdr-browser: failed to start recording (is the session running?)" >&2 - exit 3 - fi - echo "herdr-browser: recording to $f" - ;; -stop) - if ! with_timeout 15 agent-browser --session "$session" record stop >/dev/null; then - echo "herdr-browser: failed to stop recording" >&2 - exit 3 - fi - echo "herdr-browser: recording saved under $dir" - ;; +start | stop) ;; *) echo "usage: record.sh start|stop" >&2 exit 2 ;; esac + +require_agent_browser +if ! command -v node >/dev/null 2>&1; then + echo "herdr-browser: node is required." >&2 + exit 1 +fi + +HERDR_BROWSER_STATE_DIR="$(state_dir)" +HERDR_BROWSER_WORKSPACE_ID="$(ws_id)" +HERDR_BROWSER_SESSION_PINNED="$(session_name)" +export HERDR_BROWSER_STATE_DIR HERDR_BROWSER_WORKSPACE_ID HERDR_BROWSER_SESSION_PINNED +exec node bin/record.mjs "$1" diff --git a/tests/launchers.test.mjs b/tests/launchers.test.mjs index 7110184..33060f8 100644 --- a/tests/launchers.test.mjs +++ b/tests/launchers.test.mjs @@ -536,22 +536,6 @@ test("browse-pane clamps zoom to 25..500 with 100 as the garbage default", () => assert.match(run("5\n"), /--zoom=25/); }); -// --- Wave 2c: record action --- - -test("record start/stop drive the workspace session and name the file", () => { - const start = runScript("record.sh", ["start"]); - assert.equal(start.status, 0, start.stderr); - assert.match( - log(), - /agent-browser --session herdr-ws-w9 record start .*\/recordings\/herdr-ws-w9-.*\.webm/, - ); - assert.match(start.stdout, /recording to .*\.webm/); - const stop = runScript("record.sh", ["stop"]); - assert.equal(stop.status, 0, stop.stderr); - assert.match(log(), /agent-browser --session herdr-ws-w9 record stop/); - assert.equal(runScript("record.sh", ["bogus"]).status, 2); -}); - test("with_timeout kills a hung command and returns 137", () => { const r = spawnSync( "bash", diff --git a/tests/manifest.test.mjs b/tests/manifest.test.mjs index a61f382..a448154 100644 --- a/tests/manifest.test.mjs +++ b/tests/manifest.test.mjs @@ -33,6 +33,17 @@ test("repository manifest passes CI validation", () => { assert.deepEqual(validateRepository(root).errors, []); }); +test("release version and existing action IDs remain stable", () => { + const packageJson = JSON.parse(fs.readFileSync(path.join(root, "package.json"), "utf8")); + const manifest = fs.readFileSync(path.join(root, "herdr-plugin.toml"), "utf8"); + assert.equal(packageJson.version, "0.6.0"); + assert.match(manifest, /^version = "0\.6\.0"$/m); + assert.deepEqual( + [...manifest.matchAll(/^id = "([^"]+)"$/gm)].slice(1, 6).map((match) => match[1]), + ["open", "close", "browse", "record-start", "record-stop"], + ); +}); + test("manifest validation reports version, entrypoint, and executable-bit failures", (t) => { const { dir } = fixture(t); fs.writeFileSync( diff --git a/tests/record.test.mjs b/tests/record.test.mjs new file mode 100644 index 0000000..b78d4d7 --- /dev/null +++ b/tests/record.test.mjs @@ -0,0 +1,223 @@ +import assert from "node:assert/strict"; +import crypto from "node:crypto"; +import { spawnSync } from "node:child_process"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import test from "node:test"; +import { fileURLToPath } from "node:url"; +import { atomicWriteJson, MAX_RECORDING_BYTES, validateRunId } from "../bin/record.mjs"; + +const root = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); + +function fixture(t, overrides = {}) { + const base = fs.mkdtempSync(path.join(os.tmpdir(), "herdr-browser-record-")); + const state = path.join(base, "state"); + const bin = path.join(base, "bin"); + const config = path.join(base, "config"); + const calls = path.join(base, "calls"); + fs.mkdirSync(bin); + fs.mkdirSync(config); + fs.writeFileSync(path.join(bin, "agent-browser"), `#!/usr/bin/env bash +printf '%s\\n' "$*" >> "$AB_CALLS" +if [ "$4" = "start" ]; then + [ "${"$"}{AB_START_FAIL:-0}" = 1 ] && exit 9 + case "${"$"}{AB_ARTIFACT_MODE:-data}" in + data) printf 'webm-test-data' > "$5" ;; + empty) : > "$5" ;; + missing) ;; + esac +else + [ "${"$"}{AB_STOP_FAIL:-0}" = 1 ] && exit 8 +fi +exit 0 +`, { mode: 0o755 }); + const env = { + ...process.env, + PATH: `${bin}:${path.dirname(process.execPath)}:/usr/bin:/bin`, + HERDR_PLUGIN_ROOT: root, + HERDR_PLUGIN_STATE_DIR: state, + HERDR_PLUGIN_CONFIG_DIR: config, + HERDR_WORKSPACE_ID: "workspace_1", + HERDR_BROWSER_RUN_ID: "suite-run-1", + HERDR_BROWSER_SESSION: "browser-session", + AB_CALLS: calls, + ...overrides, + }; + t.after(() => fs.rmSync(base, { recursive: true, force: true })); + return { base, state, config, calls, env }; +} + +function invoke(f, mode, overrides = {}) { + return spawnSync("bash", [path.join(root, "scripts/record.sh"), mode], { + env: { ...f.env, ...overrides }, encoding: "utf8", + }); +} + +function bundle(f, runId = "suite-run-1") { + const dir = path.join(f.state, "runs", `run-${runId}`, "browser"); + return { + dir, + manifest: path.join(dir, "evidence.json"), + artifact: path.join(dir, "recording.webm"), + pointer: path.join(f.state, "runs", "active-workspace_1.json"), + }; +} + +function json(file) { return JSON.parse(fs.readFileSync(file, "utf8")); } + +test("explicit run creates a private contained recording bundle and complete digest", (t) => { + const f = fixture(t); + const legacy = path.join(f.state, "recordings", "legacy.webm"); + fs.mkdirSync(path.dirname(legacy), { recursive: true }); + fs.writeFileSync(legacy, "legacy"); + const start = invoke(f, "start"); + assert.equal(start.status, 0, start.stderr); + const b = bundle(f); + const recording = json(b.manifest); + assert.equal(recording.status, "recording"); + assert.equal(recording.review.status, "unreviewed"); + assert.equal(recording.review.attestation, "none"); + assert.equal(recording.artifact.path, "recording.webm"); + assert.ok(fs.existsSync(b.pointer)); + assert.match(fs.readFileSync(f.calls, "utf8"), /--session browser-session record start .*recording\.webm/); + assert.equal(fs.statSync(b.dir).mode & 0o777, 0o700); + assert.equal(fs.statSync(b.manifest).mode & 0o777, 0o600); + assert.equal(fs.statSync(b.pointer).mode & 0o777, 0o600); + const stop = invoke(f, "stop"); + assert.equal(stop.status, 0, stop.stderr); + const complete = json(b.manifest); + assert.equal(complete.status, "complete"); + assert.equal(complete.artifact.bytes, 14); + assert.equal(complete.artifact.sha256, crypto.createHash("sha256").update("webm-test-data").digest("hex")); + assert.equal(complete.review.attestation, "none"); + assert.equal(fs.existsSync(b.pointer), false); + assert.equal(fs.statSync(b.artifact).mode & 0o777, 0o600); + assert.equal(fs.readFileSync(legacy, "utf8"), "legacy"); +}); + +test("invalid, traversal, control, and oversized run IDs fail before the engine", (t) => { + for (const id of ["../escape", "bad/name", "bad\nname", `a${"x".repeat(128)}`]) assert.throws(() => validateRunId(id)); + const f = fixture(t, { HERDR_BROWSER_RUN_ID: "../../escape" }); + const result = invoke(f, "start"); + assert.equal(result.status, 3); + assert.equal(fs.existsSync(f.calls), false); + assert.equal(fs.existsSync(path.join(f.base, "escape")), false); +}); + +test("config run ID is used when the environment is absent", (t) => { + const f = fixture(t, { HERDR_BROWSER_RUN_ID: "" }); + fs.writeFileSync(path.join(f.config, "run-id"), "config-run.2\nignored\n"); + const result = invoke(f, "start"); + assert.equal(result.status, 0, result.stderr); + assert.ok(fs.existsSync(bundle(f, "config-run.2").manifest)); +}); + +test("generated run IDs are valid and distinct", (t) => { + const first = fixture(t, { HERDR_BROWSER_RUN_ID: "", HERDR_WORKSPACE_ID: "a" }); + const second = fixture(t, { HERDR_BROWSER_RUN_ID: "", HERDR_WORKSPACE_ID: "b" }); + assert.equal(invoke(first, "start").status, 0); + assert.equal(invoke(second, "start").status, 0); + const id1 = fs.readdirSync(path.join(first.state, "runs")).find((name) => name.startsWith("run-")).slice(4); + const id2 = fs.readdirSync(path.join(second.state, "runs")).find((name) => name.startsWith("run-")).slice(4); + assert.equal(validateRunId(id1), id1); + assert.notEqual(id1, id2); +}); + +test("session text never participates in paths and stop uses the pinned session", (t) => { + const f = fixture(t, { HERDR_BROWSER_SESSION: "../../escape" }); + assert.equal(invoke(f, "start").status, 0); + const b = bundle(f); + assert.equal(json(b.manifest).browser_session, "../../escape"); + const stopped = invoke(f, "stop", { HERDR_BROWSER_SESSION: "changed-session", HERDR_BROWSER_RUN_ID: "changed-run" }); + assert.equal(stopped.status, 0, stopped.stderr); + const calls = fs.readFileSync(f.calls, "utf8"); + assert.match(calls, /--session \.\.\/\.\.\/escape record stop/); + assert.doesNotMatch(calls, /changed-session/); + assert.equal(fs.existsSync(path.join(f.base, "escape")), false); +}); + +test("duplicate start refuses without overwriting the active state", (t) => { + const f = fixture(t); + assert.equal(invoke(f, "start").status, 0); + const b = bundle(f); + const before = fs.readFileSync(b.pointer, "utf8"); + const second = invoke(f, "start", { HERDR_BROWSER_RUN_ID: "suite-run-2" }); + assert.equal(second.status, 3); + assert.match(second.stderr, /already active/); + assert.equal(fs.readFileSync(b.pointer, "utf8"), before); + assert.equal(fs.existsSync(bundle(f, "suite-run-2").dir), false); +}); + +test("start failure records failed state but creates no active pointer", (t) => { + const f = fixture(t, { AB_START_FAIL: "1" }); + assert.equal(invoke(f, "start").status, 3); + const b = bundle(f); + assert.equal(json(b.manifest).status, "failed"); + assert.equal(fs.existsSync(b.pointer), false); +}); + +test("stop failure is retryable and never marks complete", (t) => { + const f = fixture(t); + assert.equal(invoke(f, "start").status, 0); + const b = bundle(f); + const failed = invoke(f, "stop", { AB_STOP_FAIL: "1" }); + assert.equal(failed.status, 3); + assert.ok(fs.existsSync(b.pointer)); + assert.equal(json(b.manifest).status, "recording"); + assert.match(json(b.manifest).error, /exited 8/); + assert.equal(invoke(f, "stop", { AB_STOP_FAIL: "0" }).status, 0); + assert.equal(json(b.manifest).status, "complete"); +}); + +test("missing, empty, special, and oversized artifacts cannot complete", async (t) => { + for (const [name, mutate, mode = "data"] of [ + ["missing", (b) => fs.rmSync(b.artifact), "data"], + ["empty", () => {}, "empty"], + ["directory", (b) => { fs.rmSync(b.artifact); fs.mkdirSync(b.artifact); }], + ["symlink", (b, f) => { fs.rmSync(b.artifact); fs.symlinkSync(path.join(f.base, "outside"), b.artifact); }], + ["oversized", (b) => fs.truncateSync(b.artifact, MAX_RECORDING_BYTES + 1)], + ]) { + await t.test(name, (st) => { + const f = fixture(st, { AB_ARTIFACT_MODE: mode }); + assert.equal(invoke(f, "start").status, 0); + const b = bundle(f); + mutate(b, f); + const result = invoke(f, "stop"); + assert.equal(result.status, 3); + assert.ok(fs.existsSync(b.pointer)); + assert.equal(json(b.manifest).status, "recording"); + if (name === "missing") { + fs.writeFileSync(b.artifact, "webm-after-delay"); + const retry = invoke(f, "stop", { AB_STOP_FAIL: "1" }); + assert.equal(retry.status, 0, retry.stderr); + assert.equal(json(b.manifest).status, "complete"); + } + }); + } +}); + +test("symlink and oversized control files are refused", (t) => { + const f = fixture(t); + assert.equal(invoke(f, "start").status, 0); + const b = bundle(f); + fs.rmSync(b.pointer); + fs.writeFileSync(path.join(f.base, "outside-pointer"), "{}"); + fs.symlinkSync(path.join(f.base, "outside-pointer"), b.pointer); + assert.equal(invoke(f, "stop").status, 3); + fs.rmSync(b.pointer); + fs.writeFileSync(b.pointer, "x".repeat(65 * 1024)); + assert.equal(invoke(f, "stop").status, 3); +}); + +test("atomic JSON replacement remains valid and private", (t) => { + const f = fixture(t); + fs.mkdirSync(f.state, { mode: 0o700 }); + const canonical = fs.realpathSync(f.state); + const target = path.join(canonical, "state.json"); + atomicWriteJson(canonical, target, { generation: 1 }); + atomicWriteJson(canonical, target, { generation: 2 }); + assert.deepEqual(json(target), { generation: 2 }); + assert.equal(fs.statSync(target).mode & 0o777, 0o600); + assert.deepEqual(fs.readdirSync(f.state), ["state.json"]); +}); From 4c9f75ed8e543722cc23bd09c79ae8ef95bb432c Mon Sep 17 00:00:00 2001 From: Victor Garcia Date: Tue, 28 Jul 2026 02:00:44 -0600 Subject: [PATCH 05/19] [pi] Implemented, committed, and pushed Browser 0.6.0. --- bin/record.mjs | 195 +++++++++++++++++++++++++++++++--------- tests/manifest.test.mjs | 13 ++- tests/record.test.mjs | 80 +++++++++++++---- 3 files changed, 228 insertions(+), 60 deletions(-) diff --git a/bin/record.mjs b/bin/record.mjs index 3bd841a..6ea3442 100644 --- a/bin/record.mjs +++ b/bin/record.mjs @@ -12,51 +12,69 @@ export const MAX_RECORDING_BYTES = 10 * 1024 * 1024 * 1024; export function validateRunId(value) { if (typeof value !== "string" || !RUN_ID_RE.test(value)) { - throw new Error("run ID must be 1-128 ASCII letters, digits, dots, underscores, or hyphens and start with a letter or digit"); + throw new Error( + "run ID must be 1-128 ASCII letters, digits, dots, underscores, or hyphens and start with a letter or digit", + ); } return value; } export function isContained(root, target) { const relative = path.relative(root, target); - return relative !== ".." && !relative.startsWith(`..${path.sep}`) && !path.isAbsolute(relative); + return ( + relative !== ".." && + !relative.startsWith(`..${path.sep}`) && + !path.isAbsolute(relative) + ); } function safeError(error) { - return String(error?.message ?? error).replace(/[\u0000-\u001f\u007f]/g, " ").slice(0, 512); + return String(error?.message ?? error) + .replace(/[\u0000-\u001f\u007f]/g, " ") + .slice(0, 512); } function ensureDirectory(dir) { fs.mkdirSync(dir, { recursive: true, mode: 0o700 }); const stat = fs.lstatSync(dir); - if (!stat.isDirectory() || stat.isSymbolicLink()) throw new Error(`unsafe state directory: ${dir}`); + if (!stat.isDirectory() || stat.isSymbolicLink()) + throw new Error(`unsafe state directory: ${dir}`); fs.chmodSync(dir, 0o700); } function canonicalState(rawState) { - if (!path.isAbsolute(rawState)) throw new Error("plugin state directory must be absolute"); + if (!path.isAbsolute(rawState)) + throw new Error("plugin state directory must be absolute"); ensureDirectory(rawState); const root = fs.realpathSync(rawState); - if (!path.isAbsolute(root)) throw new Error("could not canonicalize plugin state directory"); + if (!path.isAbsolute(root)) + throw new Error("could not canonicalize plugin state directory"); return root; } function assertSafeTarget(root, target, kind = "file") { - if (!isContained(root, target)) throw new Error(`${kind} escapes plugin state`); + if (!isContained(root, target)) + throw new Error(`${kind} escapes plugin state`); const parent = fs.realpathSync(path.dirname(target)); - if (!isContained(root, parent) && parent !== root) throw new Error(`${kind} parent escapes plugin state`); + if (!isContained(root, parent) && parent !== root) + throw new Error(`${kind} parent escapes plugin state`); if (fs.existsSync(target)) { const stat = fs.lstatSync(target); if (stat.isSymbolicLink()) throw new Error(`${kind} must not be a symlink`); - if (kind.includes("JSON") && !stat.isFile()) throw new Error(`${kind} must be a regular file`); + if (kind.includes("JSON") && !stat.isFile()) + throw new Error(`${kind} must be a regular file`); } } export function atomicWriteJson(root, target, value) { assertSafeTarget(root, target, "JSON file"); const data = `${JSON.stringify(value, null, 2)}\n`; - if (Buffer.byteLength(data) > JSON_LIMIT) throw new Error("JSON document is too large"); - const temp = path.join(path.dirname(target), `.${path.basename(target)}.${process.pid}.${crypto.randomBytes(6).toString("hex")}.tmp`); + if (Buffer.byteLength(data) > JSON_LIMIT) + throw new Error("JSON document is too large"); + const temp = path.join( + path.dirname(target), + `.${path.basename(target)}.${process.pid}.${crypto.randomBytes(6).toString("hex")}.tmp`, + ); assertSafeTarget(root, temp, "temporary JSON file"); let fd; try { @@ -68,22 +86,34 @@ export function atomicWriteJson(root, target, value) { fs.renameSync(temp, target); fs.chmodSync(target, 0o600); const dirFd = fs.openSync(path.dirname(target), "r"); - try { fs.fsyncSync(dirFd); } finally { fs.closeSync(dirFd); } + try { + fs.fsyncSync(dirFd); + } finally { + fs.closeSync(dirFd); + } } finally { if (fd !== undefined) fs.closeSync(fd); - try { fs.unlinkSync(temp); } catch (error) { if (error.code !== "ENOENT") throw error; } + try { + fs.unlinkSync(temp); + } catch (error) { + if (error.code !== "ENOENT") throw error; + } } } function readJsonFile(root, target, label) { assertSafeTarget(root, target, label); const stat = fs.lstatSync(target); - if (!stat.isFile() || stat.size > JSON_LIMIT) throw new Error(`${label} must be a bounded regular file`); + if (!stat.isFile() || stat.size > JSON_LIMIT) + throw new Error(`${label} must be a bounded regular file`); return JSON.parse(fs.readFileSync(target, "utf8")); } function runIdFromEnvironment() { - if (process.env.HERDR_BROWSER_RUN_ID !== undefined && process.env.HERDR_BROWSER_RUN_ID !== "") { + if ( + process.env.HERDR_BROWSER_RUN_ID !== undefined && + process.env.HERDR_BROWSER_RUN_ID !== "" + ) { return validateRunId(process.env.HERDR_BROWSER_RUN_ID); } const configDir = process.env.HERDR_PLUGIN_CONFIG_DIR; @@ -91,16 +121,25 @@ function runIdFromEnvironment() { const source = path.join(configDir, "run-id"); if (fs.existsSync(source)) { const stat = fs.lstatSync(source); - if (!stat.isFile() || stat.isSymbolicLink() || stat.size > 1024) throw new Error("run-id config must be a bounded regular file"); + if (!stat.isFile() || stat.isSymbolicLink() || stat.size > 1024) + throw new Error("run-id config must be a bounded regular file"); const firstLine = fs.readFileSync(source, "utf8").split(/\r?\n/, 1)[0]; if (firstLine) return validateRunId(firstLine); } } - const stamp = new Date().toISOString().replace(/[-:]/g, "").replace(/\.\d{3}Z$/, "Z"); + const stamp = new Date() + .toISOString() + .replace(/[-:]/g, "") + .replace(/\.\d{3}Z$/, "Z"); return `browser-${stamp}-${crypto.randomBytes(4).toString("hex")}`; } -export function buildManifest({ runId, workspaceId, session, startedAt = new Date().toISOString() }) { +export function buildManifest({ + runId, + workspaceId, + session, + startedAt = new Date().toISOString(), +}) { return { schema_version: 1, evidence_type: "browser-recording", @@ -112,18 +151,34 @@ export function buildManifest({ runId, workspaceId, session, startedAt = new Dat started_at: startedAt, completed_at: null, recording_context_reset: true, - artifact: { path: "recording.webm", media_type: "video/webm", bytes: null, sha256: null }, + artifact: { + path: "recording.webm", + media_type: "video/webm", + bytes: null, + sha256: null, + }, review: { status: "unreviewed", required: true, attestation: "none" }, error: null, }; } function engine(session, args) { - const result = spawnSync("agent-browser", ["--session", session, "record", ...args], { - stdio: ["ignore", "ignore", "pipe"], encoding: "utf8", timeout: 15_000, killSignal: "SIGKILL", - }); + const result = spawnSync( + "agent-browser", + ["--session", session, "record", ...args], + { + stdio: ["ignore", "ignore", "pipe"], + encoding: "utf8", + timeout: 15_000, + killSignal: "SIGKILL", + }, + ); if (result.error || result.status !== 0) { - throw new Error(result.error?.code === "ETIMEDOUT" ? "agent-browser timed out" : result.stderr?.trim() || `agent-browser exited ${result.status}`); + throw new Error( + result.error?.code === "ETIMEDOUT" + ? "agent-browser timed out" + : result.stderr?.trim() || `agent-browser exited ${result.status}`, + ); } } @@ -132,17 +187,32 @@ function pathsFor(root, workspaceId, runId) { ensureDirectory(runs); const bundle = path.join(runs, `run-${runId}`, "browser"); const pointer = path.join(runs, `active-${workspaceId}.json`); - return { runs, bundle, manifest: path.join(bundle, "evidence.json"), artifact: path.join(bundle, "recording.webm"), pointer }; + return { + runs, + bundle, + manifest: path.join(bundle, "evidence.json"), + artifact: path.join(bundle, "recording.webm"), + pointer, + }; } function withWorkspaceLock(root, workspaceId, callback) { const lock = path.join(root, "runs", `.record-${workspaceId}.lock`); assertSafeTarget(root, lock, "recording lock"); - try { fs.mkdirSync(lock, { mode: 0o700 }); } catch (error) { - if (error.code === "EEXIST") throw new Error("another recording action is in progress for this workspace"); + try { + fs.mkdirSync(lock, { mode: 0o700 }); + } catch (error) { + if (error.code === "EEXIST") + throw new Error( + "another recording action is in progress for this workspace", + ); throw error; } - try { return callback(); } finally { fs.rmdirSync(lock); } + try { + return callback(); + } finally { + fs.rmdirSync(lock); + } } export function sha256File(file) { @@ -150,15 +220,22 @@ export function sha256File(file) { const fd = fs.openSync(file, "r"); const buffer = Buffer.allocUnsafe(1024 * 1024); try { - for (;;) { const count = fs.readSync(fd, buffer, 0, buffer.length, null); if (count === 0) break; hash.update(buffer.subarray(0, count)); } - } finally { fs.closeSync(fd); } + for (;;) { + const count = fs.readSync(fd, buffer, 0, buffer.length, null); + if (count === 0) break; + hash.update(buffer.subarray(0, count)); + } + } finally { + fs.closeSync(fd); + } return hash.digest("hex"); } function context() { const root = canonicalState(process.env.HERDR_BROWSER_STATE_DIR ?? ""); const workspaceId = process.env.HERDR_BROWSER_WORKSPACE_ID ?? ""; - if (!/^[A-Za-z0-9_-]+$/.test(workspaceId)) throw new Error("invalid workspace ID"); + if (!/^[A-Za-z0-9_-]+$/.test(workspaceId)) + throw new Error("invalid workspace ID"); const session = process.env.HERDR_BROWSER_SESSION_PINNED ?? ""; if (!session) throw new Error("browser session is required"); return { root, workspaceId, session }; @@ -169,22 +246,32 @@ function start() { const runId = runIdFromEnvironment(); const paths = pathsFor(root, workspaceId, runId); return withWorkspaceLock(root, workspaceId, () => { - if (fs.existsSync(paths.pointer)) throw new Error("a recording is already active for this workspace"); - if (fs.existsSync(path.join(paths.runs, `run-${runId}`))) throw new Error(`recording bundle already exists for run ${runId}`); + if (fs.existsSync(paths.pointer)) + throw new Error("a recording is already active for this workspace"); + if (fs.existsSync(path.join(paths.runs, `run-${runId}`))) + throw new Error(`recording bundle already exists for run ${runId}`); ensureDirectory(path.dirname(paths.bundle)); ensureDirectory(paths.bundle); const manifest = buildManifest({ runId, workspaceId, session }); try { engine(session, ["start", paths.artifact]); atomicWriteJson(root, paths.manifest, manifest); - atomicWriteJson(root, paths.pointer, { schema_version: 1, run_id: runId, workspace_id: workspaceId, browser_session: session, engine_stopped: false }); + atomicWriteJson(root, paths.pointer, { + schema_version: 1, + run_id: runId, + workspace_id: workspaceId, + browser_session: session, + engine_stopped: false, + }); } catch (error) { manifest.status = "failed"; manifest.error = safeError(error); atomicWriteJson(root, paths.manifest, manifest); throw error; } - console.log(`herdr-browser: recording run_id=${runId} manifest=${paths.manifest}`); + console.log( + `herdr-browser: recording run_id=${runId} manifest=${paths.manifest}`, + ); }); } @@ -192,13 +279,25 @@ function stop() { const { root, workspaceId } = context(); return withWorkspaceLock(root, workspaceId, () => { const pointerPath = path.join(root, "runs", `active-${workspaceId}.json`); - if (!fs.existsSync(pointerPath)) throw new Error("no recording is active for this workspace"); + if (!fs.existsSync(pointerPath)) + throw new Error("no recording is active for this workspace"); const pointer = readJsonFile(root, pointerPath, "active recording pointer"); validateRunId(pointer.run_id); - if (pointer.workspace_id !== workspaceId || typeof pointer.browser_session !== "string" || !pointer.browser_session || typeof pointer.engine_stopped !== "boolean") throw new Error("active recording pointer is invalid"); + if ( + pointer.workspace_id !== workspaceId || + typeof pointer.browser_session !== "string" || + !pointer.browser_session || + typeof pointer.engine_stopped !== "boolean" + ) + throw new Error("active recording pointer is invalid"); const paths = pathsFor(root, workspaceId, pointer.run_id); const manifest = readJsonFile(root, paths.manifest, "evidence manifest"); - if (manifest.status !== "recording" || manifest.run_id !== pointer.run_id || manifest.browser_session !== pointer.browser_session) throw new Error("active recording state is inconsistent"); + if ( + manifest.status !== "recording" || + manifest.run_id !== pointer.run_id || + manifest.browser_session !== pointer.browser_session + ) + throw new Error("active recording state is inconsistent"); try { if (!pointer.engine_stopped) { engine(pointer.browser_session, ["stop"]); @@ -207,9 +306,11 @@ function stop() { } assertSafeTarget(root, paths.artifact, "recording artifact"); const stat = fs.lstatSync(paths.artifact); - if (!stat.isFile() || stat.isSymbolicLink()) throw new Error("recording artifact must be a regular file"); + if (!stat.isFile() || stat.isSymbolicLink()) + throw new Error("recording artifact must be a regular file"); if (stat.size === 0) throw new Error("recording artifact is empty"); - if (stat.size > MAX_RECORDING_BYTES) throw new Error("recording artifact exceeds the 10 GiB limit"); + if (stat.size > MAX_RECORDING_BYTES) + throw new Error("recording artifact exceeds the 10 GiB limit"); fs.chmodSync(paths.artifact, 0o600); manifest.status = "complete"; manifest.completed_at = new Date().toISOString(); @@ -227,7 +328,9 @@ function stop() { atomicWriteJson(root, paths.manifest, manifest); throw error; } - console.log(`herdr-browser: completed run_id=${pointer.run_id} manifest=${paths.manifest}`); + console.log( + `herdr-browser: completed run_id=${pointer.run_id} manifest=${paths.manifest}`, + ); }); } @@ -237,6 +340,14 @@ export function main(mode = process.argv[2]) { throw new Error("usage: record.mjs start|stop"); } -if (process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)) { - try { main(); } catch (error) { console.error(`herdr-browser: ${safeError(error)}`); process.exitCode = 3; } +if ( + process.argv[1] && + path.resolve(process.argv[1]) === fileURLToPath(import.meta.url) +) { + try { + main(); + } catch (error) { + console.error(`herdr-browser: ${safeError(error)}`); + process.exitCode = 3; + } } diff --git a/tests/manifest.test.mjs b/tests/manifest.test.mjs index a448154..23318f1 100644 --- a/tests/manifest.test.mjs +++ b/tests/manifest.test.mjs @@ -34,12 +34,19 @@ test("repository manifest passes CI validation", () => { }); test("release version and existing action IDs remain stable", () => { - const packageJson = JSON.parse(fs.readFileSync(path.join(root, "package.json"), "utf8")); - const manifest = fs.readFileSync(path.join(root, "herdr-plugin.toml"), "utf8"); + const packageJson = JSON.parse( + fs.readFileSync(path.join(root, "package.json"), "utf8"), + ); + const manifest = fs.readFileSync( + path.join(root, "herdr-plugin.toml"), + "utf8", + ); assert.equal(packageJson.version, "0.6.0"); assert.match(manifest, /^version = "0\.6\.0"$/m); assert.deepEqual( - [...manifest.matchAll(/^id = "([^"]+)"$/gm)].slice(1, 6).map((match) => match[1]), + [...manifest.matchAll(/^id = "([^"]+)"$/gm)] + .slice(1, 6) + .map((match) => match[1]), ["open", "close", "browse", "record-start", "record-stop"], ); }); diff --git a/tests/record.test.mjs b/tests/record.test.mjs index b78d4d7..4431480 100644 --- a/tests/record.test.mjs +++ b/tests/record.test.mjs @@ -6,7 +6,11 @@ import os from "node:os"; import path from "node:path"; import test from "node:test"; import { fileURLToPath } from "node:url"; -import { atomicWriteJson, MAX_RECORDING_BYTES, validateRunId } from "../bin/record.mjs"; +import { + atomicWriteJson, + MAX_RECORDING_BYTES, + validateRunId, +} from "../bin/record.mjs"; const root = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); @@ -18,7 +22,9 @@ function fixture(t, overrides = {}) { const calls = path.join(base, "calls"); fs.mkdirSync(bin); fs.mkdirSync(config); - fs.writeFileSync(path.join(bin, "agent-browser"), `#!/usr/bin/env bash + fs.writeFileSync( + path.join(bin, "agent-browser"), + `#!/usr/bin/env bash printf '%s\\n' "$*" >> "$AB_CALLS" if [ "$4" = "start" ]; then [ "${"$"}{AB_START_FAIL:-0}" = 1 ] && exit 9 @@ -31,7 +37,9 @@ else [ "${"$"}{AB_STOP_FAIL:-0}" = 1 ] && exit 8 fi exit 0 -`, { mode: 0o755 }); +`, + { mode: 0o755 }, + ); const env = { ...process.env, PATH: `${bin}:${path.dirname(process.execPath)}:/usr/bin:/bin`, @@ -50,7 +58,8 @@ exit 0 function invoke(f, mode, overrides = {}) { return spawnSync("bash", [path.join(root, "scripts/record.sh"), mode], { - env: { ...f.env, ...overrides }, encoding: "utf8", + env: { ...f.env, ...overrides }, + encoding: "utf8", }); } @@ -64,7 +73,9 @@ function bundle(f, runId = "suite-run-1") { }; } -function json(file) { return JSON.parse(fs.readFileSync(file, "utf8")); } +function json(file) { + return JSON.parse(fs.readFileSync(file, "utf8")); +} test("explicit run creates a private contained recording bundle and complete digest", (t) => { const f = fixture(t); @@ -80,7 +91,10 @@ test("explicit run creates a private contained recording bundle and complete dig assert.equal(recording.review.attestation, "none"); assert.equal(recording.artifact.path, "recording.webm"); assert.ok(fs.existsSync(b.pointer)); - assert.match(fs.readFileSync(f.calls, "utf8"), /--session browser-session record start .*recording\.webm/); + assert.match( + fs.readFileSync(f.calls, "utf8"), + /--session browser-session record start .*recording\.webm/, + ); assert.equal(fs.statSync(b.dir).mode & 0o777, 0o700); assert.equal(fs.statSync(b.manifest).mode & 0o777, 0o600); assert.equal(fs.statSync(b.pointer).mode & 0o777, 0o600); @@ -89,7 +103,10 @@ test("explicit run creates a private contained recording bundle and complete dig const complete = json(b.manifest); assert.equal(complete.status, "complete"); assert.equal(complete.artifact.bytes, 14); - assert.equal(complete.artifact.sha256, crypto.createHash("sha256").update("webm-test-data").digest("hex")); + assert.equal( + complete.artifact.sha256, + crypto.createHash("sha256").update("webm-test-data").digest("hex"), + ); assert.equal(complete.review.attestation, "none"); assert.equal(fs.existsSync(b.pointer), false); assert.equal(fs.statSync(b.artifact).mode & 0o777, 0o600); @@ -97,7 +114,13 @@ test("explicit run creates a private contained recording bundle and complete dig }); test("invalid, traversal, control, and oversized run IDs fail before the engine", (t) => { - for (const id of ["../escape", "bad/name", "bad\nname", `a${"x".repeat(128)}`]) assert.throws(() => validateRunId(id)); + for (const id of [ + "../escape", + "bad/name", + "bad\nname", + `a${"x".repeat(128)}`, + ]) + assert.throws(() => validateRunId(id)); const f = fixture(t, { HERDR_BROWSER_RUN_ID: "../../escape" }); const result = invoke(f, "start"); assert.equal(result.status, 3); @@ -114,12 +137,24 @@ test("config run ID is used when the environment is absent", (t) => { }); test("generated run IDs are valid and distinct", (t) => { - const first = fixture(t, { HERDR_BROWSER_RUN_ID: "", HERDR_WORKSPACE_ID: "a" }); - const second = fixture(t, { HERDR_BROWSER_RUN_ID: "", HERDR_WORKSPACE_ID: "b" }); + const first = fixture(t, { + HERDR_BROWSER_RUN_ID: "", + HERDR_WORKSPACE_ID: "a", + }); + const second = fixture(t, { + HERDR_BROWSER_RUN_ID: "", + HERDR_WORKSPACE_ID: "b", + }); assert.equal(invoke(first, "start").status, 0); assert.equal(invoke(second, "start").status, 0); - const id1 = fs.readdirSync(path.join(first.state, "runs")).find((name) => name.startsWith("run-")).slice(4); - const id2 = fs.readdirSync(path.join(second.state, "runs")).find((name) => name.startsWith("run-")).slice(4); + const id1 = fs + .readdirSync(path.join(first.state, "runs")) + .find((name) => name.startsWith("run-")) + .slice(4); + const id2 = fs + .readdirSync(path.join(second.state, "runs")) + .find((name) => name.startsWith("run-")) + .slice(4); assert.equal(validateRunId(id1), id1); assert.notEqual(id1, id2); }); @@ -129,7 +164,10 @@ test("session text never participates in paths and stop uses the pinned session" assert.equal(invoke(f, "start").status, 0); const b = bundle(f); assert.equal(json(b.manifest).browser_session, "../../escape"); - const stopped = invoke(f, "stop", { HERDR_BROWSER_SESSION: "changed-session", HERDR_BROWSER_RUN_ID: "changed-run" }); + const stopped = invoke(f, "stop", { + HERDR_BROWSER_SESSION: "changed-session", + HERDR_BROWSER_RUN_ID: "changed-run", + }); assert.equal(stopped.status, 0, stopped.stderr); const calls = fs.readFileSync(f.calls, "utf8"); assert.match(calls, /--session \.\.\/\.\.\/escape record stop/); @@ -174,8 +212,20 @@ test("missing, empty, special, and oversized artifacts cannot complete", async ( for (const [name, mutate, mode = "data"] of [ ["missing", (b) => fs.rmSync(b.artifact), "data"], ["empty", () => {}, "empty"], - ["directory", (b) => { fs.rmSync(b.artifact); fs.mkdirSync(b.artifact); }], - ["symlink", (b, f) => { fs.rmSync(b.artifact); fs.symlinkSync(path.join(f.base, "outside"), b.artifact); }], + [ + "directory", + (b) => { + fs.rmSync(b.artifact); + fs.mkdirSync(b.artifact); + }, + ], + [ + "symlink", + (b, f) => { + fs.rmSync(b.artifact); + fs.symlinkSync(path.join(f.base, "outside"), b.artifact); + }, + ], ["oversized", (b) => fs.truncateSync(b.artifact, MAX_RECORDING_BYTES + 1)], ]) { await t.test(name, (st) => { From b79c25b6c7aa4c848c6af6dc421bd3c7e1eae515 Mon Sep 17 00:00:00 2001 From: Victor Garcia Date: Tue, 28 Jul 2026 02:48:09 -0600 Subject: [PATCH 06/19] fix(recording): harden evidence state transitions Validate exact pointer and manifest generations before any Stop engine call, atomically claim run directories across workspaces, and journal Start state before spawning the recorder. Compensate failed starts or retain an explicit retryable needs-attention generation when compensation fails.\n\nUse no-follow descriptors for bounded JSON reads and stable artifact chmod/hash operations, make completion recovery idempotent with durable pointer removal, and reclaim only well-formed locks whose local owner is proven dead. Cover corrupt generations, replacement races, concurrency, crash recovery, and injected persistence failures. --- bin/record.mjs | 645 +++++++++++++++++++++++++++++++++++------- tests/record.test.mjs | 377 +++++++++++++++++++++++- 2 files changed, 916 insertions(+), 106 deletions(-) diff --git a/bin/record.mjs b/bin/record.mjs index 6ea3442..d6c8083 100644 --- a/bin/record.mjs +++ b/bin/record.mjs @@ -2,12 +2,19 @@ import { spawnSync } from "node:child_process"; import crypto from "node:crypto"; import fs from "node:fs"; +import os from "node:os"; import path from "node:path"; import { fileURLToPath } from "node:url"; +const PLUGIN_ID = "structupath.browser"; const PLUGIN_VERSION = "0.6.0"; +const SCHEMA_VERSION = 1; const RUN_ID_RE = /^[A-Za-z0-9][A-Za-z0-9._-]{0,127}$/; +const WORKSPACE_ID_RE = /^[A-Za-z0-9_-]+$/; +const LOCK_NONCE_RE = /^[a-f0-9]{32}$/; +const SHA256_RE = /^[a-f0-9]{64}$/; const JSON_LIMIT = 64 * 1024; +const NOFOLLOW = fs.constants.O_NOFOLLOW; export const MAX_RECORDING_BYTES = 10 * 1024 * 1024 * 1024; export function validateRunId(value) { @@ -34,6 +41,16 @@ function safeError(error) { .slice(0, 512); } +function pathEntryExists(target) { + try { + fs.lstatSync(target); + return true; + } catch (error) { + if (error.code === "ENOENT") return false; + throw error; + } +} + function ensureDirectory(dir) { fs.mkdirSync(dir, { recursive: true, mode: 0o700 }); const stat = fs.lstatSync(dir); @@ -52,22 +69,50 @@ function canonicalState(rawState) { return root; } -function assertSafeTarget(root, target, kind = "file") { - if (!isContained(root, target)) - throw new Error(`${kind} escapes plugin state`); +function assertSafeParent(root, target, kind) { + if (!isContained(root, target)) throw new Error(`${kind} escapes plugin state`); const parent = fs.realpathSync(path.dirname(target)); - if (!isContained(root, parent) && parent !== root) + if (parent !== root && !isContained(root, parent)) throw new Error(`${kind} parent escapes plugin state`); - if (fs.existsSync(target)) { - const stat = fs.lstatSync(target); - if (stat.isSymbolicLink()) throw new Error(`${kind} must not be a symlink`); - if (kind.includes("JSON") && !stat.isFile()) - throw new Error(`${kind} must be a regular file`); +} + +function syncDirectory(dir) { + const fd = fs.openSync(dir, fs.constants.O_RDONLY); + try { + fs.fsyncSync(fd); + } finally { + fs.closeSync(fd); + } +} + +function descriptorStat(fd) { + return fs.fstatSync(fd, { bigint: true }); +} + +function sameDescriptorGeneration(left, right) { + return ( + left.dev === right.dev && + left.ino === right.ino && + left.size === right.size && + left.mtimeNs === right.mtimeNs && + left.ctimeNs === right.ctimeNs + ); +} + +function assertPathMatchesDescriptor(target, descriptor, label) { + const current = fs.lstatSync(target, { bigint: true }); + if ( + !current.isFile() || + current.isSymbolicLink() || + current.dev !== descriptor.dev || + current.ino !== descriptor.ino + ) { + throw new Error(`${label} changed while it was being validated`); } } export function atomicWriteJson(root, target, value) { - assertSafeTarget(root, target, "JSON file"); + assertSafeParent(root, target, "JSON file"); const data = `${JSON.stringify(value, null, 2)}\n`; if (Buffer.byteLength(data) > JSON_LIMIT) throw new Error("JSON document is too large"); @@ -75,22 +120,21 @@ export function atomicWriteJson(root, target, value) { path.dirname(target), `.${path.basename(target)}.${process.pid}.${crypto.randomBytes(6).toString("hex")}.tmp`, ); - assertSafeTarget(root, temp, "temporary JSON file"); + assertSafeParent(root, temp, "temporary JSON file"); let fd; try { - fd = fs.openSync(temp, "wx", 0o600); + fd = fs.openSync( + temp, + fs.constants.O_WRONLY | fs.constants.O_CREAT | fs.constants.O_EXCL, + 0o600, + ); fs.writeFileSync(fd, data, "utf8"); + fs.fchmodSync(fd, 0o600); fs.fsyncSync(fd); fs.closeSync(fd); fd = undefined; fs.renameSync(temp, target); - fs.chmodSync(target, 0o600); - const dirFd = fs.openSync(path.dirname(target), "r"); - try { - fs.fsyncSync(dirFd); - } finally { - fs.closeSync(dirFd); - } + syncDirectory(path.dirname(target)); } finally { if (fd !== undefined) fs.closeSync(fd); try { @@ -101,12 +145,36 @@ export function atomicWriteJson(root, target, value) { } } -function readJsonFile(root, target, label) { - assertSafeTarget(root, target, label); - const stat = fs.lstatSync(target); - if (!stat.isFile() || stat.size > JSON_LIMIT) - throw new Error(`${label} must be a bounded regular file`); - return JSON.parse(fs.readFileSync(target, "utf8")); +function readDescriptor(fd, size) { + const data = Buffer.alloc(size); + let offset = 0; + while (offset < data.length) { + const count = fs.readSync(fd, data, offset, data.length - offset, null); + if (count === 0) throw new Error("file ended while it was being read"); + offset += count; + } + return data; +} + +export function readJsonFile(root, target, label, hooks = {}) { + assertSafeParent(root, target, label); + if (NOFOLLOW === undefined) + throw new Error("this platform cannot safely open state files"); + const fd = fs.openSync(target, fs.constants.O_RDONLY | NOFOLLOW); + try { + const before = descriptorStat(fd); + if (!before.isFile() || before.size > BigInt(JSON_LIMIT)) + throw new Error(`${label} must be a bounded regular file`); + hooks.afterOpen?.(); + const parsed = JSON.parse(readDescriptor(fd, Number(before.size)).toString("utf8")); + const after = descriptorStat(fd); + if (!sameDescriptorGeneration(before, after)) + throw new Error(`${label} changed while it was being read`); + assertPathMatchesDescriptor(target, after, label); + return parsed; + } finally { + fs.closeSync(fd); + } } function runIdFromEnvironment() { @@ -118,12 +186,11 @@ function runIdFromEnvironment() { } const configDir = process.env.HERDR_PLUGIN_CONFIG_DIR; if (configDir) { - const source = path.join(configDir, "run-id"); - if (fs.existsSync(source)) { - const stat = fs.lstatSync(source); - if (!stat.isFile() || stat.isSymbolicLink() || stat.size > 1024) - throw new Error("run-id config must be a bounded regular file"); - const firstLine = fs.readFileSync(source, "utf8").split(/\r?\n/, 1)[0]; + const configRoot = fs.realpathSync(configDir); + const source = path.join(configRoot, "run-id"); + if (pathEntryExists(source)) { + const firstLine = readBoundedText(configRoot, source, "run-id config", 1024) + .split(/\r?\n/, 1)[0]; if (firstLine) return validateRunId(firstLine); } } @@ -134,6 +201,26 @@ function runIdFromEnvironment() { return `browser-${stamp}-${crypto.randomBytes(4).toString("hex")}`; } +function readBoundedText(root, target, label, limit) { + assertSafeParent(root, target, label); + if (NOFOLLOW === undefined) + throw new Error("this platform cannot safely open state files"); + const fd = fs.openSync(target, fs.constants.O_RDONLY | NOFOLLOW); + try { + const before = descriptorStat(fd); + if (!before.isFile() || before.size > BigInt(limit)) + throw new Error(`${label} must be a bounded regular file`); + const text = readDescriptor(fd, Number(before.size)).toString("utf8"); + const after = descriptorStat(fd); + if (!sameDescriptorGeneration(before, after)) + throw new Error(`${label} changed while it was being read`); + assertPathMatchesDescriptor(target, after, label); + return text; + } finally { + fs.closeSync(fd); + } +} + export function buildManifest({ runId, workspaceId, @@ -141,10 +228,10 @@ export function buildManifest({ startedAt = new Date().toISOString(), }) { return { - schema_version: 1, + schema_version: SCHEMA_VERSION, evidence_type: "browser-recording", run_id: runId, - plugin: { id: "structupath.browser", version: PLUGIN_VERSION }, + plugin: { id: PLUGIN_ID, version: PLUGIN_VERSION }, workspace_id: workspaceId, browser_session: session, status: "recording", @@ -162,16 +249,23 @@ export function buildManifest({ }; } +function withPrivateUmask(callback) { + const previous = process.umask(0o077); + try { + return callback(); + } finally { + process.umask(previous); + } +} + function engine(session, args) { - const result = spawnSync( - "agent-browser", - ["--session", session, "record", ...args], - { + const result = withPrivateUmask(() => + spawnSync("agent-browser", ["--session", session, "record", ...args], { stdio: ["ignore", "ignore", "pipe"], encoding: "utf8", timeout: 15_000, killSignal: "SIGKILL", - }, + }), ); if (result.error || result.status !== 0) { throw new Error( @@ -185,149 +279,496 @@ function engine(session, args) { function pathsFor(root, workspaceId, runId) { const runs = path.join(root, "runs"); ensureDirectory(runs); - const bundle = path.join(runs, `run-${runId}`, "browser"); - const pointer = path.join(runs, `active-${workspaceId}.json`); + const run = path.join(runs, `run-${runId}`); + const bundle = path.join(run, "browser"); return { runs, + run, bundle, manifest: path.join(bundle, "evidence.json"), artifact: path.join(bundle, "recording.webm"), + pointer: path.join(runs, `active-${workspaceId}.json`), + }; +} + +function assertExactKeys(value, keys, label) { + if (value === null || typeof value !== "object" || Array.isArray(value)) + throw new Error(`${label} must be an object`); + const actual = Object.keys(value).sort(); + const expected = [...keys].sort(); + if (actual.length !== expected.length || actual.some((key, index) => key !== expected[index])) + throw new Error(`${label} has incompatible fields`); +} + +function assertIsoTimestamp(value, label) { + if ( + typeof value !== "string" || + Number.isNaN(Date.parse(value)) || + new Date(value).toISOString() !== value + ) { + throw new Error(`${label} must be an ISO 8601 UTC timestamp`); + } +} + +function assertErrorField(value) { + if ( + value !== null && + (typeof value !== "string" || value.length > 512 || /[\u0000-\u001f\u007f]/.test(value)) + ) { + throw new Error("evidence manifest error is invalid"); + } +} + +function validatePointer(pointer, identity) { + assertExactKeys( pointer, + ["schema_version", "run_id", "workspace_id", "browser_session", "engine_stopped"], + "active recording pointer", + ); + if (pointer.schema_version !== SCHEMA_VERSION) + throw new Error("active recording pointer schema is incompatible"); + validateRunId(pointer.run_id); + if ( + pointer.run_id !== identity.runId || + pointer.workspace_id !== identity.workspaceId || + typeof pointer.browser_session !== "string" || + !pointer.browser_session || + typeof pointer.engine_stopped !== "boolean" + ) { + throw new Error("active recording pointer identity is invalid"); + } +} + +function validateManifest(manifest, identity, expectedStatus) { + assertExactKeys( + manifest, + [ + "schema_version", + "evidence_type", + "run_id", + "plugin", + "workspace_id", + "browser_session", + "status", + "started_at", + "completed_at", + "recording_context_reset", + "artifact", + "review", + "error", + ], + "evidence manifest", + ); + assertExactKeys(manifest.plugin, ["id", "version"], "evidence manifest plugin"); + assertExactKeys( + manifest.artifact, + ["path", "media_type", "bytes", "sha256"], + "evidence manifest artifact", + ); + assertExactKeys( + manifest.review, + ["status", "required", "attestation"], + "evidence manifest review", + ); + if ( + manifest.schema_version !== SCHEMA_VERSION || + manifest.evidence_type !== "browser-recording" || + manifest.run_id !== identity.runId || + manifest.plugin.id !== PLUGIN_ID || + manifest.plugin.version !== PLUGIN_VERSION || + manifest.workspace_id !== identity.workspaceId || + manifest.browser_session !== identity.session || + manifest.status !== expectedStatus || + manifest.recording_context_reset !== true || + manifest.artifact.path !== "recording.webm" || + manifest.artifact.media_type !== "video/webm" || + manifest.review.status !== "unreviewed" || + manifest.review.required !== true || + manifest.review.attestation !== "none" + ) { + throw new Error("evidence manifest identity or contract is incompatible"); + } + assertIsoTimestamp(manifest.started_at, "evidence manifest started_at"); + assertErrorField(manifest.error); + if (expectedStatus === "recording") { + if ( + manifest.completed_at !== null || + manifest.artifact.bytes !== null || + manifest.artifact.sha256 !== null + ) { + throw new Error("recording evidence manifest has completion data"); + } + return; + } + assertIsoTimestamp(manifest.completed_at, "evidence manifest completed_at"); + if (Date.parse(manifest.completed_at) < Date.parse(manifest.started_at)) + throw new Error("evidence manifest completion predates its start"); + if ( + !Number.isSafeInteger(manifest.artifact.bytes) || + manifest.artifact.bytes <= 0 || + manifest.artifact.bytes > MAX_RECORDING_BYTES || + !SHA256_RE.test(manifest.artifact.sha256) || + manifest.error !== null + ) { + throw new Error("complete evidence manifest has invalid completion data"); + } +} + +function lockOwner() { + return { + schema_version: SCHEMA_VERSION, + pid: process.pid, + hostname: os.hostname(), + started_at: new Date().toISOString(), + nonce: crypto.randomBytes(16).toString("hex"), }; } -function withWorkspaceLock(root, workspaceId, callback) { - const lock = path.join(root, "runs", `.record-${workspaceId}.lock`); - assertSafeTarget(root, lock, "recording lock"); +function validateLockOwner(owner) { + assertExactKeys( + owner, + ["schema_version", "pid", "hostname", "started_at", "nonce"], + "recording lock owner", + ); + if ( + owner.schema_version !== SCHEMA_VERSION || + !Number.isSafeInteger(owner.pid) || + owner.pid <= 0 || + owner.hostname !== os.hostname() || + !LOCK_NONCE_RE.test(owner.nonce) + ) { + throw new Error("recording lock owner is malformed or belongs to another host"); + } + assertIsoTimestamp(owner.started_at, "recording lock started_at"); +} + +function processIsProvenDead(pid) { try { - fs.mkdirSync(lock, { mode: 0o700 }); + process.kill(pid, 0); + return false; } catch (error) { - if (error.code === "EEXIST") + if (error.code === "ESRCH") return true; + return false; + } +} + +function acquireWorkspaceLock(root, workspaceId) { + const runs = path.join(root, "runs"); + ensureDirectory(runs); + const lock = path.join(runs, `.record-${workspaceId}.lock`); + const ownerPath = path.join(lock, "owner.json"); + assertSafeParent(root, lock, "recording lock"); + for (;;) { + try { + fs.mkdirSync(lock, { mode: 0o700 }); + const owner = lockOwner(); + try { + atomicWriteJson(root, ownerPath, owner); + syncDirectory(runs); + return { lock, ownerPath, owner, runs }; + } catch (error) { + fs.rmSync(lock, { recursive: true, force: true }); + syncDirectory(runs); + throw error; + } + } catch (error) { + if (error.code !== "EEXIST") throw error; + } + + let owner; + try { + owner = readJsonFile(root, ownerPath, "recording lock owner"); + validateLockOwner(owner); + } catch (error) { throw new Error( - "another recording action is in progress for this workspace", + `recording lock cannot be safely reclaimed: ${safeError(error)}; inspect ${lock} manually`, ); - throw error; + } + if (!processIsProvenDead(owner.pid)) + throw new Error("another recording action is in progress for this workspace"); + + const tombstone = `${lock}.stale-${process.pid}-${crypto.randomBytes(6).toString("hex")}`; + try { + fs.renameSync(lock, tombstone); + } catch (error) { + if (error.code === "ENOENT") continue; + throw error; + } + fs.rmSync(tombstone, { recursive: true, force: true }); + syncDirectory(runs); } +} + +function releaseWorkspaceLock(root, held) { + const current = readJsonFile(root, held.ownerPath, "recording lock owner"); + validateLockOwner(current); + if (current.nonce !== held.owner.nonce || current.pid !== held.owner.pid) + throw new Error("recording lock ownership changed unexpectedly"); + fs.rmSync(held.lock, { recursive: true }); + syncDirectory(held.runs); +} + +function withWorkspaceLock(root, workspaceId, callback) { + const held = acquireWorkspaceLock(root, workspaceId); try { return callback(); } finally { - fs.rmdirSync(lock); + releaseWorkspaceLock(root, held); } } -export function sha256File(file) { - const hash = crypto.createHash("sha256"); - const fd = fs.openSync(file, "r"); - const buffer = Buffer.allocUnsafe(1024 * 1024); +function claimRun(paths, runId) { + try { + fs.mkdirSync(paths.run, { mode: 0o700 }); + } catch (error) { + if (error.code === "EEXIST") + throw new Error(`recording bundle already exists for run ${runId}`); + throw error; + } + syncDirectory(paths.runs); + fs.mkdirSync(paths.bundle, { mode: 0o700 }); + syncDirectory(paths.run); +} + +function durableUnlink(target, parent) { + fs.unlinkSync(target); + syncDirectory(parent); +} + +function pointerFor(runId, workspaceId, session) { + return { + schema_version: SCHEMA_VERSION, + run_id: runId, + workspace_id: workspaceId, + browser_session: session, + engine_stopped: false, + }; +} + +function privatizeArtifactIfPresent(root, target) { + assertSafeParent(root, target, "recording artifact"); + if (!pathEntryExists(target)) return; + if (NOFOLLOW === undefined) + throw new Error("this platform cannot safely open recording artifacts"); + const fd = fs.openSync(target, fs.constants.O_RDONLY | NOFOLLOW); try { + const stat = descriptorStat(fd); + if (!stat.isFile()) + throw new Error("recording artifact must be a regular file"); + fs.fchmodSync(fd, 0o600); + assertPathMatchesDescriptor(target, descriptorStat(fd), "recording artifact"); + } finally { + fs.closeSync(fd); + } +} + +export function inspectArtifact(root, target, hooks = {}) { + assertSafeParent(root, target, "recording artifact"); + if (NOFOLLOW === undefined) + throw new Error("this platform cannot safely open recording artifacts"); + const fd = fs.openSync(target, fs.constants.O_RDONLY | NOFOLLOW); + try { + let before = descriptorStat(fd); + if (!before.isFile()) + throw new Error("recording artifact must be a regular file"); + if (before.size === 0n) throw new Error("recording artifact is empty"); + if (before.size > BigInt(MAX_RECORDING_BYTES)) + throw new Error("recording artifact exceeds the 10 GiB limit"); + fs.fchmodSync(fd, 0o600); + before = descriptorStat(fd); + hooks.afterOpen?.(); + const hash = crypto.createHash("sha256"); + const buffer = Buffer.allocUnsafe(1024 * 1024); + let bytes = 0; for (;;) { const count = fs.readSync(fd, buffer, 0, buffer.length, null); if (count === 0) break; + bytes += count; hash.update(buffer.subarray(0, count)); } + const after = descriptorStat(fd); + if ( + !sameDescriptorGeneration(before, after) || + BigInt(bytes) !== after.size + ) { + throw new Error("recording artifact changed while it was being hashed"); + } + assertPathMatchesDescriptor(target, after, "recording artifact"); + return { bytes, sha256: hash.digest("hex") }; } finally { fs.closeSync(fd); } - return hash.digest("hex"); +} + +export function sha256File(file) { + const root = fs.realpathSync(path.dirname(file)); + return inspectArtifact(root, file).sha256; } function context() { const root = canonicalState(process.env.HERDR_BROWSER_STATE_DIR ?? ""); const workspaceId = process.env.HERDR_BROWSER_WORKSPACE_ID ?? ""; - if (!/^[A-Za-z0-9_-]+$/.test(workspaceId)) + if (!WORKSPACE_ID_RE.test(workspaceId)) throw new Error("invalid workspace ID"); const session = process.env.HERDR_BROWSER_SESSION_PINNED ?? ""; if (!session) throw new Error("browser session is required"); return { root, workspaceId, session }; } +function recordRetryError(root, manifestPath, manifest, error) { + manifest.status = "recording"; + manifest.completed_at = null; + manifest.artifact.bytes = null; + manifest.artifact.sha256 = null; + manifest.error = safeError(error); + atomicWriteJson(root, manifestPath, manifest); +} + function start() { const { root, workspaceId, session } = context(); const runId = runIdFromEnvironment(); const paths = pathsFor(root, workspaceId, runId); return withWorkspaceLock(root, workspaceId, () => { - if (fs.existsSync(paths.pointer)) + if (pathEntryExists(paths.pointer)) throw new Error("a recording is already active for this workspace"); - if (fs.existsSync(path.join(paths.runs, `run-${runId}`))) - throw new Error(`recording bundle already exists for run ${runId}`); - ensureDirectory(path.dirname(paths.bundle)); - ensureDirectory(paths.bundle); + claimRun(paths, runId); const manifest = buildManifest({ runId, workspaceId, session }); + const pointer = pointerFor(runId, workspaceId, session); try { - engine(session, ["start", paths.artifact]); atomicWriteJson(root, paths.manifest, manifest); - atomicWriteJson(root, paths.pointer, { - schema_version: 1, - run_id: runId, - workspace_id: workspaceId, - browser_session: session, - engine_stopped: false, - }); + atomicWriteJson(root, paths.pointer, pointer); } catch (error) { manifest.status = "failed"; manifest.error = safeError(error); - atomicWriteJson(root, paths.manifest, manifest); + try { + atomicWriteJson(root, paths.manifest, manifest); + } catch { + // The original recording journal, if published, remains conservative. + } + if (pathEntryExists(paths.pointer)) { + try { + durableUnlink(paths.pointer, paths.runs); + } catch { + // No engine side effect occurred; refuse rather than delete ambiguity. + } + } throw error; } + + try { + engine(session, ["start", paths.artifact]); + privatizeArtifactIfPresent(root, paths.artifact); + } catch (startError) { + let compensationError; + try { + engine(session, ["stop"]); + } catch (error) { + compensationError = error; + } + if (compensationError) { + const attention = new Error( + `recording start needs attention: ${safeError(startError)}; compensating stop failed: ${safeError(compensationError)}`, + ); + try { + recordRetryError(root, paths.manifest, manifest, attention); + } catch { + // The durable pre-engine journal and pointer remain retryable. + } + throw attention; + } + pointer.engine_stopped = true; + atomicWriteJson(root, paths.pointer, pointer); + manifest.status = "failed"; + manifest.error = safeError(startError); + atomicWriteJson(root, paths.manifest, manifest); + durableUnlink(paths.pointer, paths.runs); + throw startError; + } console.log( `herdr-browser: recording run_id=${runId} manifest=${paths.manifest}`, ); }); } +function recoverComplete(root, paths, pointer, manifest) { + const artifact = inspectArtifact(root, paths.artifact); + if ( + artifact.bytes !== manifest.artifact.bytes || + artifact.sha256 !== manifest.artifact.sha256 + ) { + throw new Error("completed recording artifact no longer matches its manifest"); + } + durableUnlink(paths.pointer, paths.runs); + console.log( + `herdr-browser: completed run_id=${pointer.run_id} manifest=${paths.manifest}`, + ); +} + function stop() { const { root, workspaceId } = context(); return withWorkspaceLock(root, workspaceId, () => { const pointerPath = path.join(root, "runs", `active-${workspaceId}.json`); - if (!fs.existsSync(pointerPath)) + if (!pathEntryExists(pointerPath)) throw new Error("no recording is active for this workspace"); const pointer = readJsonFile(root, pointerPath, "active recording pointer"); validateRunId(pointer.run_id); - if ( - pointer.workspace_id !== workspaceId || - typeof pointer.browser_session !== "string" || - !pointer.browser_session || - typeof pointer.engine_stopped !== "boolean" - ) - throw new Error("active recording pointer is invalid"); const paths = pathsFor(root, workspaceId, pointer.run_id); const manifest = readJsonFile(root, paths.manifest, "evidence manifest"); - if ( - manifest.status !== "recording" || - manifest.run_id !== pointer.run_id || - manifest.browser_session !== pointer.browser_session - ) - throw new Error("active recording state is inconsistent"); + const identity = { + runId: pointer.run_id, + workspaceId, + session: pointer.browser_session, + }; + validatePointer(pointer, identity); + + if (manifest.status === "complete") { + validateManifest(manifest, identity, "complete"); + recoverComplete(root, paths, pointer, manifest); + return; + } + validateManifest(manifest, identity, "recording"); + + let artifact; try { if (!pointer.engine_stopped) { engine(pointer.browser_session, ["stop"]); pointer.engine_stopped = true; atomicWriteJson(root, paths.pointer, pointer); } - assertSafeTarget(root, paths.artifact, "recording artifact"); - const stat = fs.lstatSync(paths.artifact); - if (!stat.isFile() || stat.isSymbolicLink()) - throw new Error("recording artifact must be a regular file"); - if (stat.size === 0) throw new Error("recording artifact is empty"); - if (stat.size > MAX_RECORDING_BYTES) - throw new Error("recording artifact exceeds the 10 GiB limit"); - fs.chmodSync(paths.artifact, 0o600); - manifest.status = "complete"; - manifest.completed_at = new Date().toISOString(); - manifest.artifact.bytes = stat.size; - manifest.artifact.sha256 = sha256File(paths.artifact); - manifest.error = null; - atomicWriteJson(root, paths.manifest, manifest); - fs.unlinkSync(paths.pointer); + artifact = inspectArtifact(root, paths.artifact); } catch (error) { - manifest.status = "recording"; - manifest.completed_at = null; - manifest.artifact.bytes = null; - manifest.artifact.sha256 = null; - manifest.error = safeError(error); + recordRetryError(root, paths.manifest, manifest, error); + throw error; + } + + manifest.status = "complete"; + manifest.completed_at = new Date().toISOString(); + manifest.artifact.bytes = artifact.bytes; + manifest.artifact.sha256 = artifact.sha256; + manifest.error = null; + try { atomicWriteJson(root, paths.manifest, manifest); + } catch (error) { + let published = false; + try { + const current = readJsonFile(root, paths.manifest, "evidence manifest"); + validateManifest(current, identity, "complete"); + published = + current.artifact.bytes === artifact.bytes && + current.artifact.sha256 === artifact.sha256; + } catch { + // Keep the recording generation retryable below. + } + if (!published) { + manifest.status = "recording"; + manifest.completed_at = null; + manifest.artifact.bytes = null; + manifest.artifact.sha256 = null; + recordRetryError(root, paths.manifest, manifest, error); + } throw error; } + durableUnlink(paths.pointer, paths.runs); console.log( `herdr-browser: completed run_id=${pointer.run_id} manifest=${paths.manifest}`, ); diff --git a/tests/record.test.mjs b/tests/record.test.mjs index 4431480..cccb815 100644 --- a/tests/record.test.mjs +++ b/tests/record.test.mjs @@ -1,6 +1,6 @@ import assert from "node:assert/strict"; import crypto from "node:crypto"; -import { spawnSync } from "node:child_process"; +import { spawn, spawnSync } from "node:child_process"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; @@ -8,7 +8,10 @@ import test from "node:test"; import { fileURLToPath } from "node:url"; import { atomicWriteJson, + inspectArtifact, + main, MAX_RECORDING_BYTES, + readJsonFile, validateRunId, } from "../bin/record.mjs"; @@ -27,14 +30,21 @@ function fixture(t, overrides = {}) { `#!/usr/bin/env bash printf '%s\\n' "$*" >> "$AB_CALLS" if [ "$4" = "start" ]; then - [ "${"$"}{AB_START_FAIL:-0}" = 1 ] && exit 9 + if [ "${"$"}{AB_START_FAIL:-0}" = 1 ]; then + [ -n "${"$"}{AB_ACTIVE:-}" ] && printf active > "$AB_ACTIVE" + exit 9 + fi + [ -n "${"$"}{AB_START_DELAY:-}" ] && sleep "$AB_START_DELAY" case "${"$"}{AB_ARTIFACT_MODE:-data}" in data) printf 'webm-test-data' > "$5" ;; empty) : > "$5" ;; missing) ;; + symlink) ln -s "$AB_OUTSIDE" "$5" ;; esac + [ -n "${"$"}{AB_ACTIVE:-}" ] && printf active > "$AB_ACTIVE" else [ "${"$"}{AB_STOP_FAIL:-0}" = 1 ] && exit 8 + [ -n "${"$"}{AB_ACTIVE:-}" ] && rm -f "$AB_ACTIVE" fi exit 0 `, @@ -63,13 +73,31 @@ function invoke(f, mode, overrides = {}) { }); } -function bundle(f, runId = "suite-run-1") { +function invokeAsync(f, mode, overrides = {}) { + return new Promise((resolve) => { + const child = spawn("bash", [path.join(root, "scripts/record.sh"), mode], { + env: { ...f.env, ...overrides }, + stdio: ["ignore", "pipe", "pipe"], + }); + let stdout = ""; + let stderr = ""; + child.stdout.setEncoding("utf8").on("data", (chunk) => { + stdout += chunk; + }); + child.stderr.setEncoding("utf8").on("data", (chunk) => { + stderr += chunk; + }); + child.on("close", (status) => resolve({ status, stdout, stderr })); + }); +} + +function bundle(f, runId = "suite-run-1", workspaceId = "workspace_1") { const dir = path.join(f.state, "runs", `run-${runId}`, "browser"); return { dir, manifest: path.join(dir, "evidence.json"), artifact: path.join(dir, "recording.webm"), - pointer: path.join(f.state, "runs", "active-workspace_1.json"), + pointer: path.join(f.state, "runs", `active-${workspaceId}.json`), }; } @@ -98,6 +126,7 @@ test("explicit run creates a private contained recording bundle and complete dig assert.equal(fs.statSync(b.dir).mode & 0o777, 0o700); assert.equal(fs.statSync(b.manifest).mode & 0o777, 0o600); assert.equal(fs.statSync(b.pointer).mode & 0o777, 0o600); + assert.equal(fs.statSync(b.artifact).mode & 0o777, 0o600); const stop = invoke(f, "stop"); assert.equal(stop.status, 0, stop.stderr); const complete = json(b.manifest); @@ -271,3 +300,343 @@ test("atomic JSON replacement remains valid and private", (t) => { assert.equal(fs.statSync(target).mode & 0o777, 0o600); assert.deepEqual(fs.readdirSync(f.state), ["state.json"]); }); + +function writeLockOwner(f, value) { + const runs = path.join(f.state, "runs"); + const lock = path.join(runs, ".record-workspace_1.lock"); + fs.mkdirSync(lock, { recursive: true, mode: 0o700 }); + fs.writeFileSync(path.join(lock, "owner.json"), `${JSON.stringify(value)}\n`, { + mode: 0o600, + }); + return lock; +} + +function validLockOwner(pid) { + return { + schema_version: 1, + pid, + hostname: os.hostname(), + started_at: new Date().toISOString(), + nonce: "a".repeat(32), + }; +} + +test("Stop rejects every incompatible pointer and manifest field before engine side effects", async (t) => { + const mutations = [ + ["pointer schema", "pointer", (value) => (value.schema_version = 999)], + ["pointer unknown field", "pointer", (value) => (value.extra = true)], + ["pointer run", "pointer", (value) => (value.run_id = "foreign-run")], + ["pointer workspace", "pointer", (value) => (value.workspace_id = "foreign")], + ["pointer session", "pointer", (value) => (value.browser_session = "foreign")], + ["pointer stop flag", "pointer", (value) => (value.engine_stopped = "false")], + ["manifest schema", "manifest", (value) => (value.schema_version = 999)], + ["manifest unknown field", "manifest", (value) => (value.extra = true)], + ["manifest evidence type", "manifest", (value) => (value.evidence_type = "not-browser")], + ["manifest run", "manifest", (value) => (value.run_id = "foreign-run")], + ["plugin id", "manifest", (value) => (value.plugin.id = "foreign")], + ["plugin version", "manifest", (value) => (value.plugin.version = "999")], + ["plugin unknown field", "manifest", (value) => (value.plugin.extra = true)], + ["manifest workspace", "manifest", (value) => (value.workspace_id = "foreign")], + ["manifest session", "manifest", (value) => (value.browser_session = "foreign")], + ["manifest status", "manifest", (value) => (value.status = "failed")], + ["start timestamp", "manifest", (value) => (value.started_at = "yesterday")], + ["premature completion timestamp", "manifest", (value) => (value.completed_at = new Date().toISOString())], + ["context reset", "manifest", (value) => (value.recording_context_reset = false)], + ["artifact path", "manifest", (value) => (value.artifact.path = "../../outside")], + ["artifact media", "manifest", (value) => (value.artifact.media_type = "text/plain")], + ["artifact bytes", "manifest", (value) => (value.artifact.bytes = 14)], + ["artifact digest", "manifest", (value) => (value.artifact.sha256 = "a".repeat(64))], + ["artifact unknown field", "manifest", (value) => (value.artifact.extra = true)], + ["review status", "manifest", (value) => (value.review.status = "reviewed")], + ["review required", "manifest", (value) => (value.review.required = false)], + ["review attestation", "manifest", (value) => (value.review.attestation = "self-attested")], + ["review unknown field", "manifest", (value) => (value.review.extra = true)], + ["error type", "manifest", (value) => (value.error = { message: "hostile" })], + ]; + + for (const [name, target, mutate] of mutations) { + await t.test(name, (st) => { + const f = fixture(st); + assert.equal(invoke(f, "start").status, 0); + const b = bundle(f); + const file = target === "pointer" ? b.pointer : b.manifest; + const value = json(file); + mutate(value); + fs.writeFileSync(file, `${JSON.stringify(value, null, 2)}\n`); + const beforeFile = fs.readFileSync(file); + const beforeCalls = fs.readFileSync(f.calls); + const stopped = invoke(f, "stop"); + assert.equal(stopped.status, 3, name); + assert.deepEqual(fs.readFileSync(file), beforeFile, name); + assert.deepEqual(fs.readFileSync(f.calls), beforeCalls, name); + }); + } +}); + +test("one run ID is atomically claimed across different workspaces", async (t) => { + const f = fixture(t, { AB_START_DELAY: "0.2" }); + const [first, second] = await Promise.all([ + invokeAsync(f, "start", { + HERDR_WORKSPACE_ID: "workspace_a", + HERDR_BROWSER_SESSION: "session-a", + HERDR_BROWSER_RUN_ID: "shared-run", + }), + invokeAsync(f, "start", { + HERDR_WORKSPACE_ID: "workspace_b", + HERDR_BROWSER_SESSION: "session-b", + HERDR_BROWSER_RUN_ID: "shared-run", + }), + ]); + assert.deepEqual([first.status, second.status].sort(), [0, 3]); + const calls = fs.readFileSync(f.calls, "utf8").trim().split("\n"); + assert.equal(calls.filter((line) => line.includes(" record start ")).length, 1); + const pointers = fs + .readdirSync(path.join(f.state, "runs")) + .filter((name) => name.startsWith("active-")); + assert.equal(pointers.length, 1); + const b = bundle(f, "shared-run"); + const manifest = json(b.manifest); + assert.ok(["workspace_a", "workspace_b"].includes(manifest.workspace_id)); + assert.equal(manifest.browser_session, manifest.workspace_id === "workspace_a" ? "session-a" : "session-b"); + assert.equal(fs.readFileSync(b.artifact, "utf8"), "webm-test-data"); +}); + +test("failed post-start validation compensates and only then records terminal failure", (t) => { + const f = fixture(t, { + AB_ACTIVE: path.join(os.tmpdir(), `herdr-active-${crypto.randomUUID()}`), + AB_ARTIFACT_MODE: "symlink", + AB_OUTSIDE: path.join(os.tmpdir(), `herdr-outside-${crypto.randomUUID()}`), + }); + fs.writeFileSync(f.env.AB_OUTSIDE, "outside", { mode: 0o644 }); + t.after(() => { + fs.rmSync(f.env.AB_ACTIVE, { force: true }); + fs.rmSync(f.env.AB_OUTSIDE, { force: true }); + }); + const result = invoke(f, "start"); + assert.equal(result.status, 3); + const b = bundle(f); + assert.equal(json(b.manifest).status, "failed"); + assert.equal(fs.existsSync(b.pointer), false); + assert.equal(fs.existsSync(f.env.AB_ACTIVE), false); + assert.equal(fs.statSync(f.env.AB_OUTSIDE).mode & 0o777, 0o644); + const calls = fs.readFileSync(f.calls, "utf8"); + assert.match(calls, /record start/); + assert.match(calls, /record stop/); +}); + +test("failed compensation retains truthful retryable needs-attention state", (t) => { + const outside = path.join(os.tmpdir(), `herdr-outside-${crypto.randomUUID()}`); + const active = path.join(os.tmpdir(), `herdr-active-${crypto.randomUUID()}`); + const f = fixture(t, { + AB_ACTIVE: active, + AB_ARTIFACT_MODE: "symlink", + AB_OUTSIDE: outside, + AB_STOP_FAIL: "1", + }); + fs.writeFileSync(outside, "outside"); + t.after(() => { + fs.rmSync(active, { force: true }); + fs.rmSync(outside, { force: true }); + }); + const result = invoke(f, "start"); + assert.equal(result.status, 3); + const b = bundle(f); + assert.equal(json(b.manifest).status, "recording"); + assert.match(json(b.manifest).error, /needs attention/); + assert.equal(fs.existsSync(b.pointer), true); + assert.equal(fs.existsSync(active), true); + fs.rmSync(b.artifact); + fs.writeFileSync(b.artifact, "recovered-webm"); + const retry = invoke(f, "stop", { AB_STOP_FAIL: "0" }); + assert.equal(retry.status, 0, retry.stderr); + assert.equal(json(b.manifest).status, "complete"); + assert.equal(fs.existsSync(active), false); +}); + +function withMainEnvironment(f, callback) { + const values = { + PATH: f.env.PATH, + HERDR_BROWSER_STATE_DIR: f.state, + HERDR_BROWSER_WORKSPACE_ID: "workspace_1", + HERDR_BROWSER_SESSION_PINNED: "browser-session", + HERDR_BROWSER_RUN_ID: f.env.HERDR_BROWSER_RUN_ID, + HERDR_PLUGIN_CONFIG_DIR: f.config, + AB_CALLS: f.calls, + }; + const previous = Object.fromEntries( + Object.keys(values).map((key) => [key, process.env[key]]), + ); + Object.assign(process.env, values); + try { + return callback(); + } finally { + for (const [key, value] of Object.entries(previous)) { + if (value === undefined) delete process.env[key]; + else process.env[key] = value; + } + } +} + +function failOperationNumber(f, method, number, message) { + const original = fs[method]; + let calls = 0; + fs[method] = (...args) => { + calls++; + if (calls === number) { + const error = new Error(message); + error.code = "EIO"; + throw error; + } + return original(...args); + }; + try { + assert.throws( + () => withMainEnvironment(f, () => main("start")), + new RegExp(message), + ); + } finally { + fs[method] = original; + } + return calls; +} + +test("manifest and pointer write/fsync faults occur before the engine can start", async (t) => { + for (const [name, method, operationNumber] of [ + ["manifest write", "writeFileSync", 2], + ["pointer write", "writeFileSync", 3], + ["manifest fsync", "fsyncSync", 6], + ["pointer fsync", "fsyncSync", 8], + ]) { + await t.test(name, (st) => { + const runId = `fault-${name.replace(" ", "-")}`; + const f = fixture(st, { HERDR_BROWSER_RUN_ID: runId }); + assert.ok( + failOperationNumber( + f, + method, + operationNumber, + `injected ${name} failure`, + ) >= operationNumber, + ); + assert.equal(fs.existsSync(f.calls), false); + assert.equal(fs.existsSync(bundle(f, runId).pointer), false); + }); + } +}); + +test("complete-manifest crash recovery verifies the artifact and unlinks without stopping twice", (t) => { + const f = fixture(t); + assert.equal(invoke(f, "start").status, 0); + const b = bundle(f); + const pointer = fs.readFileSync(b.pointer); + assert.equal(invoke(f, "stop").status, 0); + const calls = fs.readFileSync(f.calls); + fs.writeFileSync(b.pointer, pointer, { mode: 0o600 }); + const recovered = invoke(f, "stop"); + assert.equal(recovered.status, 0, recovered.stderr); + assert.equal(fs.existsSync(b.pointer), false); + assert.deepEqual(fs.readFileSync(f.calls), calls); + assert.equal(json(b.manifest).status, "complete"); +}); + +test("complete recovery fails closed when the artifact no longer matches", (t) => { + const f = fixture(t); + assert.equal(invoke(f, "start").status, 0); + const b = bundle(f); + const pointer = fs.readFileSync(b.pointer); + assert.equal(invoke(f, "stop").status, 0); + fs.writeFileSync(b.pointer, pointer, { mode: 0o600 }); + fs.writeFileSync(b.artifact, "replacement"); + const calls = fs.readFileSync(f.calls); + assert.equal(invoke(f, "stop").status, 3); + assert.equal(fs.existsSync(b.pointer), true); + assert.deepEqual(fs.readFileSync(f.calls), calls); +}); + +test("descriptor-based JSON validation detects coordinated pathname replacement", (t) => { + const f = fixture(t); + fs.mkdirSync(f.state, { mode: 0o700 }); + const canonical = fs.realpathSync(f.state); + const target = path.join(canonical, "control.json"); + const held = path.join(canonical, "held.json"); + fs.writeFileSync(target, '{"generation":1}\n'); + assert.throws( + () => + readJsonFile(canonical, target, "control JSON", { + afterOpen() { + fs.renameSync(target, held); + fs.writeFileSync(target, '{"generation":2}\n'); + }, + }), + /changed while/, + ); +}); + +test("artifact hash and chmod stay on one no-follow descriptor during replacement", (t) => { + const f = fixture(t); + fs.mkdirSync(f.state, { mode: 0o700 }); + const canonical = fs.realpathSync(f.state); + const target = path.join(canonical, "recording.webm"); + const held = path.join(canonical, "held.webm"); + const outside = path.join(f.base, "outside.webm"); + fs.writeFileSync(target, "original", { mode: 0o644 }); + fs.writeFileSync(outside, "outside", { mode: 0o644 }); + assert.throws( + () => + inspectArtifact(canonical, target, { + afterOpen() { + fs.renameSync(target, held); + fs.symlinkSync(outside, target); + }, + }), + /changed while/, + ); + assert.equal(fs.statSync(held).mode & 0o777, 0o600); + assert.equal(fs.statSync(outside).mode & 0o777, 0o644); +}); + +test("workspace locks fail closed for live and malformed owners", async (t) => { + await t.test("live owner", (st) => { + const f = fixture(st); + writeLockOwner(f, validLockOwner(process.pid)); + const result = invoke(f, "start"); + assert.equal(result.status, 3); + assert.match(result.stderr, /another recording action is in progress/); + assert.equal(fs.existsSync(f.calls), false); + }); + await t.test("malformed owner", (st) => { + const f = fixture(st); + const lock = writeLockOwner(f, { pid: 999999 }); + const result = invoke(f, "start"); + assert.equal(result.status, 3); + assert.match(result.stderr, /cannot be safely reclaimed/); + assert.match(result.stderr, /inspect .*\.record-workspace_1\.lock manually/); + assert.equal(fs.existsSync(lock), true); + assert.equal(fs.existsSync(f.calls), false); + }); +}); + +test("well-formed locks are reclaimed only after their owner is proven dead", (t) => { + const f = fixture(t); + const lock = writeLockOwner(f, validLockOwner(2_147_483_647)); + const result = invoke(f, "start"); + assert.equal(result.status, 0, result.stderr); + assert.equal(fs.existsSync(lock), false); + assert.equal(json(bundle(f).manifest).status, "recording"); +}); + +test("concurrent stale-lock reclamation still permits only one workspace action", async (t) => { + const f = fixture(t, { AB_START_DELAY: "0.2" }); + writeLockOwner(f, validLockOwner(2_147_483_647)); + const [first, second] = await Promise.all([ + invokeAsync(f, "start", { HERDR_BROWSER_RUN_ID: "reclaim-a" }), + invokeAsync(f, "start", { HERDR_BROWSER_RUN_ID: "reclaim-b" }), + ]); + assert.deepEqual([first.status, second.status].sort(), [0, 3]); + const calls = fs.readFileSync(f.calls, "utf8").trim().split("\n"); + assert.equal(calls.filter((line) => line.includes(" record start ")).length, 1); + const pointers = fs + .readdirSync(path.join(f.state, "runs")) + .filter((name) => name.startsWith("active-")); + assert.deepEqual(pointers, ["active-workspace_1.json"]); +}); From d91341483e578132755e2cf7d625cc806cab5001 Mon Sep 17 00:00:00 2001 From: Victor Garcia Date: Tue, 28 Jul 2026 02:48:17 -0600 Subject: [PATCH 07/19] chore(browser): close Wave 0 drift and CI gaps Correct the renderer ownership and navigation description, replace the literal source NUL with an escaped separator, and align the Carbonyl installation guidance with the documented next channel.\n\nRun ShellCheck in hosted CI and keep these source-level drift contracts under repository tests. --- .github/workflows/ci.yml | 5 ++++- bin/renderer.mjs | 8 ++++---- scripts/browse-pane.sh | 2 +- tests/manifest.test.mjs | 16 ++++++++++++++++ 4 files changed, 25 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2167fe5..61464c8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,6 +1,7 @@ +--- name: CI -on: +"on": pull_request: push: @@ -24,5 +25,7 @@ jobs: run: npm test - name: Validate shell syntax run: bash -n scripts/*.sh + - name: Run ShellCheck + run: shellcheck scripts/*.sh - name: Validate plugin manifest run: node scripts/check-manifest.mjs diff --git a/bin/renderer.mjs b/bin/renderer.mjs index fb1a9c7..f912c36 100644 --- a/bin/renderer.mjs +++ b/bin/renderer.mjs @@ -1,7 +1,7 @@ #!/usr/bin/env node -// herdr-browser pane renderer: a passive viewer of an agent-browser session. -// It never navigates, never clears the console buffer, and never creates or -// destroys the browser session — those belong to the user and their agent. +// herdr-browser pane renderer: an attached view of an agent-browser session. +// It stays passive until explicit pane input, never clears the console buffer, +// and closes only sessions that its own successful navigation created. import { execFile, spawnSync } from "node:child_process"; import { promisify } from "node:util"; import { createHash } from "node:crypto"; @@ -1047,7 +1047,7 @@ export class Renderer { this.lastHash, this.consolePushes, this.frameSeq, - ].join(""); + ].join("\0"); } // --- live stream (push) --- diff --git a/scripts/browse-pane.sh b/scripts/browse-pane.sh index 847bd35..1321844 100755 --- a/scripts/browse-pane.sh +++ b/scripts/browse-pane.sh @@ -7,7 +7,7 @@ cd "${HERDR_PLUGIN_ROOT:-$(dirname "$0")/..}" || exit 1 if ! command -v carbonyl >/dev/null 2>&1; then echo "herdr-browser: carbonyl not found." - echo "Install it with: npm install -g carbonyl" + echo "Install it with: npm install -g carbonyl@next" echo "Then reopen this pane." sleep 600 exit 1 diff --git a/tests/manifest.test.mjs b/tests/manifest.test.mjs index 23318f1..d26611a 100644 --- a/tests/manifest.test.mjs +++ b/tests/manifest.test.mjs @@ -33,6 +33,22 @@ test("repository manifest passes CI validation", () => { assert.deepEqual(validateRepository(root).errors, []); }); +test("Wave 0 drift contracts stay represented in source and hosted CI", () => { + const renderer = fs.readFileSync(path.join(root, "bin", "renderer.mjs")); + const browsePane = fs.readFileSync( + path.join(root, "scripts", "browse-pane.sh"), + "utf8", + ); + const workflow = fs.readFileSync( + path.join(root, ".github", "workflows", "ci.yml"), + "utf8", + ); + assert.equal(renderer.includes(0), false); + assert.match(renderer.toString("utf8"), /stays passive until explicit pane input/); + assert.match(browsePane, /npm install -g carbonyl@next/); + assert.match(workflow, /run: shellcheck scripts\/\*\.sh/); +}); + test("release version and existing action IDs remain stable", () => { const packageJson = JSON.parse( fs.readFileSync(path.join(root, "package.json"), "utf8"), From e857e5c7e685048a592cb73c0f750f91f5a7268f Mon Sep 17 00:00:00 2001 From: Victor Garcia Date: Tue, 28 Jul 2026 02:52:43 -0600 Subject: [PATCH 08/19] test(renderer): isolate optional runtime dependencies Make the live-stream cooldown unit supply its own WebSocket capability and establish an actual live renderer state for the action-failure banner unit. This keeps Node 20 CI and machines without agent-browser from changing the behavior under test. --- tests/renderer.test.mjs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/tests/renderer.test.mjs b/tests/renderer.test.mjs index 2646447..718b393 100644 --- a/tests/renderer.test.mjs +++ b/tests/renderer.test.mjs @@ -1194,10 +1194,17 @@ test("polling tick tries goLive once, then respects the cooldown", async () => { }, sessionExists: async () => true, }; - await r.tick(); - assert.equal(enables, 1, "one live attempt on the first attached tick"); - await r.tick(); - assert.equal(enables, 1, "cooldown suppresses immediate retries"); + const saved = global.WebSocket; + global.WebSocket = class {}; + try { + await r.tick(); + assert.equal(enables, 1, "one live attempt on the first attached tick"); + await r.tick(); + assert.equal(enables, 1, "cooldown suppresses immediate retries"); + } finally { + if (saved === undefined) delete global.WebSocket; + else global.WebSocket = saved; + } }); // End-to-end against a real agent-browser session; skips when the engine is @@ -1401,6 +1408,9 @@ test('goLive rejects hostile or malformed ports without throwing', async () => { test('userAction surfaces failures in the banner (live mode has no tick report)', async () => { const r = quiet(mkRenderer()); + r.attached = true; + r.live = { ws: { close: () => {} } }; + r.lastLiveCheck = Date.now(); let headers = 0; r.header = () => { headers++; }; r.userAction(async () => { throw new Error('daemon hung'); }); From c0672b1140fb62d75934ce536ce23bfeedb37f91 Mon Sep 17 00:00:00 2001 From: Victor Garcia Date: Tue, 28 Jul 2026 02:56:27 -0600 Subject: [PATCH 09/19] [pi] Implemented, committed, and pushed the Browser rep... --- bin/record.mjs | 56 +++++++-- tests/manifest.test.mjs | 5 +- tests/record.test.mjs | 143 +++++++++++++++++---- tests/renderer.test.mjs | 267 +++++++++++++++++++++++----------------- 4 files changed, 320 insertions(+), 151 deletions(-) diff --git a/bin/record.mjs b/bin/record.mjs index d6c8083..c3dd40e 100644 --- a/bin/record.mjs +++ b/bin/record.mjs @@ -70,7 +70,8 @@ function canonicalState(rawState) { } function assertSafeParent(root, target, kind) { - if (!isContained(root, target)) throw new Error(`${kind} escapes plugin state`); + if (!isContained(root, target)) + throw new Error(`${kind} escapes plugin state`); const parent = fs.realpathSync(path.dirname(target)); if (parent !== root && !isContained(root, parent)) throw new Error(`${kind} parent escapes plugin state`); @@ -166,7 +167,9 @@ export function readJsonFile(root, target, label, hooks = {}) { if (!before.isFile() || before.size > BigInt(JSON_LIMIT)) throw new Error(`${label} must be a bounded regular file`); hooks.afterOpen?.(); - const parsed = JSON.parse(readDescriptor(fd, Number(before.size)).toString("utf8")); + const parsed = JSON.parse( + readDescriptor(fd, Number(before.size)).toString("utf8"), + ); const after = descriptorStat(fd); if (!sameDescriptorGeneration(before, after)) throw new Error(`${label} changed while it was being read`); @@ -189,8 +192,12 @@ function runIdFromEnvironment() { const configRoot = fs.realpathSync(configDir); const source = path.join(configRoot, "run-id"); if (pathEntryExists(source)) { - const firstLine = readBoundedText(configRoot, source, "run-id config", 1024) - .split(/\r?\n/, 1)[0]; + const firstLine = readBoundedText( + configRoot, + source, + "run-id config", + 1024, + ).split(/\r?\n/, 1)[0]; if (firstLine) return validateRunId(firstLine); } } @@ -296,7 +303,10 @@ function assertExactKeys(value, keys, label) { throw new Error(`${label} must be an object`); const actual = Object.keys(value).sort(); const expected = [...keys].sort(); - if (actual.length !== expected.length || actual.some((key, index) => key !== expected[index])) + if ( + actual.length !== expected.length || + actual.some((key, index) => key !== expected[index]) + ) throw new Error(`${label} has incompatible fields`); } @@ -313,7 +323,9 @@ function assertIsoTimestamp(value, label) { function assertErrorField(value) { if ( value !== null && - (typeof value !== "string" || value.length > 512 || /[\u0000-\u001f\u007f]/.test(value)) + (typeof value !== "string" || + value.length > 512 || + /[\u0000-\u001f\u007f]/.test(value)) ) { throw new Error("evidence manifest error is invalid"); } @@ -322,7 +334,13 @@ function assertErrorField(value) { function validatePointer(pointer, identity) { assertExactKeys( pointer, - ["schema_version", "run_id", "workspace_id", "browser_session", "engine_stopped"], + [ + "schema_version", + "run_id", + "workspace_id", + "browser_session", + "engine_stopped", + ], "active recording pointer", ); if (pointer.schema_version !== SCHEMA_VERSION) @@ -359,7 +377,11 @@ function validateManifest(manifest, identity, expectedStatus) { ], "evidence manifest", ); - assertExactKeys(manifest.plugin, ["id", "version"], "evidence manifest plugin"); + assertExactKeys( + manifest.plugin, + ["id", "version"], + "evidence manifest plugin", + ); assertExactKeys( manifest.artifact, ["path", "media_type", "bytes", "sha256"], @@ -437,7 +459,9 @@ function validateLockOwner(owner) { owner.hostname !== os.hostname() || !LOCK_NONCE_RE.test(owner.nonce) ) { - throw new Error("recording lock owner is malformed or belongs to another host"); + throw new Error( + "recording lock owner is malformed or belongs to another host", + ); } assertIsoTimestamp(owner.started_at, "recording lock started_at"); } @@ -485,7 +509,9 @@ function acquireWorkspaceLock(root, workspaceId) { ); } if (!processIsProvenDead(owner.pid)) - throw new Error("another recording action is in progress for this workspace"); + throw new Error( + "another recording action is in progress for this workspace", + ); const tombstone = `${lock}.stale-${process.pid}-${crypto.randomBytes(6).toString("hex")}`; try { @@ -556,7 +582,11 @@ function privatizeArtifactIfPresent(root, target) { if (!stat.isFile()) throw new Error("recording artifact must be a regular file"); fs.fchmodSync(fd, 0o600); - assertPathMatchesDescriptor(target, descriptorStat(fd), "recording artifact"); + assertPathMatchesDescriptor( + target, + descriptorStat(fd), + "recording artifact", + ); } finally { fs.closeSync(fd); } @@ -696,7 +726,9 @@ function recoverComplete(root, paths, pointer, manifest) { artifact.bytes !== manifest.artifact.bytes || artifact.sha256 !== manifest.artifact.sha256 ) { - throw new Error("completed recording artifact no longer matches its manifest"); + throw new Error( + "completed recording artifact no longer matches its manifest", + ); } durableUnlink(paths.pointer, paths.runs); console.log( diff --git a/tests/manifest.test.mjs b/tests/manifest.test.mjs index d26611a..cd61175 100644 --- a/tests/manifest.test.mjs +++ b/tests/manifest.test.mjs @@ -44,7 +44,10 @@ test("Wave 0 drift contracts stay represented in source and hosted CI", () => { "utf8", ); assert.equal(renderer.includes(0), false); - assert.match(renderer.toString("utf8"), /stays passive until explicit pane input/); + assert.match( + renderer.toString("utf8"), + /stays passive until explicit pane input/, + ); assert.match(browsePane, /npm install -g carbonyl@next/); assert.match(workflow, /run: shellcheck scripts\/\*\.sh/); }); diff --git a/tests/record.test.mjs b/tests/record.test.mjs index cccb815..4c8c442 100644 --- a/tests/record.test.mjs +++ b/tests/record.test.mjs @@ -305,9 +305,13 @@ function writeLockOwner(f, value) { const runs = path.join(f.state, "runs"); const lock = path.join(runs, ".record-workspace_1.lock"); fs.mkdirSync(lock, { recursive: true, mode: 0o700 }); - fs.writeFileSync(path.join(lock, "owner.json"), `${JSON.stringify(value)}\n`, { - mode: 0o600, - }); + fs.writeFileSync( + path.join(lock, "owner.json"), + `${JSON.stringify(value)}\n`, + { + mode: 0o600, + }, + ); return lock; } @@ -326,32 +330,104 @@ test("Stop rejects every incompatible pointer and manifest field before engine s ["pointer schema", "pointer", (value) => (value.schema_version = 999)], ["pointer unknown field", "pointer", (value) => (value.extra = true)], ["pointer run", "pointer", (value) => (value.run_id = "foreign-run")], - ["pointer workspace", "pointer", (value) => (value.workspace_id = "foreign")], - ["pointer session", "pointer", (value) => (value.browser_session = "foreign")], - ["pointer stop flag", "pointer", (value) => (value.engine_stopped = "false")], + [ + "pointer workspace", + "pointer", + (value) => (value.workspace_id = "foreign"), + ], + [ + "pointer session", + "pointer", + (value) => (value.browser_session = "foreign"), + ], + [ + "pointer stop flag", + "pointer", + (value) => (value.engine_stopped = "false"), + ], ["manifest schema", "manifest", (value) => (value.schema_version = 999)], ["manifest unknown field", "manifest", (value) => (value.extra = true)], - ["manifest evidence type", "manifest", (value) => (value.evidence_type = "not-browser")], + [ + "manifest evidence type", + "manifest", + (value) => (value.evidence_type = "not-browser"), + ], ["manifest run", "manifest", (value) => (value.run_id = "foreign-run")], ["plugin id", "manifest", (value) => (value.plugin.id = "foreign")], ["plugin version", "manifest", (value) => (value.plugin.version = "999")], - ["plugin unknown field", "manifest", (value) => (value.plugin.extra = true)], - ["manifest workspace", "manifest", (value) => (value.workspace_id = "foreign")], - ["manifest session", "manifest", (value) => (value.browser_session = "foreign")], + [ + "plugin unknown field", + "manifest", + (value) => (value.plugin.extra = true), + ], + [ + "manifest workspace", + "manifest", + (value) => (value.workspace_id = "foreign"), + ], + [ + "manifest session", + "manifest", + (value) => (value.browser_session = "foreign"), + ], ["manifest status", "manifest", (value) => (value.status = "failed")], - ["start timestamp", "manifest", (value) => (value.started_at = "yesterday")], - ["premature completion timestamp", "manifest", (value) => (value.completed_at = new Date().toISOString())], - ["context reset", "manifest", (value) => (value.recording_context_reset = false)], - ["artifact path", "manifest", (value) => (value.artifact.path = "../../outside")], - ["artifact media", "manifest", (value) => (value.artifact.media_type = "text/plain")], + [ + "start timestamp", + "manifest", + (value) => (value.started_at = "yesterday"), + ], + [ + "premature completion timestamp", + "manifest", + (value) => (value.completed_at = new Date().toISOString()), + ], + [ + "context reset", + "manifest", + (value) => (value.recording_context_reset = false), + ], + [ + "artifact path", + "manifest", + (value) => (value.artifact.path = "../../outside"), + ], + [ + "artifact media", + "manifest", + (value) => (value.artifact.media_type = "text/plain"), + ], ["artifact bytes", "manifest", (value) => (value.artifact.bytes = 14)], - ["artifact digest", "manifest", (value) => (value.artifact.sha256 = "a".repeat(64))], - ["artifact unknown field", "manifest", (value) => (value.artifact.extra = true)], - ["review status", "manifest", (value) => (value.review.status = "reviewed")], + [ + "artifact digest", + "manifest", + (value) => (value.artifact.sha256 = "a".repeat(64)), + ], + [ + "artifact unknown field", + "manifest", + (value) => (value.artifact.extra = true), + ], + [ + "review status", + "manifest", + (value) => (value.review.status = "reviewed"), + ], ["review required", "manifest", (value) => (value.review.required = false)], - ["review attestation", "manifest", (value) => (value.review.attestation = "self-attested")], - ["review unknown field", "manifest", (value) => (value.review.extra = true)], - ["error type", "manifest", (value) => (value.error = { message: "hostile" })], + [ + "review attestation", + "manifest", + (value) => (value.review.attestation = "self-attested"), + ], + [ + "review unknown field", + "manifest", + (value) => (value.review.extra = true), + ], + [ + "error type", + "manifest", + (value) => (value.error = { message: "hostile" }), + ], ]; for (const [name, target, mutate] of mutations) { @@ -389,7 +465,10 @@ test("one run ID is atomically claimed across different workspaces", async (t) = ]); assert.deepEqual([first.status, second.status].sort(), [0, 3]); const calls = fs.readFileSync(f.calls, "utf8").trim().split("\n"); - assert.equal(calls.filter((line) => line.includes(" record start ")).length, 1); + assert.equal( + calls.filter((line) => line.includes(" record start ")).length, + 1, + ); const pointers = fs .readdirSync(path.join(f.state, "runs")) .filter((name) => name.startsWith("active-")); @@ -397,7 +476,10 @@ test("one run ID is atomically claimed across different workspaces", async (t) = const b = bundle(f, "shared-run"); const manifest = json(b.manifest); assert.ok(["workspace_a", "workspace_b"].includes(manifest.workspace_id)); - assert.equal(manifest.browser_session, manifest.workspace_id === "workspace_a" ? "session-a" : "session-b"); + assert.equal( + manifest.browser_session, + manifest.workspace_id === "workspace_a" ? "session-a" : "session-b", + ); assert.equal(fs.readFileSync(b.artifact, "utf8"), "webm-test-data"); }); @@ -425,7 +507,10 @@ test("failed post-start validation compensates and only then records terminal fa }); test("failed compensation retains truthful retryable needs-attention state", (t) => { - const outside = path.join(os.tmpdir(), `herdr-outside-${crypto.randomUUID()}`); + const outside = path.join( + os.tmpdir(), + `herdr-outside-${crypto.randomUUID()}`, + ); const active = path.join(os.tmpdir(), `herdr-active-${crypto.randomUUID()}`); const f = fixture(t, { AB_ACTIVE: active, @@ -610,7 +695,10 @@ test("workspace locks fail closed for live and malformed owners", async (t) => { const result = invoke(f, "start"); assert.equal(result.status, 3); assert.match(result.stderr, /cannot be safely reclaimed/); - assert.match(result.stderr, /inspect .*\.record-workspace_1\.lock manually/); + assert.match( + result.stderr, + /inspect .*\.record-workspace_1\.lock manually/, + ); assert.equal(fs.existsSync(lock), true); assert.equal(fs.existsSync(f.calls), false); }); @@ -634,7 +722,10 @@ test("concurrent stale-lock reclamation still permits only one workspace action" ]); assert.deepEqual([first.status, second.status].sort(), [0, 3]); const calls = fs.readFileSync(f.calls, "utf8").trim().split("\n"); - assert.equal(calls.filter((line) => line.includes(" record start ")).length, 1); + assert.equal( + calls.filter((line) => line.includes(" record start ")).length, + 1, + ); const pointers = fs .readdirSync(path.join(f.state, "runs")) .filter((name) => name.startsWith("active-")); diff --git a/tests/renderer.test.mjs b/tests/renderer.test.mjs index 718b393..3777bda 100644 --- a/tests/renderer.test.mjs +++ b/tests/renderer.test.mjs @@ -1325,121 +1325,156 @@ test("header is repaint-gated but always paints real changes", () => { // --- Wave 2d: fleet-re-review regression tests --- -test('poll tick after a stream drop renders the fresh PNG, not the stale JPEG', async () => { - const r = quiet(mkRenderer()); - r.attached = true; - r.fitViewport = async () => false; - // Simulate a prior live stream: a jpg frame landed, then the stream died. - r.onStreamMessage({ type: 'frame', data: jpeg(1280, 720).toString('base64') }); - await flush(); - assert.equal(r.shotFormat, 'jpg'); - r.live = { ws: { close: () => {} } }; - r.dropLive(); - assert.equal(r.shotFormat, 'png', 'dropLive points the renderer at the poll path frame'); - let renderedPath = null; - const origExists = fs.existsSync; - r.renderImage = async () => { renderedPath = r.shotFormat === 'jpg' ? r.shotJpg : r.shot; }; - r.browser = { - snapshot: async f => { - fs.writeFileSync(f, PNG_1PX); - return { url: 'https://x', title: '', entries: [] }; - }, - sessionExists: async () => true, - }; - await r.tick(); - assert.equal(r.shotFormat, 'png'); - assert.equal(renderedPath, r.shot, 'poll frame rendered, not the frozen stream frame'); - assert.ok(origExists(r.shot)); -}); - -test('malformed stream messages never crash the dispatch', async () => { - const r = quiet(mkRenderer()); - const bad = [ - { type: 'tabs', tabs: {} }, - { type: 'tabs', tabs: 42 }, - { type: 'tabs', tabs: 'x' }, - { type: 'frame', data: 12345 }, - { type: 'frame' }, - { type: 'console' }, - { type: 'url' }, - {}, - { type: null }, - ]; - for (const m of bad) r.onStreamMessage(m); - await flush(); - assert.equal(r.frameSeq, 0); - // And the dispatch wrapper in goLive counts instead of throwing: - assert.ok(r.paintErrors >= 0); -}); - -test('a successful goLive from the attach path repaints the header', async () => { - const r = quiet(mkRenderer()); - let headers = 0; - const realHeader = Object.getPrototypeOf(r).header.bind(r); - r.header = () => { headers++; realHeader(); }; - r.banner = 'waiting for session …'; - r.goLive = async () => true; - r.browser = { sessionExists: async () => true }; - await r.tick(); - assert.equal(r.attached, true); - assert.ok(headers >= 1, 'header repainted after goLive, stale banner cleared'); -}); - -test('goLive stays on the poll path for kitty mode without chafa', async () => { - const r = quiet(mkRenderer()); - r.mode = 'kitty'; - r.chafa = false; - let enabled = 0; - r.browser = { streamEnable: async () => { enabled++; } }; - assert.equal(await r.goLive(), false); - assert.equal(enabled, 0, 'no stream attempt when stream frames could not render'); -}); - -test('goLive rejects hostile or malformed ports without throwing', async () => { - const r = quiet(mkRenderer()); - for (const port of ['evil.com/x', 0, -1, 70000, 3.5, null, undefined]) { - r.browser = { - streamEnable: async () => {}, - streamStatus: async () => ({ port }), - }; - assert.equal(await r.goLive(), false, `port ${JSON.stringify(port)} refused`); - } -}); - -test('userAction surfaces failures in the banner (live mode has no tick report)', async () => { - const r = quiet(mkRenderer()); - r.attached = true; - r.live = { ws: { close: () => {} } }; - r.lastLiveCheck = Date.now(); - let headers = 0; - r.header = () => { headers++; }; - r.userAction(async () => { throw new Error('daemon hung'); }); - await flush(); - assert.match(r.banner, /command failed/); - assert.ok(headers >= 1, 'failure banner painted'); -}); - -test('feed holds a split ESC-[ but a bare ESC still dispatches immediately', () => { - const r = quiet(mkRenderer()); - const keys = []; - r.onKey = ch => keys.push(ch); - r.feed('\x1b['); - assert.deepEqual(keys, [], 'ESC-[ held for the next chunk'); - r.feed('<0;10;5M'); - assert.deepEqual(keys, [], 'completed report went to the mouse path'); - r.feed('\x1b'); - // bare ESC reaches the key path at once (prompt cancel must not lag); - // onKey ignores it when no prompt is open and it is not a mapped key. - assert.equal(r.inputBuf, ''); +test("poll tick after a stream drop renders the fresh PNG, not the stale JPEG", async () => { + const r = quiet(mkRenderer()); + r.attached = true; + r.fitViewport = async () => false; + // Simulate a prior live stream: a jpg frame landed, then the stream died. + r.onStreamMessage({ + type: "frame", + data: jpeg(1280, 720).toString("base64"), + }); + await flush(); + assert.equal(r.shotFormat, "jpg"); + r.live = { ws: { close: () => {} } }; + r.dropLive(); + assert.equal( + r.shotFormat, + "png", + "dropLive points the renderer at the poll path frame", + ); + let renderedPath = null; + const origExists = fs.existsSync; + r.renderImage = async () => { + renderedPath = r.shotFormat === "jpg" ? r.shotJpg : r.shot; + }; + r.browser = { + snapshot: async (f) => { + fs.writeFileSync(f, PNG_1PX); + return { url: "https://x", title: "", entries: [] }; + }, + sessionExists: async () => true, + }; + await r.tick(); + assert.equal(r.shotFormat, "png"); + assert.equal( + renderedPath, + r.shot, + "poll frame rendered, not the frozen stream frame", + ); + assert.ok(origExists(r.shot)); +}); + +test("malformed stream messages never crash the dispatch", async () => { + const r = quiet(mkRenderer()); + const bad = [ + { type: "tabs", tabs: {} }, + { type: "tabs", tabs: 42 }, + { type: "tabs", tabs: "x" }, + { type: "frame", data: 12345 }, + { type: "frame" }, + { type: "console" }, + { type: "url" }, + {}, + { type: null }, + ]; + for (const m of bad) r.onStreamMessage(m); + await flush(); + assert.equal(r.frameSeq, 0); + // And the dispatch wrapper in goLive counts instead of throwing: + assert.ok(r.paintErrors >= 0); +}); + +test("a successful goLive from the attach path repaints the header", async () => { + const r = quiet(mkRenderer()); + let headers = 0; + const realHeader = Object.getPrototypeOf(r).header.bind(r); + r.header = () => { + headers++; + realHeader(); + }; + r.banner = "waiting for session …"; + r.goLive = async () => true; + r.browser = { sessionExists: async () => true }; + await r.tick(); + assert.equal(r.attached, true); + assert.ok( + headers >= 1, + "header repainted after goLive, stale banner cleared", + ); +}); + +test("goLive stays on the poll path for kitty mode without chafa", async () => { + const r = quiet(mkRenderer()); + r.mode = "kitty"; + r.chafa = false; + let enabled = 0; + r.browser = { + streamEnable: async () => { + enabled++; + }, + }; + assert.equal(await r.goLive(), false); + assert.equal( + enabled, + 0, + "no stream attempt when stream frames could not render", + ); +}); + +test("goLive rejects hostile or malformed ports without throwing", async () => { + const r = quiet(mkRenderer()); + for (const port of ["evil.com/x", 0, -1, 70000, 3.5, null, undefined]) { + r.browser = { + streamEnable: async () => {}, + streamStatus: async () => ({ port }), + }; + assert.equal( + await r.goLive(), + false, + `port ${JSON.stringify(port)} refused`, + ); + } +}); + +test("userAction surfaces failures in the banner (live mode has no tick report)", async () => { + const r = quiet(mkRenderer()); + r.attached = true; + r.live = { ws: { close: () => {} } }; + r.lastLiveCheck = Date.now(); + let headers = 0; + r.header = () => { + headers++; + }; + r.userAction(async () => { + throw new Error("daemon hung"); + }); + await flush(); + assert.match(r.banner, /command failed/); + assert.ok(headers >= 1, "failure banner painted"); +}); + +test("feed holds a split ESC-[ but a bare ESC still dispatches immediately", () => { + const r = quiet(mkRenderer()); + const keys = []; + r.onKey = (ch) => keys.push(ch); + r.feed("\x1b["); + assert.deepEqual(keys, [], "ESC-[ held for the next chunk"); + r.feed("<0;10;5M"); + assert.deepEqual(keys, [], "completed report went to the mouse path"); + r.feed("\x1b"); + // bare ESC reaches the key path at once (prompt cancel must not lag); + // onKey ignores it when no prompt is open and it is not a mapped key. + assert.equal(r.inputBuf, ""); }); // --- Pane viewport fitting --- test("viewportForPane preserves width and fills the image-cell aspect", () => { - assert.deepEqual( - viewportForPane(1280, { cols: 80, imageRows: 30 }), - { w: 1280, h: 960 }, - ); + assert.deepEqual(viewportForPane(1280, { cols: 80, imageRows: 30 }), { + w: 1280, + h: 960, + }); assert.deepEqual( viewportForPane(390, { cols: 80, imageRows: 30 }), { w: 390, h: 293 }, @@ -1459,7 +1494,11 @@ test("fitViewport sets the browser height once per pane geometry", async () => { assert.equal(await r.fitViewport(1280, 577), true); assert.deepEqual(calls, [[1280, 960]]); assert.deepEqual(r.lastImageDims, { w: 1280, h: 577 }); - assert.equal(await r.fitViewport(1280, 577), false, "same geometry is cached"); + assert.equal( + await r.fitViewport(1280, 577), + false, + "same geometry is cached", + ); assert.equal(calls.length, 1); r.lastViewportRequest = ""; // pane resize invalidates the cache @@ -1472,7 +1511,11 @@ test("fitViewport avoids resize churn when the frame already fits", async () => const r = quiet(mkRenderer()); r.size = () => ({ cols: 80, imageRows: 30 }); let calls = 0; - r.browser = { setViewport: async () => { calls++; } }; + r.browser = { + setViewport: async () => { + calls++; + }, + }; assert.equal(await r.fitViewport(1280, 959), false); assert.equal(calls, 0); }); From b8a1d0e1debd364ec6c8f7d104001bfc1d3e422f Mon Sep 17 00:00:00 2001 From: Victor Garcia Date: Tue, 28 Jul 2026 03:42:59 -0600 Subject: [PATCH 10/19] chore(browser): revert local formatting-only changes Restore the reviewed e857e5c source and test tree before applying the lifecycle fix. This preserves c0672b1 in forward history without publishing its unrelated formatting churn as part of the functional repair. --- bin/record.mjs | 56 ++------- tests/manifest.test.mjs | 5 +- tests/record.test.mjs | 143 ++++----------------- tests/renderer.test.mjs | 267 +++++++++++++++++----------------------- 4 files changed, 151 insertions(+), 320 deletions(-) diff --git a/bin/record.mjs b/bin/record.mjs index c3dd40e..d6c8083 100644 --- a/bin/record.mjs +++ b/bin/record.mjs @@ -70,8 +70,7 @@ function canonicalState(rawState) { } function assertSafeParent(root, target, kind) { - if (!isContained(root, target)) - throw new Error(`${kind} escapes plugin state`); + if (!isContained(root, target)) throw new Error(`${kind} escapes plugin state`); const parent = fs.realpathSync(path.dirname(target)); if (parent !== root && !isContained(root, parent)) throw new Error(`${kind} parent escapes plugin state`); @@ -167,9 +166,7 @@ export function readJsonFile(root, target, label, hooks = {}) { if (!before.isFile() || before.size > BigInt(JSON_LIMIT)) throw new Error(`${label} must be a bounded regular file`); hooks.afterOpen?.(); - const parsed = JSON.parse( - readDescriptor(fd, Number(before.size)).toString("utf8"), - ); + const parsed = JSON.parse(readDescriptor(fd, Number(before.size)).toString("utf8")); const after = descriptorStat(fd); if (!sameDescriptorGeneration(before, after)) throw new Error(`${label} changed while it was being read`); @@ -192,12 +189,8 @@ function runIdFromEnvironment() { const configRoot = fs.realpathSync(configDir); const source = path.join(configRoot, "run-id"); if (pathEntryExists(source)) { - const firstLine = readBoundedText( - configRoot, - source, - "run-id config", - 1024, - ).split(/\r?\n/, 1)[0]; + const firstLine = readBoundedText(configRoot, source, "run-id config", 1024) + .split(/\r?\n/, 1)[0]; if (firstLine) return validateRunId(firstLine); } } @@ -303,10 +296,7 @@ function assertExactKeys(value, keys, label) { throw new Error(`${label} must be an object`); const actual = Object.keys(value).sort(); const expected = [...keys].sort(); - if ( - actual.length !== expected.length || - actual.some((key, index) => key !== expected[index]) - ) + if (actual.length !== expected.length || actual.some((key, index) => key !== expected[index])) throw new Error(`${label} has incompatible fields`); } @@ -323,9 +313,7 @@ function assertIsoTimestamp(value, label) { function assertErrorField(value) { if ( value !== null && - (typeof value !== "string" || - value.length > 512 || - /[\u0000-\u001f\u007f]/.test(value)) + (typeof value !== "string" || value.length > 512 || /[\u0000-\u001f\u007f]/.test(value)) ) { throw new Error("evidence manifest error is invalid"); } @@ -334,13 +322,7 @@ function assertErrorField(value) { function validatePointer(pointer, identity) { assertExactKeys( pointer, - [ - "schema_version", - "run_id", - "workspace_id", - "browser_session", - "engine_stopped", - ], + ["schema_version", "run_id", "workspace_id", "browser_session", "engine_stopped"], "active recording pointer", ); if (pointer.schema_version !== SCHEMA_VERSION) @@ -377,11 +359,7 @@ function validateManifest(manifest, identity, expectedStatus) { ], "evidence manifest", ); - assertExactKeys( - manifest.plugin, - ["id", "version"], - "evidence manifest plugin", - ); + assertExactKeys(manifest.plugin, ["id", "version"], "evidence manifest plugin"); assertExactKeys( manifest.artifact, ["path", "media_type", "bytes", "sha256"], @@ -459,9 +437,7 @@ function validateLockOwner(owner) { owner.hostname !== os.hostname() || !LOCK_NONCE_RE.test(owner.nonce) ) { - throw new Error( - "recording lock owner is malformed or belongs to another host", - ); + throw new Error("recording lock owner is malformed or belongs to another host"); } assertIsoTimestamp(owner.started_at, "recording lock started_at"); } @@ -509,9 +485,7 @@ function acquireWorkspaceLock(root, workspaceId) { ); } if (!processIsProvenDead(owner.pid)) - throw new Error( - "another recording action is in progress for this workspace", - ); + throw new Error("another recording action is in progress for this workspace"); const tombstone = `${lock}.stale-${process.pid}-${crypto.randomBytes(6).toString("hex")}`; try { @@ -582,11 +556,7 @@ function privatizeArtifactIfPresent(root, target) { if (!stat.isFile()) throw new Error("recording artifact must be a regular file"); fs.fchmodSync(fd, 0o600); - assertPathMatchesDescriptor( - target, - descriptorStat(fd), - "recording artifact", - ); + assertPathMatchesDescriptor(target, descriptorStat(fd), "recording artifact"); } finally { fs.closeSync(fd); } @@ -726,9 +696,7 @@ function recoverComplete(root, paths, pointer, manifest) { artifact.bytes !== manifest.artifact.bytes || artifact.sha256 !== manifest.artifact.sha256 ) { - throw new Error( - "completed recording artifact no longer matches its manifest", - ); + throw new Error("completed recording artifact no longer matches its manifest"); } durableUnlink(paths.pointer, paths.runs); console.log( diff --git a/tests/manifest.test.mjs b/tests/manifest.test.mjs index cd61175..d26611a 100644 --- a/tests/manifest.test.mjs +++ b/tests/manifest.test.mjs @@ -44,10 +44,7 @@ test("Wave 0 drift contracts stay represented in source and hosted CI", () => { "utf8", ); assert.equal(renderer.includes(0), false); - assert.match( - renderer.toString("utf8"), - /stays passive until explicit pane input/, - ); + assert.match(renderer.toString("utf8"), /stays passive until explicit pane input/); assert.match(browsePane, /npm install -g carbonyl@next/); assert.match(workflow, /run: shellcheck scripts\/\*\.sh/); }); diff --git a/tests/record.test.mjs b/tests/record.test.mjs index 4c8c442..cccb815 100644 --- a/tests/record.test.mjs +++ b/tests/record.test.mjs @@ -305,13 +305,9 @@ function writeLockOwner(f, value) { const runs = path.join(f.state, "runs"); const lock = path.join(runs, ".record-workspace_1.lock"); fs.mkdirSync(lock, { recursive: true, mode: 0o700 }); - fs.writeFileSync( - path.join(lock, "owner.json"), - `${JSON.stringify(value)}\n`, - { - mode: 0o600, - }, - ); + fs.writeFileSync(path.join(lock, "owner.json"), `${JSON.stringify(value)}\n`, { + mode: 0o600, + }); return lock; } @@ -330,104 +326,32 @@ test("Stop rejects every incompatible pointer and manifest field before engine s ["pointer schema", "pointer", (value) => (value.schema_version = 999)], ["pointer unknown field", "pointer", (value) => (value.extra = true)], ["pointer run", "pointer", (value) => (value.run_id = "foreign-run")], - [ - "pointer workspace", - "pointer", - (value) => (value.workspace_id = "foreign"), - ], - [ - "pointer session", - "pointer", - (value) => (value.browser_session = "foreign"), - ], - [ - "pointer stop flag", - "pointer", - (value) => (value.engine_stopped = "false"), - ], + ["pointer workspace", "pointer", (value) => (value.workspace_id = "foreign")], + ["pointer session", "pointer", (value) => (value.browser_session = "foreign")], + ["pointer stop flag", "pointer", (value) => (value.engine_stopped = "false")], ["manifest schema", "manifest", (value) => (value.schema_version = 999)], ["manifest unknown field", "manifest", (value) => (value.extra = true)], - [ - "manifest evidence type", - "manifest", - (value) => (value.evidence_type = "not-browser"), - ], + ["manifest evidence type", "manifest", (value) => (value.evidence_type = "not-browser")], ["manifest run", "manifest", (value) => (value.run_id = "foreign-run")], ["plugin id", "manifest", (value) => (value.plugin.id = "foreign")], ["plugin version", "manifest", (value) => (value.plugin.version = "999")], - [ - "plugin unknown field", - "manifest", - (value) => (value.plugin.extra = true), - ], - [ - "manifest workspace", - "manifest", - (value) => (value.workspace_id = "foreign"), - ], - [ - "manifest session", - "manifest", - (value) => (value.browser_session = "foreign"), - ], + ["plugin unknown field", "manifest", (value) => (value.plugin.extra = true)], + ["manifest workspace", "manifest", (value) => (value.workspace_id = "foreign")], + ["manifest session", "manifest", (value) => (value.browser_session = "foreign")], ["manifest status", "manifest", (value) => (value.status = "failed")], - [ - "start timestamp", - "manifest", - (value) => (value.started_at = "yesterday"), - ], - [ - "premature completion timestamp", - "manifest", - (value) => (value.completed_at = new Date().toISOString()), - ], - [ - "context reset", - "manifest", - (value) => (value.recording_context_reset = false), - ], - [ - "artifact path", - "manifest", - (value) => (value.artifact.path = "../../outside"), - ], - [ - "artifact media", - "manifest", - (value) => (value.artifact.media_type = "text/plain"), - ], + ["start timestamp", "manifest", (value) => (value.started_at = "yesterday")], + ["premature completion timestamp", "manifest", (value) => (value.completed_at = new Date().toISOString())], + ["context reset", "manifest", (value) => (value.recording_context_reset = false)], + ["artifact path", "manifest", (value) => (value.artifact.path = "../../outside")], + ["artifact media", "manifest", (value) => (value.artifact.media_type = "text/plain")], ["artifact bytes", "manifest", (value) => (value.artifact.bytes = 14)], - [ - "artifact digest", - "manifest", - (value) => (value.artifact.sha256 = "a".repeat(64)), - ], - [ - "artifact unknown field", - "manifest", - (value) => (value.artifact.extra = true), - ], - [ - "review status", - "manifest", - (value) => (value.review.status = "reviewed"), - ], + ["artifact digest", "manifest", (value) => (value.artifact.sha256 = "a".repeat(64))], + ["artifact unknown field", "manifest", (value) => (value.artifact.extra = true)], + ["review status", "manifest", (value) => (value.review.status = "reviewed")], ["review required", "manifest", (value) => (value.review.required = false)], - [ - "review attestation", - "manifest", - (value) => (value.review.attestation = "self-attested"), - ], - [ - "review unknown field", - "manifest", - (value) => (value.review.extra = true), - ], - [ - "error type", - "manifest", - (value) => (value.error = { message: "hostile" }), - ], + ["review attestation", "manifest", (value) => (value.review.attestation = "self-attested")], + ["review unknown field", "manifest", (value) => (value.review.extra = true)], + ["error type", "manifest", (value) => (value.error = { message: "hostile" })], ]; for (const [name, target, mutate] of mutations) { @@ -465,10 +389,7 @@ test("one run ID is atomically claimed across different workspaces", async (t) = ]); assert.deepEqual([first.status, second.status].sort(), [0, 3]); const calls = fs.readFileSync(f.calls, "utf8").trim().split("\n"); - assert.equal( - calls.filter((line) => line.includes(" record start ")).length, - 1, - ); + assert.equal(calls.filter((line) => line.includes(" record start ")).length, 1); const pointers = fs .readdirSync(path.join(f.state, "runs")) .filter((name) => name.startsWith("active-")); @@ -476,10 +397,7 @@ test("one run ID is atomically claimed across different workspaces", async (t) = const b = bundle(f, "shared-run"); const manifest = json(b.manifest); assert.ok(["workspace_a", "workspace_b"].includes(manifest.workspace_id)); - assert.equal( - manifest.browser_session, - manifest.workspace_id === "workspace_a" ? "session-a" : "session-b", - ); + assert.equal(manifest.browser_session, manifest.workspace_id === "workspace_a" ? "session-a" : "session-b"); assert.equal(fs.readFileSync(b.artifact, "utf8"), "webm-test-data"); }); @@ -507,10 +425,7 @@ test("failed post-start validation compensates and only then records terminal fa }); test("failed compensation retains truthful retryable needs-attention state", (t) => { - const outside = path.join( - os.tmpdir(), - `herdr-outside-${crypto.randomUUID()}`, - ); + const outside = path.join(os.tmpdir(), `herdr-outside-${crypto.randomUUID()}`); const active = path.join(os.tmpdir(), `herdr-active-${crypto.randomUUID()}`); const f = fixture(t, { AB_ACTIVE: active, @@ -695,10 +610,7 @@ test("workspace locks fail closed for live and malformed owners", async (t) => { const result = invoke(f, "start"); assert.equal(result.status, 3); assert.match(result.stderr, /cannot be safely reclaimed/); - assert.match( - result.stderr, - /inspect .*\.record-workspace_1\.lock manually/, - ); + assert.match(result.stderr, /inspect .*\.record-workspace_1\.lock manually/); assert.equal(fs.existsSync(lock), true); assert.equal(fs.existsSync(f.calls), false); }); @@ -722,10 +634,7 @@ test("concurrent stale-lock reclamation still permits only one workspace action" ]); assert.deepEqual([first.status, second.status].sort(), [0, 3]); const calls = fs.readFileSync(f.calls, "utf8").trim().split("\n"); - assert.equal( - calls.filter((line) => line.includes(" record start ")).length, - 1, - ); + assert.equal(calls.filter((line) => line.includes(" record start ")).length, 1); const pointers = fs .readdirSync(path.join(f.state, "runs")) .filter((name) => name.startsWith("active-")); diff --git a/tests/renderer.test.mjs b/tests/renderer.test.mjs index 3777bda..718b393 100644 --- a/tests/renderer.test.mjs +++ b/tests/renderer.test.mjs @@ -1325,156 +1325,121 @@ test("header is repaint-gated but always paints real changes", () => { // --- Wave 2d: fleet-re-review regression tests --- -test("poll tick after a stream drop renders the fresh PNG, not the stale JPEG", async () => { - const r = quiet(mkRenderer()); - r.attached = true; - r.fitViewport = async () => false; - // Simulate a prior live stream: a jpg frame landed, then the stream died. - r.onStreamMessage({ - type: "frame", - data: jpeg(1280, 720).toString("base64"), - }); - await flush(); - assert.equal(r.shotFormat, "jpg"); - r.live = { ws: { close: () => {} } }; - r.dropLive(); - assert.equal( - r.shotFormat, - "png", - "dropLive points the renderer at the poll path frame", - ); - let renderedPath = null; - const origExists = fs.existsSync; - r.renderImage = async () => { - renderedPath = r.shotFormat === "jpg" ? r.shotJpg : r.shot; - }; - r.browser = { - snapshot: async (f) => { - fs.writeFileSync(f, PNG_1PX); - return { url: "https://x", title: "", entries: [] }; - }, - sessionExists: async () => true, - }; - await r.tick(); - assert.equal(r.shotFormat, "png"); - assert.equal( - renderedPath, - r.shot, - "poll frame rendered, not the frozen stream frame", - ); - assert.ok(origExists(r.shot)); -}); - -test("malformed stream messages never crash the dispatch", async () => { - const r = quiet(mkRenderer()); - const bad = [ - { type: "tabs", tabs: {} }, - { type: "tabs", tabs: 42 }, - { type: "tabs", tabs: "x" }, - { type: "frame", data: 12345 }, - { type: "frame" }, - { type: "console" }, - { type: "url" }, - {}, - { type: null }, - ]; - for (const m of bad) r.onStreamMessage(m); - await flush(); - assert.equal(r.frameSeq, 0); - // And the dispatch wrapper in goLive counts instead of throwing: - assert.ok(r.paintErrors >= 0); -}); - -test("a successful goLive from the attach path repaints the header", async () => { - const r = quiet(mkRenderer()); - let headers = 0; - const realHeader = Object.getPrototypeOf(r).header.bind(r); - r.header = () => { - headers++; - realHeader(); - }; - r.banner = "waiting for session …"; - r.goLive = async () => true; - r.browser = { sessionExists: async () => true }; - await r.tick(); - assert.equal(r.attached, true); - assert.ok( - headers >= 1, - "header repainted after goLive, stale banner cleared", - ); -}); - -test("goLive stays on the poll path for kitty mode without chafa", async () => { - const r = quiet(mkRenderer()); - r.mode = "kitty"; - r.chafa = false; - let enabled = 0; - r.browser = { - streamEnable: async () => { - enabled++; - }, - }; - assert.equal(await r.goLive(), false); - assert.equal( - enabled, - 0, - "no stream attempt when stream frames could not render", - ); -}); - -test("goLive rejects hostile or malformed ports without throwing", async () => { - const r = quiet(mkRenderer()); - for (const port of ["evil.com/x", 0, -1, 70000, 3.5, null, undefined]) { - r.browser = { - streamEnable: async () => {}, - streamStatus: async () => ({ port }), - }; - assert.equal( - await r.goLive(), - false, - `port ${JSON.stringify(port)} refused`, - ); - } -}); - -test("userAction surfaces failures in the banner (live mode has no tick report)", async () => { - const r = quiet(mkRenderer()); - r.attached = true; - r.live = { ws: { close: () => {} } }; - r.lastLiveCheck = Date.now(); - let headers = 0; - r.header = () => { - headers++; - }; - r.userAction(async () => { - throw new Error("daemon hung"); - }); - await flush(); - assert.match(r.banner, /command failed/); - assert.ok(headers >= 1, "failure banner painted"); -}); - -test("feed holds a split ESC-[ but a bare ESC still dispatches immediately", () => { - const r = quiet(mkRenderer()); - const keys = []; - r.onKey = (ch) => keys.push(ch); - r.feed("\x1b["); - assert.deepEqual(keys, [], "ESC-[ held for the next chunk"); - r.feed("<0;10;5M"); - assert.deepEqual(keys, [], "completed report went to the mouse path"); - r.feed("\x1b"); - // bare ESC reaches the key path at once (prompt cancel must not lag); - // onKey ignores it when no prompt is open and it is not a mapped key. - assert.equal(r.inputBuf, ""); +test('poll tick after a stream drop renders the fresh PNG, not the stale JPEG', async () => { + const r = quiet(mkRenderer()); + r.attached = true; + r.fitViewport = async () => false; + // Simulate a prior live stream: a jpg frame landed, then the stream died. + r.onStreamMessage({ type: 'frame', data: jpeg(1280, 720).toString('base64') }); + await flush(); + assert.equal(r.shotFormat, 'jpg'); + r.live = { ws: { close: () => {} } }; + r.dropLive(); + assert.equal(r.shotFormat, 'png', 'dropLive points the renderer at the poll path frame'); + let renderedPath = null; + const origExists = fs.existsSync; + r.renderImage = async () => { renderedPath = r.shotFormat === 'jpg' ? r.shotJpg : r.shot; }; + r.browser = { + snapshot: async f => { + fs.writeFileSync(f, PNG_1PX); + return { url: 'https://x', title: '', entries: [] }; + }, + sessionExists: async () => true, + }; + await r.tick(); + assert.equal(r.shotFormat, 'png'); + assert.equal(renderedPath, r.shot, 'poll frame rendered, not the frozen stream frame'); + assert.ok(origExists(r.shot)); +}); + +test('malformed stream messages never crash the dispatch', async () => { + const r = quiet(mkRenderer()); + const bad = [ + { type: 'tabs', tabs: {} }, + { type: 'tabs', tabs: 42 }, + { type: 'tabs', tabs: 'x' }, + { type: 'frame', data: 12345 }, + { type: 'frame' }, + { type: 'console' }, + { type: 'url' }, + {}, + { type: null }, + ]; + for (const m of bad) r.onStreamMessage(m); + await flush(); + assert.equal(r.frameSeq, 0); + // And the dispatch wrapper in goLive counts instead of throwing: + assert.ok(r.paintErrors >= 0); +}); + +test('a successful goLive from the attach path repaints the header', async () => { + const r = quiet(mkRenderer()); + let headers = 0; + const realHeader = Object.getPrototypeOf(r).header.bind(r); + r.header = () => { headers++; realHeader(); }; + r.banner = 'waiting for session …'; + r.goLive = async () => true; + r.browser = { sessionExists: async () => true }; + await r.tick(); + assert.equal(r.attached, true); + assert.ok(headers >= 1, 'header repainted after goLive, stale banner cleared'); +}); + +test('goLive stays on the poll path for kitty mode without chafa', async () => { + const r = quiet(mkRenderer()); + r.mode = 'kitty'; + r.chafa = false; + let enabled = 0; + r.browser = { streamEnable: async () => { enabled++; } }; + assert.equal(await r.goLive(), false); + assert.equal(enabled, 0, 'no stream attempt when stream frames could not render'); +}); + +test('goLive rejects hostile or malformed ports without throwing', async () => { + const r = quiet(mkRenderer()); + for (const port of ['evil.com/x', 0, -1, 70000, 3.5, null, undefined]) { + r.browser = { + streamEnable: async () => {}, + streamStatus: async () => ({ port }), + }; + assert.equal(await r.goLive(), false, `port ${JSON.stringify(port)} refused`); + } +}); + +test('userAction surfaces failures in the banner (live mode has no tick report)', async () => { + const r = quiet(mkRenderer()); + r.attached = true; + r.live = { ws: { close: () => {} } }; + r.lastLiveCheck = Date.now(); + let headers = 0; + r.header = () => { headers++; }; + r.userAction(async () => { throw new Error('daemon hung'); }); + await flush(); + assert.match(r.banner, /command failed/); + assert.ok(headers >= 1, 'failure banner painted'); +}); + +test('feed holds a split ESC-[ but a bare ESC still dispatches immediately', () => { + const r = quiet(mkRenderer()); + const keys = []; + r.onKey = ch => keys.push(ch); + r.feed('\x1b['); + assert.deepEqual(keys, [], 'ESC-[ held for the next chunk'); + r.feed('<0;10;5M'); + assert.deepEqual(keys, [], 'completed report went to the mouse path'); + r.feed('\x1b'); + // bare ESC reaches the key path at once (prompt cancel must not lag); + // onKey ignores it when no prompt is open and it is not a mapped key. + assert.equal(r.inputBuf, ''); }); // --- Pane viewport fitting --- test("viewportForPane preserves width and fills the image-cell aspect", () => { - assert.deepEqual(viewportForPane(1280, { cols: 80, imageRows: 30 }), { - w: 1280, - h: 960, - }); + assert.deepEqual( + viewportForPane(1280, { cols: 80, imageRows: 30 }), + { w: 1280, h: 960 }, + ); assert.deepEqual( viewportForPane(390, { cols: 80, imageRows: 30 }), { w: 390, h: 293 }, @@ -1494,11 +1459,7 @@ test("fitViewport sets the browser height once per pane geometry", async () => { assert.equal(await r.fitViewport(1280, 577), true); assert.deepEqual(calls, [[1280, 960]]); assert.deepEqual(r.lastImageDims, { w: 1280, h: 577 }); - assert.equal( - await r.fitViewport(1280, 577), - false, - "same geometry is cached", - ); + assert.equal(await r.fitViewport(1280, 577), false, "same geometry is cached"); assert.equal(calls.length, 1); r.lastViewportRequest = ""; // pane resize invalidates the cache @@ -1511,11 +1472,7 @@ test("fitViewport avoids resize churn when the frame already fits", async () => const r = quiet(mkRenderer()); r.size = () => ({ cols: 80, imageRows: 30 }); let calls = 0; - r.browser = { - setViewport: async () => { - calls++; - }, - }; + r.browser = { setViewport: async () => { calls++; } }; assert.equal(await r.fitViewport(1280, 959), false); assert.equal(calls, 0); }); From 8ccc8b8764ad269ba4e7c4468bff3a0f21dde4e0 Mon Sep 17 00:00:00 2001 From: Victor Garcia Date: Tue, 28 Jul 2026 03:51:17 -0600 Subject: [PATCH 11/19] fix(recording): close confirmed stop publication window Journal a stop-pending state before invoking the non-idempotent engine and refuse automatic recovery when an interrupted outcome is unknown. Once Stop succeeds in-process, continue artifact validation, manifest finalization, and durable pointer removal even if the intermediate stopped-pointer publication fails. Apply the same transition to successful Start compensation and cover write, fsync, and rename faults with a non-idempotent recorder stub. --- README.md | 6 ++- bin/record.mjs | 115 +++++++++++++++++++++++++++++++++++---- tests/record.test.mjs | 122 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 231 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 5252213..d24e5e6 100644 --- a/README.md +++ b/README.md @@ -250,7 +250,11 @@ The bundle is an **unreviewed, operator-reviewable observation**, not a test result, acceptance decision, provenance claim, or cryptographic attestation. Its digest detects later content changes but does not identify who recorded or reviewed it. A missing, empty, non-regular, symlinked, or oversized WebM is not -marked complete. Failed stops retain the active pointer so Stop can be retried. +marked complete. A confirmed failed Stop retains a retryable active pointer. +Before invoking Stop, Browser durably marks the attempt pending; if the process +is interrupted while its outcome is unknown, later Stop actions fail closed +without calling the non-idempotent engine again or marking evidence complete. +That pending pointer remains occupied for manual inspection and reconciliation. Starting a recording creates a fresh browser context: the page reloads, while cookies and localStorage are preserved. Start recording before the flow you diff --git a/bin/record.mjs b/bin/record.mjs index d6c8083..7fe167c 100644 --- a/bin/record.mjs +++ b/bin/record.mjs @@ -322,7 +322,14 @@ function assertErrorField(value) { function validatePointer(pointer, identity) { assertExactKeys( pointer, - ["schema_version", "run_id", "workspace_id", "browser_session", "engine_stopped"], + [ + "schema_version", + "run_id", + "workspace_id", + "browser_session", + "engine_stopped", + "stop_pending", + ], "active recording pointer", ); if (pointer.schema_version !== SCHEMA_VERSION) @@ -333,7 +340,9 @@ function validatePointer(pointer, identity) { pointer.workspace_id !== identity.workspaceId || typeof pointer.browser_session !== "string" || !pointer.browser_session || - typeof pointer.engine_stopped !== "boolean" + typeof pointer.engine_stopped !== "boolean" || + typeof pointer.stop_pending !== "boolean" || + (pointer.engine_stopped && pointer.stop_pending) ) { throw new Error("active recording pointer identity is invalid"); } @@ -542,6 +551,7 @@ function pointerFor(runId, workspaceId, session) { workspace_id: workspaceId, browser_session: session, engine_stopped: false, + stop_pending: false, }; } @@ -661,23 +671,41 @@ function start() { } catch (startError) { let compensationError; try { + pointer.stop_pending = true; + atomicWriteJson(root, paths.pointer, pointer); engine(session, ["stop"]); } catch (error) { compensationError = error; } if (compensationError) { + pointer.stop_pending = false; + let retryStateError; + try { + atomicWriteJson(root, paths.pointer, pointer); + } catch (error) { + retryStateError = error; + } const attention = new Error( - `recording start needs attention: ${safeError(startError)}; compensating stop failed: ${safeError(compensationError)}`, + `recording start needs attention: ${safeError(startError)}; compensating stop was not confirmed: ${safeError(compensationError)}${ + retryStateError + ? `; retry state could not be published: ${safeError(retryStateError)}; inspect the recording manually` + : "" + }`, ); try { recordRetryError(root, paths.manifest, manifest, attention); } catch { - // The durable pre-engine journal and pointer remain retryable. + // The durable pre-engine journal and pointer remain conservative. } throw attention; } pointer.engine_stopped = true; - atomicWriteJson(root, paths.pointer, pointer); + pointer.stop_pending = false; + try { + atomicWriteJson(root, paths.pointer, pointer); + } catch { + // Stop succeeded here; terminal publication and pointer removal continue. + } manifest.status = "failed"; manifest.error = safeError(startError); atomicWriteJson(root, paths.manifest, manifest); @@ -728,17 +756,82 @@ function stop() { } validateManifest(manifest, identity, "recording"); - let artifact; - try { - if (!pointer.engine_stopped) { + if (pointer.stop_pending) { + const interrupted = new Error( + "recording stop outcome is unknown after an interrupted stop; no engine action or evidence completion was attempted; inspect the recording manually", + ); + try { + recordRetryError(root, paths.manifest, manifest, interrupted); + } catch { + // The stop-pending pointer remains the authoritative manual state. + } + throw interrupted; + } + + let stoppedPublicationError; + if (!pointer.engine_stopped) { + pointer.stop_pending = true; + try { + atomicWriteJson(root, paths.pointer, pointer); + } catch (error) { + pointer.stop_pending = false; + let retryStateError; + try { + atomicWriteJson(root, paths.pointer, pointer); + } catch (stateError) { + retryStateError = stateError; + } + const publicationError = retryStateError + ? new Error( + `recording stop did not start because its pending state could not be published: ${safeError(error)}; retry state could not be restored: ${safeError(retryStateError)}; inspect the recording manually`, + ) + : error; + recordRetryError(root, paths.manifest, manifest, publicationError); + throw publicationError; + } + try { engine(pointer.browser_session, ["stop"]); - pointer.engine_stopped = true; + } catch (error) { + pointer.stop_pending = false; + let retryStateError; + try { + atomicWriteJson(root, paths.pointer, pointer); + } catch (stateError) { + retryStateError = stateError; + } + const stopError = retryStateError + ? new Error( + `recording stop was not confirmed: ${safeError(error)}; retry state could not be published: ${safeError(retryStateError)}; inspect the recording manually`, + ) + : error; + recordRetryError(root, paths.manifest, manifest, stopError); + throw stopError; + } + pointer.engine_stopped = true; + pointer.stop_pending = false; + try { atomicWriteJson(root, paths.pointer, pointer); + } catch (error) { + stoppedPublicationError = error; } + } + + let artifact; + try { artifact = inspectArtifact(root, paths.artifact); } catch (error) { - recordRetryError(root, paths.manifest, manifest, error); - throw error; + let retryError = error; + if (stoppedPublicationError) { + try { + atomicWriteJson(root, paths.pointer, pointer); + } catch (stateError) { + retryError = new Error( + `recording engine stopped, but its retry state could not be published: ${safeError(stateError)}; inspect the recording manually`, + ); + } + } + recordRetryError(root, paths.manifest, manifest, retryError); + throw retryError; } manifest.status = "complete"; diff --git a/tests/record.test.mjs b/tests/record.test.mjs index cccb815..d54e358 100644 --- a/tests/record.test.mjs +++ b/tests/record.test.mjs @@ -44,6 +44,10 @@ if [ "$4" = "start" ]; then [ -n "${"$"}{AB_ACTIVE:-}" ] && printf active > "$AB_ACTIVE" else [ "${"$"}{AB_STOP_FAIL:-0}" = 1 ] && exit 8 + if [ "${"$"}{AB_NON_IDEMPOTENT_STOP:-0}" = 1 ] && [ -n "${"$"}{AB_ACTIVE:-}" ] && [ ! -e "$AB_ACTIVE" ]; then + printf 'No recording in progress\n' >&2 + exit 10 + fi [ -n "${"$"}{AB_ACTIVE:-}" ] && rm -f "$AB_ACTIVE" fi exit 0 @@ -329,6 +333,15 @@ test("Stop rejects every incompatible pointer and manifest field before engine s ["pointer workspace", "pointer", (value) => (value.workspace_id = "foreign")], ["pointer session", "pointer", (value) => (value.browser_session = "foreign")], ["pointer stop flag", "pointer", (value) => (value.engine_stopped = "false")], + ["pointer pending flag", "pointer", (value) => (value.stop_pending = "false")], + [ + "pointer contradictory stop state", + "pointer", + (value) => { + value.engine_stopped = true; + value.stop_pending = true; + }, + ], ["manifest schema", "manifest", (value) => (value.schema_version = 999)], ["manifest unknown field", "manifest", (value) => (value.extra = true)], ["manifest evidence type", "manifest", (value) => (value.evidence_type = "not-browser")], @@ -463,6 +476,16 @@ function withMainEnvironment(f, callback) { HERDR_PLUGIN_CONFIG_DIR: f.config, AB_CALLS: f.calls, }; + for (const key of [ + "AB_ACTIVE", + "AB_ARTIFACT_MODE", + "AB_NON_IDEMPOTENT_STOP", + "AB_OUTSIDE", + "AB_START_FAIL", + "AB_STOP_FAIL", + ]) { + if (f.env[key] !== undefined) values[key] = f.env[key]; + } const previous = Object.fromEntries( Object.keys(values).map((key) => [key, process.env[key]]), ); @@ -524,6 +547,105 @@ test("manifest and pointer write/fsync faults occur before the engine can start" } }); +function failFirstOperationAfterConfirmedStop(f, method, callback) { + const original = fs[method]; + let injected = false; + fs[method] = (...args) => { + const stopped = + !fs.existsSync(f.env.AB_ACTIVE) && + fs.existsSync(f.calls) && + fs.readFileSync(f.calls, "utf8").includes(" record stop"); + if (!injected && stopped) { + injected = true; + const error = new Error(`injected post-stop ${method} failure`); + error.code = "EIO"; + throw error; + } + return original(...args); + }; + try { + callback(); + } finally { + fs[method] = original; + } + assert.equal(injected, true, `${method} fault was not injected`); +} + +function countStopCalls(f) { + return fs + .readFileSync(f.calls, "utf8") + .trim() + .split("\n") + .filter((line) => line.endsWith(" record stop")).length; +} + +test("confirmed Stop finalizes through stopped-pointer publication faults", async (t) => { + for (const method of ["writeFileSync", "fsyncSync", "renameSync"]) { + await t.test(method, (st) => { + const f = fixture(st, { AB_NON_IDEMPOTENT_STOP: "1" }); + const active = path.join(f.base, `active-${method}`); + f.env.AB_ACTIVE = active; + assert.equal(invoke(f, "start").status, 0); + failFirstOperationAfterConfirmedStop(f, method, () => + withMainEnvironment(f, () => main("stop")), + ); + const b = bundle(f); + assert.equal(json(b.manifest).status, "complete"); + assert.equal(fs.existsSync(b.pointer), false); + assert.equal(fs.existsSync(active), false); + assert.equal(countStopCalls(f), 1); + }); + } +}); + +test("confirmed Start compensation finalizes through stopped-pointer publication faults", async (t) => { + for (const method of ["writeFileSync", "fsyncSync", "renameSync"]) { + await t.test(method, (st) => { + const f = fixture(st, { + AB_ARTIFACT_MODE: "symlink", + AB_NON_IDEMPOTENT_STOP: "1", + }); + const active = path.join(f.base, `active-${method}`); + const outside = path.join(f.base, `outside-${method}`); + fs.writeFileSync(outside, "outside", { mode: 0o644 }); + f.env.AB_ACTIVE = active; + f.env.AB_OUTSIDE = outside; + failFirstOperationAfterConfirmedStop(f, method, () => + assert.throws( + () => withMainEnvironment(f, () => main("start")), + /ELOOP|recording artifact must be a regular file/, + ), + ); + const b = bundle(f); + assert.equal(json(b.manifest).status, "failed"); + assert.equal(fs.existsSync(b.pointer), false); + assert.equal(fs.existsSync(active), false); + assert.equal(countStopCalls(f), 1); + }); + } +}); + +test("interrupted stop state fails closed without inferring engine success", (t) => { + const f = fixture(t, { AB_NON_IDEMPOTENT_STOP: "1" }); + const active = path.join(f.base, "active-interrupted"); + f.env.AB_ACTIVE = active; + assert.equal(invoke(f, "start").status, 0); + const b = bundle(f); + const pointer = json(b.pointer); + pointer.stop_pending = true; + fs.writeFileSync(b.pointer, `${JSON.stringify(pointer, null, 2)}\n`); + fs.rmSync(active); + const calls = fs.readFileSync(f.calls); + + const retry = invoke(f, "stop"); + assert.equal(retry.status, 3); + assert.match(retry.stderr, /outcome is unknown/); + assert.equal(json(b.manifest).status, "recording"); + assert.match(json(b.manifest).error, /inspect the recording manually/); + assert.equal(fs.existsSync(b.pointer), true); + assert.deepEqual(fs.readFileSync(f.calls), calls); +}); + test("complete-manifest crash recovery verifies the artifact and unlinks without stopping twice", (t) => { const f = fixture(t); assert.equal(invoke(f, "start").status, 0); From b09a56781af17db507d4b86c9cbd0489dcb76e73 Mon Sep 17 00:00:00 2001 From: Victor Garcia Date: Tue, 4 Aug 2026 16:54:17 -0600 Subject: [PATCH 12/19] feat(network): pure diff and formatting helpers for failed requests --- bin/renderer.mjs | 83 ++++++++ ...-001-feat-network-failures-console-plan.md | 186 ++++++++++++++++++ tests/renderer.test.mjs | 135 +++++++++++++ 3 files changed, 404 insertions(+) create mode 100644 docs/plans/2026-08-03-001-feat-network-failures-console-plan.md diff --git a/bin/renderer.mjs b/bin/renderer.mjs index f912c36..1d39c7b 100644 --- a/bin/renderer.mjs +++ b/bin/renderer.mjs @@ -312,6 +312,89 @@ export function viewportForPane(frameWidth, { cols, imageRows }) { return { w, h }; } +// Failed-request diffing over `network requests --json`. The daemon's log is +// append-only from the pane's perspective but can be wiped without notice +// (browser relaunch, external --clear), and pid.N-format requestIds can +// restart after a relaunch — so membership sets are pruned to the ids present +// in the current log every poll instead of trusting counts or id uniqueness. +// Seen-set membership means "already reported OR resolved to a non-failure +// status": an in-flight request must stay classifiable until its status +// lands, or a 500 arriving one poll late would be swallowed forever. +export function newNetworkState() { + return { + seen: new Set(), // requestIds reported or resolved 2xx/3xx + pending: new Set(), // requestIds observed with no status yet + recent: new Map(), // dedupe key -> last emit/suppress time (ms) + }; +} + +export function diffNetworkFailures(state, entries, nowMs, opts = {}) { + const ageThresholdMs = opts.ageThresholdMs ?? 15_000; + const dedupeWindowMs = opts.dedupeWindowMs ?? 60_000; + const maxPerPoll = opts.maxPerPoll ?? 5; + const currentIds = new Set(); + const candidates = []; + for (const e of entries) { + const id = e?.requestId; + if (typeof id !== "string" || !id) continue; + currentIds.add(id); + if (opts.baseline) { + state.seen.add(id); + continue; + } + if (state.seen.has(id)) continue; + const status = typeof e.status === "number" ? e.status : null; + if (status !== null) { + state.pending.delete(id); + state.seen.add(id); + if (status >= 400 && status <= 599) + candidates.push({ method: e.method ?? "GET", url: e.url ?? "", status }); + continue; + } + state.pending.add(id); + // No error detail exists anywhere for connection-level failures (the + // daemon drops loadingFailed), so age past threshold is the only signal + // — and it must be entry age, not poll count: pollDelay stretches ticks. + if (typeof e.timestamp === "number" && nowMs - e.timestamp > ageThresholdMs) { + state.pending.delete(id); + state.seen.add(id); + candidates.push({ + method: e.method ?? "GET", + url: e.url ?? "", + status: null, + }); + } + } + // Prune both sets to the live log so wipes and id reuse stay harmless. + for (const id of state.seen) if (!currentIds.has(id)) state.seen.delete(id); + for (const id of state.pending) + if (!currentIds.has(id)) state.pending.delete(id); + for (const [k, t] of state.recent) + if (nowMs - t > dedupeWindowMs) state.recent.delete(k); + // Chrome retries a failed navigation with fresh requestIds and app retry + // loops mint new ids per attempt — dedupe by shape, refreshing the window + // on suppressed hits so a steady loop paints once, not once per minute. + const failures = []; + let overflow = 0; + for (const c of candidates) { + const key = `${c.method} ${c.url} ${c.status ?? "no response"}`; + const last = state.recent.get(key); + state.recent.set(key, nowMs); + if (last !== undefined && nowMs - last <= dedupeWindowMs) continue; + if (failures.length < maxPerPoll) failures.push(c); + else overflow++; + } + return { failures, overflow }; +} + +// Display text for one failure ("✖ " comes from pushConsole's error prefix). +// URLs are page-controlled and unbounded (data: URLs carry payloads) — +// sanitize and hard-cap before the line enters the 500-line store. +export function formatNetworkFailure({ method, url, status }) { + const shownUrl = truncate(sanitizeText(url), 200); + return `${status ?? "no response"} ${method} ${shownUrl}`; +} + // --- agent-browser access --- export function makeBrowser(session, bin = "agent-browser") { diff --git a/docs/plans/2026-08-03-001-feat-network-failures-console-plan.md b/docs/plans/2026-08-03-001-feat-network-failures-console-plan.md new file mode 100644 index 0000000..3247026 --- /dev/null +++ b/docs/plans/2026-08-03-001-feat-network-failures-console-plan.md @@ -0,0 +1,186 @@ +--- +title: "feat: Surface failed network requests in the console region" +type: feat +date: 2026-08-03 +--- + +# feat: Surface failed network requests in the console region + +## Summary + +Closes issue #1: failed network requests (4xx/5xx and no-response failures) appear in the pane's console region with the `✖ ` error prefix, in both poll and live-stream modes. Implemented as a requestId-set diff over `agent-browser network requests --json` — agent-browser 0.33.2's push stream carries no network event, so polling is the only path. + +## Problem Frame + +The console region shows console API output and page errors but never failed network requests — a 404 or connection-refused fetch is visible in `agent-browser network requests` yet invisible in the pane. The data already exists in the shared daemon session; it just isn't painted. The pane's core audience (localhost dev debugging) hits this constantly. + +--- + +## Requirements + +**Display** + +- R1. HTTP failures (status 400–599) on xhr/fetch/document requests appear in the console region as `✖ ` within one poll interval. +- R2. Connection-level failures (entries whose status never arrives) appear as `✖ no response ` once the entry is older than the age threshold. +- R3. Failure lines appear in live-stream mode too, via a low-cadence dedicated poll, since the WebSocket stream has no network event type. + +**Passivity and safety** + +- R4. The pane issues no network call before `attached` is true, never passes `--clear`, and never causes session auto-creation — the existing passivity contract holds unchanged. +- R5. URLs are display-sanitized (`sanitizeText`) and hard-truncated before storage; failure lines flow through `pushConsole` so the 500-line cap, lazy region open, and repaint gating keep working. + +**Noise control** + +- R6. Attaching to a session with existing failures in the log produces no replay wall — the first read seeds a silent baseline. Re-attach after session death re-baselines. +- R7. Repeated identical failures (Chrome nav retries, app retry loops) are deduped, and per-poll output is capped with a `…and N more failed requests` summary line. + +**Degradation** + +- R8. A failing or unsupported `network` subcommand degrades only network reporting — screenshots and console keep painting, and older agent-browser versions without the command leave the pane fully functional. +- R9. A failed navigation the pane itself initiated still surfaces (the baseline must not swallow the user's own 404). + +--- + +## Key Technical Decisions + +- **Poll-diff, not stream.** Verified against the vercel-labs/agent-browser 0.33.2 source: the per-session WebSocket emits only `url`, `frame`, `console`, `page_error`, `tabs`. Issue approach 1 (consume a network stream event) is impossible today; a follow-up upstream feature request is deferred work. +- **Separate exec, not the snapshot batch.** The snapshot batch uses `--bail` and throws on a short result array — an agent-browser version lacking the `network` subcommand would brick every tick (no frames, no console) if the command joined the batch. A separate `network requests --type xhr,fetch,document --json` exec degrades independently, and live mode needs a standalone call anyway. `--type` also bounds payload (spike observed `data:` URLs with full base64 payloads in the log; `maxBuffer` is 16 MiB). +- **Optional duck-typed `network()` method on `makeBrowser`.** Every call site guards `typeof this.browser.network === "function"` — the `streamEnable` precedent (`bin/renderer.mjs` `goLive()`) — so the dozens of existing object-literal test fakes stay green. +- **Diff by requestId with prune-to-current-log, not count-based reconcile.** A live spike on 0.33.2 observed the request log wiped without `--clear` (browser relaunch after a failed navigation recreated the page target), so `reconcileConsole`-style count/tail matching is unsafe here. Seen-set membership = ids already reported OR resolved to a non-failure status (2xx/3xx); unreported null-status ids live only in a pending-candidates state and are reclassified every poll — report immediately when status lands at 400–599, drop at 200–399, report as "no response" past the age threshold. This closes the slow-failure hole where a request observed in-flight at poll N would swallow its 500 arriving at poll N+1. Both sets are pruned to ids present in the current log (bounds growth; `pid.N`-format requestIds can restart after relaunch, so pruning makes stale collisions harmless). +- **"Failed" = status 400–599, plus null-status entries older than 15 s.** Spike-verified: entries carry a ms-epoch `timestamp`, and null-status entries have no error field anywhere (the daemon drops `Network.loadingFailed` detail), so lines can only say "no response". Age-based detection beats poll-count heuristics because `pollDelay` backoff stretches ticks up to 30×. The `--type xhr,fetch,document` filter excludes EventSource/WebSocket, whose held-open connections would otherwise false-positive every localhost HMR/SSE stream. +- **Silent baseline on attach — a deliberate divergence from console behavior.** The console region replays full history on attach (`reconcileConsole` from `count: 0`); the network feed starts silent instead, because stale failures from an hours-old agent session are noise, not signal. Re-baseline everywhere `attached` flips. On the pane's own `navigate()` path (which sets `attached` directly), split by the existing `existed` check: if the session already exists, run a real baseline read before `open` (safe — no auto-creation); if it does not exist yet, seed state-only (empty seen-set, baseline marked consumed, **no CLI call** — a pre-`open` read would auto-create a session and break the ownership invariant). R9 holds on the fresh-session path because streaming daemons arm request tracking at spawn, so the navigation's own failure is in the log for the first post-attach poll. +- **One shared poll function for both modes, with an in-flight guard.** Poll mode calls it from `tick()` after a successful snapshot; live mode runs it on its own timer with `pollDelay`-style backoff. A single guard prevents the dropLive-transition race where a timer poll and a tick poll diff the same seen-set concurrently. The live timer is cleared as the first statement of `dropLive()` and each firing checks `this.live && this.attached` before executing (R4). +- **Zero new dependencies, Node 20 floor.** Repo policy — only `node:` builtins, `node --test` runner. + +--- + +## High-Level Technical Design + +Directional guidance, not implementation specification. + +```mermaid +flowchart TB + subgraph shared [Shared] + PN[pollNetwork - in-flight guarded] + DIFF[diffNetworkFailures - pure, exported] + PC[pushConsole with error prefix] + PN --> DIFF --> PC + end + TICK[poll mode: tick after snapshot ok] --> PN + TIMER[live mode: timer with backoff] --> PN + ATTACH[attached flips true] -->|reset seen-set, set baseline-pending| PN + DROP[dropLive] -->|clear timer first| TIMER + NAV[navigate: real baseline read if session existed, state-only seed if creating] --> ATTACH +``` + +Line format: `✖ 404 GET https://api.example.com/users` / `✖ no response GET http://localhost:3000/api` — URL sanitized and truncated to ~200 chars before storage; per-poll overflow collapses into `✖ …and N more failed requests`. + +--- + +## Implementation Units + +### U1. Pure diff and formatting helpers + +- **Goal:** The diff/classification/dedupe/format logic exists as exported pure functions, fully unit-testable without a Renderer. +- **Requirements:** R1, R2, R6, R7 +- **Dependencies:** none +- **Files:** `bin/renderer.mjs` (helpers band), `tests/renderer.test.mjs` +- **Approach:** A `diffNetworkFailures(state, entries, nowMs, opts)`-shaped function taking the previous state (seen-set of reported-or-resolved-OK ids, pending null-status candidates, recent-failure memory) and the current entry list, returning new failure descriptors plus the next state (both id sets pruned to the current log). Classification per entry: status 400–599 → report unless already reported; status 200–399 → move to seen, never report; null status → pending, reported as "no response" only when `nowMs - timestamp > ageThreshold` (default 15 000 ms), and reclassified on every poll so a late-arriving status wins. Dedupe by `method + url + (status ?? "no response")`: within the returned batch, and across polls via a 60 s suppression window refreshed on each suppressed hit (a steady retry loop paints once, not every poll). Cap output at N (~5) per poll with an overflow count. A small formatter builds the display line (sanitize + truncate URL ~200 chars). +- **Patterns to follow:** `reconcileConsole` / `pollDelay` — exported pure helpers in the labeled helpers band; tabs, double quotes, why-not-what comments naming the failure mode. +- **Test scenarios:** + - New 404 entry → one `✖ 404 GET ` descriptor; same entry next poll → nothing (seen). + - Entry with null status and age 5 s → nothing; same entry at 20 s → `no response` descriptor; entry that gains status 200 before threshold → never reported. + - Entry with null status at poll 1, status 500 at poll 2 → exactly one `✖ 500` descriptor, at poll 2 (late-arriving failure status is not swallowed by the seen-set). + - Same failing `method + url + status` key on 5 consecutive polls → one line total (60 s cross-poll suppression window). + - Log wipe (current entries no longer contain seen ids) → seen-set pruned, no replay of surviving-but-already-seen ids re-added later with same id. + - Chrome nav retry shape: 3 entries, distinct requestIds, same URL + null status → one line, not three. + - 12 distinct failures in one poll with cap 5 → 5 lines + `…and 7 more failed requests`. + - Baseline call (`state` empty, `baseline: true` or equivalent) → zero descriptors, seen-set populated with all current ids. + - URL with ANSI escape bytes and 5 000 chars → sanitized, truncated to ~200 chars. +- **Verification:** `npm test` green; every scenario above has a direct assertion on the returned descriptors/state. + +### U2. `network()` method on the browser wrapper + +- **Goal:** `makeBrowser` exposes an optional `network()` returning the parsed request list, with the CLI contract pinned by stub tests. +- **Requirements:** R4, R8 +- **Dependencies:** none +- **Files:** `bin/renderer.mjs` (agent-browser access band), `tests/renderer.test.mjs` +- **Approach:** `network: async () => run("network", "requests", "--type", "xhr,fetch,document")`, normalizing the result to an array. No `--clear`, ever. +- **Patterns to follow:** the `streamEnable`/`run` wrappers; bash-stub CLI-contract tests in the existing batch-shape test style. +- **Test scenarios:** + - Stub returns `{requests: [...]}` → method resolves to the array; malformed JSON → rejects (caller degrades). + - Stub logs argv → assert exact args include `--type xhr,fetch,document` and `--json`, and assert `--clear` never appears (extend the existing never-destructive contract test). +- **Verification:** `npm test` green; argv assertions pass against the stub. + +### U3. Poll-mode integration in `tick()` + +- **Goal:** Poll mode paints failure lines; a broken network poll never affects frame/console painting. +- **Requirements:** R1, R2, R4, R5, R6, R8, R9 +- **Dependencies:** U1, U2 +- **Files:** `bin/renderer.mjs` (Renderer: constructor state, `tick()`, attach transition, `navigate()`), `tests/renderer.test.mjs` +- **Approach:** New constructor state (`networkState`, baseline-pending flag) following the `` naming convention. In `tick()`, after the snapshot succeeds, run the shared poll: duck-type guard, own try/catch that swallows into a counter (no banner — the feature is best-effort), baseline consumed on first read after any `attached` flip. On a maxBuffer/timeout-class failure from `network()`, stop polling for the rest of the attach and push a one-time `✖ network reporting off — request log too large` line — the daemon log is unbounded (verified: append-only Vec, `--clear` is the only eviction), so retrying a known-fatal multi-MiB exec every tick is pure waste. Push descriptors via `pushConsole([{text, type: "error"}], false)` so `consolePushes`/`sig()` backoff-reset work unchanged; use the tick's existing console layout dance for the first-line region open. In `navigate()`, split baseline seeding by the `existed` check per the KTD: real read before `open` when the session existed, state-only seed (no CLI call) when the pane is creating it. +- **Patterns to follow:** `tick()`'s swallow-and-degrade error idiom (counter, no unguarded paths); `suppressConsoleOnce` one-shot consumption shape (but a separate flag — do not reuse the console's). +- **Test scenarios:** + - Fake browser with `network` returning a 404 entry → after two ticks (baseline, then report… first tick baselines silently, entry present at baseline is NOT reported; a new failing id on tick 2 → `✖ 404` line appears in `consoleLines`). + - Fake browser without a `network` method (existing object-literal fakes) → all current tick tests pass unmodified. + - `network()` rejecting every tick → screenshots/console still paint, no banner, no uncaught rejection. + - Session death → `attached` false → recreation → re-attach → failures from before re-attach not replayed; new ones are. + - `network()` rejecting with a maxBuffer/timeout-class error → polling stops for the rest of the attach, exactly one `request log too large` line; a fresh re-attach resumes polling. + - `navigate()` to a refused port on the self-created path → no `network` call before `open` (call-log assertion), and the navigation's own failure line appears on the first post-attach poll. + - `navigate()` into an already-existing session → baseline read fires before `open`; prior failures not replayed, the navigation's own failure reports. + - Update `"tick stays passive when the session is missing"` — call sequence stays `["sessionExists"]` (no network call while unattached). + - Network lines in `consoleLines` do not perturb console reconcile: console entries still diff correctly afterward (guards the `consoleState`-vs-display separation). +- **Verification:** `npm test` green including the two deliberately-updated passivity tests; manual check with a linked pane (close/reopen the pane after edits — a running pane keeps the old renderer). + +### U4. Live-mode timer + +- **Goal:** Failure lines appear while the WebSocket stream is active, without violating passivity or the unwatched-pane-costs-nothing principle. +- **Requirements:** R3, R4, R7 +- **Dependencies:** U3 +- **Files:** `bin/renderer.mjs` (`goLive()`, `dropLive()`, live branch of `tick()` or a dedicated timer), `tests/renderer.test.mjs` +- **Approach:** Start a timer on successful `goLive()` (base ~4 s) that fires the shared poll with `pollDelay`-style backoff, reset by frame/console stream activity or by a poll that returned failure descriptors (a silently failing background fetch on a static page produces neither frames nor console entries — the failures themselves must hold the cadence). Guards: clear the timer as the first statement of `dropLive()`; each firing checks `this.live && this.attached`; shared in-flight guard prevents a timer poll and a tick poll racing across the dropLive transition. Paint via `queueConsolePaint(hadConsole)` (handles the console-region-opens layout change in live mode). +- **Patterns to follow:** `streamCooldownUntil`/`lastLiveCheck` cadence state; stream handlers' `queueConsolePaint` usage. +- **Test scenarios:** + - Live mode faked (`r.live = {ws: ...}`) with a failing entry appearing in `network()` → line lands in `consoleLines` after the timer fires (drive the timer hook directly rather than real time). + - `dropLive()` → timer cleared; an in-flight poll resolving after the drop pushes nothing twice (in-flight guard) and the post-drop tick does not re-report entries the timer already showed. + - Update `"live tick only watches liveness"` deliberately for the new call pattern. + - Timer firing after session death (`sessionExists` false path) → no session-creating call. +- **Test expectation note:** backoff-cadence exactness is not asserted (timing-flaky); assert the reset-on-activity state transitions instead. +- **Verification:** `npm test` green; a live-mode manual session shows failure lines within a few seconds. + +### U5. README documentation + +- **Goal:** The console-region docs describe failure lines and their limits. +- **Requirements:** R1, R2, R3 +- **Dependencies:** U3, U4 +- **Files:** `README.md` +- **Approach:** Extend the console-region description (Highlights + relevant Troubleshooting notes): what shows (`✖ 404 GET …`, `✖ no response …`), the xhr/fetch/document scope, the ~15 s delay on no-response detection, that history before pane attach is intentionally not replayed, and that failure reporting turns itself off on very long sessions once the daemon's unbounded request log outgrows the read buffer. +- **Test scenarios:** Test expectation: none — documentation only. +- **Verification:** README reads accurately against shipped behavior. + +--- + +## Scope Boundaries + +**In scope:** failure display in the console region, both modes, tests, README. + +**Not in scope:** + +- Successful-request display, request detail drill-down, HAR anything. +- Merging failure lines chronologically with console entries — polling makes interleaving inherently approximate; accepted. +- The pane calling `network requests --clear` to bound daemon log growth — the pane is passive and external agents may depend on the log. + +**Deferred to follow-up work:** + +- Upstream feature request to vercel-labs/agent-browser for a `network` stream event (read their CONTRIBUTING and templates first), which would replace the live-mode timer entirely. +- Upstream issue for `tracked_requests` growth: verified unbounded in 0.33.2 (append-only Vec, `--clear` is the only eviction, armed at daemon spawn on streaming daemons) — request a cap or eviction policy. + +--- + +## Risks & Dependencies + +- **agent-browser flag churn.** A `-labs` project; every CLI fact here was verified against 0.33.2 (installed) via the upstream source and a live spike. The separate-exec + duck-type design means a future breaking change degrades network reporting only. Re-verify `network requests` argv against the installed version at implementation time. +- **Arming semantics (verified in 0.33.2 source).** Daemons started with a stream server arm request tracking at startup (`new_with_stream` sets `request_tracking = true`); non-stream daemons lazy-arm on the first `network requests` call. herdr sessions run streaming daemons, which is why the spike saw pre-existing requests tracked. Both paths are handled by the unconditional baseline read. +- **Unbounded daemon request log.** On a long-lived, request-heavy session the `network requests` payload grows monotonically toward the 16 MiB `maxBuffer` / 10 s exec timeout; when either trips, the pane disables network reporting for the rest of the attach with a one-time console note (U3). The real fix is upstream (deferred follow-up). +- **Long-poll XHR false positives.** An XHR held open past 15 s with no response reports as failed. The type filter removes the worst offenders (SSE/WS); dedupe and the per-poll cap bound the residual noise. Revisit the threshold if real-world use complains. +- **Console-line eviction.** A burst of failures (dead API server) can evict genuine console lines from the 500-line ring; the per-poll cap is the mitigation, accepted as sufficient. diff --git a/tests/renderer.test.mjs b/tests/renderer.test.mjs index 718b393..faa7253 100644 --- a/tests/renderer.test.mjs +++ b/tests/renderer.test.mjs @@ -24,6 +24,9 @@ import { safeWsId, kittyImageSequence, viewportForPane, + newNetworkState, + diffNetworkFailures, + formatNetworkFailure, } from "../bin/renderer.mjs"; const repoRoot = path.resolve( @@ -1489,3 +1492,135 @@ test("empty console gives its rows to the browser until output arrives", () => { assert.ok(visible.consoleRows >= 4); assert.equal(visible.imageRows, empty.imageRows - visible.consoleRows); }); + +// --- Wave 3: failed network requests in the console region --- + +const req = (id, over = {}) => ({ + requestId: id, + url: `https://api.test/${id}`, + method: "GET", + resourceType: "Fetch", + timestamp: 1_000_000, + ...over, +}); +const T0 = 1_000_000; + +test("network diff: new 404 reported once, then seen", () => { + const st = newNetworkState(); + const first = diffNetworkFailures(st, [req("a", { status: 404 })], T0 + 10); + assert.equal(first.failures.length, 1); + assert.equal(first.failures[0].status, 404); + const second = diffNetworkFailures(st, [req("a", { status: 404 })], T0 + 20); + assert.equal(second.failures.length, 0, "same entry must not re-report"); +}); + +test("network diff: null status ages into no-response, 200 never reports", () => { + const st = newNetworkState(); + const young = diffNetworkFailures(st, [req("a")], T0 + 5_000); + assert.equal(young.failures.length, 0, "5s old in-flight is not a failure"); + const aged = diffNetworkFailures(st, [req("a")], T0 + 20_000); + assert.equal(aged.failures.length, 1); + assert.equal(aged.failures[0].status, null); + const st2 = newNetworkState(); + diffNetworkFailures(st2, [req("b")], T0 + 5_000); + const ok = diffNetworkFailures(st2, [req("b", { status: 200 })], T0 + 9_000); + assert.equal(ok.failures.length, 0); + const later = diffNetworkFailures(st2, [req("b", { status: 200 })], T0 + 60_000); + assert.equal(later.failures.length, 0, "resolved-OK id stays swallowed"); +}); + +test("network diff: failure status arriving one poll late still reports", () => { + const st = newNetworkState(); + const inflight = diffNetworkFailures(st, [req("a")], T0 + 1_000); + assert.equal(inflight.failures.length, 0); + const landed = diffNetworkFailures(st, [req("a", { status: 500 })], T0 + 3_000); + assert.equal(landed.failures.length, 1, "late 500 must not be swallowed"); + assert.equal(landed.failures[0].status, 500); +}); + +test("network diff: log wipe prunes state without replay", () => { + const st = newNetworkState(); + diffNetworkFailures(st, [req("a", { status: 404 })], T0 + 10); + assert.ok(st.seen.has("a")); + // Wipe: log now holds only a fresh id; 'a' evaporates from state. + const after = diffNetworkFailures(st, [req("z", { status: 200 })], T0 + 20); + assert.equal(after.failures.length, 0); + assert.ok(!st.seen.has("a"), "seen pruned to current log"); + // Reused id after relaunch is a new request, judged on its own status — + // dedupe by shape (not id) is what suppresses the repeat line. + const reused = diffNetworkFailures( + st, + [req("z", { status: 200 }), req("a", { status: 404 })], + T0 + 30, + ); + assert.equal(reused.failures.length, 0, "same shape within window dedupes"); + assert.ok(st.seen.has("a"), "reused id still classified and tracked"); +}); + +test("network diff: nav-retry burst dedupes to one line", () => { + const st = newNetworkState(); + const out = diffNetworkFailures( + st, + [ + req("r1", { url: "https://x.invalid/", timestamp: T0 - 60_000 }), + req("r2", { url: "https://x.invalid/", timestamp: T0 - 60_000 }), + req("r3", { url: "https://x.invalid/", timestamp: T0 - 60_000 }), + ], + T0, + ); + assert.equal(out.failures.length, 1, "3 retry entries paint one line"); + assert.equal(out.overflow, 0, "deduped entries are not overflow"); +}); + +test("network diff: cross-poll retry loop stays suppressed within window", () => { + const st = newNetworkState(); + let lines = 0; + for (let i = 0; i < 5; i++) { + const out = diffNetworkFailures( + st, + [req(`try${i}`, { url: "https://api.test/beacon", status: 502 })], + T0 + i * 5_000, + ); + lines += out.failures.length; + } + assert.equal(lines, 1, "steady 5s retry loop paints once inside 60s window"); +}); + +test("network diff: per-poll cap emits overflow count", () => { + const st = newNetworkState(); + const entries = []; + for (let i = 0; i < 12; i++) + entries.push(req(`e${i}`, { url: `https://api.test/${i}`, status: 500 })); + const out = diffNetworkFailures(st, entries, T0); + assert.equal(out.failures.length, 5); + assert.equal(out.overflow, 7); +}); + +test("network diff: baseline swallows everything silently", () => { + const st = newNetworkState(); + const out = diffNetworkFailures( + st, + [req("a", { status: 404 }), req("b"), req("c", { status: 500 })], + T0, + { baseline: true }, + ); + assert.equal(out.failures.length, 0); + const next = diffNetworkFailures( + st, + [req("a", { status: 404 }), req("b"), req("c", { status: 500 })], + T0 + 1_000, + ); + assert.equal(next.failures.length, 0, "baselined ids never replay"); +}); + +test("network format: sanitizes and hard-caps page-controlled URLs", () => { + const nasty = `https://api.test/${"\x1b[2J"}${"x".repeat(5000)}`; + const line = formatNetworkFailure({ method: "GET", url: nasty, status: 404 }); + assert.ok(line.startsWith("404 GET https://api.test/")); + assert.ok(!line.includes("\x1b"), "escape bytes stripped"); + assert.ok(line.length <= 220, "stored line is capped"); + assert.equal( + formatNetworkFailure({ method: "POST", url: "http://l:3000/a", status: null }), + "no response POST http://l:3000/a", + ); +}); From 29129429bb8cf4776f9ab86bea4446cb2921b57f Mon Sep 17 00:00:00 2001 From: Victor Garcia Date: Tue, 4 Aug 2026 16:54:53 -0600 Subject: [PATCH 13/19] feat(network): optional network() reader on the browser wrapper --- bin/renderer.mjs | 14 ++++++++++++++ tests/renderer.test.mjs | 31 +++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/bin/renderer.mjs b/bin/renderer.mjs index 1d39c7b..15daf32 100644 --- a/bin/renderer.mjs +++ b/bin/renderer.mjs @@ -504,6 +504,20 @@ export function makeBrowser(session, bin = "agent-browser") { } }, streamStatus: async () => run("stream", "status"), + // Failed-request source (agent-browser >= 0.33). Read-only: never pass + // --clear — external agents share the daemon's request log. --type + // bounds payload (data: URLs and SSE/WS noise stay out) and is the + // signal filter: failed xhr/fetch/document is what a dev wants to see. + network: async () => { + const data = await run( + "network", + "requests", + "--type", + "xhr,fetch,document", + ); + if (Array.isArray(data?.requests)) return data.requests; + return Array.isArray(data) ? data : []; + }, sessionExists: async () => { try { const { stdout } = await pExecFile(bin, ["session", "list", "--json"], { diff --git a/tests/renderer.test.mjs b/tests/renderer.test.mjs index faa7253..0cf3a8f 100644 --- a/tests/renderer.test.mjs +++ b/tests/renderer.test.mjs @@ -1624,3 +1624,34 @@ test("network format: sanitizes and hard-caps page-controlled URLs", () => { "no response POST http://l:3000/a", ); }); + +test("makeBrowser.network passes the type filter, never --clear", async () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "hb-net-")); + const logf = path.join(dir, "log"); + const stub = path.join(dir, "ab-stub"); + fs.writeFileSync( + stub, + `#!/usr/bin/env bash +echo "$@" >> "${logf}" +printf '%s' '{"success":true,"data":{"requests":[{"requestId":"r1","url":"https://x/a","method":"GET","status":404,"timestamp":1000,"resourceType":"Fetch"}]}}' +`, + ); + fs.chmodSync(stub, 0o755); + const reqs = await makeBrowser("s", stub).network(); + assert.equal(reqs.length, 1); + assert.equal(reqs[0].requestId, "r1"); + const logged = fs.readFileSync(logf, "utf8"); + assert.match( + logged, + /--session s network requests --type xhr,fetch,document --json/, + ); + assert.ok(!logged.includes("--clear"), "pane must never clear the shared log"); +}); + +test("makeBrowser.network rejects on malformed output so callers degrade", async () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "hb-netbad-")); + const stub = path.join(dir, "ab-stub"); + fs.writeFileSync(stub, `#!/usr/bin/env bash\nprintf 'not json'\n`); + fs.chmodSync(stub, 0o755); + await assert.rejects(makeBrowser("s", stub).network(), /non-JSON/); +}); From c3b7652355e5475e0314c84d883e436eff7655f4 Mon Sep 17 00:00:00 2001 From: Victor Garcia Date: Tue, 4 Aug 2026 16:58:26 -0600 Subject: [PATCH 14/19] feat(network): poll-mode failure feed in tick() with silent baseline --- bin/renderer.mjs | 94 +++++++++++++++++++ tests/renderer.test.mjs | 197 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 291 insertions(+) diff --git a/bin/renderer.mjs b/bin/renderer.mjs index 15daf32..c0c97b8 100644 --- a/bin/renderer.mjs +++ b/bin/renderer.mjs @@ -602,6 +602,18 @@ export class Renderer { `shot-${safeWsId(env.HERDR_WORKSPACE_ID)}.jpg`, ); this.lastLiveCheck = 0; + // Failed-request feed (see pollNetwork). The baseline flag makes the + // first read after an attach swallow pre-attach history silently; the + // off latch stops polling for the rest of the attach once the daemon's + // unbounded log outgrows the exec limits — retrying a known-fatal + // multi-MiB read every tick would waste CPU forever with no output. + this.networkState = newNetworkState(); + this.networkBaselinePending = true; + this.networkOff = false; + this.networkPollBusy = false; + this.networkPollErrors = 0; + this.networkTimer = null; // live-mode cadence (see goLive/dropLive) + this.networkIdleTicks = 0; this.kittyAnon = false; // chafa emitted anonymous kitty placements this.lastImageDims = null; this.lastViewportRequest = ""; @@ -660,6 +672,64 @@ export class Renderer { } } + // Shared failure-feed poll for both modes (tick calls it after a healthy + // snapshot; the live-mode timer calls it directly). Returns true when it + // painted failures, so the live timer can hold its base cadence on a page + // whose only activity is failing requests. The in-flight guard prevents a + // timer poll and a tick poll from racing the same state across the + // dropLive transition. Never throws. + async pollNetwork(baseline = false) { + if (this.networkOff || this.networkPollBusy || !this.attached) + return false; + if (typeof this.browser.network !== "function") return false; // test doubles / older engines + this.networkPollBusy = true; + try { + const entries = await this.browser.network(); + const { failures, overflow } = diffNetworkFailures( + this.networkState, + entries, + Date.now(), + { baseline: baseline || this.networkBaselinePending }, + ); + this.networkBaselinePending = false; + this.networkPollErrors = 0; + if (!failures.length) return false; + const hadConsole = this.consoleLines.length > 0; + const lines = failures.map((f) => ({ + text: formatNetworkFailure(f), + type: "error", + })); + if (overflow) + lines.push({ + text: `…and ${overflow} more failed requests`, + type: "error", + }); + this.pushConsole(lines, false); + this.queueConsolePaint(hadConsole); + return true; + } catch (err) { + // The daemon log is unbounded and --clear is not ours to send: once + // the payload exceeds maxBuffer or the exec timeout, every retry is + // guaranteed to fail the same way. Latch off with one visible line. + const fatal = + /maxBuffer/i.test(err?.message ?? "") || err?.killed === true; + if (fatal) { + this.networkOff = true; + const hadConsole = this.consoleLines.length > 0; + this.pushConsole( + [{ text: "network reporting off — request log too large", type: "error" }], + false, + ); + this.queueConsolePaint(hadConsole); + } else { + this.networkPollErrors++; + } + return false; + } finally { + this.networkPollBusy = false; + } + } + queueConsolePaint(hadConsole) { const layoutChanged = this.mode !== "text" && !hadConsole && this.consoleLines.length > 0; @@ -871,6 +941,11 @@ export class Renderer { this.banner = ""; this.streamCooldownUntil = 0; // try the live stream right away this.lastViewportRequest = ""; // fit the new session once + // A (re-)attach may be a different session under the same name: + // start the failure feed from a clean silent baseline every time. + this.networkState = newNetworkState(); + this.networkBaselinePending = true; + this.networkOff = false; } if (this.live) { // Event-driven: the stream paints everything; the poll loop only @@ -948,6 +1023,9 @@ export class Renderer { failed = true; this.failures++; } + // Best-effort by design: a broken failure feed must never take the + // frame/console paint path down with it (pollNetwork never throws). + if (!failed) await this.pollNetwork(); if (failed && this.failures >= 3) { if (await this.browser.sessionExists()) { this.banner = "agent-browser not responding — retrying"; @@ -1357,6 +1435,22 @@ export class Renderer { // and ownership is claimed only after the open actually succeeds, so a // failed navigate can never make us kill someone else's session. const existed = await this.browser.sessionExists(); + // Failure-feed baseline splits on the same check: an existing session + // gets a real silent-baseline read before open (so only the navigation's + // own failures paint), while a not-yet-existing session gets state-only + // seeding — a pre-open network read would auto-create the session and + // break the ownership rule below. Fresh daemons arm request tracking at + // spawn, so the navigation's failures are in the log for the first + // post-attach poll either way. + this.networkState = newNetworkState(); + this.networkOff = false; + if (existed) { + this.networkBaselinePending = false; + this.attached = true; // pollNetwork requires it; the session exists + await this.pollNetwork(true); + } else { + this.networkBaselinePending = false; // nothing to swallow: empty log + } await this.browser.open(u); if (!existed) this.selfCreated = true; this.attached = true; // the user is explicitly starting/driving the session diff --git a/tests/renderer.test.mjs b/tests/renderer.test.mjs index 0cf3a8f..5a63cd0 100644 --- a/tests/renderer.test.mjs +++ b/tests/renderer.test.mjs @@ -1655,3 +1655,200 @@ test("makeBrowser.network rejects on malformed output so callers degrade", async fs.chmodSync(stub, 0o755); await assert.rejects(makeBrowser("s", stub).network(), /non-JSON/); }); + +// U3: poll-mode failure feed integration. + +const netEntry = (id, over = {}) => ({ + requestId: id, + url: `https://api.test/${id}`, + method: "GET", + resourceType: "Fetch", + timestamp: Date.now(), + ...over, +}); +const pollFake = (netQueue, calls = []) => ({ + sessionExists: async () => { + calls.push("sessionExists"); + return true; + }, + snapshot: async (f) => { + calls.push("snapshot"); + fs.writeFileSync(f, PNG_1PX); + return { url: "https://x/", title: "T", entries: [] }; + }, + network: async () => { + calls.push("network"); + const next = netQueue.length > 1 ? netQueue.shift() : netQueue[0]; + if (next instanceof Error) throw next; + return next; + }, +}); +const quietPoll = (r) => { + quiet(r); + r.redrawAll = async () => {}; + r.fitViewport = async () => false; + r.streamCooldownUntil = Number.MAX_SAFE_INTEGER; // stay in poll mode + return r; +}; + +test("tick: first poll baselines silently, later failure paints with ✖", async () => { + const r = quietPoll(mkRenderer()); + r.browser = pollFake([ + [netEntry("old", { status: 404 })], + [netEntry("old", { status: 404 }), netEntry("fresh", { status: 500 })], + ]); + await r.tick(); // attach + baseline: 'old' swallowed + await flush(); + assert.deepEqual(r.consoleLines, [], "baseline paints nothing"); + await r.tick(); + await flush(); + assert.equal(r.consoleLines.length, 1); + assert.match(r.consoleLines[0], /^✖ 500 GET https:\/\/api\.test\/fresh/); +}); + +test("tick: broken network feed degrades silently, pane keeps painting", async () => { + const r = quietPoll(mkRenderer()); + r.browser = pollFake([new Error("weird transient failure")]); + await r.tick(); + await r.tick(); + await flush(); + assert.deepEqual(r.consoleLines, []); + assert.equal(r.banner, "", "no banner for a best-effort feature"); + assert.equal(r.failures, 0, "tick failure counter untouched"); + assert.ok(r.networkPollErrors >= 2); +}); + +test("tick: maxBuffer-class failure latches the feed off with one line", async () => { + const r = quietPoll(mkRenderer()); + const calls = []; + r.browser = pollFake( + [new Error("stdout maxBuffer length exceeded")], + calls, + ); + await r.tick(); + await r.tick(); + await r.tick(); + await flush(); + assert.equal(r.networkOff, true); + assert.deepEqual( + r.consoleLines, + ["✖ network reporting off — request log too large"], + "exactly one visible off note", + ); + assert.equal( + calls.filter((c) => c === "network").length, + 1, + "no retries after the latch", + ); +}); + +test("tick: re-attach re-baselines instead of replaying", async () => { + const r = quietPoll(mkRenderer()); + let alive = true; + const netQueue = [[netEntry("preexisting", { status: 503 })]]; + r.browser = { + ...pollFake(netQueue), + sessionExists: async () => alive, + }; + await r.tick(); // attach + baseline + await flush(); + // Session dies: three failed snapshots detach the pane. + const goodSnapshot = r.browser.snapshot; + r.browser.snapshot = async () => { + throw new Error("session gone"); + }; + alive = false; + await r.tick(); + await r.tick(); + await r.tick(); + assert.equal(r.attached, false, "death detaches"); + // Session comes back under the same name with old failures in its log. + alive = true; + r.browser.snapshot = goodSnapshot; + await r.tick(); // re-attach: baseline swallows 'preexisting' again + await flush(); + assert.deepEqual(r.consoleLines, [], "no replay across re-attach"); + netQueue[0] = [netEntry("preexisting", { status: 503 }), netEntry("new1", { status: 404 })]; + await r.tick(); + await flush(); + assert.equal(r.consoleLines.length, 1); + assert.match(r.consoleLines[0], /404 GET https:\/\/api\.test\/new1/); +}); + +test("navigate: existing session baselines before open; nav failure still reports", async () => { + const r = quietPoll(mkRenderer()); + const calls = []; + const netQueue = [[netEntry("stale", { status: 500 })]]; + r.browser = { + ...pollFake(netQueue, calls), + open: async () => { + calls.push("open"); + }, + }; + await r.navigate("https://localhost:3000/"); + assert.ok( + calls.indexOf("network") < calls.indexOf("open"), + "baseline read fires before open on an existing session", + ); + assert.deepEqual(r.consoleLines, [], "stale failure swallowed"); + netQueue[0] = [netEntry("stale", { status: 500 }), netEntry("nav", { status: 404 })]; + await r.tick(); + await flush(); + assert.equal(r.consoleLines.length, 1); + assert.match(r.consoleLines[0], /404 GET https:\/\/api\.test\/nav/); +}); + +test("navigate: fresh session gets no pre-open network call, nav failure reports", async () => { + const r = quietPoll(mkRenderer()); + const calls = []; + let exists = false; + const netQueue = [[]]; + r.browser = { + ...pollFake(netQueue, calls), + sessionExists: async () => { + calls.push("sessionExists"); + return exists; + }, + open: async () => { + calls.push("open"); + exists = true; + }, + }; + await r.navigate("https://localhost:3000/"); + assert.ok( + !calls.slice(0, calls.indexOf("open")).includes("network"), + "no session-creating read before open", + ); + assert.equal(r.selfCreated, true); + netQueue[0] = [netEntry("nav", { status: null, timestamp: Date.now() - 20_000 })]; + await r.tick(); + await flush(); + assert.equal(r.consoleLines.length, 1); + assert.match(r.consoleLines[0], /^✖ no response GET/); +}); + +test("network lines in consoleLines do not perturb console reconcile", async () => { + const r = quietPoll(mkRenderer()); + let consoleEntries = []; + const netQueue = [[]]; + r.browser = { + ...pollFake(netQueue), + snapshot: async (f) => { + fs.writeFileSync(f, PNG_1PX); + return { url: "https://x/", title: "T", entries: consoleEntries }; + }, + }; + await r.tick(); // baseline + netQueue[0] = [netEntry("bad", { status: 500 })]; + await r.tick(); // paints the failure line + await flush(); + assert.equal(r.consoleLines.length, 1); + consoleEntries = [{ text: "page says hi", type: "log" }]; + await r.tick(); + await flush(); + assert.equal(r.consoleLines.length, 2, "console entry appended once"); + assert.equal(r.consoleLines.at(-1), " page says hi"); + await r.tick(); + await flush(); + assert.equal(r.consoleLines.length, 2, "no duplicate on the next tick"); +}); From a1b3d43b965ff8b1afeb8f2fb6d2bb38582eeef2 Mon Sep 17 00:00:00 2001 From: Victor Garcia Date: Tue, 4 Aug 2026 17:00:34 -0600 Subject: [PATCH 15/19] feat(network): live-mode failure poll timer with activity-held cadence --- bin/renderer.mjs | 40 +++++++++++++++++++++ tests/renderer.test.mjs | 79 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 119 insertions(+) diff --git a/bin/renderer.mjs b/bin/renderer.mjs index c0c97b8..b493d1c 100644 --- a/bin/renderer.mjs +++ b/bin/renderer.mjs @@ -730,6 +730,38 @@ export class Renderer { } } + // Live mode has no snapshot tick and the push stream carries no network + // events, so failures need their own low-cadence poll. pollDelay-style + // backoff keeps an unwatched live pane near-free; the idle counter resets + // on stream activity AND on painted failures — a background retry loop on + // a visually static page produces neither frames nor console entries, so + // the failures themselves must hold the base cadence. + startNetworkTimer(baseMs = 4_000) { + if (this.networkTimer || typeof this.browser.network !== "function") + return; + const fire = async () => { + this.networkTimer = null; + if (!this.live || !this.attached || this.networkOff) return; + if (await this.pollNetwork()) this.networkIdleTicks = 0; + else this.networkIdleTicks++; + if (!this.live || this.networkOff) return; // dropped or latched mid-poll + this.networkTimer = setTimeout( + fire, + pollDelay(baseMs, this.networkIdleTicks), + ); + this.networkTimer.unref?.(); + }; + this.networkTimer = setTimeout(fire, baseMs); + this.networkTimer.unref?.(); + } + + stopNetworkTimer() { + if (this.networkTimer) { + clearTimeout(this.networkTimer); + this.networkTimer = null; + } + } + queueConsolePaint(hadConsole) { const layoutChanged = this.mode !== "text" && !hadConsole && this.consoleLines.length > 0; @@ -1291,10 +1323,15 @@ export class Renderer { ws.onclose = drop; ws.onerror = drop; this.banner = ""; + this.networkIdleTicks = 0; + this.startNetworkTimer(); return true; } dropLive(note) { + // First: a timer poll must not fire into the poll-mode transition and + // race tick's poll over the same diff state. + this.stopNetworkTimer(); const wasLive = !!this.live; if (this.live) { try { @@ -1334,6 +1371,7 @@ export class Renderer { } this.shotFormat = "jpg"; this.frameSeq++; + this.networkIdleTicks = 0; // page activity: keep failure polls prompt this.enqueue(async () => { await this.renderImage(); await this.fitViewport(dims.w, dims.h); @@ -1341,6 +1379,7 @@ export class Renderer { break; } case "console": { + this.networkIdleTicks = 0; // page activity: keep failure polls prompt const hadConsole = this.consoleLines.length > 0; this.pushConsole( [{ text: m.text ?? "", type: m.level ?? "log" }], @@ -1489,6 +1528,7 @@ export class Renderer { cleanup() { if (this.mode === "kitty") process.stdout.write(KITTY_DELETE_ALL); + this.stopNetworkTimer(); try { this.live?.ws.close(); } catch { diff --git a/tests/renderer.test.mjs b/tests/renderer.test.mjs index 5a63cd0..3278d19 100644 --- a/tests/renderer.test.mjs +++ b/tests/renderer.test.mjs @@ -1852,3 +1852,82 @@ test("network lines in consoleLines do not perturb console reconcile", async () await flush(); assert.equal(r.consoleLines.length, 2, "no duplicate on the next tick"); }); + +// U4: live-mode network timer. + +test("live timer: fires the shared poll and paints while streaming", async () => { + const r = quietPoll(mkRenderer()); + r.attached = true; + r.live = { ws: { close: () => {} } }; + r.networkBaselinePending = false; + const calls = []; + r.browser = pollFake([[netEntry("bad", { status: 500 })]], calls); + r.startNetworkTimer(5); + await new Promise((res) => setTimeout(res, 60)); + r.stopNetworkTimer(); + await flush(); + assert.ok(calls.includes("network"), "timer polled the daemon"); + assert.equal(r.consoleLines.length, 1); + assert.match(r.consoleLines[0], /^✖ 500 GET/); +}); + +test("live timer: painted polls hold base cadence, empty polls back off", async () => { + const r = quietPoll(mkRenderer()); + r.attached = true; + r.live = { ws: { close: () => {} } }; + r.browser = { network: async () => [] }; + let painted = true; + r.pollNetwork = async () => painted; + r.startNetworkTimer(5); + await new Promise((res) => setTimeout(res, 40)); + assert.equal(r.networkIdleTicks, 0, "painted failures reset the counter"); + painted = false; + await new Promise((res) => setTimeout(res, 40)); + r.stopNetworkTimer(); + assert.ok(r.networkIdleTicks > 0, "quiet polls accumulate idle ticks"); +}); + +test("live timer: dropLive clears it first; no fire after drop", async () => { + const r = quietPoll(mkRenderer()); + r.attached = true; + r.live = { ws: { close: () => {} } }; + r.networkBaselinePending = false; + const calls = []; + r.browser = pollFake([[]], calls); + r.startNetworkTimer(20); + r.dropLive(); + assert.equal(r.networkTimer, null, "timer cleared on drop"); + await new Promise((res) => setTimeout(res, 60)); + assert.ok(!calls.includes("network"), "no poll after the stream dropped"); +}); + +test("live timer: in-flight guard collapses concurrent polls", async () => { + const r = quietPoll(mkRenderer()); + r.attached = true; + r.networkBaselinePending = false; + let netCalls = 0; + let release; + r.browser = { + network: async () => { + netCalls++; + await new Promise((res) => { + release = res; + }); + return []; + }, + }; + const p1 = r.pollNetwork(); + const p2 = r.pollNetwork(); + release([]); + const [r1, r2] = await Promise.all([p1, p2]); + assert.equal(netCalls, 1, "second poll skipped while one is in flight"); + assert.equal(r2, false); + assert.equal(r1, false); +}); + +test("live timer: never starts for browsers without network()", () => { + const r = quietPoll(mkRenderer()); + r.browser = { sessionExists: async () => true }; + r.startNetworkTimer(5); + assert.equal(r.networkTimer, null); +}); From 9759a85b04cef6ddf49c59e57002408e4e32706b Mon Sep 17 00:00:00 2001 From: Victor Garcia Date: Tue, 4 Aug 2026 17:01:02 -0600 Subject: [PATCH 16/19] docs: describe failed-request lines in the console region --- README.md | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index d24e5e6..505dd6f 100644 --- a/README.md +++ b/README.md @@ -19,6 +19,9 @@ Conductor). - **Shared agent sessions** — one isolated browser session per Herdr workspace. - **Live push streaming** — frames, URL/title changes, console messages, and page errors arrive over WebSocket, with transparent polling fallback. +- **Failed network requests** — 4xx/5xx and no-response xhr/fetch/document + requests appear in the console region as `✖ 404 GET ` lines, in both + streaming and polling modes. - **Pane-aware layout** — the browser viewport fits the pane without stretching or changing its responsive width; the console opens only when output exists. - **Real interaction** — clicks use Chrome mouse events rather than DOM selector @@ -197,8 +200,17 @@ viewport width—and therefore its responsive breakpoint—while fitting only th height to the pane's image area. The frame fills that area without stretching. On a quiet page, the image uses all rows between the header and controls. The -console region appears only after a console message or page error arrives; the -viewport then refits to the remaining image area. +console region appears only after a console message, page error, or failed +network request arrives; the viewport then refits to the remaining image area. + +Failed network requests paint as `✖ 404 GET ` (HTTP 400–599) or +`✖ no response GET ` (connection-level failures, detected after ~15 +seconds without a status). Only xhr, fetch, and document requests are watched — +images, stylesheets, and held-open streams (SSE, WebSocket) stay out. Failures +from before the pane attached are intentionally not replayed, repeated +identical failures are collapsed within a 60-second window, and on very long +sessions the feed turns itself off with a one-time note once the daemon's +request log outgrows the pane's read buffer. ## Session model From e14aed38075b4daf66c5132a3520220803205a2c Mon Sep 17 00:00:00 2001 From: Victor Garcia Date: Tue, 4 Aug 2026 17:04:20 -0600 Subject: [PATCH 17/19] refactor(network): drop write-only pending set from diff state --- bin/renderer.mjs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/bin/renderer.mjs b/bin/renderer.mjs index b493d1c..18dd1ec 100644 --- a/bin/renderer.mjs +++ b/bin/renderer.mjs @@ -323,7 +323,6 @@ export function viewportForPane(frameWidth, { cols, imageRows }) { export function newNetworkState() { return { seen: new Set(), // requestIds reported or resolved 2xx/3xx - pending: new Set(), // requestIds observed with no status yet recent: new Map(), // dedupe key -> last emit/suppress time (ms) }; } @@ -345,18 +344,17 @@ export function diffNetworkFailures(state, entries, nowMs, opts = {}) { if (state.seen.has(id)) continue; const status = typeof e.status === "number" ? e.status : null; if (status !== null) { - state.pending.delete(id); state.seen.add(id); if (status >= 400 && status <= 599) candidates.push({ method: e.method ?? "GET", url: e.url ?? "", status }); continue; } - state.pending.add(id); - // No error detail exists anywhere for connection-level failures (the - // daemon drops loadingFailed), so age past threshold is the only signal - // — and it must be entry age, not poll count: pollDelay stretches ticks. + // Null status = still in flight OR failed at the connection level; the + // daemon drops loadingFailed detail, so entry age past the threshold is + // the only failure signal — and it must be entry age, not poll count: + // pollDelay stretches ticks. Unaged ids stay out of `seen` so a status + // arriving on a later poll is still classified. if (typeof e.timestamp === "number" && nowMs - e.timestamp > ageThresholdMs) { - state.pending.delete(id); state.seen.add(id); candidates.push({ method: e.method ?? "GET", @@ -365,10 +363,8 @@ export function diffNetworkFailures(state, entries, nowMs, opts = {}) { }); } } - // Prune both sets to the live log so wipes and id reuse stay harmless. + // Prune to the live log so wipes and id reuse stay harmless. for (const id of state.seen) if (!currentIds.has(id)) state.seen.delete(id); - for (const id of state.pending) - if (!currentIds.has(id)) state.pending.delete(id); for (const [k, t] of state.recent) if (nowMs - t > dedupeWindowMs) state.recent.delete(k); // Chrome retries a failed navigation with fresh requestIds and app retry From f87fe6908ca208fe9acbc689980223ea25bd6ee4 Mon Sep 17 00:00:00 2001 From: Victor Garcia Date: Tue, 4 Aug 2026 17:28:58 -0600 Subject: [PATCH 18/19] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- scripts/check-manifest.mjs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/scripts/check-manifest.mjs b/scripts/check-manifest.mjs index ad014e6..b0e73e5 100755 --- a/scripts/check-manifest.mjs +++ b/scripts/check-manifest.mjs @@ -23,9 +23,21 @@ function parseManifest(manifestPath) { ); } if (result.status !== 0) { - throw new Error(`manifest is not valid TOML: ${result.stderr.trim()}`); + const stderr = (result.stderr ?? "").trim(); + if (/No module named ['"]tomllib['"]/.test(stderr)) { + throw new Error( + "python3 >= 3.11 is required to parse herdr-plugin.toml (tomllib missing)", + ); + } + throw new Error(`manifest is not valid TOML: ${stderr}`); + } + try { + return JSON.parse(result.stdout); + } catch (error) { + throw new Error( + `manifest parser returned non-JSON output: ${error.message}`, + ); } - return JSON.parse(result.stdout); } function manifestCommands(manifest, errors) { From 42d4d1a75f5cfec695f8a8524105223f427fc423 Mon Sep 17 00:00:00 2001 From: Victor Garcia Date: Tue, 4 Aug 2026 17:29:14 -0600 Subject: [PATCH 19/19] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .github/workflows/ci.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 61464c8..95d2129 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -16,12 +16,15 @@ jobs: - name: Check out repository # v4.2.2 uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: "3.11" - name: Set up Node.js # v4.0.3 uses: actions/setup-node@1e60f620b9541d16bece96c5465dc8ee9832be0b with: node-version: 20 - - name: Run tests run: npm test - name: Validate shell syntax run: bash -n scripts/*.sh