From f96b0969e068397b4a265266405aa08753b7c532 Mon Sep 17 00:00:00 2001 From: Drew Stone Date: Tue, 1 Sep 2026 13:48:51 -0600 Subject: [PATCH 1/3] refactor(claim-evidence): consume agent-eval runBoundedProcess; delete the private runner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The claim grader spawned its own checks through `execFile`, which puts the check and every descendant it starts in the GRADER's process group. A deadline therefore killed the shell alone: a check that backgrounds its real work — `solver & wait` — left that work running, holding the stdout and stderr pipes open, so the grader observed no result either. Three solver processes outlived their grading parent by five days, and a grading loop hung twice on the same cause, for 8 hours and for 1.9 hours. agent-eval owns the runner that closes this. `runBoundedProcess` gives the check its own process group and kills the group, so a deadline reaches the whole tree, and it forces a non-zero status on a killed run so a SIGKILLed child that closes with 0 cannot read as a pass. The 32-line private `runBash` is deleted. Both call sites — the `bash -n` parse and the execution — go through the shared runner, and the check body now reaches bash as an argument vector rather than as shell text, so no quoting stands between an author's check and the interpreter. That needed `args` on `runBoundedProcess`, added upstream in agent-eval 0.172.1. Grading is unchanged for a check that runs and finishes; every existing caller test passes with its fixtures untouched. Three runner-side conditions are restored into the vocabulary `UNRUNNABLE_SIGNATURES` is calibrated on, because none of them is a verdict on the claim: a run the caller aborted, a run whose capture overflowed `maxBufferBytes`, and a run that could not be spawned. A deadline now reports 124, this file's own `DEADLINE_EXIT_CODE`, where the old wrapper reported 127. Three tests are added for what the move buys, each of which fails against the deleted runner: a backgrounded descendant is gone after the deadline, an overflowed capture is not graded, and an aborted check is not graded. --- CHANGELOG.md | 15 +++++ package.json | 2 +- src/claim-evidence.test.ts | 109 +++++++++++++++++++++++++++++++++++++ src/claim-evidence.ts | 107 +++++++++++++++++++++++++----------- 4 files changed, 200 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b4569a7..3a4b964 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,20 @@ # Changelog +## 12.1.0 — 2026-09-01 + +### Changed + +- `verifyGradeableEvidence` runs a claim's check through agent-eval's `runBoundedProcess` instead of this package's own `execFile` wrapper. +The private wrapper spawned the check in the GRADER's process group, so a deadline killed the shell alone. +A check that starts its real work in the background — `solver & wait` — left that work running, holding the stdout and stderr pipes open, and the grader observed no result either. +Three solver processes outlived their grading parent by five days, and a grading loop hung twice on the same cause, for 8 hours and for 1.9 hours. +The shared runner gives the check its own process group and kills the group, so a deadline reaches the whole tree. +- The check body reaches bash as an argument vector and never as shell text, so no quoting stands between an author's check and the interpreter that reads it. +This needed `args` on `runBoundedProcess`, added in agent-eval 0.172.1. +- The peer range on `@tangle-network/agent-eval` moves to `>=0.172.1 <0.173.0`. +- Grading is unchanged for a check that runs and finishes. Three runner-side conditions are now reported in the vocabulary `UNRUNNABLE_SIGNATURES` is calibrated on, so none of them can be read as a verdict on the claim: a run the caller aborted, a run whose capture overflowed `maxBufferBytes`, and a run that could not be spawned. +A deadline now reports exit status 124, this package's own `DEADLINE_EXIT_CODE`, where the old wrapper reported 127. + ## 12.0.2 — 2026-09-01 ### Changed diff --git a/package.json b/package.json index 4f74429..8f770b9 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@tangle-network/agent-knowledge", - "version": "12.0.2", + "version": "12.1.0", "description": "Build, search, evaluate, and improve source-backed knowledge bases.", "homepage": "https://github.com/tangle-network/agent-knowledge#readme", "repository": { diff --git a/src/claim-evidence.test.ts b/src/claim-evidence.test.ts index 686b1b8..692bb77 100644 --- a/src/claim-evidence.test.ts +++ b/src/claim-evidence.test.ts @@ -1,3 +1,4 @@ +import { execFile } from 'node:child_process' import { tmpdir } from 'node:os' import { describe, expect, it } from 'vitest' import { @@ -603,3 +604,111 @@ describe('verifyGradeableEvidence — the executor reports its own deadline', () expect(verified.grade).toEqual({ verdict: 'verified' }) }) }) + +/** Pids still in a process group, read through `pgrep`, whose "no match" exit is 1. */ +async function pidsInGroup(pgid: number): Promise { + return await new Promise((resolve, reject) => { + execFile('pgrep', ['-g', String(pgid)], (error, stdout) => { + const found = stdout.split('\n').filter((line) => line.trim().length > 0) + if (error && (error as { code?: unknown }).code !== 1) { + reject(error) + return + } + resolve(found) + }) + }) +} + +describe('verifyGradeableEvidence — a deadline reaches the descendants, not just the shell', () => { + it.skipIf(process.platform === 'win32')( + 'leaves nothing running when the check backgrounds its real work', + async () => { + // The defect this file's runner was moved to close. A check that starts a solver in the + // background puts the real work in a GRANDCHILD of the grader. A deadline aimed at the + // shell alone leaves that grandchild running, holding the stdout and stderr pipes open, so + // the grader never observes a result either. Three solver processes outlived their grading + // parent by five days on this exact shape, and a grading loop hung twice on it, for 8 hours + // and for 1.9 hours. Both writes below are shell builtins, so the two pids reach the pipe + // with no fork and no PATH lookup. + const started = Date.now() + const verified = await verifyGradeableEvidence( + { rung: 4, check: 'echo $$; sleep 60 & echo $!; wait', expect: 'never printed' }, + { cwd: tmpdir(), env: { PATH: process.env.PATH ?? '' }, timeoutMs: 200 }, + ) + + // The check asks for 60s of work. Returning near that means the kill missed the descendant. + expect(Date.now() - started).toBeLessThan(15_000) + expect(verified.execution.timedOut).toBe(true) + expect(verified.grade.verdict).toBe('unrunnable') + + const [leader, descendant] = verified.execution.stdout + .split('\n') + .map((line) => Number.parseInt(line.trim(), 10)) + expect(Number.isInteger(leader) && (leader as number) > 1).toBe(true) + expect(Number.isInteger(descendant) && (descendant as number) > 1).toBe(true) + + const deadline = Date.now() + 5000 + let survivors = await pidsInGroup(leader as number) + while (survivors.length > 0 && Date.now() < deadline) { + await new Promise((resolve) => setTimeout(resolve, 50)) + survivors = await pidsInGroup(leader as number) + } + expect(survivors).toEqual([]) + // Asserted directly, because a surviving `sleep` answers `kill(pid, 0)` whatever pgrep says. + let alive = true + try { + process.kill(descendant as number, 0) + } catch { + alive = false + } + expect(alive).toBe(false) + }, + 40_000, + ) + + it.skipIf(process.platform === 'win32')( + 'refuses to grade a check whose output the runner could not read in full', + async () => { + // A capture that overflowed is not a reading of the check. Grading the truncated text would + // report `contradicted` for an expectation that was printed in the bytes nobody kept, which + // is a verdict on the grader and not on the claim. + const verified = await verifyGradeableEvidence( + { + rung: 4, + check: 'head -c 200000 /dev/zero | tr "\\0" "x"; echo "done=$((0 + 1))"', + expect: 'done=1', + }, + { + cwd: tmpdir(), + env: { PATH: process.env.PATH ?? '' }, + timeoutMs: 20_000, + maxBufferBytes: 4096, + }, + ) + expect(verified.execution.exitCode).not.toBe(0) + expect(verified.execution.stderr).toContain('ERR_CHILD_PROCESS_STDIO_MAXBUFFER') + expect(verified.grade.verdict).toBe('unrunnable') + }, + 40_000, + ) + + it.skipIf(process.platform === 'win32')( + 'refuses to grade a check the caller aborted', + async () => { + const controller = new AbortController() + setTimeout(() => controller.abort(), 100) + const verified = await verifyGradeableEvidence( + { rung: 4, check: 'sleep 30; echo "done=$((0 + 1))"', expect: 'done=1' }, + { + cwd: tmpdir(), + env: { PATH: process.env.PATH ?? '' }, + timeoutMs: 20_000, + signal: controller.signal, + }, + ) + expect(verified.execution.exitCode).not.toBe(0) + expect(verified.grade.verdict).toBe('unrunnable') + }, + 40_000, + ) +}) diff --git a/src/claim-evidence.ts b/src/claim-evidence.ts index 5a5169f..ae3363c 100644 --- a/src/claim-evidence.ts +++ b/src/claim-evidence.ts @@ -24,6 +24,11 @@ * Reporting a low rung in the vocabulary of a high one is the most expensive error available to a * knowledge system: it is indistinguishable from success and propagates as settled provenance. */ + +// Type-only: erased at build time, so a consumer that never grades a check still imports this +// module without pulling agent-eval — the same property the runtime `import()` below preserves. +import type { BoundedProcessResult } from '@tangle-network/agent-eval' + export type EvidenceRung = 1 | 2 | 3 | 4 | 5 /** The rung at and above which a claim must be machine-checkable to be recorded at that rung. */ @@ -393,9 +398,9 @@ export interface VerifiedGradeableEvidence { * will use later. * * This function executes `evidence.check` verbatim. It is intentionally opt-in, Node-only at call - * time, and dynamically imports `node:child_process` so edge consumers that never invoke it remain - * importable. Callers must apply their own sandbox and capability policy before passing untrusted - * commands here. + * time, and dynamically imports `@tangle-network/agent-eval` so edge consumers that never invoke + * it remain importable. Callers must apply their own sandbox and capability policy before passing + * untrusted commands here. */ export async function verifyGradeableEvidence( evidence: ClaimEvidence, @@ -403,13 +408,13 @@ export async function verifyGradeableEvidence( ): Promise { const accepted = assertGradeableEvidence(evidence) const check = accepted.check as string - const syntax = await runBash(['-n', '-c', check], options) + const syntax = await runCheckProcess(['-n', '-c', check], options) if (syntax.exitCode !== 0) { const detail = `${syntax.stdout}\n${syntax.stderr}`.trim() const note = detail ? `${INVALID_SHELL_SYNTAX_NOTE}: ${detail}` : INVALID_SHELL_SYNTAX_NOTE throw new UncheckableClaimError(accepted.rung, note) } - const execution = await runBash(['-c', check], options) + const execution = await runCheckProcess(['-c', check], options) return { evidence: accepted, execution, @@ -463,35 +468,73 @@ function expectationRefusalNote(expect: string | undefined): string | undefined return undefined } -async function runBash( +/** + * The exit status reported for a run that produced no status of its own because the RUNNER, and + * not the check, ended it. It is `execFile`'s status for an error carrying no numeric `code`, + * which is the status the grades in `UNRUNNABLE_SIGNATURES` were swept from. + */ +const RUNNER_FAILURE_EXIT_CODE = 127 + +/** + * What the runner, rather than the check, has to say about a run — in the vocabulary + * `UNRUNNABLE_SIGNATURES` is calibrated on. + * + * `execFile` reported each of these conditions as an `Error` whose text the old runner appended to + * stderr, and the signature list was swept over 272 grade files produced that way. The shared + * runner reports them as fields instead, so the words are restored here: dropping them would turn + * a run the grader could not observe into a verdict on the claim. + * + * The order is the order of dominance. A killed run has no output worth reading, so the kill is + * reported even when the capture also overflowed. + */ +function runnerDiagnosis(result: BoundedProcessResult): string | undefined { + if (result.runnerError) return result.runnerError + if (result.killedBySignal) return "AbortError: the check was killed by the caller's abort signal" + if (result.outputTruncated) { + return `ERR_CHILD_PROCESS_STDIO_MAXBUFFER: the check printed more than the ${ + result.stdout.length + result.stderr.length + } bytes captured, so its output was not read in full` + } + return undefined +} + +/** + * Run one bash invocation under agent-eval's bounded process runner and report it as a + * `CheckExecution`. + * + * The runner is shared on purpose. This file used to spawn through `execFile`, which puts the + * check and every descendant it starts in THIS process's group: a deadline killed the shell alone, + * the descendants kept the stdout and stderr pipes open, and the callback never fired. Three + * solver processes outlived their grading parent by five days, and a grading loop hung twice on + * the same cause, for 8 hours and for 1.9 hours. `runBoundedProcess` gives the check its own + * process group and kills the group, so a deadline reaches the whole tree. + * + * The check body is passed as an argument vector and never as shell text, so no quoting stands + * between an author's check and the interpreter that reads it. + */ +async function runCheckProcess( args: string[], options: VerifyGradeableEvidenceOptions, ): Promise { - const { execFile } = await import('node:child_process') - return new Promise((resolve) => { - execFile( - options.bashPath ?? 'bash', - args, - { - cwd: options.cwd, - env: options.env ?? {}, - timeout: options.timeoutMs ?? 30_000, - maxBuffer: options.maxBufferBytes ?? 1024 * 1024, - signal: options.signal, - encoding: 'utf8', - }, - (error, stdout, stderr) => { - const diagnostic = error && typeof error.code !== 'number' ? `${stderr}\n${error}` : stderr - // A process killed at the deadline reports no exit status of its own, and its name - // separates the deadline from a caller's abort, which is a different verdict. - const timedOut = error?.killed === true && error.name !== 'AbortError' - resolve({ - exitCode: typeof error?.code === 'number' ? error.code : error ? 127 : 0, - stdout: String(stdout ?? ''), - stderr: String(diagnostic ?? ''), - ...(timedOut ? { timedOut: true } : {}), - }) - }, - ) + const { runBoundedProcess } = await import('@tangle-network/agent-eval') + const result = await runBoundedProcess({ + command: options.bashPath ?? 'bash', + args, + cwd: options.cwd, + // The exact environment the caller named, and nothing else: a check is graded on what it could + // read, so an inherited variable would make the grade depend on this process. + env: options.env ?? {}, + envMode: 'replace', + timeoutMs: options.timeoutMs ?? 30_000, + maxOutputBytes: options.maxBufferBytes ?? 1024 * 1024, + signal: options.signal, }) + const diagnosis = runnerDiagnosis(result) + return { + exitCode: + result.exitCode === 0 && diagnosis !== undefined ? RUNNER_FAILURE_EXIT_CODE : result.exitCode, + stdout: result.stdout, + stderr: diagnosis === undefined ? result.stderr : `${result.stderr}\n${diagnosis}`.trim(), + ...(result.killedByTimeout ? { timedOut: true } : {}), + } } From 26bdede453f1b6b0fea1f2d61987a65d11a2dec6 Mon Sep 17 00:00:00 2001 From: Drew Stone Date: Tue, 1 Sep 2026 14:01:40 -0600 Subject: [PATCH 2/3] refactor(claim-evidence): grade the executor's own report from fields, not from prose MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first commit restored three runner-side conditions into the error text `UNRUNNABLE_SIGNATURES` was calibrated on, so the shared runner's fields would keep classifying the way the old `execFile` wrapper's `Error` text did. That works, and it is the wrong shape: it makes this file's grading depend on the words a reconstructed error happens to use, and a drift on either side turns an environment failure into `contradicted` silently. `CheckExecution` now carries `killedBySignal` and `outputTruncated` beside `timedOut`, and `gradeFor` reads all three before it reads the exit status. Neither new field is an observation of the claim: the first says the caller withdrew the run, the second says the executor stopped keeping the output before the check stopped printing, so a comparison would answer about a fragment and could refute a claim the check established. `UNRUNNABLE_SIGNATURES` stays what it was calibrated for — reading a failure out of what the CHECK printed. It no longer has to recognise conditions the executor caused and can simply state. Nothing is manufactured now: `stderr` carries the runner's own `runnerError` text and nothing else, and the exit status is the one the run actually produced. Raised by the PR reviewer audit as the durable fix; it is right, so it is taken here rather than left as a follow-up. --- CHANGELOG.md | 13 ++++-- api-surface.json | 2 +- package.json | 2 +- src/claim-evidence.test.ts | 10 +++-- src/claim-evidence.ts | 86 +++++++++++++++----------------------- 5 files changed, 53 insertions(+), 60 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a4b964..b1b9367 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog -## 12.1.0 — 2026-09-01 +## 13.0.0 — 2026-09-01 ### Changed @@ -11,10 +11,17 @@ Three solver processes outlived their grading parent by five days, and a grading The shared runner gives the check its own process group and kills the group, so a deadline reaches the whole tree. - The check body reaches bash as an argument vector and never as shell text, so no quoting stands between an author's check and the interpreter that reads it. This needed `args` on `runBoundedProcess`, added in agent-eval 0.172.1. -- The peer range on `@tangle-network/agent-eval` moves to `>=0.172.1 <0.173.0`. -- Grading is unchanged for a check that runs and finishes. Three runner-side conditions are now reported in the vocabulary `UNRUNNABLE_SIGNATURES` is calibrated on, so none of them can be read as a verdict on the claim: a run the caller aborted, a run whose capture overflowed `maxBufferBytes`, and a run that could not be spawned. +- Grading is unchanged for a check that runs and finishes. A deadline now reports exit status 124, this package's own `DEADLINE_EXIT_CODE`, where the old wrapper reported 127. +### Added + +- `CheckExecution` gains `killedBySignal` and `outputTruncated`, and `gradeFor` grades both `unrunnable` before it reads the exit status. +**This is why the release is a major.** Reading a `CheckExecution` needs no change, because both fields are optional. Producing one does: an executor that kills a check on a caller's signal, or that stops keeping the check's output, must now say so, or `gradeFor` will grade a fragment as if it were the whole reading. Every executor in this package is updated; a consumer with its own executor sets the two fields. +Neither is an observation of the claim: the first says the caller withdrew the run, the second says the executor stopped keeping the output before the check stopped printing, so the comparison would answer about a fragment. +`UNRUNNABLE_SIGNATURES` stays what it was calibrated for — reading a failure out of what the CHECK printed — and no longer has to recognise the executor's own conditions from the words an error object happened to use. +The old `execFile` wrapper reported both as `Error` text appended to stderr, which is why the signature list carried `AbortError` and `ERR_CHILD_PROCESS_STDIO_MAXBUFFER`; an executor that caused a condition now states it. + ## 12.0.2 — 2026-09-01 ### Changed diff --git a/api-surface.json b/api-surface.json index 39cfafe..91cda25 100644 --- a/api-surface.json +++ b/api-surface.json @@ -78,7 +78,7 @@ "BuildRetrievalEvalDispatchOptions": "value 2a1fe2b2da73", "CHECKABLE_RUNG_THRESHOLD": "value 534bcde62c80", "CITES_INVALIDATED_FIELD": "value 3fff28ee6d1f", - "CheckExecution": "value f8a48e381773", + "CheckExecution": "value 884df3b223af", "ChunkingOptions": "value 00fb66d7d155", "ClaimEvidence": "value f780da49a3ef", "ClaimGrade": "value a9e6edbc7877", diff --git a/package.json b/package.json index 8f770b9..759bfaf 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@tangle-network/agent-knowledge", - "version": "12.1.0", + "version": "13.0.0", "description": "Build, search, evaluate, and improve source-backed knowledge bases.", "homepage": "https://github.com/tangle-network/agent-knowledge#readme", "repository": { diff --git a/src/claim-evidence.test.ts b/src/claim-evidence.test.ts index 692bb77..17920bf 100644 --- a/src/claim-evidence.test.ts +++ b/src/claim-evidence.test.ts @@ -685,9 +685,11 @@ describe('verifyGradeableEvidence — a deadline reaches the descendants, not ju maxBufferBytes: 4096, }, ) - expect(verified.execution.exitCode).not.toBe(0) - expect(verified.execution.stderr).toContain('ERR_CHILD_PROCESS_STDIO_MAXBUFFER') + // The check itself exits 0 here. The refusal comes from the executor's own report that + // it stopped keeping the output, not from anything the check printed. + expect(verified.execution.outputTruncated).toBe(true) expect(verified.grade.verdict).toBe('unrunnable') + expect(verified.grade.note).toContain('printed more than the executor kept') }, 40_000, ) @@ -706,8 +708,10 @@ describe('verifyGradeableEvidence — a deadline reaches the descendants, not ju signal: controller.signal, }, ) - expect(verified.execution.exitCode).not.toBe(0) + expect(verified.execution.killedBySignal).toBe(true) + expect(verified.execution.timedOut).toBeUndefined() expect(verified.grade.verdict).toBe('unrunnable') + expect(verified.grade.note).toContain('stopped by the caller') }, 40_000, ) diff --git a/src/claim-evidence.ts b/src/claim-evidence.ts index ae3363c..eb332a5 100644 --- a/src/claim-evidence.ts +++ b/src/claim-evidence.ts @@ -25,10 +25,6 @@ * knowledge system: it is indistinguishable from success and propagates as settled provenance. */ -// Type-only: erased at build time, so a consumer that never grades a check still imports this -// module without pulling agent-eval — the same property the runtime `import()` below preserves. -import type { BoundedProcessResult } from '@tangle-network/agent-eval' - export type EvidenceRung = 1 | 2 | 3 | 4 | 5 /** The rung at and above which a claim must be machine-checkable to be recorded at that rung. */ @@ -219,6 +215,14 @@ const DEADLINE_NOTE = 'the check was killed at its deadline, so it never tested the claim — a deadline is a budget, ' + 'not a verdict; raise the budget or record a check that decides within it' +const ABORTED_NOTE = + 'the check was stopped by the caller before it finished, so it never tested the claim — ' + + 'this is a decision about the run and not a verdict on the claim' + +const TRUNCATED_OUTPUT_NOTE = + 'the check printed more than the executor kept, so the claim was graded against a fragment — ' + + 'raise the output ceiling or record a check that prints only the decisive value' + const UNREACHED_INPUT_NOTE = 'the check never reached what the claim is about, so this is a verdict on the environment ' + 'and not on the claim' @@ -232,6 +236,18 @@ export interface CheckExecution { * verdict: a check that ran out of time never tested the claim. */ timedOut?: boolean + /** + * True when the caller withdrew the run before it finished. Like a deadline, this is a + * decision about the budget and not an observation of the claim. + */ + killedBySignal?: boolean + /** + * True when the executor stopped keeping the check's output before the check stopped printing. + * The kept text is a fragment, so a comparison against it answers about the fragment: an + * expectation printed in the discarded bytes reads as absent, which would refute a claim the + * check may well have established. + */ + outputTruncated?: boolean } /** A verdict with the reason a grader may report to the claim's author. */ @@ -275,6 +291,12 @@ export function gradeFor( if (execution.timedOut || execution.exitCode === DEADLINE_EXIT_CODE) { return { verdict: 'unrunnable', note: DEADLINE_NOTE } } + // Both are reports from the EXECUTOR about the run, not observations of the claim, and both + // are read before the exit status because a killed or half-captured run has no status worth + // reading. They are fields rather than text in `stderr` on purpose: an executor that reports + // the condition it caused must not have to spell it in words a regex happens to know. + if (execution.killedBySignal) return { verdict: 'unrunnable', note: ABORTED_NOTE } + if (execution.outputTruncated) return { verdict: 'unrunnable', note: TRUNCATED_OUTPUT_NOTE } if (execution.exitCode !== 0) return refutation(output, evidence.expect) if (mustBeCheckable) { const note = expectationRefusalNote(evidence.expect) @@ -468,50 +490,6 @@ function expectationRefusalNote(expect: string | undefined): string | undefined return undefined } -/** - * The exit status reported for a run that produced no status of its own because the RUNNER, and - * not the check, ended it. It is `execFile`'s status for an error carrying no numeric `code`, - * which is the status the grades in `UNRUNNABLE_SIGNATURES` were swept from. - */ -const RUNNER_FAILURE_EXIT_CODE = 127 - -/** - * What the runner, rather than the check, has to say about a run — in the vocabulary - * `UNRUNNABLE_SIGNATURES` is calibrated on. - * - * `execFile` reported each of these conditions as an `Error` whose text the old runner appended to - * stderr, and the signature list was swept over 272 grade files produced that way. The shared - * runner reports them as fields instead, so the words are restored here: dropping them would turn - * a run the grader could not observe into a verdict on the claim. - * - * The order is the order of dominance. A killed run has no output worth reading, so the kill is - * reported even when the capture also overflowed. - */ -function runnerDiagnosis(result: BoundedProcessResult): string | undefined { - if (result.runnerError) return result.runnerError - if (result.killedBySignal) return "AbortError: the check was killed by the caller's abort signal" - if (result.outputTruncated) { - return `ERR_CHILD_PROCESS_STDIO_MAXBUFFER: the check printed more than the ${ - result.stdout.length + result.stderr.length - } bytes captured, so its output was not read in full` - } - return undefined -} - -/** - * Run one bash invocation under agent-eval's bounded process runner and report it as a - * `CheckExecution`. - * - * The runner is shared on purpose. This file used to spawn through `execFile`, which puts the - * check and every descendant it starts in THIS process's group: a deadline killed the shell alone, - * the descendants kept the stdout and stderr pipes open, and the callback never fired. Three - * solver processes outlived their grading parent by five days, and a grading loop hung twice on - * the same cause, for 8 hours and for 1.9 hours. `runBoundedProcess` gives the check its own - * process group and kills the group, so a deadline reaches the whole tree. - * - * The check body is passed as an argument vector and never as shell text, so no quoting stands - * between an author's check and the interpreter that reads it. - */ async function runCheckProcess( args: string[], options: VerifyGradeableEvidenceOptions, @@ -529,12 +507,16 @@ async function runCheckProcess( maxOutputBytes: options.maxBufferBytes ?? 1024 * 1024, signal: options.signal, }) - const diagnosis = runnerDiagnosis(result) return { - exitCode: - result.exitCode === 0 && diagnosis !== undefined ? RUNNER_FAILURE_EXIT_CODE : result.exitCode, + exitCode: result.exitCode, stdout: result.stdout, - stderr: diagnosis === undefined ? result.stderr : `${result.stderr}\n${diagnosis}`.trim(), + // `runnerError` is the runner's own text for a run that never started — a missing + // interpreter, a signal that had already fired. It is appended rather than replacing + // stderr, because a caller reading a failed check wants both what the check said and why + // the runner could not run it. + stderr: result.runnerError ? `${result.stderr}\n${result.runnerError}`.trim() : result.stderr, ...(result.killedByTimeout ? { timedOut: true } : {}), + ...(result.killedBySignal ? { killedBySignal: true } : {}), + ...(result.outputTruncated ? { outputTruncated: true } : {}), } } From d36576af1f5256ce640b2f7fb8f384992bd24ddc Mon Sep 17 00:00:00 2001 From: Drew Stone Date: Tue, 1 Sep 2026 14:11:36 -0600 Subject: [PATCH 3/3] fix(claim-evidence): a parse the executor stopped is not a parse failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `verifyGradeableEvidence` raised `UncheckableClaimError` on any non-zero status from the `bash -n` pass, so a parse killed at its deadline, or by the caller's own signal, blamed the author for a command bash never finished reading. It is the same shape every other rule in this file exists to refuse: a budget, or a caller's teardown, reported as a verdict on the claim. Measured on the pinned runner before the fix: a signal already aborted at call time produced "the recorded check does not parse under bash". A stopped parse is now graded rather than raised, and `gradeFor` reports it `unrunnable` from the field that says what stopped it. A parse that really ran and failed still raises, because a command that cannot run anywhere is a record-time defect the author must fix. That also removes a race the audit measured in the abort test: the first dynamic import of the runner takes 175-186ms cold, so a 100ms abort could land on the parse rather than the execution. Both landings now reach the same verdict, so the test does not have to beat the import. A second test aborts before the call with no timer at all, which is the pre-abort path the runner reports with both `killedBySignal` and a `runnerError`. Found by the PR reviewer audit, whose blocking finding — an abort graded `contradicted` — was already closed by grading from the field instead of from error prose. Reproduced both against HEAD before acting. --- CHANGELOG.md | 7 +++++++ src/claim-evidence.test.ts | 19 +++++++++++++++++++ src/claim-evidence.ts | 12 ++++++++++++ 3 files changed, 38 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b1b9367..1a4db59 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,13 @@ This needed `args` on `runBoundedProcess`, added in agent-eval 0.172.1. - Grading is unchanged for a check that runs and finishes. A deadline now reports exit status 124, this package's own `DEADLINE_EXIT_CODE`, where the old wrapper reported 127. +### Fixed + +- A `bash -n` parse that the executor stopped is no longer reported as a check that does not parse. +`verifyGradeableEvidence` raised `UncheckableClaimError` on any non-zero status from the parse pass, so a parse killed at its deadline, or by the caller's own signal, blamed the author for a command bash never finished reading. +Measured on the pinned runner: a signal already aborted at call time produced "the recorded check does not parse under bash". +A stopped parse now grades `unrunnable` and raises nothing; a parse that really ran and failed still raises, because a command that cannot run anywhere is a record-time defect the author must fix. + ### Added - `CheckExecution` gains `killedBySignal` and `outputTruncated`, and `gradeFor` grades both `unrunnable` before it reads the exit status. diff --git a/src/claim-evidence.test.ts b/src/claim-evidence.test.ts index 17920bf..1bdab1c 100644 --- a/src/claim-evidence.test.ts +++ b/src/claim-evidence.test.ts @@ -694,9 +694,28 @@ describe('verifyGradeableEvidence — a deadline reaches the descendants, not ju 40_000, ) + it('refuses to grade a check whose signal had already fired', async () => { + // No timer, so the landing is not a race: the signal is aborted before the call. The abort + // reaches the `bash -n` parse rather than the execution, and a parse that never ran is not a + // verdict on the check's grammar — it must not surface as "does not parse under bash". + const controller = new AbortController() + controller.abort() + const verified = await verifyGradeableEvidence( + { rung: 4, check: 'sleep 30; echo "done=$((0 + 1))"', expect: 'done=1' }, + { cwd: tmpdir(), env: { PATH: process.env.PATH ?? '' }, signal: controller.signal }, + ) + expect(verified.execution.killedBySignal).toBe(true) + expect(verified.grade.verdict).toBe('unrunnable') + expect(verified.grade.note).toContain('stopped by the caller') + }, 20_000) + it.skipIf(process.platform === 'win32')( 'refuses to grade a check the caller aborted', async () => { + // The abort may land on either phase — the parse or the execution — because the first + // dynamic import of the runner is measured at 175-186ms cold. Both landings must reach the + // same verdict, which is what the assertions below check; a timer that had to beat the + // import would be the flake, not the test. const controller = new AbortController() setTimeout(() => controller.abort(), 100) const verified = await verifyGradeableEvidence( diff --git a/src/claim-evidence.ts b/src/claim-evidence.ts index eb332a5..fe8c3a5 100644 --- a/src/claim-evidence.ts +++ b/src/claim-evidence.ts @@ -419,6 +419,11 @@ export interface VerifiedGradeableEvidence { * Parse, execute, and grade evidence at intake using the same cwd and environment the blind grader * will use later. * + * A check that bash cannot parse raises `UncheckableClaimError`, because a command that cannot run + * anywhere is a record-time defect the author must fix. A parse the EXECUTOR stopped — its + * deadline, or the caller's signal — raises nothing and is graded `unrunnable`: nothing was + * learned about the check's grammar, so there is nothing to refuse it for. + * * This function executes `evidence.check` verbatim. It is intentionally opt-in, Node-only at call * time, and dynamically imports `@tangle-network/agent-eval` so edge consumers that never invoke * it remain importable. Callers must apply their own sandbox and capability policy before passing @@ -431,6 +436,13 @@ export async function verifyGradeableEvidence( const accepted = assertGradeableEvidence(evidence) const check = accepted.check as string const syntax = await runCheckProcess(['-n', '-c', check], options) + // A parse that the executor stopped is not a reading of the check's grammar. Reporting it as a + // parse failure would blame the author for a deadline or for the caller's own teardown, which + // is the shape every other rule in this file exists to refuse. The run is graded instead, and + // `gradeFor` reports it `unrunnable` from the field that says what stopped it. + if (syntax.timedOut || syntax.killedBySignal) { + return { evidence: accepted, execution: syntax, grade: gradeFor(accepted, syntax) } + } if (syntax.exitCode !== 0) { const detail = `${syntax.stdout}\n${syntax.stderr}`.trim() const note = detail ? `${INVALID_SHELL_SYNTAX_NOTE}: ${detail}` : INVALID_SHELL_SYNTAX_NOTE