From f002e08014596750cccc686c4511f6f739a6834c Mon Sep 17 00:00:00 2001 From: Dat Date: Sun, 23 Aug 2026 08:36:58 +0700 Subject: [PATCH 01/34] fix(adapters): kill the process group so timeoutSec actually aborts a run execa's timeout signals the direct child only. A grandchild inherits the stdout pipe, the pipe never closes, and the run stays blocked for the command's full duration - timeoutSec: 2 on a command that backgrounds sleep 30 finished at +30s and only relabelled the result as a timeout. runProcess spawns detached so the child leads its own process group, then SIGTERMs the group at the deadline and escalates to SIGKILL. timedOut is now the helper's own flag: the shell often exits 0 before the deadline while an orphan keeps the run alive, so the exit code cannot be trusted to say whether the run completed. Co-Authored-By: Claude Opus 5 (1M context) --- src/adapters/command.test.ts | 12 ++++ src/adapters/command.ts | 18 +++--- src/adapters/types.test.ts | 54 +++++++++++++++++ src/adapters/types.ts | 112 +++++++++++++++++++++++++++++++++++ 4 files changed, 187 insertions(+), 9 deletions(-) create mode 100644 src/adapters/types.test.ts diff --git a/src/adapters/command.test.ts b/src/adapters/command.test.ts index 415935a..18f056a 100644 --- a/src/adapters/command.test.ts +++ b/src/adapters/command.test.ts @@ -27,6 +27,18 @@ describe("CommandAdapter", () => { expect(out.error).toMatch(/timeout/i); }); + it("fails at the timeout even when a grandchild outlives the shell", async () => { + // `sleep 30 &` leaves an orphan holding the stdout pipe. The shell exits 0 + // immediately, but the pipe stays open, so the node used to stay blocked for + // the command's full 30s and only then relabel the result as a timeout. + const started = Date.now(); + const out = await adapter.run({ prompt: "sleep 30 & echo started", cwd: tmpdir(), timeoutSec: 2 }); + const elapsed = Date.now() - started; + expect(out.ok).toBe(false); + expect(out.error).toMatch(/timeout/i); + expect(elapsed).toBeLessThan(3500); + }, 20000); + it("runs the command in the requested cwd", async () => { const out = await adapter.run({ prompt: "pwd", cwd: tmpdir(), timeoutSec: 10 }); expect(out.ok).toBe(true); diff --git a/src/adapters/command.ts b/src/adapters/command.ts index d5c6814..058fe83 100644 --- a/src/adapters/command.ts +++ b/src/adapters/command.ts @@ -1,4 +1,4 @@ -import { execa } from "execa"; +import { runProcess } from "./types.js"; import type { Adapter, AdapterInput, AdapterOutput } from "./types.js"; /** @@ -9,17 +9,17 @@ export class CommandAdapter implements Adapter { readonly name = "command"; async run(input: AdapterInput): Promise { - const result = await execa(input.prompt, { - shell: true, + const result = await runProcess(input.prompt, [], { cwd: input.cwd, - timeout: input.timeoutSec * 1000, - reject: false, - all: false, + timeoutSec: input.timeoutSec, + shell: true, }); - const stdout = typeof result.stdout === "string" ? result.stdout : ""; - const stderr = typeof result.stderr === "string" ? result.stderr : ""; + const { stdout, stderr } = result; + // The timeout check comes first on purpose: the shell can exit 0 long + // before the deadline while a backgrounded grandchild keeps the run alive, + // so a zero exit code says nothing about whether the run completed. if (result.timedOut) { return { ok: false, @@ -30,7 +30,7 @@ export class CommandAdapter implements Adapter { }; } - if (result.failed || result.exitCode !== 0) { + if (result.exitCode !== 0) { const code = result.exitCode ?? "unknown"; return { ok: false, diff --git a/src/adapters/types.test.ts b/src/adapters/types.test.ts new file mode 100644 index 0000000..0de447d --- /dev/null +++ b/src/adapters/types.test.ts @@ -0,0 +1,54 @@ +import { describe, it, expect } from "vitest"; +import { tmpdir } from "node:os"; +import { runProcess } from "./types.js"; + +describe("runProcess", () => { + it("returns stdout, stderr and the exit code for a normal run", async () => { + const result = await runProcess("echo hi; echo bad >&2", [], { + cwd: tmpdir(), + timeoutSec: 10, + shell: true, + }); + expect(result.exitCode).toBe(0); + expect(result.stdout.trim()).toBe("hi"); + expect(result.stderr.trim()).toBe("bad"); + expect(result.timedOut).toBe(false); + expect(result.spawnErrorCode).toBeNull(); + }); + + it("reports ENOENT rather than an empty stdout when the binary is missing", async () => { + const result = await runProcess("/nonexistent/loomgraph-not-a-real-binary", ["--version"], { + cwd: tmpdir(), + timeoutSec: 10, + }); + expect(result.spawnErrorCode).toBe("ENOENT"); + expect(result.timedOut).toBe(false); + }); + + it("kills the whole process group so a grandchild holding stdout cannot outlive the timeout", async () => { + // The regression: `sleep 30 &` leaves an orphan holding the stdout pipe. The + // direct child exits immediately with code 0, but the pipe never closes, so + // the run stayed blocked for the full 30s no matter what timeoutSec said. + const started = Date.now(); + const result = await runProcess("sleep 30 & echo started", [], { + cwd: tmpdir(), + timeoutSec: 2, + shell: true, + }); + const elapsed = Date.now() - started; + expect(result.timedOut).toBe(true); + expect(elapsed).toBeLessThan(3500); + }, 20000); + + it("kills a grandchild the direct child is waiting on", async () => { + const started = Date.now(); + const result = await runProcess("sleep 30 & wait", [], { + cwd: tmpdir(), + timeoutSec: 2, + shell: true, + }); + const elapsed = Date.now() - started; + expect(result.timedOut).toBe(true); + expect(elapsed).toBeLessThan(3500); + }, 20000); +}); diff --git a/src/adapters/types.ts b/src/adapters/types.ts index ee4d138..ac8f08b 100644 --- a/src/adapters/types.ts +++ b/src/adapters/types.ts @@ -1,3 +1,5 @@ +import { execa } from "execa"; + export interface AdapterInput { prompt: string; cwd: string; @@ -19,3 +21,113 @@ export interface Adapter { name: string; run(input: AdapterInput): Promise; } + +export interface ProcessRunOptions { + cwd: string; + timeoutSec: number; + /** Run the command line through a shell. Used by the command adapter only. */ + shell?: boolean; + /** Written to the child's stdin, then stdin is closed. Defaults to "". */ + input?: string; +} + +export interface ProcessRunResult { + stdout: string; + stderr: string; + /** Undefined when the child was killed by a signal or never started. */ + exitCode: number | undefined; + /** True when this helper killed the process group because timeoutSec elapsed. */ + timedOut: boolean; + /** + * The errno code when the binary itself could not be spawned - "ENOENT" for a + * missing binary. Null on any run that actually started. + */ + spawnErrorCode: string | null; +} + +/** How long a SIGTERM'd process group gets before it is SIGKILLed. */ +const SIGKILL_GRACE_MS = 2_000; + +function killGroup(pid: number | undefined, signal: NodeJS.Signals): void { + if (pid === undefined) return; + try { + // Negative pid = the whole process group. `detached: true` below made the + // child the group leader, so this reaches every descendant. + process.kill(-pid, signal); + } catch { + try { + process.kill(pid, signal); + } catch { + // Already gone. Nothing to do. + } + } +} + +/** + * Spawn a child process with a timeout that actually aborts the run. + * + * execa's own `timeout` option signals the direct child only. That is not + * enough: a grandchild inherits the stdout pipe, the pipe never closes, and + * execa stays unresolved for the command's full duration. `timeoutSec: 2` on + * something that backgrounds `sleep 30` finished at +30s and merely relabelled + * the result as a timeout after the fact. + * + * So the child is spawned detached - making it a process-group leader - and on + * timeout the whole group is SIGTERMed, then SIGKILLed if it is still alive. + * Killing the group closes the inherited pipes, which is what lets the await + * resolve at the deadline instead of at the command's natural end. + * + * `timedOut` is this helper's own flag, not execa's: the direct child often + * exits 0 well before the deadline while an orphan keeps the run alive, so an + * exit code of 0 says nothing about whether the run completed. Callers must + * check `timedOut` before they interpret `exitCode`. + */ +export async function runProcess( + file: string, + args: string[], + options: ProcessRunOptions, +): Promise { + const child = execa(file, args, { + cwd: options.cwd, + reject: false, + all: false, + shell: options.shell ?? false, + // Keep every run non-interactive: an open stdin can stall a CLI until the + // timeout fires, which is indistinguishable from a hung model call. + input: options.input ?? "", + detached: true, + }); + + let timedOut = false; + let escalation: NodeJS.Timeout | undefined; + + const deadline = setTimeout( + () => { + timedOut = true; + killGroup(child.pid, "SIGTERM"); + escalation = setTimeout(() => killGroup(child.pid, "SIGKILL"), SIGKILL_GRACE_MS); + escalation.unref(); + }, + Math.max(1, Math.round(options.timeoutSec * 1000)), + ); + deadline.unref(); + + try { + const result = await child; + const stdout = typeof result.stdout === "string" ? result.stdout : ""; + const stderr = typeof result.stderr === "string" ? result.stderr : ""; + const errno = (result as { code?: unknown }).code; + const signal = (result as { signal?: unknown }).signal; + // A spawn failure is the only case with no exit code, no signal and an + // errno string - a timeout kill produces a signal instead. + const spawnErrorCode = + !timedOut && result.exitCode === undefined && signal === undefined && typeof errno === "string" + ? errno + : null; + + return { stdout, stderr, exitCode: result.exitCode, timedOut, spawnErrorCode }; + } finally { + clearTimeout(deadline); + if (escalation !== undefined) clearTimeout(escalation); + } +} From f5bfb9d834b3b82c5b5e47b1eaf94c5bb39b83e1 Mon Sep 17 00:00:00 2001 From: Dat Date: Sun, 23 Aug 2026 08:40:02 +0700 Subject: [PATCH 02/34] fix(adapters): abort agent runs at the timeout, name a missing binary, clamp costs H1: claude and opencode now go through runProcess, so timeoutSec kills the whole process group. A CLI that leaves a grandchild holding stdout used to keep the node blocked for the command's full duration. M11: execa with reject:false swallowed the spawn error, so a missing binary surfaced as "could not parse claude json output:" and never said which binary was missing. ENOENT now fails with " not found on PATH". L3: a negative or non-finite reported price is recorded as 0 instead of driving the run budget backwards. Co-Authored-By: Claude Opus 5 (1M context) --- src/adapters/claude.test.ts | 51 ++++++++++++++++++++++++++++-- src/adapters/claude.ts | 29 ++++++++++-------- src/adapters/opencode.test.ts | 58 +++++++++++++++++++++++++++++++++-- src/adapters/opencode.ts | 22 +++++++------ src/adapters/types.ts | 10 ++++++ 5 files changed, 145 insertions(+), 25 deletions(-) diff --git a/src/adapters/claude.test.ts b/src/adapters/claude.test.ts index 1804fc6..a5ba610 100644 --- a/src/adapters/claude.test.ts +++ b/src/adapters/claude.test.ts @@ -1,5 +1,8 @@ -import { describe, it, expect } from "vitest"; -import { buildClaudeArgs, parseClaudeJson } from "./claude.js"; +import { describe, it, expect, beforeAll, afterAll } from "vitest"; +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { buildClaudeArgs, parseClaudeJson, ClaudeAdapter } from "./claude.js"; const SUCCESS = `{"type":"result","subtype":"success","result":"done","session_id":"abc","num_turns":3,"total_cost_usd":0.0787}`; const MAX_TURNS = `{"type":"result","subtype":"error_max_turns","result":"","total_cost_usd":0.5}`; @@ -147,4 +150,48 @@ describe("parseClaudeJson", () => { expect(Array.isArray(out.raw)).toBe(true); expect((out.raw as unknown[]).length).toBe(4); }); + + it("clamps a negative total_cost_usd to 0", () => { + const out = parseClaudeJson(`{"subtype":"success","result":"ok","total_cost_usd":-1.5}`); + expect(out.ok).toBe(true); + expect(out.costUsd).toBe(0); + }); + + it("clamps a non-finite total_cost_usd to 0", () => { + const out = parseClaudeJson(`{"subtype":"success","result":"ok","total_cost_usd":1e999}`); + expect(out.costUsd).toBe(0); + }); +}); + +describe("ClaudeAdapter process handling", () => { + // Every test here points the adapter at a throwaway stub script by absolute + // path. No real agent CLI is ever spawned and PATH is never touched. + let dir: string; + + beforeAll(async () => { + dir = await mkdtemp(join(tmpdir(), "lg-claude-stub-")); + }); + + afterAll(async () => { + await rm(dir, { recursive: true, force: true }); + }); + + it("names the binary when it is missing instead of blaming the json parser", async () => { + const bin = join(dir, "definitely-not-installed"); + const out = await new ClaudeAdapter(bin).run({ prompt: "hi", cwd: dir, timeoutSec: 10 }); + expect(out.ok).toBe(false); + expect(out.error).toContain(bin); + expect(out.error).toMatch(/not found on PATH/); + expect(out.error).not.toMatch(/could not parse/); + }); + + it("times out when a grandchild outlives the CLI", async () => { + const bin = join(dir, "slow-stub.sh"); + await writeFile(bin, `#!/bin/sh\nsleep 30 &\necho '{"subtype":"success","result":"ok"}'\n`, { mode: 0o755 }); + const started = Date.now(); + const out = await new ClaudeAdapter(bin).run({ prompt: "hi", cwd: dir, timeoutSec: 2 }); + expect(out.ok).toBe(false); + expect(out.error).toMatch(/timeout after 2s/); + expect(Date.now() - started).toBeLessThan(3500); + }, 20000); }); diff --git a/src/adapters/claude.ts b/src/adapters/claude.ts index c1bf1b2..f02fc10 100644 --- a/src/adapters/claude.ts +++ b/src/adapters/claude.ts @@ -1,4 +1,4 @@ -import { execa } from "execa"; +import { clampCostUsd, runProcess } from "./types.js"; import type { Adapter, AdapterInput, AdapterOutput } from "./types.js"; /** @@ -59,7 +59,7 @@ export function parseClaudeJson(stdout: string): AdapterOutput { } // Cost is harvested even on failure - budget accounting depends on it. - const costUsd = typeof result.total_cost_usd === "number" ? result.total_cost_usd : 0; + const costUsd = clampCostUsd(result.total_cost_usd); const text = typeof result.result === "string" ? result.result : ""; // Claude Code can report `subtype: "success"` while `is_error` is true - an @@ -91,17 +91,22 @@ export class ClaudeAdapter implements Adapter { async run(input: AdapterInput): Promise { const args = buildClaudeArgs(input.prompt, input.maxTurns, input.model); - const result = await execa(this.bin, args, { - cwd: input.cwd, - timeout: input.timeoutSec * 1000, - reject: false, - // Keep the run non-interactive: an open stdin can stall the CLI until the - // node's timeout fires, which is indistinguishable from a hung model call. - input: "", - }); + const result = await runProcess(this.bin, args, { cwd: input.cwd, timeoutSec: input.timeoutSec }); - const stdout = typeof result.stdout === "string" ? result.stdout : ""; - const stderr = typeof result.stderr === "string" ? result.stderr : ""; + const { stdout, stderr } = result; + + // Without this the spawn error is swallowed, the adapter parses an empty + // stdout, and the run fails with "could not parse claude json output:" - + // which never names the binary that is missing. + if (result.spawnErrorCode === "ENOENT") { + return { + ok: false, + text: "", + costUsd: 0, + raw: { stdout, stderr }, + error: `${this.bin} not found on PATH`, + }; + } if (result.timedOut) { return { ok: false, text: stdout, costUsd: 0, raw: { stdout, stderr }, error: `timeout after ${input.timeoutSec}s` }; diff --git a/src/adapters/opencode.test.ts b/src/adapters/opencode.test.ts index badf55b..7410d7b 100644 --- a/src/adapters/opencode.test.ts +++ b/src/adapters/opencode.test.ts @@ -1,5 +1,8 @@ -import { describe, expect, it } from "vitest"; -import { buildOpencodeArgs, parseOpencodeJsonl } from "./opencode.js"; +import { describe, expect, it, beforeAll, afterAll } from "vitest"; +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { buildOpencodeArgs, parseOpencodeJsonl, OpenCodeAdapter } from "./opencode.js"; /** * Captured by hand from opencode 1.18.17 on 2026-08-15: @@ -104,4 +107,55 @@ describe("parseOpencodeJsonl", () => { it("keeps the raw stdout so a run can be audited", () => { expect(parseOpencodeJsonl(SUCCESS_JSONL, 0).raw).toBe(SUCCESS_JSONL); }); + + it("clamps a negative reported cost to 0", () => { + const stdout = [ + JSON.stringify({ type: "text", part: { text: "hello" } }), + JSON.stringify({ type: "step_finish", part: { cost: -0.25 } }), + ].join("\n"); + const out = parseOpencodeJsonl(stdout, 0); + expect(out.ok).toBe(true); + expect(out.costUsd).toBe(0); + }); + + it("ignores a negative cost but still sums the positive ones", () => { + const stdout = [ + JSON.stringify({ type: "text", part: { text: "hello" } }), + JSON.stringify({ type: "step_finish", part: { cost: -0.25 } }), + JSON.stringify({ type: "step_finish", part: { cost: 0.5 } }), + ].join("\n"); + expect(parseOpencodeJsonl(stdout, 0).costUsd).toBeCloseTo(0.5, 10); + }); +}); + +describe("OpenCodeAdapter process handling", () => { + // Stub scripts only, invoked by absolute path. No real agent CLI is spawned + // and PATH is never touched. + let dir: string; + + beforeAll(async () => { + dir = await mkdtemp(join(tmpdir(), "lg-opencode-stub-")); + }); + + afterAll(async () => { + await rm(dir, { recursive: true, force: true }); + }); + + it("names the binary when it is missing", async () => { + const bin = join(dir, "definitely-not-installed"); + const out = await new OpenCodeAdapter(bin).run({ prompt: "hi", cwd: dir, timeoutSec: 10 }); + expect(out.ok).toBe(false); + expect(out.error).toContain(bin); + expect(out.error).toMatch(/not found on PATH/); + }); + + it("times out when a grandchild outlives the CLI", async () => { + const bin = join(dir, "slow-stub.sh"); + await writeFile(bin, `#!/bin/sh\nsleep 30 &\necho '{"type":"text","part":{"text":"ok"}}'\n`, { mode: 0o755 }); + const started = Date.now(); + const out = await new OpenCodeAdapter(bin).run({ prompt: "hi", cwd: dir, timeoutSec: 2 }); + expect(out.ok).toBe(false); + expect(out.error).toMatch(/timeout after 2s/); + expect(Date.now() - started).toBeLessThan(3500); + }, 20000); }); diff --git a/src/adapters/opencode.ts b/src/adapters/opencode.ts index 041ab61..f0cd7da 100644 --- a/src/adapters/opencode.ts +++ b/src/adapters/opencode.ts @@ -1,4 +1,4 @@ -import { execa } from "execa"; +import { clampCostUsd, runProcess } from "./types.js"; import type { Adapter, AdapterInput, AdapterOutput } from "./types.js"; /** @@ -62,7 +62,9 @@ export function parseOpencodeJsonl(stdout: string, exitCode: number | null): Ada for (const event of events) { const part = event.part; if (event.type === "text" && part && typeof part.text === "string") text += part.text; - if (part && typeof part.cost === "number") costUsd += part.cost; + // A negative or non-finite reported price contributes 0 - it must never + // drive the run budget backwards. + if (part) costUsd += clampCostUsd(part.cost); } if (events.length === 0) { @@ -83,16 +85,18 @@ export class OpenCodeAdapter implements Adapter { constructor(private readonly bin = "opencode") {} async run(input: AdapterInput): Promise { - const result = await execa(this.bin, buildOpencodeArgs(input.prompt, input.model), { + const result = await runProcess(this.bin, buildOpencodeArgs(input.prompt, input.model), { cwd: input.cwd, - timeout: input.timeoutSec * 1000, - reject: false, - // Keep the run non-interactive - an open stdin can stall the CLI. - input: "", + timeoutSec: input.timeoutSec, }); - const stdout = typeof result.stdout === "string" ? result.stdout : ""; - const stderr = typeof result.stderr === "string" ? result.stderr : ""; + const { stdout, stderr } = result; + + // Name the binary rather than letting the empty stdout surface as a parse + // failure - the same shape lg-handoff uses. + if (result.spawnErrorCode === "ENOENT") { + return { ok: false, text: "", costUsd: 0, raw: stdout, error: `${this.bin} not found on PATH` }; + } if (result.timedOut) { return { ok: false, text: stdout, costUsd: 0, raw: { stdout, stderr }, error: `timeout after ${input.timeoutSec}s` }; diff --git a/src/adapters/types.ts b/src/adapters/types.ts index ac8f08b..90e05a2 100644 --- a/src/adapters/types.ts +++ b/src/adapters/types.ts @@ -22,6 +22,16 @@ export interface Adapter { run(input: AdapterInput): Promise; } +/** + * A reported price is only usable if it is a finite, non-negative number. A + * negative report would drive the run budget backwards and let a run outlive + * its ceiling, so it is treated as 0 rather than trusted. + */ +export function clampCostUsd(value: unknown): number { + if (typeof value !== "number" || !Number.isFinite(value) || value < 0) return 0; + return value; +} + export interface ProcessRunOptions { cwd: string; timeoutSec: number; From c811511e2d3ed821ac4c34f2db822cc9823ef439 Mon Sep 17 00:00:00 2001 From: Dat Date: Sun, 23 Aug 2026 08:41:04 +0700 Subject: [PATCH 03/34] fix: catch the auth-header, .netrc and auth.json shapes the scanner missed The JSON-encoded Authorization header could not be seen at all: the rule went straight from the header name to the scheme, so the quote after the colon in {"Authorization":"Bearer ..."} ended the match before it started. A .netrc row carries no separator any assignment rule can key on, and an opencode auth.json stores its OAuth material under refresh/access/credential. Every one of these produced "scan clean" on a bundle that was carrying a live credential. Excerpts stay masked to four characters, so a finding still cannot be pasted into a report and read as the secret it warns about. Co-Authored-By: Claude Opus 5 (1M context) --- src/handoff/scan.test.ts | 41 ++++++++++++++++++++++++++++++++++++++++ src/handoff/scan.ts | 18 +++++++++++++++++- 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/src/handoff/scan.test.ts b/src/handoff/scan.test.ts index bd2881f..90f0e65 100644 --- a/src/handoff/scan.test.ts +++ b/src/handoff/scan.test.ts @@ -60,6 +60,8 @@ describe("SCAN_RULES", () => { "huggingface-token", "google-oauth-secret", "auth-header", + "netrc-credentials", + "auth-json-credential", "abs-home-path", ]); }); @@ -169,6 +171,35 @@ describe("scanText — hits and near-misses per rule", () => { expect(fires("Authorization: " + "Bearer", "auth-header")).toBe(false); }); + it("auth-header also matches the JSON-quoted form", () => { + const jsonBearer = '{"Authorization":"' + "Bearer " + shaped("FAKE", "faketoken0000") + '"}'; + const jsonBasic = '{"Authorization": "' + "Basic " + shaped("dXNl", "cjpwYXNz") + '"}'; + expect(fires(jsonBearer, "auth-header")).toBe(true); + expect(fires(jsonBasic, "auth-header")).toBe(true); + // The plain-text form must keep working. + expect(fires("Authorization: " + "Bearer " + shaped("FAKE", "faketoken0000"), "auth-header")).toBe(true); + // Scheme with no credential is still not a finding. + expect(fires('{"Authorization":"' + 'Bearer"}', "auth-header")).toBe(false); + }); + + it("netrc-credentials", () => { + const row = "machine api.example.com login alice password " + shaped("FAKE", "fakesecret0000"); + expect(fires(row, "netrc-credentials")).toBe(true); + expect(fires(" machine gitlab.example.com login bot password " + shaped("FAKE", "fakesecret0000"), "netrc-credentials")).toBe(true); + // Prose that merely mentions the words is not a .netrc row. + expect(fires("the machine has a login and a password", "netrc-credentials")).toBe(false); + expect(fires("machine api.example.com login alice", "netrc-credentials")).toBe(false); + }); + + it("auth-json-credential covers opencode auth.json shapes", () => { + expect(fires('{"refresh":"' + shaped("FAKE", "fakerefresh0000") + '"}', "auth-json-credential")).toBe(true); + expect(fires('{"access": "' + shaped("FAKE", "fakeaccess0000") + '"}', "auth-json-credential")).toBe(true); + expect(fires('{"credential":"' + shaped("FAKE", "fakecred0000") + '"}', "auth-json-credential")).toBe(true); + // Empty value is a placeholder, and prose is not a JSON credential. + expect(fires('{"access":""}', "auth-json-credential")).toBe(false); + expect(fires("refresh the access credential in the browser", "auth-json-credential")).toBe(false); + }); + it("env-assignment is case-insensitive and covers a json key", () => { expect(fires("database_password=hunter2xyz", "env-assignment")).toBe(true); expect(fires('"password": "hunter2xyz"', "env-assignment")).toBe(true); @@ -226,6 +257,16 @@ describe("scanText — shape of a finding", () => { expect(JSON.stringify(findings)).not.toContain("FAKEfake"); }); + it("masks the excerpt of the new credential shapes", () => { + const row = "machine api.example.com login alice password " + shaped("FAKE", "fakesecret0000"); + const netrc = scanText(row, "f.md").find((f) => f.rule === "netrc-credentials"); + expect(netrc?.excerpt).toBe("mach..."); + const json = '{"refresh":"' + shaped("FAKE", "fakerefresh0000") + '"}'; + const hit = scanText(json, "f.md").find((f) => f.rule === "auth-json-credential"); + expect(hit?.excerpt.length).toBeLessThanOrEqual(7); + expect(hit?.excerpt).not.toContain("fakerefresh"); + }); + it("reports 1-based line numbers and echoes the file back", () => { const text = ["clean", "clean", shaped("AKIA", "FAKEFAKEFAKE0000")].join("\n"); expect(scanText(text, "nested/dirty.md")).toEqual([ diff --git a/src/handoff/scan.ts b/src/handoff/scan.ts index 6e4a297..60c26f9 100644 --- a/src/handoff/scan.ts +++ b/src/handoff/scan.ts @@ -126,9 +126,25 @@ export const SCAN_RULES: ReadonlyArray<{ }, { name: "auth-header", - pattern: /\bAuthorization\s*:\s*(?:Bearer|Basic)\s+\S+/i, + // Optional quotes around the name and the scheme cover the JSON-encoded + // form of the same header, e.g. `{"Authorization":"Bearer ..."}`. + pattern: /\bAuthorization"?\s*:\s*"?(?:Bearer|Basic)\s+\S+/i, description: "Authorization Bearer or Basic header", }, + { + name: "netrc-credentials", + // A .netrc row is space-separated with no `=` or `:`, so no assignment rule + // can see it. Requiring all three keywords in order keeps prose out. + pattern: /\bmachine\s+\S+\s+login\s+\S+\s+password\s+\S+/i, + description: ".netrc machine/login/password row", + }, + { + name: "auth-json-credential", + // The shapes an opencode auth.json uses for stored OAuth material. The key + // must be JSON-quoted and the value non-empty, so prose cannot fire. + pattern: /"(?:refresh|access|credential)"\s*:\s*"[^"\s]+"/i, + description: "OAuth refresh/access/credential value in a JSON auth file", + }, { name: "abs-home-path", // Residual absolute home directory left after rewritePaths ran. From 8776e78535e7b08bff54eead2b970e8be7f63eaa Mon Sep 17 00:00:00 2001 From: Dat Date: Sun, 23 Aug 2026 08:41:08 +0700 Subject: [PATCH 04/34] fix(graph): reject adapter on command and human nodes zod strips unknown keys, so `adapter:` on a command or human node was silently accepted and the node ran as a plain shell command with no warning. Extend the existing `model` guard to cover `adapter`. Co-Authored-By: Claude Opus 5 (1M context) --- src/core/graph.test.ts | 44 ++++++++++++++++++++++++++++++++++++++++++ src/core/graph.ts | 9 ++++++--- 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/src/core/graph.test.ts b/src/core/graph.test.ts index e7841f6..99ccc5f 100644 --- a/src/core/graph.test.ts +++ b/src/core/graph.test.ts @@ -250,6 +250,50 @@ edges: expect(() => parseGraph(src)).toThrow(/model/); }); + it("rejects a command node that declares an adapter", () => { + const src = graph(`nodes: + a: + type: command + run: "echo hi" + adapter: claude +edges: + - from: a + to: END +`); + expect(() => parseGraph(src)).toThrow(/a/); + expect(() => parseGraph(src)).toThrow(/adapter/); + }); + + it("rejects a human node that declares an adapter", () => { + const src = graph(`nodes: + a: + type: human + question: "ok?" + adapter: claude +edges: + - from: a + to: END +`); + expect(() => parseGraph(src)).toThrow(/a/); + expect(() => parseGraph(src)).toThrow(/adapter/); + }); + + it("accepts an agent node with an explicit adapter", () => { + const src = graph(`nodes: + a: + type: agent + adapter: claude + prompt: "hi" +edges: + - from: a + to: END +`); + const parsed = parseGraph(src); + const node = parsed.nodes["a"]; + expect(node?.type).toBe("agent"); + if (node?.type === "agent") expect(node.adapter).toBe("claude"); + }); + it("rejects an agent node with an unknown adapter", () => { const src = graph(`nodes: a: diff --git a/src/core/graph.ts b/src/core/graph.ts index 32222d0..5de58f6 100644 --- a/src/core/graph.ts +++ b/src/core/graph.ts @@ -153,12 +153,15 @@ export function parseGraph(source: string, sourceName = "graph"): Graph { if (typeof type !== "string" || !(NODE_TYPES as readonly string[]).includes(type)) { fail(`node "${id}" has unknown type "${String(type)}" - valid types are ${NODE_TYPES.join(", ")}`); } - // zod strips unknown keys, so a `model` on a command or human node would - // vanish silently rather than fail. Only the two node types that dispatch a - // prompt to an agent CLI can carry one. + // zod strips unknown keys, so a `model` or `adapter` on a command or human + // node would vanish silently rather than fail. Only the two node types that + // dispatch a prompt to an agent CLI can carry either. if ((value as Record).model !== undefined && type !== "agent" && type !== "verifier") { fail(`node "${id}" of type "${type}" cannot declare a model - only agent and verifier nodes dispatch to an adapter`); } + if ((value as Record).adapter !== undefined && type !== "agent" && type !== "verifier") { + fail(`node "${id}" of type "${type}" cannot declare a adapter - only agent and verifier nodes dispatch to an adapter`); + } const parsed = nodeSchema.safeParse(value); if (!parsed.success) fail(issuesToMessage(`node "${id}"`, parsed.error)); nodes[id] = parsed.data; From 1927463c9ddb5979ff6f757848d914722a832e37 Mon Sep 17 00:00:00 2001 From: Dat Date: Sun, 23 Aug 2026 08:42:21 +0700 Subject: [PATCH 05/34] fix(codex): fail a verifier whose sandbox is broken, and validate the sandbox env H2: bwrap fails per tool call, not at startup, so a broken sandbox produces codex exit 0 plus a confident agent message from a run that read nothing - 'I could not read any files, but nothing looks wrong. PASS' was scored as a pass. detectSandboxFailure now looks for a line starting with 'bwrap:' on stderr AND stdout, regardless of exit code, and fails the node with that line. Anchoring to the line prefix keeps an agent message that merely discusses bwrap from failing the node. L2: LOOMGRAPH_CODEX_SANDBOX was cast, not validated - 'danger-full-access' read as a wider policy that was never applied and 'READ-ONLY' failed inside codex. resolveCodexSandbox rejects anything but the three documented values. H1/M11/L3 for codex: runProcess for the group-kill timeout, ENOENT reported as ' not found on PATH', reported cost clamped to >= 0. Co-Authored-By: Claude Opus 5 (1M context) --- src/adapters/codex.test.ts | 130 ++++++++++++++++++++++++++++++++++++- src/adapters/codex.ts | 99 +++++++++++++++++++++++----- 2 files changed, 210 insertions(+), 19 deletions(-) diff --git a/src/adapters/codex.test.ts b/src/adapters/codex.test.ts index d5e2726..aa30ed4 100644 --- a/src/adapters/codex.test.ts +++ b/src/adapters/codex.test.ts @@ -1,5 +1,15 @@ -import { describe, it, expect } from "vitest"; -import { buildCodexArgs, decideCodexResult, parseCodexJsonl } from "./codex.js"; +import { describe, it, expect, beforeAll, afterAll } from "vitest"; +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { + buildCodexArgs, + decideCodexResult, + detectSandboxFailure, + parseCodexJsonl, + resolveCodexSandbox, + CodexAdapter, +} from "./codex.js"; const EVENTS = [ `{"id":"0","msg":{"type":"task_started"}}`, @@ -119,4 +129,120 @@ describe("decideCodexResult", () => { expect(decided.error).toMatch(/no agent message/i); expect(decided.error).toMatch(/auth expired/); }); + + it("fails a verifier whose sandbox is broken even when the pass string appears and codex exits 0", () => { + // A verifier that could not read the tree must not pass: codex keeps + // talking and exits 0, and the agent message can still contain the pass + // string the engine is looking for. + const parsed = parseCodexJsonl(`{"msg":{"type":"agent_message","message":"looks good - PASS"}}`); + const decided = decideCodexResult(parsed, 0, "bwrap: loopback: Failed RTM_NEWADDR: Operation not permitted"); + expect(decided.ok).toBe(false); + expect(decided.error).toMatch(/sandbox/i); + expect(decided.error).toMatch(/bwrap/i); + }); + + it("explains the bypass escape hatch when the sandbox failed", () => { + const parsed = parseCodexJsonl(`{"msg":{"type":"agent_message","message":"PASS"}}`); + const decided = decideCodexResult(parsed, 0, "bwrap: loopback: Failed RTM_NEWADDR: Operation not permitted"); + expect(decided.error).toMatch(/LOOMGRAPH_CODEX_SANDBOX=bypass/); + }); + + it("reports the sandbox failure instead of a missing-message error", () => { + const parsed = parseCodexJsonl(`{"msg":{"type":"task_started"}}`); + const decided = decideCodexResult(parsed, 0, "bwrap: loopback: Failed RTM_NEWADDR: Operation not permitted"); + expect(decided.ok).toBe(false); + expect(decided.error).toMatch(/sandbox failed/i); + expect(decided.error).not.toMatch(/no agent message/i); + }); + + it("does not flag a clean run whose stderr merely mentions the word pass", () => { + const decided = decideCodexResult(parseCodexJsonl(EVENTS), 0, "all checks pass"); + expect(decided.ok).toBe(true); + expect(decided.text).toBe("final answer"); + }); + + it("catches the bwrap diagnostic when codex writes it to stdout instead", () => { + const stdout = [ + "bwrap: loopback: Failed RTM_NEWADDR: Operation not permitted", + `{"msg":{"type":"agent_message","message":"I could not read any files, but nothing looks wrong. PASS"}}`, + ].join("\n"); + const decided = decideCodexResult(parseCodexJsonl(stdout), 0, "", stdout); + expect(decided.ok).toBe(false); + expect(decided.error).toMatch(/sandbox failed/i); + }); + + it("does not flag an agent message that merely discusses bwrap", () => { + const stdout = `{"msg":{"type":"agent_message","message":"the script calls bwrap for isolation - looks fine, PASS"}}`; + const decided = decideCodexResult(parseCodexJsonl(stdout), 0, "", stdout); + expect(decided.ok).toBe(true); + expect(decided.error).toBeNull(); + }); +}); + +describe("detectSandboxFailure", () => { + it("returns null for ordinary output", () => { + expect(detectSandboxFailure("warning: something harmless\nall good\n")).toBeNull(); + }); + + it("returns the offending line", () => { + const line = "bwrap: loopback: Failed RTM_NEWADDR: Operation not permitted"; + expect(detectSandboxFailure(`noise\n ${line} \nmore noise`)).toBe(line); + }); +}); + +describe("resolveCodexSandbox", () => { + it("defaults to read-only when unset or empty", () => { + expect(resolveCodexSandbox(undefined)).toBe("read-only"); + expect(resolveCodexSandbox("")).toBe("read-only"); + expect(resolveCodexSandbox(" ")).toBe("read-only"); + }); + + it("accepts each documented value", () => { + expect(resolveCodexSandbox("read-only")).toBe("read-only"); + expect(resolveCodexSandbox("workspace-write")).toBe("workspace-write"); + expect(resolveCodexSandbox("bypass")).toBe("bypass"); + }); + + it("rejects an undocumented value and names the valid ones", () => { + expect(() => resolveCodexSandbox("danger-full-access")).toThrow(/LOOMGRAPH_CODEX_SANDBOX/); + expect(() => resolveCodexSandbox("danger-full-access")).toThrow(/read-only, workspace-write, bypass/); + }); + + it("rejects a wrong-case value rather than passing it through to codex", () => { + expect(() => resolveCodexSandbox("READ-ONLY")).toThrow(/LOOMGRAPH_CODEX_SANDBOX/); + }); +}); + +describe("CodexAdapter process handling", () => { + // Stub scripts only, invoked by absolute path. No real agent CLI is spawned + // and PATH is never touched. + let dir: string; + + beforeAll(async () => { + dir = await mkdtemp(join(tmpdir(), "lg-codex-stub-")); + }); + + afterAll(async () => { + await rm(dir, { recursive: true, force: true }); + }); + + it("names the binary when it is missing", async () => { + const bin = join(dir, "definitely-not-installed"); + const out = await new CodexAdapter(bin).run({ prompt: "hi", cwd: dir, timeoutSec: 10 }); + expect(out.ok).toBe(false); + expect(out.error).toContain(bin); + expect(out.error).toMatch(/not found on PATH/); + }); + + it("times out when a grandchild outlives the CLI", async () => { + const bin = join(dir, "slow-stub.sh"); + await writeFile(bin, `#!/bin/sh\nsleep 30 &\necho '{"msg":{"type":"agent_message","message":"ok"}}'\n`, { + mode: 0o755, + }); + const started = Date.now(); + const out = await new CodexAdapter(bin).run({ prompt: "hi", cwd: dir, timeoutSec: 2 }); + expect(out.ok).toBe(false); + expect(out.error).toMatch(/timeout after 2s/); + expect(Date.now() - started).toBeLessThan(3500); + }, 20000); }); diff --git a/src/adapters/codex.ts b/src/adapters/codex.ts index 19bbaf9..db518b0 100644 --- a/src/adapters/codex.ts +++ b/src/adapters/codex.ts @@ -1,4 +1,4 @@ -import { execa } from "execa"; +import { clampCostUsd, runProcess } from "./types.js"; import type { Adapter, AdapterInput, AdapterOutput } from "./types.js"; /** @@ -27,7 +27,28 @@ import type { Adapter, AdapterInput, AdapterOutput } from "./types.js"; * "bwrap: loopback: Failed RTM_NEWADDR: Operation not permitted", which makes * every repository read fail and the review useless. */ -export type CodexSandbox = "read-only" | "workspace-write" | "bypass"; +const CODEX_SANDBOXES = ["read-only", "workspace-write", "bypass"] as const; + +export type CodexSandbox = (typeof CODEX_SANDBOXES)[number]; + +/** + * Validate the LOOMGRAPH_CODEX_SANDBOX override rather than casting it. + * An unvalidated cast lets a typo through in both directions: an invented + * value like "danger-full-access" reads as a wider policy that is never + * applied, and "READ-ONLY" reaches codex verbatim and fails inside it. Fail + * here instead, naming the three values that work. + */ +export function resolveCodexSandbox(raw: string | undefined | null): CodexSandbox { + const value = (raw ?? "").trim(); + if (value.length === 0) return "read-only"; + const match = CODEX_SANDBOXES.find((candidate) => candidate === value); + if (match === undefined) { + throw new Error( + `invalid LOOMGRAPH_CODEX_SANDBOX "${raw}" - valid values are ${CODEX_SANDBOXES.join(", ")}`, + ); + } + return match; +} export function buildCodexArgs( prompt: string, @@ -91,7 +112,8 @@ export function parseCodexJsonl(stdout: string): AdapterOutput { } // Codex normally reports no price at all. Record 0 rather than estimating one. - const costUsd = cumulativeUsd ?? deltaUsd; + // A negative report is treated as 0 - it must never drive the budget back. + const costUsd = clampCostUsd(cumulativeUsd ?? deltaUsd); if (text === null) { return { @@ -106,11 +128,52 @@ export function parseCodexJsonl(stdout: string): AdapterOutput { return { ok: true, text, costUsd, raw: events, error: null }; } +/** + * A broken codex sandbox makes every repository read fail with a bubblewrap + * diagnostic like "bwrap: loopback: Failed RTM_NEWADDR: Operation not + * permitted". bwrap fails per TOOL CALL rather than at startup, so codex keeps + * talking, exits 0, and can hand back a confident "PASS" from a verifier that + * read nothing at all. Trusting the exit code lets that through, so the + * diagnostic is looked for explicitly on both streams. + * + * Matching is anchored to a line that starts with "bwrap:" - that is the shape + * bubblewrap writes - so an agent message that merely discusses bwrap in prose + * does not fail the node. + */ +export function detectSandboxFailure(text: string): string | null { + for (const line of text.split("\n")) { + const trimmed = line.trim(); + if (trimmed.startsWith("bwrap:")) return trimmed; + } + return null; +} + +function sandboxFailureMessage(stderr: string, stdout: string): string | null { + const line = detectSandboxFailure(stderr) ?? detectSandboxFailure(stdout); + if (line === null) return null; + return ( + `codex sandbox failed (${line}) - the verifier could not read the tree, so its verdict means nothing; ` + + `set LOOMGRAPH_CODEX_SANDBOX=bypass only on an already isolated host` + ); +} + /** * Decide the final result once the process has exited. Kept separate from * `run` so the exit-code policy is unit-testable without spawning codex. */ -export function decideCodexResult(parsed: AdapterOutput, exitCode: number | undefined, stderr: string): AdapterOutput { +export function decideCodexResult( + parsed: AdapterOutput, + exitCode: number | undefined, + stderr: string, + stdout = "", +): AdapterOutput { + // Check the sandbox before the exit code: a broken sandbox fails every read + // even when codex exits 0 and the agent message contains the pass string. + const sandbox = sandboxFailureMessage(stderr, stdout); + if (sandbox !== null) { + return { ...parsed, ok: false, error: sandbox }; + } + const trimmed = stderr.trim(); const failed = exitCode !== 0 && exitCode !== undefined; @@ -130,26 +193,28 @@ export class CodexAdapter implements Adapter { constructor( private readonly bin = "codex", - private readonly sandbox: CodexSandbox = (process.env.LOOMGRAPH_CODEX_SANDBOX as CodexSandbox) ?? "read-only", + private readonly sandbox: CodexSandbox = resolveCodexSandbox(process.env.LOOMGRAPH_CODEX_SANDBOX), ) {} async run(input: AdapterInput): Promise { - const result = await execa(this.bin, buildCodexArgs(input.prompt, input.cwd, this.sandbox, input.model), { - cwd: input.cwd, - timeout: input.timeoutSec * 1000, - reject: false, - // Codex waits on stdin when it stays open, which stalls the node until the - // timeout fires. Close it so the run is genuinely non-interactive. - input: "", - }); - - const stdout = typeof result.stdout === "string" ? result.stdout : ""; - const stderr = typeof result.stderr === "string" ? result.stderr : ""; + const result = await runProcess( + this.bin, + buildCodexArgs(input.prompt, input.cwd, this.sandbox, input.model), + { cwd: input.cwd, timeoutSec: input.timeoutSec }, + ); + + const { stdout, stderr } = result; + + // Name the binary rather than letting an empty stdout surface as a parse + // failure with no clue about what is missing. + if (result.spawnErrorCode === "ENOENT") { + return { ok: false, text: "", costUsd: 0, raw: { stdout, stderr }, error: `${this.bin} not found on PATH` }; + } if (result.timedOut) { return { ok: false, text: stdout, costUsd: 0, raw: { stdout, stderr }, error: `timeout after ${input.timeoutSec}s` }; } - return decideCodexResult(parseCodexJsonl(stdout), result.exitCode, stderr); + return decideCodexResult(parseCodexJsonl(stdout), result.exitCode, stderr, stdout); } } From 5d549146f661dfc70348e6667ea5886406eff535 Mon Sep 17 00:00:00 2001 From: Dat Date: Sun, 23 Aug 2026 08:43:22 +0700 Subject: [PATCH 06/34] fix(resume): reject --answer for unknown or non-paused node ids An --answer for a node id that does not exist, or that exists but is not awaiting an answer, was silently discarded (exit 4, no warning), so a typo was indistinguishable from a correct answer. Validate both before resuming and exit 1 naming the node id. Also stop delegating --answer pair parsing to the shared --var parser, which leaked "--var expects key=value" into the --answer error message. Co-Authored-By: Claude Opus 5 (1M context) --- src/commands/plan.test.ts | 89 ++++++++++++++++++++++++++++++++++++++- src/commands/resume.ts | 26 ++++++++++-- 2 files changed, 109 insertions(+), 6 deletions(-) diff --git a/src/commands/plan.test.ts b/src/commands/plan.test.ts index 31d3435..01c76c8 100644 --- a/src/commands/plan.test.ts +++ b/src/commands/plan.test.ts @@ -1,7 +1,12 @@ -import { describe, it, expect } from "vitest"; +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { mkdtempSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { parseGraph } from "../core/graph.js"; -import { planLevels } from "../core/engine.js"; +import { planLevels, newRunState } from "../core/engine.js"; +import { CheckpointStore } from "../core/store.js"; import { renderPlan } from "./render.js"; +import { resumeCommand } from "./resume.js"; const FANOUT = ` name: fanout @@ -32,3 +37,83 @@ describe("renderPlan", () => { expect(out).toContain("command"); }); }); + +const GATED = ` +name: gated +budget: { maxUsd: 1, maxWallClockSec: 60, maxNodeRuns: 9 } +nodes: + before: { type: command, run: "echo before" } + approve: { type: human, question: "Ship it?" } + later: { type: human, question: "Later?" } +edges: + - { from: before, to: approve } + - { from: approve, to: later } + - { from: later, to: END } +`; + +describe("resumeCommand --answer validation", () => { + let dir: string; + let originalCwd: string; + let store: CheckpointStore; + + beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), "lg-resume-")); + originalCwd = process.cwd(); + process.chdir(dir); + store = new CheckpointStore(join(dir, ".loomgraph", "runs")); + }); + + afterEach(() => { + process.chdir(originalCwd); + }); + + function seedPausedRun(runId: string, graphSrc: string): void { + const graph = parseGraph(graphSrc); + const state = newRunState(graph, { runId, cwd: dir }); + state.status = "paused"; + state.nodes = { + before: { + nodeId: "before", status: "succeeded", startedAt: "", endedAt: null, + attempts: 1, output: "", error: null, costUsd: 0, + }, + }; + state.completed = ["before"]; + store.save(state); + store.saveGraphSource(runId, graphSrc); + } + + async function captureError(fn: () => Promise): Promise<{ code: number; errors: string[] }> { + const errors: string[] = []; + const original = console.error; + console.error = (...args: unknown[]) => void errors.push(args.join(" ")); + try { + const code = await fn(); + return { code, errors }; + } finally { + console.error = original; + } + } + + it("exits 1 and names the node when --answer names a node that is not in the graph", async () => { + seedPausedRun("gated-unknown", GATED); + const { code, errors } = await captureError(() => resumeCommand("gated-unknown", { answer: ["nosuch=hi"] })); + expect(code).toBe(1); + expect(errors.join("\n")).toContain('--answer: no node "nosuch" in this run'); + }); + + it("exits 1 and names the node when --answer names a node that is not awaiting an answer", async () => { + seedPausedRun("gated-notpaused", GATED); + const { code, errors } = await captureError(() => resumeCommand("gated-notpaused", { answer: ["later=hi"] })); + expect(code).toBe(1); + expect(errors.join("\n")).toContain('--answer: node "later" is not awaiting an answer'); + }); + + it("reports a malformed --answer pair without mentioning --var", async () => { + seedPausedRun("gated-malformed", GATED); + const { code, errors } = await captureError(() => resumeCommand("gated-malformed", { answer: ["approve"] })); + expect(code).toBe(1); + const message = errors.join("\n"); + expect(message).toContain('--answer expects nodeId=text, got "approve"'); + expect(message).not.toContain("--var"); + }); +}); diff --git a/src/commands/resume.ts b/src/commands/resume.ts index 10c023f..4918465 100644 --- a/src/commands/resume.ts +++ b/src/commands/resume.ts @@ -1,8 +1,8 @@ import { defaultRegistry } from "../adapters/registry.js"; -import { execute } from "../core/engine.js"; +import { execute, readySet } from "../core/engine.js"; import { parseGraph } from "../core/graph.js"; import { formatEventLine, openLog, openStore } from "./context.js"; -import { exitCodeFor, parseVars, renderStatus } from "./render.js"; +import { exitCodeFor, renderStatus } from "./render.js"; export interface ResumeOptions { answer: string[]; @@ -30,13 +30,31 @@ export async function resumeCommand(runId: string, options: ResumeOptions): Prom let humanAnswers: Record; try { - humanAnswers = parseVars(options.answer ?? []); + humanAnswers = {}; + for (const pair of options.answer ?? []) { + const eq = pair.indexOf("="); + if (eq <= 0) throw new Error(`--answer expects nodeId=text, got "${pair}"`); + humanAnswers[pair.slice(0, eq)] = pair.slice(eq + 1); + } } catch (err) { - console.error(`--answer expects nodeId=text: ${(err as Error).message}`); + console.error((err as Error).message); return 1; } const graph = parseGraph(source, `${runId}/graph.yaml`); + + const awaiting = readySet(graph, state).filter((id) => graph.nodes[id]!.type === "human"); + for (const nodeId of Object.keys(humanAnswers)) { + if (graph.nodes[nodeId] === undefined) { + console.error(`--answer: no node "${nodeId}" in this run`); + return 1; + } + if (!awaiting.includes(nodeId)) { + console.error(`--answer: node "${nodeId}" is not awaiting an answer`); + return 1; + } + } + const final = await execute(graph, state, { store, log, From e0018238b53f45e672674fea640d9222c1e87e75 Mon Sep 17 00:00:00 2001 From: Dat Date: Sun, 23 Aug 2026 08:43:26 +0700 Subject: [PATCH 07/34] fix: treat a bare `key` assignment as a credential The assignment rule only knew names ending in TOKEN, SECRET, PASSWD, PASSWORD or API_KEY, so `key=...` and `{"key": "..."}` - the shape several agent config files actually use - walked past the scanner. API-KEY with a hyphen was missed for the same reason. Co-Authored-By: Claude Opus 5 (1M context) --- src/handoff/scan.test.ts | 12 ++++++++++++ src/handoff/scan.ts | 8 +++++--- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/handoff/scan.test.ts b/src/handoff/scan.test.ts index 90f0e65..d5c4928 100644 --- a/src/handoff/scan.test.ts +++ b/src/handoff/scan.test.ts @@ -231,6 +231,18 @@ describe("scanText — hits and near-misses per rule", () => { expect(fires("rotate the ENCLAVE_TOKEN weekly", "env-assignment")).toBe(false); }); + it("env-assignment catches a bare key assignment in both shell and json shapes", () => { + expect(fires("key=" + shaped("FAKE", "fakevalue0000"), "env-assignment")).toBe(true); + expect(fires('{"key": "' + shaped("FAKE", "fakevalue0000") + '"}', "env-assignment")).toBe(true); + expect(fires('{"key":"' + shaped("FAKE", "fakevalue0000") + '"}', "env-assignment")).toBe(true); + expect(fires("MY_SERVICE_KEY=" + shaped("FAKE", "fakevalue0000"), "env-assignment")).toBe(true); + // The hyphenated spelling of the api key name. + expect(fires('API-KEY: "' + shaped("FAKE", "fakevalue0000") + '"', "env-assignment")).toBe(true); + // Placeholders stay placeholders. + expect(fires("key=", "env-assignment")).toBe(false); + expect(fires('key=""', "env-assignment")).toBe(false); + }); + it("abs-home-path", () => { expect(fires("/Users/someone/Documents/notes.md", "abs-home-path")).toBe(true); expect(fires("/home/someone/src/app.ts", "abs-home-path")).toBe(true); diff --git a/src/handoff/scan.ts b/src/handoff/scan.ts index 60c26f9..9c8136a 100644 --- a/src/handoff/scan.ts +++ b/src/handoff/scan.ts @@ -78,10 +78,12 @@ export const SCAN_RULES: ReadonlyArray<{ name: "env-assignment", // Case-insensitive: a .env line is upper-case by convention but a JSON key // or a lower-case shell export is the same secret. The name may also BE the - // word (`password: x`), not just end in it. The value must be non-empty: - // `FOO_TOKEN=` and `FOO_TOKEN=""` are placeholders, not secrets. + // word (`password: x`), not just end in it. A bare `key` name counts too, + // because an opencode / .netrc style config writes the credential under + // exactly that name. The value must be non-empty: `FOO_TOKEN=` and + // `FOO_TOKEN=""` are placeholders, not secrets. pattern: - /(?:[A-Za-z0-9_]*_)?(?:TOKEN|SECRET|PASSWD|PASSWORD|API_?KEY)"?\s*[:=]\s*(?:"[^"\s]+"|'[^'\s]+'|[^\s"';,]+)/i, + /(?:[A-Za-z0-9_]*_)?(?:TOKEN|SECRET|PASSWD|PASSWORD|API[_-]?KEY|KEY)"?\s*[:=]\s*(?:"[^"\s]+"|'[^'\s]+'|[^\s"';,]+)/i, description: "Assignment to a token / secret / password / api-key name", }, { From fb84b9d27ff463f8749523b53ded3642fb356e78 Mon Sep 17 00:00:00 2001 From: Dat Date: Sun, 23 Aug 2026 08:48:16 +0700 Subject: [PATCH 08/34] fix(graph): reject unknown template node references at validate time lg validate accepted {{nodes.nope.output}} and exited 0; the run then died mid-flight with 'unknown template reference' and was left stranded. Check every {{...}} reference at graph load time against the resolver's own grammar and the graph's declared node ids. Var references are deliberately not checked: lg run --var injects vars the graph never declares, so an undeclared var reference is not statically decidable. Also fixes 'a adapter' -> 'an adapter' in the node-type guard. Co-Authored-By: Claude Opus 5 (1M context) --- src/core/graph.test.ts | 95 ++++++++++++++++++++++++++++++++++++++++++ src/core/graph.ts | 39 ++++++++++++++++- 2 files changed, 133 insertions(+), 1 deletion(-) diff --git a/src/core/graph.test.ts b/src/core/graph.test.ts index 99ccc5f..a530d06 100644 --- a/src/core/graph.test.ts +++ b/src/core/graph.test.ts @@ -503,3 +503,98 @@ edges: expect(() => parseGraph(src)).toThrow(/reserved/); }); }); + +describe("template references", () => { + it("accepts declared bare, dotted and node-output references", () => { + const src = graph(`vars: + ticket: ABC-1 +nodes: + a: + type: command + run: "echo {{ticket}} {{vars.ticket}}" + b: + type: agent + adapter: claude + prompt: "Use {{nodes.a.output}}" +edges: + - from: a + to: b + - from: b + to: END +`); + expect(parseGraph(src).nodes.a).toMatchObject({ type: "command" }); + expect(parseGraph(src).nodes.b).toMatchObject({ type: "agent" }); + }); + + it("rejects a reference to an undeclared node, naming node and reference", () => { + const src = graph(`vars: + ticket: ABC-1 +nodes: + a: + type: command + run: "echo {{ticket}}" + b: + type: command + run: "echo {{nodes.nope.output}}" +edges: + - from: a + to: b + - from: b + to: END +`); + expect(() => parseGraph(src)).toThrow(GraphValidationError); + expect(() => parseGraph(src)).toThrow(/node "b"/); + expect(() => parseGraph(src)).toThrow(/nodes\.nope\.output/); + }); + + it("accepts a dotted var reference to an undeclared var (--var supplies it at runtime)", () => { + const src = graph(`nodes: + a: + type: command + run: "echo {{vars.nosuchvar}}" +edges: + - from: a + to: END +`); + expect(parseGraph(src).nodes.a).toMatchObject({ type: "command", run: "echo {{vars.nosuchvar}}" }); + }); + + it("accepts a bare reference that is not a declared var (--var supplies it at runtime)", () => { + const src = graph(`nodes: + a: + type: command + run: "echo {{nosuchvar}}" +edges: + - from: a + to: END +`); + expect(parseGraph(src).nodes.a).toMatchObject({ type: "command", run: "echo {{nosuchvar}}" }); + }); + + it("rejects a reference shape the runtime resolver would reject", () => { + const src = graph(`nodes: + a: + type: command + run: "echo {{vars.ticket.extra}}" +edges: + - from: a + to: END +`); + expect(() => parseGraph(src)).toThrow(/vars\.ticket\.extra/); + }); + + it("validates verifier prompts too", () => { + const src = graph(`nodes: + a: + type: verifier + adapter: codex + prompt: "Check {{nodes.ghost.output}}" + pass: "PASS" +edges: + - from: a + to: END +`); + expect(() => parseGraph(src)).toThrow(/node "a"/); + expect(() => parseGraph(src)).toThrow(/ghost/); + }); +}); diff --git a/src/core/graph.ts b/src/core/graph.ts index 5de58f6..073c268 100644 --- a/src/core/graph.ts +++ b/src/core/graph.ts @@ -110,6 +110,42 @@ function asList(value: unknown, where: string): string[] { return fail(`${where} must be a node id or a list of node ids`); } +/** + * The same `{{...}}` grammar the engine's `interpolate` resolver accepts: + * a bare shorthand `{{x}}` (meaning `{{vars.x}}`), `{{vars.x}}`, or + * `{{nodes..output}}`. Anything else, or a `nodes..output` reference + * to an undeclared node id, is a static validation error. + * + * Var references are deliberately NOT checked against the graph's `vars:` + * block: `lg run --var name=value` injects vars at runtime that the block + * never declares, so an undeclared var is not statically decidable. Only node + * ids, which come exclusively from the graph itself, are load-time decidable. + */ +const templateRefPattern = /\{\{\s*([A-Za-z0-9_.\-]+)\s*\}\}/g; + +function checkTemplateReferences(nodes: Record): void { + for (const [id, def] of Object.entries(nodes)) { + const template = + def.type === "command" ? def.run : def.type === "agent" || def.type === "verifier" ? def.prompt : null; + if (template === null) continue; + for (const match of template.matchAll(templateRefPattern)) { + const ref = match[1]!; + const parts = ref.split("."); + let ok: boolean; + if (parts.length === 1) { + ok = true; + } else if (parts[0] === "vars" && parts.length === 2) { + ok = true; + } else if (parts[0] === "nodes" && parts.length === 3 && parts[2] === "output") { + ok = parts[1]! in nodes; + } else { + ok = false; + } + if (!ok) fail(`node "${id}": unknown template reference "{{${ref}}}"`); + } + } +} + /** Parse a graph from YAML text and validate it structurally. */ export function parseGraph(source: string, sourceName = "graph"): Graph { let raw: unknown; @@ -160,13 +196,14 @@ export function parseGraph(source: string, sourceName = "graph"): Graph { fail(`node "${id}" of type "${type}" cannot declare a model - only agent and verifier nodes dispatch to an adapter`); } if ((value as Record).adapter !== undefined && type !== "agent" && type !== "verifier") { - fail(`node "${id}" of type "${type}" cannot declare a adapter - only agent and verifier nodes dispatch to an adapter`); + fail(`node "${id}" of type "${type}" cannot declare an adapter - only agent and verifier nodes dispatch to an adapter`); } const parsed = nodeSchema.safeParse(value); if (!parsed.success) fail(issuesToMessage(`node "${id}"`, parsed.error)); nodes[id] = parsed.data; } if (Object.keys(nodes).length === 0) fail("graph has no nodes"); + checkTemplateReferences(nodes); const rawEdges = doc.edges; if (!Array.isArray(rawEdges) || rawEdges.length === 0) fail("edges is required and must be a non-empty list"); From f65f410928afc0521fa9e0d8cb90e3fba488458e Mon Sep 17 00:00:00 2001 From: Dat Date: Sun, 23 Aug 2026 08:48:19 +0700 Subject: [PATCH 09/34] fix: make the assignment rule linear and stop it firing on prose The optional name prefix was unanchored, so at every offset in a line the engine consumed to end-of-line looking for an underscore. A 64k single-character line took 8.3 seconds and doubled fourfold per doubling; it now takes 2 milliseconds. A lookbehind pins the name to a real word start, which fixes the cost and also stops `monkey=` being read as `key=`. An unquoted value must now be at least eight characters. A quoted value is a config value at any length, but a short bare word is prose: the line "Standalone token: user" was reported as a secret and blocked a pack. Co-Authored-By: Claude Opus 5 (1M context) --- src/handoff/scan.test.ts | 25 +++++++++++++++++++++++++ src/handoff/scan.ts | 11 ++++++++--- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/src/handoff/scan.test.ts b/src/handoff/scan.test.ts index d5c4928..cc155ce 100644 --- a/src/handoff/scan.test.ts +++ b/src/handoff/scan.test.ts @@ -231,6 +231,18 @@ describe("scanText — hits and near-misses per rule", () => { expect(fires("rotate the ENCLAVE_TOKEN weekly", "env-assignment")).toBe(false); }); + it("env-assignment does not fire on prose that merely mentions a secret name", () => { + // This exact line blocked a real bundle. A short unquoted word is prose, not a secret. + expect(fires("Standalone token: user", "env-assignment")).toBe(false); + expect(fires("the secret: none", "env-assignment")).toBe(false); + expect(fires("password: yes", "env-assignment")).toBe(false); + // A secret name embedded in a longer word is not an assignment either. + expect(fires("monkey=" + shaped("FAKE", "fakevalue0000"), "env-assignment")).toBe(false); + // Real assignments must survive. + expect(fires("MY_SERVICE_TOKEN=" + shaped("FAKE", "fakevalue0000"), "env-assignment")).toBe(true); + expect(fires('APP_SECRET: "' + shaped("FAKE", "fakevalue0000") + '"', "env-assignment")).toBe(true); + }); + it("env-assignment catches a bare key assignment in both shell and json shapes", () => { expect(fires("key=" + shaped("FAKE", "fakevalue0000"), "env-assignment")).toBe(true); expect(fires('{"key": "' + shaped("FAKE", "fakevalue0000") + '"}', "env-assignment")).toBe(true); @@ -512,3 +524,16 @@ describe("stripUrlCredentials", () => { expect(scanText(tokenAsUser, "meta.json").map((f) => f.rule)).toContain("github-token"); }); }); + +describe("scanText - performance", () => { + it("scans a 64k single-character line in well under two seconds", () => { + // Regression guard: the env-assignment prefix used to rescan to end-of-line at + // every offset, which made this line take ~9 seconds. + const line = "a".repeat(65536); + const started = performance.now(); + const findings = scanText(line, "big.md"); + const elapsedMs = performance.now() - started; + expect(findings).toEqual([]); + expect(elapsedMs).toBeLessThan(2000); + }); +}); diff --git a/src/handoff/scan.ts b/src/handoff/scan.ts index 9c8136a..36d03d4 100644 --- a/src/handoff/scan.ts +++ b/src/handoff/scan.ts @@ -80,10 +80,15 @@ export const SCAN_RULES: ReadonlyArray<{ // or a lower-case shell export is the same secret. The name may also BE the // word (`password: x`), not just end in it. A bare `key` name counts too, // because an opencode / .netrc style config writes the credential under - // exactly that name. The value must be non-empty: `FOO_TOKEN=` and - // `FOO_TOKEN=""` are placeholders, not secrets. + // exactly that name. The lookbehind anchors the name to a real word start, + // so `monkey=x` is not a `key=x` assignment and the prefix never gets to + // rescan to end-of-line at every offset - that was a quadratic blow-up: a + // 64k line took 9 seconds. The value must be non-empty, and an unquoted + // value must be at least 8 characters: `FOO_TOKEN=` and `FOO_TOKEN=""` are + // placeholders, a quoted value is a config value, but a short bare word is + // prose - `Standalone token: user` used to block a bundle. pattern: - /(?:[A-Za-z0-9_]*_)?(?:TOKEN|SECRET|PASSWD|PASSWORD|API[_-]?KEY|KEY)"?\s*[:=]\s*(?:"[^"\s]+"|'[^'\s]+'|[^\s"';,]+)/i, + /(? Date: Sun, 23 Aug 2026 08:51:06 +0700 Subject: [PATCH 10/34] fix(engine): L4 match verifier pass string on word boundaries A raw substring test let PASSWORD and BYPASS satisfy pass: "PASS". Match the pass string as a whole token instead, escaping regex metacharacters so a value like v1.0 stays literal. Case-sensitive, as documented. --- src/core/engine.test.ts | 43 ++++++++++++++++++++++++++++++++++++++++- src/core/engine.ts | 18 ++++++++++++++++- 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/src/core/engine.test.ts b/src/core/engine.test.ts index d735f81..040c7a9 100644 --- a/src/core/engine.test.ts +++ b/src/core/engine.test.ts @@ -5,7 +5,7 @@ import { join } from "node:path"; import { parseGraph } from "./graph.js"; import { CheckpointStore } from "./store.js"; import { EventLog } from "./events.js"; -import { execute, newRunState, interpolate, readySet, EngineError } from "./engine.js"; +import { execute, newRunState, interpolate, readySet, containsPassToken, EngineError } from "./engine.js"; import * as engine from "./engine.js"; import type { EngineDeps } from "./engine.js"; import type { Adapter, AdapterInput, AdapterOutput } from "../adapters/types.js"; @@ -169,6 +169,25 @@ describe("checkCommandExpectations", () => { }); }); +describe("containsPassToken", () => { + it("matches a whole word but not a substring of a longer word", () => { + expect(containsPassToken("looks good - PASS", "PASS")).toBe(true); + expect(containsPassToken("PASS", "PASS")).toBe(true); + expect(containsPassToken("conclusion: PASS.", "PASS")).toBe(true); + expect(containsPassToken("(PASS)", "PASS")).toBe(true); + expect(containsPassToken("PASSWORD", "PASS")).toBe(false); + expect(containsPassToken("BYPASS", "PASS")).toBe(false); + expect(containsPassToken("xPASS", "PASS")).toBe(false); + expect(containsPassToken("PASSx", "PASS")).toBe(false); + }); + + it("matches the pass string literally, including regex metacharacters", () => { + expect(containsPassToken("status is ok? - done", "ok?")).toBe(true); + expect(containsPassToken("okay?", "ok?")).toBe(false); + expect(containsPassToken("[DONE]", "[DONE]")).toBe(true); + }); +}); + describe("execute", () => { it("runs a linear graph in order and succeeds", async () => { const order: string[] = []; @@ -402,6 +421,28 @@ edges: expect(final.nodes.v!.error).toMatch(/PASS/); }); + it("does not let a substring of a longer word satisfy a verifier's pass string", async () => { + const src = ` +name: verify +budget: { maxUsd: 10, maxWallClockSec: 600, maxNodeRuns: 20 } +nodes: + v: { type: verifier, adapter: codex, prompt: "review", pass: "PASS" } +edges: + - { from: v, to: END } +`; + for (const output of ["password confirmed", "BYPASS every check", "xPASS", "PASSx", "mismatch: password"]) { + const registry = { codex: stub("codex", () => ok(output)) }; + const final = await execute(parseGraph(src), start(src, `run-${output}`), deps(registry)); + expect(final.status, `output "${output}" must fail`).toBe("failed"); + expect(final.nodes.v!.error).toMatch(/did not report the pass string/); + } + + const boundary = { codex: stub("codex", () => ok("PASS")) }; + expect((await execute(parseGraph(src), start(src, "boundary-run"), deps(boundary))).status).toBe("succeeded"); + const ending = { codex: stub("codex", () => ok("conclusion: PASS.")) }; + expect((await execute(parseGraph(src), start(src, "ending-run"), deps(ending))).status).toBe("succeeded"); + }); + it("fails a command node whose output does not meet its expectations", async () => { const src = ` name: expect diff --git a/src/core/engine.ts b/src/core/engine.ts index 93cb262..2c927da 100644 --- a/src/core/engine.ts +++ b/src/core/engine.ts @@ -124,6 +124,22 @@ export function checkCommandExpectations( return null; } +/** Escape a literal string so it can appear inside a RegExp. */ +function escapeRegExp(s: string): string { + return s.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +} + +/** + * True when `pass` appears in `text` as a whole token, not as a raw substring. + * The token must not be glued to word characters on either side, so "PASS" + * matches "looks good - PASS" but not "PASSWORD" or "BYPASS". The pass string + * itself is otherwise matched literally. + */ +export function containsPassToken(text: string, pass: string): boolean { + const boundary = "(?:^|[^A-Za-z0-9_])"; + return new RegExp(`${boundary}${escapeRegExp(pass)}(?:$|[^A-Za-z0-9_])`).test(text); +} + /** * The batches the scheduler would dispatch, in order. Pure - used by * `lg run --dry-run`, which must not spawn anything. @@ -244,7 +260,7 @@ export async function execute(graph: Graph, initial: RunState, deps: EngineDeps) timeoutSec: def.timeoutSec, }); - if (def.type === "verifier" && out.ok && !out.text.includes(def.pass)) { + if (def.type === "verifier" && out.ok && !containsPassToken(out.text, def.pass)) { return { ...out, ok: false, error: `verifier "${id}" did not report the pass string "${def.pass}"` }; } return out; From a433872f214d034a598ae38e1b6141f1282270dd Mon Sep 17 00:00:00 2001 From: Dat Date: Sun, 23 Aug 2026 08:51:35 +0700 Subject: [PATCH 11/34] fix: rewrite the machine hostname out of a handoff bundle Path rewriting removed the repo root, the home directory and the username but left the machine name in place, so every bundle published the box it was built on. Both forms go: what `hostname` prints and what `hostname -s` prints, the long form first so no dangling `.local` is left behind. The boundary is host-shaped rather than path-shaped, because a hostname may contain `.` and `-` - `web1` must not be pulled out of `web10` or `xweb1`. The option is optional, so callers that do not pass one are unaffected. Co-Authored-By: Claude Opus 5 (1M context) --- src/handoff/scan.test.ts | 41 ++++++++++++++++++++++++++++++++++++++++ src/handoff/scan.ts | 37 +++++++++++++++++++++++++++++++++--- 2 files changed, 75 insertions(+), 3 deletions(-) diff --git a/src/handoff/scan.test.ts b/src/handoff/scan.test.ts index cc155ce..708b855 100644 --- a/src/handoff/scan.test.ts +++ b/src/handoff/scan.test.ts @@ -412,6 +412,47 @@ describe("rewritePaths", () => { }); expect(fires(out, "abs-home-path")).toBe(false); }); + + it("rewrites the machine hostname in both its long and short forms", () => { + const out = rewritePaths( + "built on dats-macbook.local, short form dats-macbook, and dats-macbookpro stays", + { + home: "/Users/dat", + username: "dat", + repoRoot: "/Users/dat/app", + hostname: "dats-macbook.local", + }, + ); + expect(out).toBe( + "built on ${HOSTNAME}, short form ${HOSTNAME}, and dats-macbookpro stays", + ); + expect(out).not.toContain("dats-macbook.local"); + }); + + it("does not rewrite a hostname that is a prefix or suffix of a longer token", () => { + const out = rewritePaths("host web1 vs web10 vs web1a vs xweb1", { + home: "/opt/h", + username: "", + repoRoot: "/opt/r", + hostname: "web1", + }); + expect(out).toBe("host ${HOSTNAME} vs web10 vs web1a vs xweb1"); + }); + + it("leaves the text alone when no hostname is supplied", () => { + const input = "built on dats-macbook.local"; + expect( + rewritePaths(input, { home: "/Users/dat", username: "dat", repoRoot: "/Users/dat/app" }), + ).toBe(input); + expect( + rewritePaths(input, { + home: "/Users/dat", + username: "dat", + repoRoot: "/Users/dat/app", + hostname: "", + }), + ).toBe(input); + }); }); describe("scanBundleDir", () => { diff --git a/src/handoff/scan.ts b/src/handoff/scan.ts index 36d03d4..692286c 100644 --- a/src/handoff/scan.ts +++ b/src/handoff/scan.ts @@ -300,6 +300,20 @@ function replaceUsernameToken(text: string, username: string): string { return text.replace(re, "$1user"); } +/** + * Replace `host` only when it stands alone as a host token. + * + * Bounded the same way `replaceUsernameToken` is bounded, but with a host-shaped + * character class: a hostname may legitimately contain `.` and `-`, so `web1` + * must not be pulled out of `web10`, `web1a` or `xweb1`. + */ +function replaceHostnameToken(text: string, host: string): string { + if (host.length === 0) return text; + const escaped = host.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + const re = new RegExp(`(^|[^A-Za-z0-9.-])${escaped}(?![A-Za-z0-9.-])`, "g"); + return text.replace(re, (_match, prefix: string) => `${prefix}${HOSTNAME_PLACEHOLDER}`); +} + /** * Rewrite machine-specific absolute paths into placeholders. * @@ -309,12 +323,14 @@ function replaceUsernameToken(text: string, username: string): string { * username are rewritten too, because a transcript can quote a path from * another machine. A standalone username token becomes `user`, but only at * path / whitespace / quote / `@` / `:` boundaries so `dataset` and - * `dat-laptop` stay intact. Residual `/Users//` paths are still caught - * by the `abs-home-path` scan rule. + * `dat-laptop` stay intact. The machine hostname is rewritten to `${HOSTNAME}` + * in both its long and short forms, bounded so it cannot be pulled out of a + * longer token. Residual `/Users//` paths are still caught by the + * `abs-home-path` scan rule. */ export function rewritePaths( text: string, - opts: { home: string; username: string; repoRoot: string }, + opts: { home: string; username: string; repoRoot: string; hostname?: string }, ): string { const roots: Array<{ from: string; to: string }> = [ { from: normalizeRoot(opts.repoRoot), to: "${REPO_ROOT}" }, @@ -329,6 +345,18 @@ export function rewritePaths( out = replaceLiteral(out, root.from, root.to); } + // The full form first: rewriting the short form first would leave a dangling + // `.local` behind. `hostname` prints the long form, `hostname -s` the short one, + // and a transcript can quote either. + const host = opts.hostname ?? ""; + if (host.length > 0) { + out = replaceHostnameToken(out, host); + const short = host.split(".")[0] ?? ""; + if (short.length > 0 && short !== host) { + out = replaceHostnameToken(out, short); + } + } + if (opts.username.length > 0) { for (const shape of [ `/home/${opts.username}`, @@ -371,3 +399,6 @@ export function stripUrlCredentials(text: string): string { /** What replaces a stripped `user:pass` pair, so the removal is visible. */ const CREDENTIAL_PLACEHOLDER = "${CREDENTIALS_REMOVED}"; + +/** What replaces the machine hostname, so the removal is visible in a published bundle. */ +const HOSTNAME_PLACEHOLDER = "${HOSTNAME}"; From 23770048ece7a345016270f2f327e9b2db02b838 Mon Sep 17 00:00:00 2001 From: Dat Date: Sun, 23 Aug 2026 08:56:02 +0700 Subject: [PATCH 12/34] fix(report): publish only the report, not its parent directory M8. `lg report --out ./r1.html --publish` handed dirname(--out) to enclave, which in practice meant the whole working directory: the graph yaml, the entire .loomgraph state dir, and every other report. One flag, a mass upload. Publish now stages a fresh mkdtemp directory the tool owns, copies only the generated report into it, hands that to enclave, and removes it in a finally so a failed push does not leak the staging dir. Co-Authored-By: Claude Opus 5 (1M context) --- src/commands/report.test.ts | 114 +++++++++++++++++++++++++++++++++++- src/commands/report.ts | 64 ++++++++++++-------- 2 files changed, 152 insertions(+), 26 deletions(-) diff --git a/src/commands/report.test.ts b/src/commands/report.test.ts index 5cee137..15aaddc 100644 --- a/src/commands/report.test.ts +++ b/src/commands/report.test.ts @@ -1,5 +1,10 @@ -import { describe, it, expect } from "vitest"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { chmodSync, existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { delimiter, dirname, join } from "node:path"; +import { CheckpointStore } from "../core/store.js"; import { escapeHtml, renderReportHtml } from "./render.js"; +import { reportCommand } from "./report.js"; import type { LgEvent } from "../core/events.js"; import type { RunState } from "../core/types.js"; @@ -108,3 +113,110 @@ describe("renderReportHtml", () => { expect(out).toContain("run_finished"); }); }); + +describe("reportCommand --publish", () => { + const runId = "demo-20260814-120000-ab12"; + const title = `loomgraph run ${runId}`; + const pushJson = JSON.stringify({ + artifactId: "art-1", + versionId: "v1", + versionNo: 1, + viewUrl: "https://enclave.example/v/1", + uploaded: [], + skipped: [], + }); + + let work: string; + let fakeBin: string; + let capturedDir: string; + let capturedListing: string; + let originalCwd: string; + let originalPath: string | undefined; + + beforeEach(() => { + work = mkdtempSync(join(tmpdir(), "lg-report-work-")); + fakeBin = mkdtempSync(join(tmpdir(), "lg-report-bin-")); + capturedDir = join(fakeBin, "dir.txt"); + capturedListing = join(fakeBin, "listing.txt"); + originalCwd = process.cwd(); + originalPath = process.env.PATH; + process.chdir(work); + + // A real run record, so reportCommand can find the run. + new CheckpointStore(join(work, ".loomgraph", "runs")).save(makeState()); + + // Decoys sitting in the working directory: the exact files that must never + // be uploaded when --publish is used. + writeFileSync(join(work, "hello.yaml"), "graph: {}\n", "utf8"); + mkdirSync(join(work, ".loomgraph", "secrets"), { recursive: true }); + writeFileSync(join(work, ".loomgraph", "secrets", "token.txt"), "token", "utf8"); + writeFileSync(join(work, "other-report.html"), "old", "utf8"); + + // FAKE enclave: a stub script that records the directory it is told to + // publish and the listing of that directory, then answers like the real + // binary would. The real enclave on the host PATH is never touched. + const stub = join(fakeBin, "enclave"); + writeFileSync( + stub, + [ + "#!/bin/sh", + `printf '%s\\n' "$2" > "${capturedDir}"`, + `ls -A "$2" > "${capturedListing}"`, + `printf '%s' '${pushJson}'`, + "", + ].join("\n"), + "utf8", + ); + chmodSync(stub, 0o755); + process.env.PATH = `${fakeBin}${delimiter}${originalPath}`; + }); + + afterEach(() => { + process.chdir(originalCwd); + if (originalPath === undefined) delete process.env.PATH; + else process.env.PATH = originalPath; + rmSync(work, { recursive: true, force: true }); + rmSync(fakeBin, { recursive: true, force: true }); + }); + + it("publishes only the generated report, never the working directory", async () => { + const out = join(work, "r1.html"); + const code = await reportCommand(runId, { out, publish: true, title }); + + expect(code).toBe(0); + expect(existsSync(out)).toBe(true); + + // The directory handed to the publisher must be neither the working + // directory nor the report's own parent (they are the same dir here). + const pushedDir = readFileSync(capturedDir, "utf8").trim(); + expect(pushedDir).not.toBe(work); + expect(pushedDir).not.toBe(dirname(out)); + + // And it must contain exactly one entry: the report itself, none of the + // decoys (hello.yaml, .loomgraph/…, other-report.html). + const listing = readFileSync(capturedListing, "utf8") + .split("\n") + .filter((l) => l !== ""); + expect(listing).toEqual(["r1.html"]); + }); + + it("removes the staging directory after publishing", async () => { + const out = join(work, "r1.html"); + const code = await reportCommand(runId, { out, publish: true, title }); + + expect(code).toBe(0); + const pushedDir = readFileSync(capturedDir, "utf8").trim(); + expect(pushedDir).not.toBe(work); + expect(existsSync(pushedDir)).toBe(false); + }); + + it("does not call the publisher at all without --publish", async () => { + const out = join(work, "r1.html"); + const code = await reportCommand(runId, { out, title }); + + expect(code).toBe(0); + expect(existsSync(out)).toBe(true); + // The fake stub records nothing: never invoked. + expect(existsSync(capturedDir)).toBe(false); + }); +}); diff --git a/src/commands/report.ts b/src/commands/report.ts index 7dad981..db90e47 100644 --- a/src/commands/report.ts +++ b/src/commands/report.ts @@ -1,6 +1,7 @@ import { execa } from "execa"; -import { dirname, join, resolve } from "node:path"; -import { mkdirSync, writeFileSync } from "node:fs"; +import { basename, dirname, join, resolve } from "node:path"; +import { copyFileSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; import { buildEnclavePushArgs, parseEnclavePushJson } from "../adapters/enclave.js"; import { openLog, openStore, runsDir } from "./context.js"; import { renderReportHtml } from "./render.js"; @@ -30,32 +31,45 @@ export async function reportCommand(runId: string, opts: ReportOptions = {}): Pr if (opts.publish !== true) return 0; - const args = buildEnclavePushArgs( - dirname(htmlPath), - opts.title ?? `loomgraph run ${runId}`, - opts.visibility ?? "private", - ); + // Publish ONLY the generated report. Stage a fresh directory we own under + // os.tmpdir(), copy the single report file into it, and hand that directory + // to enclave. Never pass the report's parent directory (which may contain + // the whole working tree, .loomgraph state, other reports, notes...) to the + // publisher. Enclave only ever sees the one file we put in the staging dir. + let stagingDir: string | undefined; + try { + stagingDir = mkdtempSync(join(tmpdir(), "lg-report-")); + copyFileSync(htmlPath, join(stagingDir, basename(htmlPath))); - const result = await execa("enclave", args, { reject: false }); + const args = buildEnclavePushArgs( + stagingDir, + opts.title ?? `loomgraph run ${runId}`, + opts.visibility ?? "private", + ); - if (result.failed && result.code === "ENOENT") { - console.error("enclave not found on PATH - the report was written but not published"); - return 0; - } + const result = await execa("enclave", args, { reject: false }); - if (result.exitCode !== 0) { - const stderr = typeof result.stderr === "string" ? result.stderr.trim() : ""; - if (stderr) console.error(stderr); - return 2; - } + if (result.failed && result.code === "ENOENT") { + console.error("enclave not found on PATH - the report was written but not published"); + return 0; + } - const stdout = typeof result.stdout === "string" ? result.stdout : ""; - const parsed = parseEnclavePushJson(stdout); - if (!parsed.ok) { - console.error(parsed.error); - return 2; - } + if (result.exitCode !== 0) { + const stderr = typeof result.stderr === "string" ? result.stderr.trim() : ""; + if (stderr) console.error(stderr); + return 2; + } - console.log(parsed.viewUrl); - return 0; + const stdout = typeof result.stdout === "string" ? result.stdout : ""; + const parsed = parseEnclavePushJson(stdout); + if (!parsed.ok) { + console.error(parsed.error); + return 2; + } + + console.log(parsed.viewUrl); + return 0; + } finally { + if (stagingDir) rmSync(stagingDir, { recursive: true, force: true }); + } } From 53d6c15b510dc818173c187288719d78ad1dfde9 Mon Sep 17 00:00:00 2001 From: Dat Date: Sun, 23 Aug 2026 08:58:37 +0700 Subject: [PATCH 13/34] fix(budget): H3 make the three ceilings exclusive maxNodeRuns N now permits N runs and maxUsd 2.00 permits exactly 2.00; only going over a ceiling stops the run. Owner decision. Updates the tests that encoded the old inclusive rule. --- src/core/budget.test.ts | 23 ++++++++++-- src/core/budget.ts | 10 +++--- src/core/engine.test.ts | 79 ++++++++++++++++++++++++++++++++++++++--- 3 files changed, 100 insertions(+), 12 deletions(-) diff --git a/src/core/budget.test.ts b/src/core/budget.test.ts index 0e57372..7e10876 100644 --- a/src/core/budget.test.ts +++ b/src/core/budget.test.ts @@ -20,18 +20,35 @@ describe("checkBudget", () => { expect(res.ok).toBe(true); }); - it("fails exactly at the usd ceiling", () => { + it("passes at exactly the usd ceiling (exclusive)", () => { const res = checkBudget(makeState({ spent: { usd: 1, wallClockSec: 0, nodeRuns: 0 } })); + expect(res.ok).toBe(true); + }); + + it("passes at exactly the node-run ceiling (exclusive)", () => { + const res = checkBudget(makeState({ spent: { usd: 0, wallClockSec: 0, nodeRuns: 5 } })); + expect(res.ok).toBe(true); + }); + + it("fails just over the usd ceiling", () => { + const res = checkBudget(makeState({ spent: { usd: 1.01, wallClockSec: 0, nodeRuns: 0 } })); expect(res.ok).toBe(false); expect(res.ok === false && res.reason).toMatch(/maxUsd/); }); - it("fails exactly at the node-run ceiling", () => { - const res = checkBudget(makeState({ spent: { usd: 0, wallClockSec: 0, nodeRuns: 5 } })); + it("fails just over the node-run ceiling", () => { + const res = checkBudget(makeState({ spent: { usd: 0, wallClockSec: 0, nodeRuns: 6 } })); expect(res.ok).toBe(false); expect(res.ok === false && res.reason).toMatch(/maxNodeRuns/); }); + it("passes at exactly the wall-clock ceiling (exclusive)", () => { + const now = Date.now(); + const createdAt = new Date(now - 60_000).toISOString(); + const res = checkBudget(makeState({ createdAt }), now); + expect(res.ok).toBe(true); + }); + it("fails exactly at the wall-clock ceiling measured from createdAt", () => { const createdAt = new Date(Date.now() - 61_000).toISOString(); const res = checkBudget(makeState({ createdAt })); diff --git a/src/core/budget.ts b/src/core/budget.ts index afa3704..7563020 100644 --- a/src/core/budget.ts +++ b/src/core/budget.ts @@ -9,20 +9,20 @@ export function elapsedSec(state: RunState, now = Date.now()): number { } /** - * Checked before every dispatch batch. Ceilings are inclusive: hitting the - * limit exactly stops the run. + * Checked before every dispatch batch. Ceilings are exclusive: hitting the + * limit exactly is allowed, only going over stops the run. */ export function checkBudget(state: RunState, now = Date.now()): BudgetCheck { const { budget, spent } = state; - if (spent.usd >= budget.maxUsd) { + if (spent.usd > budget.maxUsd) { return { ok: false, reason: `maxUsd exceeded: spent ${spent.usd} of ${budget.maxUsd} usd` }; } - if (spent.nodeRuns >= budget.maxNodeRuns) { + if (spent.nodeRuns > budget.maxNodeRuns) { return { ok: false, reason: `maxNodeRuns exceeded: ran ${spent.nodeRuns} of ${budget.maxNodeRuns} nodes` }; } const elapsed = elapsedSec(state, now); - if (elapsed >= budget.maxWallClockSec) { + if (elapsed > budget.maxWallClockSec) { return { ok: false, reason: `maxWallClockSec exceeded: ${Math.round(elapsed)}s elapsed of ${budget.maxWallClockSec}s`, diff --git a/src/core/engine.test.ts b/src/core/engine.test.ts index 040c7a9..b07d509 100644 --- a/src/core/engine.test.ts +++ b/src/core/engine.test.ts @@ -504,9 +504,11 @@ edges: const registry = { command: stub("command", (i) => ok(i.prompt)) }; const final = await execute(parseGraph(src), start(src), deps(registry)); - expect(final.spent.nodeRuns).toBe(2); + // H3: ceilings are exclusive, so maxNodeRuns 2 permits runs 1 and 2. The + // third node still overshoots and stops the run. C2 tightens this further + // by refusing to dispatch node c at all. + expect(final.spent.nodeRuns).toBe(3); expect(final.status).toBe("failed"); - expect(final.nodes.c).toBeUndefined(); }); it("fails a run whose final spend exceeds the usd ceiling", async () => { @@ -596,7 +598,7 @@ edges: expect(log.read("run1").filter((e) => e.kind === "budget_exceeded")).toHaveLength(0); }); - it("treats a final spend exactly at the usd ceiling as a breach", async () => { + it("treats a final spend exactly at the usd ceiling as within budget", async () => { const src = ` name: exactly budget: { maxUsd: 0.30, maxWallClockSec: 600, maxNodeRuns: 20 } @@ -613,10 +615,34 @@ edges: const final = await execute(parseGraph(src), start(src), deps(registry)); + // H3: ceilings are exclusive. Spending exactly maxUsd is permitted. + expect(final.status).toBe("succeeded"); + expect(log.read("run1").filter((e) => e.kind === "budget_exceeded")).toHaveLength(0); + }); + + it("fails a run whose final spend is a cent over the usd ceiling", async () => { + const src = ` +name: overby +budget: { maxUsd: 0.30, maxWallClockSec: 600, maxNodeRuns: 20 } +nodes: + a: { type: command, run: "echo a" } + b: { type: command, run: "echo b" } +edges: + - { from: a, to: b } + - { from: b, to: END } +`; + const registry = { + command: stub("command", (i) => (i.prompt === "echo a" ? ok("a", 0.15) : ok("b", 0.16))), + }; + + const final = await execute(parseGraph(src), start(src), deps(registry)); + expect(final.status).toBe("failed"); + const exceeded = log.read("run1").filter((e) => e.kind === "budget_exceeded"); + expect(String(exceeded[0]!.data.reason)).toMatch(/maxUsd/); }); - it("fails a run whose final node-run count exceeds maxNodeRuns", async () => { + it("allows a run whose final node-run count is exactly maxNodeRuns", async () => { const src = ` name: runcap budget: { maxUsd: 10, maxWallClockSec: 600, maxNodeRuns: 2 } @@ -631,6 +657,51 @@ edges: const final = await execute(parseGraph(src), start(src), deps(registry)); + // H3: ceilings are exclusive, so maxNodeRuns 2 permits exactly 2 runs. + expect(final.status).toBe("succeeded"); + expect(final.spent.nodeRuns).toBe(2); + expect(log.read("run1").filter((e) => e.kind === "budget_exceeded")).toHaveLength(0); + }); + + it("permits exactly maxNodeRuns node runs across a chain", async () => { + const src = ` +name: runcap-exact +budget: { maxUsd: 10, maxWallClockSec: 600, maxNodeRuns: 3 } +nodes: + a: { type: command, run: "echo a" } + b: { type: command, run: "echo b" } + c: { type: command, run: "echo c" } +edges: + - { from: a, to: b } + - { from: b, to: c } + - { from: c, to: END } +`; + const registry = { command: stub("command", (i) => ok(i.prompt)) }; + + const final = await execute(parseGraph(src), start(src), deps(registry)); + + expect(final.status).toBe("succeeded"); + expect(final.spent.nodeRuns).toBe(3); + expect(final.nodes.c!.status).toBe("succeeded"); + }); + + it("fails a run whose final node-run count exceeds maxNodeRuns", async () => { + const src = ` +name: runcap-over +budget: { maxUsd: 10, maxWallClockSec: 600, maxNodeRuns: 2 } +nodes: + a: { type: command, run: "echo a" } + b: { type: command, run: "echo b" } + c: { type: command, run: "echo c" } +edges: + - { from: a, to: b } + - { from: b, to: c } + - { from: c, to: END } +`; + const registry = { command: stub("command", (i) => ok(i.prompt)) }; + + const final = await execute(parseGraph(src), start(src), deps(registry)); + expect(final.status).toBe("failed"); const exceeded = log.read("run1").filter((e) => e.kind === "budget_exceeded"); expect(exceeded).toHaveLength(1); From 8b8f3886d694c6fc52883c486d12ae6df105120f Mon Sep 17 00:00:00 2001 From: Dat Date: Sun, 23 Aug 2026 09:05:56 +0700 Subject: [PATCH 14/34] fix(engine): C2 gate the node-run ceiling before every dispatch maxNodeRuns bounded only batch boundaries, so a 4-way fan-out ran every node and landed every side effect before the run reported it was over budget. Admit nodes one at a time against a projected spend and stop dispatching the moment the ceiling would be crossed. --- src/core/engine.test.ts | 60 +++++++++++++++++++++++++++++++++++++---- src/core/engine.ts | 26 ++++++++++++++++-- 2 files changed, 79 insertions(+), 7 deletions(-) diff --git a/src/core/engine.test.ts b/src/core/engine.test.ts index b07d509..bce3103 100644 --- a/src/core/engine.test.ts +++ b/src/core/engine.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, beforeEach } from "vitest"; -import { mkdtempSync } from "node:fs"; +import { mkdtempSync, mkdirSync, existsSync, readdirSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { parseGraph } from "./graph.js"; @@ -9,6 +9,7 @@ import { execute, newRunState, interpolate, readySet, containsPassToken, EngineE import * as engine from "./engine.js"; import type { EngineDeps } from "./engine.js"; import type { Adapter, AdapterInput, AdapterOutput } from "../adapters/types.js"; +import { CommandAdapter } from "../adapters/command.js"; import type { RunState } from "./types.js"; function ok(text: string, costUsd = 0): AdapterOutput { @@ -504,11 +505,60 @@ edges: const registry = { command: stub("command", (i) => ok(i.prompt)) }; const final = await execute(parseGraph(src), start(src), deps(registry)); - // H3: ceilings are exclusive, so maxNodeRuns 2 permits runs 1 and 2. The - // third node still overshoots and stops the run. C2 tightens this further - // by refusing to dispatch node c at all. - expect(final.spent.nodeRuns).toBe(3); + // H3: ceilings are exclusive, so maxNodeRuns 2 permits runs 1 and 2. C2 + // goes further and refuses to dispatch node c at all, so the recorded + // spend stays exactly at the ceiling instead of overshooting to 3. + expect(final.spent.nodeRuns).toBe(2); + expect(final.nodes.c).toBeUndefined(); + expect(final.status).toBe("failed"); + const exceeded = log.read("run1").filter((e) => e.kind === "budget_exceeded"); + expect(exceeded).toHaveLength(1); + expect(String(exceeded[0]!.data.reason)).toMatch(/maxNodeRuns/); + }); + + it("refuses to dispatch the fan-out nodes that would exceed maxNodeRuns, leaving no side effects (C2)", async () => { + const sent = join(dir, "sent"); + mkdirSync(sent); + + const src = ` +name: c2-admission +budget: { maxUsd: 999, maxWallClockSec: 999999, maxNodeRuns: 2 } +nodes: + root: { type: command, run: "touch sent/root" } + a: { type: command, run: "touch sent/a" } + b: { type: command, run: "touch sent/b" } + c: { type: command, run: "touch sent/c" } + d: { type: command, run: "touch sent/d" } +edges: + - { from: root, to: [a, b, c, d] } + - { from: [a, b, c, d], to: END } +`; + // Real command adapter: command nodes spawn `touch` inside the temporary + // directory (never an agent CLI), so side effects really land on disk. + const registry = { command: new CommandAdapter() }; + + const final = await execute(parseGraph(src), start(src, "c2-run"), deps(registry)); + expect(final.status).toBe("failed"); + expect(final.spent.nodeRuns).toBe(2); + expect(existsSync(join(sent, "root"))).toBe(true); + + // Proof that three of the four fan-out nodes were never dispatched: the + // sent directory holds exactly two entries, and only one of a/d-d exists. + const entries = readdirSync(sent); + expect(entries).toHaveLength(2); + expect(entries).toContain("root"); + const survivor = entries.find((f) => f !== "root"); + expect(survivor).toBeDefined(); + expect(["a", "b", "c", "d"]).toContain(survivor); + for (const name of ["a", "b", "c", "d"]) { + expect(existsSync(join(sent, name))).toBe(name === survivor); + } + + const exceeded = log.read("c2-run").filter((e) => e.kind === "budget_exceeded"); + expect(exceeded).toHaveLength(1); + expect(String(exceeded[0]!.data.reason)).toMatch(/maxNodeRuns/); + expect((exceeded[0]!.data.spent as { nodeRuns: number }).nodeRuns).toBe(2); }); it("fails a run whose final spend exceeds the usd ceiling", async () => { diff --git a/src/core/engine.ts b/src/core/engine.ts index 2c927da..b2d76e6 100644 --- a/src/core/engine.ts +++ b/src/core/engine.ts @@ -327,9 +327,26 @@ export async function execute(graph: Graph, initial: RunState, deps: EngineDeps) const batch = ready.filter((id) => graph.nodes[id]!.type !== "human"); if (batch.length > 0) { - // Fan-out: the whole ready set runs concurrently, checkpointing as each lands. + // Admission pass: maxNodeRuns must bound work, not just batch boundaries. + // Project the spend one node at a time and ask checkBudget before each + // dispatch, so a node whose run would push the spent node-run count over + // the ceiling is never dispatched and leaves no side effect. Everything + // admitted still runs concurrently, checkpointing as each lands. + let projection = state; + const admitted: string[] = []; + let budgetStopReason: string | null = null; + for (const id of batch) { + projection = recordSpend(projection, { usd: 0, nodeRuns: 1 }); + const admission = checkBudget(projection); + if (!admission.ok) { + budgetStopReason = admission.reason; + break; + } + admitted.push(id); + } + const results = await Promise.all( - batch.map(async (id) => { + admitted.map(async (id) => { const result = await executeNode(id, graph.nodes[id]!); commit(result); return result; @@ -341,6 +358,11 @@ export async function execute(graph: Graph, initial: RunState, deps: EngineDeps) const detail = failed.map((r) => `${r.nodeId}: ${r.error}`).join("; "); return finish("failed", `node failed after ${failed[0]!.attempts} attempt(s) - ${detail}`); } + + if (budgetStopReason !== null) { + emit({ kind: "budget_exceeded", data: { reason: budgetStopReason, spent: state.spent, budget: state.budget } }); + return finish("failed", budgetStopReason); + } } } } From 568d2dd9ba1080c540f00117b931cd7fbb61ea92 Mon Sep 17 00:00:00 2001 From: Dat Date: Sun, 23 Aug 2026 09:07:09 +0700 Subject: [PATCH 15/34] fix(report): validate --visibility at runtime, not just in the type M7. The "private" | "org" union was compile-time only, so `--visibility public` and `--visibility hackerman` were forwarded verbatim to enclave and the command exited 0. Publishing a run report as public is a leak. Refuse anything but private or org before the staging dir is created or enclave is spawned, exit 1, and match the refusal wording lg-handoff push already uses. Co-Authored-By: Claude Opus 5 (1M context) --- src/commands/report.test.ts | 32 ++++++++++++++++++++++++++++++++ src/commands/report.ts | 10 +++++++++- 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/src/commands/report.test.ts b/src/commands/report.test.ts index 15aaddc..668b00d 100644 --- a/src/commands/report.test.ts +++ b/src/commands/report.test.ts @@ -219,4 +219,36 @@ describe("reportCommand --publish", () => { // The fake stub records nothing: never invoked. expect(existsSync(capturedDir)).toBe(false); }); + + it.each(["public", "hackerman"])( + "refuses --visibility %s with exit code 1 and never spawns enclave", + async (visibility) => { + const out = join(work, "r1.html"); + const code = await reportCommand(runId, { + out, + publish: true, + visibility: visibility as "private" | "org", + }); + + expect(code).toBe(1); + expect(existsSync(out)).toBe(true); + // The fake stub records nothing: never invoked. + expect(existsSync(capturedDir)).toBe(false); + }, + ); + + it.each(["private", "org"])("still accepts --visibility %s", async (visibility) => { + const out = join(work, "r1.html"); + const code = await reportCommand(runId, { + out, + publish: true, + title, + visibility: visibility as "private" | "org", + }); + + expect(code).toBe(0); + expect(existsSync(out)).toBe(true); + // The fake stub was invoked and recorded the pushed directory. + expect(existsSync(capturedDir)).toBe(true); + }); }); diff --git a/src/commands/report.ts b/src/commands/report.ts index db90e47..6d54c22 100644 --- a/src/commands/report.ts +++ b/src/commands/report.ts @@ -31,6 +31,14 @@ export async function reportCommand(runId: string, opts: ReportOptions = {}): Pr if (opts.publish !== true) return 0; + const visibility = opts.visibility ?? "private"; + if (visibility !== "private" && visibility !== "org") { + console.error( + `refusing --visibility ${visibility}: only private and org are allowed.`, + ); + return 1; + } + // Publish ONLY the generated report. Stage a fresh directory we own under // os.tmpdir(), copy the single report file into it, and hand that directory // to enclave. Never pass the report's parent directory (which may contain @@ -44,7 +52,7 @@ export async function reportCommand(runId: string, opts: ReportOptions = {}): Pr const args = buildEnclavePushArgs( stagingDir, opts.title ?? `loomgraph run ${runId}`, - opts.visibility ?? "private", + visibility, ); const result = await execa("enclave", args, { reject: false }); From 6b82da5ee2a8365f0ef84b6544e528b3648887ce Mon Sep 17 00:00:00 2001 From: Dat Date: Sun, 23 Aug 2026 09:11:59 +0700 Subject: [PATCH 16/34] fix(engine): C2 budget-check every retry attempt A retry is a node run, but the retry loop never consulted the budget, so maxNodeRuns 2 with retries 2 reached 4 runs and emitted no budget_exceeded. Check the projected spend before each attempt and stop through the same budget-exceeded path the admission pass uses. --- src/core/engine.test.ts | 28 ++++++++++++++++++++++++++++ src/core/engine.ts | 29 +++++++++++++++++++++++------ 2 files changed, 51 insertions(+), 6 deletions(-) diff --git a/src/core/engine.test.ts b/src/core/engine.test.ts index bce3103..9dd5c8b 100644 --- a/src/core/engine.test.ts +++ b/src/core/engine.test.ts @@ -561,6 +561,34 @@ edges: expect((exceeded[0]!.data.spent as { nodeRuns: number }).nodeRuns).toBe(2); }); + it("refuses a retry attempt that would exceed maxNodeRuns, reporting budget_exceeded (C2 part B)", async () => { + const src = ` +name: c2b-retry +budget: { maxUsd: 999, maxWallClockSec: 999999, maxNodeRuns: 2 } +nodes: + a: { type: command, run: "always broken", retries: 2 } +edges: + - { from: a, to: END } +`; + // The node always fails, so with retries 2 it would normally make 3 attempts. + const registry = { command: stub("command", () => bad("still broken")) }; + const final = await execute(parseGraph(src), start(src, "c2b-run"), deps(registry)); + + expect(final.status).toBe("failed"); + expect(final.nodes.a!.status).toBe("failed"); + // The second attempt is the last one the budget permits; the third is refused. + expect(final.nodes.a!.attempts).toBe(2); + expect(final.nodes.a!.attempts).not.toBe(3); + expect(final.nodes.a!.attempts).not.toBe(4); + const started = log.read("c2b-run").filter((e) => e.kind === "node_started" && e.nodeId === "a"); + expect(started).toHaveLength(2); + expect(final.spent.nodeRuns).toBe(2); + + const exceeded = log.read("c2b-run").filter((e) => e.kind === "budget_exceeded"); + expect(exceeded).toHaveLength(1); + expect(String(exceeded[0]!.data.reason)).toMatch(/maxNodeRuns/); + }); + it("fails a run whose final spend exceeds the usd ceiling", async () => { const src = ` name: overshoot diff --git a/src/core/engine.ts b/src/core/engine.ts index b2d76e6..f46c490 100644 --- a/src/core/engine.ts +++ b/src/core/engine.ts @@ -186,6 +186,10 @@ export async function execute(graph: Graph, initial: RunState, deps: EngineDeps) const answers = deps.humanAnswers ?? {}; let state = initial; + // Shared with the admission pass below: set by both the batch admission and + // a retry refused inside executeNode, and always consumed by the budget + // stop path (emit budget_exceeded + finish("failed", reason)). + let budgetStopReason: string | null = null; if (state.status === "succeeded" || state.status === "failed") { throw new EngineError(`run ${state.runId} is already ${state.status} and cannot be executed again`); } @@ -217,6 +221,19 @@ export async function execute(graph: Graph, initial: RunState, deps: EngineDeps) let lastText = ""; while (attempts < maxAttempts) { + // C2 part B: a retry is a node run like any other, so one must never be + // spent once the ceilings would be crossed. Check the budget before each + // attempt and stop retrying the moment it refuses. The first attempt of + // an admitted node always passes: part A already admitted it with a + // projection of one run, and attempts + 1 equals 1 here. + const projected = recordSpend(state, { usd: 0, nodeRuns: attempts + 1 }); + const admission = checkBudget(projected); + if (!admission.ok) { + budgetStopReason = admission.reason; + lastError = admission.reason; + break; + } + attempts += 1; emit({ kind: "node_started", nodeId: id, data: { attempt: attempts, type: def.type } }); @@ -334,7 +351,6 @@ export async function execute(graph: Graph, initial: RunState, deps: EngineDeps) // admitted still runs concurrently, checkpointing as each lands. let projection = state; const admitted: string[] = []; - let budgetStopReason: string | null = null; for (const id of batch) { projection = recordSpend(projection, { usd: 0, nodeRuns: 1 }); const admission = checkBudget(projection); @@ -354,15 +370,16 @@ export async function execute(graph: Graph, initial: RunState, deps: EngineDeps) ); const failed = results.filter((r) => r.status === "failed"); - if (failed.length > 0) { - const detail = failed.map((r) => `${r.nodeId}: ${r.error}`).join("; "); - return finish("failed", `node failed after ${failed[0]!.attempts} attempt(s) - ${detail}`); - } - + // C2 part B: a retry refused by the budget must surface as budget_exceeded, + // exactly like the part A admission stop, not as a generic node failure. if (budgetStopReason !== null) { emit({ kind: "budget_exceeded", data: { reason: budgetStopReason, spent: state.spent, budget: state.budget } }); return finish("failed", budgetStopReason); } + if (failed.length > 0) { + const detail = failed.map((r) => `${r.nodeId}: ${r.error}`).join("; "); + return finish("failed", `node failed after ${failed[0]!.attempts} attempt(s) - ${detail}`); + } } } } From bbe7b3a008cb4d329aa78018f02d671ba8c0c67b Mon Sep 17 00:00:00 2001 From: Dat Date: Sun, 23 Aug 2026 09:17:55 +0700 Subject: [PATCH 17/34] fix(report): declare charset and language in the report HTML L9. The generated run report had no and no lang on , so any non-ASCII character in a prompt, an agent's output or an error message was left to browser encoding guesswork, and screen readers had no language to pick a voice from. charset is now the first child of , well inside the first 1024 bytes where it is actually honoured. Two existing assertions on the lowercase were updated to the canonical uppercase form. Co-Authored-By: Claude Opus 5 (1M context) --- src/commands/render.ts | 5 ++-- src/commands/report.test.ts | 48 +++++++++++++++++++++++++++++++++++-- 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/src/commands/render.ts b/src/commands/render.ts index 52f15f0..2b45e23 100644 --- a/src/commands/render.ts +++ b/src/commands/render.ts @@ -140,9 +140,10 @@ export function renderReportHtml(state: RunState, events: LgEvent[]): string { `${state.spent.nodeRuns}/${state.budget.maxNodeRuns} node runs`; return ( - "" + - "" + + "" + + '' + "" + + '' + `loomgraph run ${esc(state.runId)}` + "