refactor(claim-evidence): consume agent-eval runBoundedProcess; delete the private runner - #185
Conversation
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — 87f47b7b
This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.
This approval is provisional. It rests on the audit running. If the audit cannot run — for example the CLI bridge rejects it — this approval is dismissed rather than left standing, so an unrun check never reads as a passing one.
tangletools · auto-approval · reason: drewstone_author · 2026-09-01T19:50:57Z
tangletools
left a comment
There was a problem hiding this comment.
🟡 Value Audit — sound-with-nits
| Verdict | sound-with-nits |
| Coverage | 2 of 2 lenses (value, usefulness) |
| Concerns | 1 (1 weak-concern) |
| Heuristic | 0.0s |
| Duplication | 0.0s |
| Interrogation | 271.4s (2 bridge agents) |
| Total | 271.4s |
💰 Value — sound-with-nits
Deletes this package's only private process spawner and runs claim checks through agent-eval's group-killing runBoundedProcess, closing a real orphaned-descendant defect in the grain of the repo's own layering doctrine; ship.
- What it does: verifyGradeableEvidence's two bash invocations (the
bash -n -cparse refusal at src/claim-evidence.ts:411 and thebash -cexecution at :417) now go through a new runCheckProcess (:515) that calls runBoundedProcess from @tangle-network/agent-eval instead of a private execFile wrapper. Concrete deltas: the check runs in its own process group and a deadline/abort SIGKILLs the whole group; a kille - Goals it achieves: Read from the code and its history: (1) close the descendant-kill defect — the old execFile path left a backgrounded
solver & waitin the grader's process group, so a deadline killed the shell alone while grandchildren held the pipes open, hanging the grader (documented at :505-510 and reproduced by the new pgrep/kill(pid,0) test); (2) delete a private copy of a primitive the layering already as - Assessment: Good, and squarely in the grain. The package layering in AGENTS.md explicitly permits agent-knowledge → agent-eval and assigns shared run primitives to agent-eval; the old runBash was the only child_process spawner in shipped src (verified: rg over src/ leaves only comments in claim-evidence.ts and build scripts under scripts/). The change is behavior-preserving where it should be (argv delivery,
- Better / existing approach: none — this is the right approach. Searched: (a) rg 'child_process|execFile|spawn(' across src/ and scripts/ — no other runner exists in this package to reuse, only build scripts; (b) verified the upstream primitive in the sibling checkout tangle-network-agent-eval-pr-716/src/bounded-process.ts owns the group-kill semantics this needs; (c) considered teaching gradeFor to consume structured runner
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 2
- Bridge warning: opencode/kimi-for-coding/k2p7: opencode: opencode error event without a message: {"type":"error","timestamp":1788292370346,"sessionID":"ses_fa177f1b3ffebJq0L46mwwbPqX","error":{"name":"UnknownError","data":{"message":"Unexpected server error. Check server logs for details.","ref":"err_d99a26c9"}}}
🎯 Usefulness — sound
Deletes the package's only private process spawner and rewires both call sites to the shared agent-eval runner that was extracted upstream precisely for this consumer; the pinned version exists on npm with the needed argv support, and the defect it closes (deadline misses background descendants, gra
- Integration: Fully wired and reachable. verifyGradeableEvidence is exported public API (src/index.ts:9, api-surface.json:772) and its two spawn call sites (src/claim-evidence.ts:411,417) both moved to runCheckProcess, which calls agent-eval's runBoundedProcess (claim-evidence.ts:519-531). The peer pin >=0.172.1 <0.173.0 resolves: I pulled the published 0.172.1 tarball from npm and confirmed dist/bounded-proces
- Fit with existing patterns: Exactly the codebase's grain, not a competing pattern. The layering diagram in AGENTS.md has agent-knowledge importing agent-eval, and agent-eval's own changelog (0.171.1 entry) names agent-knowledge's claim grader as the motive for extracting runBoundedProcess and the first consumer of the argv form — the shared runner exists because of this caller. A grep for child_process|execFile|spawn( across
- Real-world viability: Verified live in this sandbox against the installed 0.172.1, not just on paper. (1) Group-kill: ran the exact shape from the incident — bash -c 'echo $$; sleep 60 & echo $!; wait' with a 300ms deadline — and got exit 124, killedByTimeout true, resolution in 303ms, pgrep showing an empty process group, and the descendant pid confirmed dead via kill(pid,0). This is the exact path that hung the gradi
- Model: opencode/zai-coding-plan/glm-5.2
- Bridge attempts: 1
💰 Value Audit
🟡 runnerDiagnosis manufactures synthetic execFile-era error strings to feed a regex [maintenance] ``
runnerDiagnosis (src/claim-evidence.ts:490-499) reconstructs strings like "AbortError: ..." and "ERR_CHILD_PROCESS_STDIO_MAXBUFFER: ..." so UNRUNNABLE_SIGNATURES (:208) keeps classifying them as unrunnable. This couples this file to upstream field semantics via hand-written pseudo-error text; if either side drifts, an environment failure could silently read as 'contradicted'. The durable fix is for gradeFor to consume structured fields (timedOut/outputTruncated/aborted) rather than regexing rest
What this audit checks
It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.
| Pass | What it asks |
|---|---|
| Heuristic | Vague title? Whitespace-only or cruft-bearing diff? (content signals only) |
| Duplication | Do added function/class names already exist elsewhere in the repo? |
| Value Audit | What does it do? What goal does it achieve? Is it good? Better architecture or already-exists? |
| Usefulness Audit | Does it integrate and fit? Will it hold up in real use and actually get used? |
Findings are concerns, not blocks — the human reviewer decides what to do with them.
…e the private runner 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.
…, not from prose 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.
❌ Needs Work —
|
| opencode GLM 5.2 | opencode DeepSeek v4 Pro | opencode DeepSeek v4 Flash | aggregate | |
|---|---|---|---|---|
| Readiness | 55 | 73 | 60 | 55 |
| Confidence | 85 | 85 | 85 | 85 |
| Correctness | 55 | 73 | 60 | 55 |
| Security | 55 | 73 | 60 | 55 |
| Testing | 55 | 73 | 60 | 55 |
| Architecture | 55 | 73 | 60 | 55 |
Reviewer score is advisory once the run is complete and the verdict has no blockers.
Full multi-shot audit completed 5/5 planned shots over 5 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 5/5 planned shots over 5 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 5/5 planned shots over 5 changed files. Global verifier still owns final merge decision.
Blocking
🔴 HIGH Abort landing at the execution-phase boundary grades a caller teardown as a claim refutation ('contradicted' instead of 'unrunnable') — src/claim-evidence.ts
agent-eval 0.172.1's aborted-before-spawn path returns BOTH runnerError ('aborted before spawn: the AbortSignal was already aborted') and killedBySignal: true (bounded-process.ts:179-190). runnerDiagnosis checks runnerError first, so the token 'AbortError' never lands in stderr, and 'aborted before spawn' matches no UNRUNNABLE_SIGNATURES entry; gradeFor's nonzero-exit path then returns 'contradicted'. Reproduced against the pinned runner: input {exitCode:137, stderr:'aborted before spawn: the AbortSignal was already aborted'} -> verdict 'contradicted'. Exposure: verifyGradeableEvidence runs a syntax phase then an execution phase with the same options.signal; if the signal aborts after the syntax spawn completes but before the execution spawn (teardown race in a grading loop passing one sig
Other
🟠 MEDIUM Abort test races the cold agent-eval import and fails intermittently — src/claim-evidence.test.ts
The 100ms setTimeout abort (line 698-699) races the first cold dynamic import of '@tangle-network/agent-eval' inside runCheckProcess (claim-evidence.ts:519, measured 175-186ms cold), plus the bash -n syntax spawn. When the signal aborts before that first spawn, runBoundedProcess returns its pre-aborted result (exit 137), the syntax check at claim-evidence.ts:411 is non-zero, and verifyGradeableEvidence throws UncheckableClaimError at claim-evidence.ts:415 instead of returning VerifiedGradeableEvidence. Reproduced:
vitest run src/claim-evidence.test.ts -t "caller aborted"failed 3/6 and 5/5 across two batches with exactly that error; a full-file run also
🟠 MEDIUM Pre-aborted AbortSignal grades 'contradicted' instead of 'unrunnable' — src/claim-evidence.ts
runnerDiagnosis checks result.runnerError before result.killedBySignal. For an AbortSignal already aborted at call time, runBoundedProcess (agent-eval@0.172.1) returns killedBySignal:true AND runnerError:'aborted before spawn: the AbortSignal was already aborted'. runnerDiagnosis therefore returns the runnerError string, which lacks the 'AbortError' token that UNRUNNABLE_SIGNATURES matches. gradeFor then sees exitCode 137 with no matching signature and returns 'contradicted', falsely refuting the claim. The old execFile path produced an AbortError whose stringified name 'AbortError' matched the signature -> 'unrunnable'. Empirically reproduced: stderr 'aborted before spawn: the AbortSignal was already aborted' -> UNRUNNABLE_SIGNATURES.test() === false. Fix: reorder so killedBySignal is che
🟠 MEDIUM Pre-spawn abort grades the claim contradicted on a run that never executed — src/claim-evidence.ts
For an already-aborted signal runBoundedProcess returns { exitCode: 137, killedBySignal: true, runnerError: 'aborted before spawn: the AbortSignal was already aborted' }. runnerDiagnosis checks result.runnerError first and returns its text, which contains no UNRUNNABLE_SIGNATURES token ('AbortError' is not in it). Because exitCode is already 137 (nonzero), the
exitCode === 0 && diagnosis !== undefinedforcing at line 534 does not apply. gradeFor then takes the exitCode!==0 branch -> refutation() -> no signature match -> verdictcontradicted. Reproduced: gradeFor({rung:4,check:'echo "done=$((0 + 1))"',expect:'done=1'}, {exitCode:137,stdout:'',stderr:'aborted befo
🟡 LOW Output cap semantics changed from per-stream to combined without documentation — CHANGELOG.md
The old execFile wrapper capped stdout and stderr EACH at maxBufferBytes (options.maxBufferBytes ?? 1MB); runBoundedProcess caps COMBINED stdout+stderr at maxOutputBytes (verified in bounded-process source: outputBytes across both streams, one cap). A check emitting ~700KB to stdout and ~700KB to stderr previously graded normally and now truncates and grades unrunnable. This is a conservative direction (unrunnable, never a false pass) and matches the changelog's 'capture overflowed maxBufferBytes' framing, but the trigger boundary changed silently; a check that used to pass on large-but-under-1MB-per-stream output now refuses. Worth one line in the changelog or a comment, not a blocker.
🟡 LOW Pre-aborted signal is mislabeled as invalid shell syntax, not an abort — CHANGELOG.md
CHANGELOG claims a 'run the caller aborted' is surfaced in the UNRUNNABLE_SIGNATURES vocabulary. Verified against the installed runner: when options.signal is already aborted at call time, runBoundedProcess returns runnerError 'aborted before spawn: the AbortSignal was already aborted' with exitCode 137 and killedBySignal=true (empirically reproduced). runnerDiagnosis (claim-evidence.ts:495) returns that text before the AbortError branch, and the text matches no UNRUNNABLE_SIGNATURES token, so the syntax pre-check throws UncheckableClaimError with INVALID_SHELL_SYNTAX_NOTE — an abort reported as a syntax error. It fails loud (never a wrong verdict), so eval validity is intact, but the error message misleads the claim author and the changelog's blanket statement overstates this edge. Fix in
🟡 LOW 200ms budget is shared with the bash -n syntax pre-check — src/claim-evidence.test.ts
verifyGradeableEvidence runs
bash -n -c <check>and the real execution through the same options, so timeoutMs:200 applies to both. Ifbash -never exceeds 200ms on a loaded CI host, runCheckProcess returns a nonzero/timedOut syntax result and verifyGradeableEvidence throws UncheckableClaimError before grading, making the test fail spuriously (timedOut/grade assertions never reached). bash -n on a 30-char script is normally single-digit ms, so this is a low-probability flake, not a product bug. Consider a separate, larger syntax budget or a higher timeoutMs with a shorter-than-sleep work body.
🟡 LOW Abort test does not pin timedOut===undefined, so an abort misread as a deadline passes — src/claim-evidence.test.ts
The test asserts exitCode!==0 and verdict==='unrunnable' but not that execution.timedOut is undefined. If runCheckProcess ever mapped a caller abort to killedByTimeout (the old execFile runner distinguished them via error.name!=='AbortError'), the grade would silently change from the AbortError-signature unrunnable to DEADLINE_NOTE unrunnable and this test would still pass, since both lattice members are 'unrunnable'. I verified the current behavior is correct (timedOut unset on abort), but the regression is unguarded. Fix: add expect(verified.execution.timedOut).toBeUndefined().
🟡 LOW No test for the pre-aborted / runnerError abort path — src/claim-evidence.test.ts
The abort test aborts at 100ms via setTimeout, so the syntax check (bash -n) completes and the execution spawn aborts mid-run — the killedBySignal path, not the runnerError path. No test calls verifyGradeableEvidence with a signal already aborted at call time, which is exactly the case that now misgrades. Add a case with controller.abort() before the call asserting grade.verdict === 'unrunnable'.
🟡 LOW Survivor poll assumes orphaned descendants are reaped within 5s; flakes under non-reaping pid 1 — src/claim-evidence.test.ts
After the group SIGKILL, the reparented sleep is a zombie until init reaps it, and both pgrep -g and kill(pid,0) see zombies. On GitHub Actions ubuntu VMs init reaps promptly (verified: passes in 376ms), but in containerized local dev where node itself is pid 1 and does not reap SIGCHLD, the 5s polling loop exhausts and both survivor assertions fail. Failure is loud, not silent, and the assertion direction (dead) is correct — this is an environment-limited flake vector, not a wrong claim. Optional hardening: treat zombie states as dead when polling (e.g. read /proc//stat), or document the container caveat next to the skipIf.
🟡 LOW failing test leaks a live sleep 60 orphan — src/claim-evidence.test.ts
If the fix regresses, the deadline misses the descendant and the test leaves a 60s
sleeprunning after the assertions fail (the assertion failures do not kill the group). This is inherent to testing exactly this defect, so it is acceptable, but a teardown that kills the leader's group would prevent orphan accumulation during a red CI loop.
🟡 LOW group-empty assertion is vacuous if the runner wraps the shell — src/claim-evidence.test.ts
expect(survivors).toEqual([]) reads the group by pgid
$$, which only equals the process-group id if runBoundedProcess spawns bash as the group leader. I confirmed it does under a detached/own-group spawn, but if that contract ever changed (e.g. a wrapper process becomes leader), pgrep -g <$$> would return [] immediately and the group assertion passes without exercising the group-kill. The regression is still guarded by the process.kill(descendant,0) check at line 664, which is the real protection, so this only weakens the group-level assertion rather than hiding the defect.
🟡 LOW pidsInGroup depends on pgrep (procps), unguarded on POSIX hosts — src/claim-evidence.test.ts
it.skipIf(process.platform === 'win32') (line 623) does not guard against a POSIX host that lacks pgrep; execFile('pgrep', ...) then rejects with code 'ENOENT' (≠ 1) at line 613, failing the test with an error unrelated to the SUT. CI on ubuntu-latest ships procps so this is latent, but a contributor on a minimal container (e.g. alpine devcontainer) gets a hard failure. Fix: treat ENOENT as a skip or fall back to reading /proc, or gate the whole describe on a capability probe.
🟡 LOW Output cap is now combined stdout+stderr; overflowed checks run to the deadline instead of being killed — src/claim-evidence.ts
execFile's maxBuffer capped stdout and stderr separately and killed the child on overflow; runBoundedProcess caps stdout+stderr combined (maxOutputBytes) and keeps draining, discarding bytes, without killing. Two behavior deltas: (1) a check writing ~600KB to each stream graded
verifiedunder the old per-stream 1 MiB cap and now truncates (combined >1 MiB) and is forced to exit 127 ->unrunnable; the verdict change is safe-direction but contradicts the changelog's 'Grading is unchanged for a check that runs and finishes'. (2) a check that spews output is no longer terminated at the cap — it runs until the 30s deadline, so a noisy check occupies a grading slot far longer than before. Consider documenting the combined-cap change and/or passing a larger explicit cap. Low severity; verdict
🟡 LOW Phase-1 runner failures are reported to authors as 'does not parse under bash' — src/claim-evidence.ts
Any nonzero syntax-phase exit throws UncheckableClaimError prefixed INVALID_SHELL_SYNTAX_NOTE. A missing bashPath (stderr 'Error: spawn ... ENOENT'), a NUL byte in the check (TypeError ERR_INVALID_ARG_VALUE), or a pre-call aborted signal all produce notes telling the author to rewrite a check that bash never saw. Reproduced: runCheckProcess with bashPath '/no/such/bash-binary' returns exit 127 + ENOENT, which phase 1 converts to 'the recorded check does not parse under bash ... Error: spawn /no/such/bash-binary ENOENT'. Fix: when the phase-1 stderr carries a runnerDiagnosis rather than bash's own diagnostic, refuse with a runner-failure note instead of the syntax note.
🟡 LOW Spawn failures only grade unrunnable when the error text hits a signature token — src/claim-evidence.ts
The changelog claims 'a run that could not be spawned' is now reported in the UNRUNNABLE_SIGNATURES vocabulary. That holds for runnerError text containing ENOENT/EACCES, but not for other spawn failures: this shot's own test runs hit
Error: spawn bash EAGAIN(process limit), which contains no signature token, so a spawn failure of that kind gradescontradicted— a claim verdict on a check that never ran. The old execFile wrapper had identical behavior (it appended the same String(err)), so this is not a regression, but the new explicit runner-diagnosis layer is the natural place to close it: prepend or append a signature-matching token to runnerError text, or add EAGAIN/E2BIG to UNRUNNABLE_SIGNATURES. Low severity given reachability requires resource exhaustion at spawn time.
🟡 LOW Truncation diagnosis reports captured UTF-16 code units as 'bytes' — src/claim-evidence.ts
The message interpolates result.stdout.length + result.stderr.length — JavaScript string lengths — as 'the N bytes captured'. For multibyte output the number understates the byte count, though it is internally consistent with the runner's own length-based cap accounting. Diagnostic-note accuracy only; fix by dropping the number or naming it characters.
🟡 LOW Verdict correctness now depends on agent-eval's runnerError prose containing a signature word — src/claim-evidence.ts
CheckExecution has no first-class field for 'the runner, not the check, failed', so runCheckProcess folds runner failures into exit 127 + stderr text and relies on UNRUNNABLE_SIGNATURES matching that text. Two of the three runnerError strings in 0.172.1 contain a signature word (spawn ENOENT; the args+shell refusal does not but is unreachable here), and finding 1 shows the one that does not silently becomes 'contradicted'. Any future agent-eval release that rewords a runnerError breaks the lattice again with no type-level signal. Fix: propagate runner failures structurally (a refusal flag on CheckExecution, or map any runnerError to an explicit unrunnable note) instead of by string calibration.
tangletools · 2026-09-01T20:06:02Z · trace
tangletools
left a comment
There was a problem hiding this comment.
❌ 1 Blocking Finding — 87f47b7b
Full multi-shot audit completed 5/5 planned shots over 5 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 5/5 planned shots over 5 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 5/5 planned shots over 5 changed files. Global verifier still owns final merge decision.
Full immutable report for this review: trace
Summary comment for this run: full summary
tangletools · 2026-09-01T20:06:02Z · immutable trace
87f47b7 to
26bdede
Compare
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — 26bdede4
This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.
This approval is provisional. It rests on the audit running. If the audit cannot run — for example the CLI bridge rejects it — this approval is dismissed rather than left standing, so an unrun check never reads as a passing one.
tangletools · auto-approval · reason: drewstone_author · 2026-09-01T20:11:08Z
`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.
|
The blocking finding was raised against Blocker: an abort graded
The medium about the abort test racing the cold import was a real defect, and a deeper one than a flaky test. Chasing it found that
Verified on HEAD The version is 13.0.0, not 12.1.0: |
tangletools
left a comment
There was a problem hiding this comment.
✅ Auto-approved drewstone PR — d36576af
This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.
This approval is provisional. It rests on the audit running. If the audit cannot run — for example the CLI bridge rejects it — this approval is dismissed rather than left standing, so an unrun check never reads as a passing one.
tangletools · auto-approval · reason: drewstone_author · 2026-09-01T20:21:59Z
The duplicate
src/claim-evidence.tscarried a private 32-linerunBash, the only process spawner in this package's shipped source. It ran the claim grader's checks throughexecFile, 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 puts its real work in the background —
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; a grading loop hung twice on the same cause, for 8 hours and for 1.9 hours.agent-eval owns the runner that closes exactly this (agent-eval #710, extracted for this reason). The private copy is deleted.
What changed
execFilein this packagerunBoundedProcessfrom agent-evalexecFileDEADLINE_EXIT_CODEBoth call sites moved: the
bash -n -cparse that refuses a check before running it, and thebash -cexecution. 32 lines deleted, 2 callers rewired, 1 private runner gone. No other spawner remains insrc/— swept withgrep -rn "child_process\|execFile\|spawn("; the remaining hits are build scripts and tests.The upstream gap this needed
runBoundedProcesscould only be handed a command line for a shell to read, sobash -n -c <check>was not expressible:shell: 'bash'runsbash -c <check>, which executes it. The only alternative was quoting an author's untrusted check into a shell string — a hand-rolled quoter, which is the class of private copy this change exists to remove.Filed as agent-eval #715, fixed by agent-eval #716 (
args?: string[]), released as agent-eval 0.172.1. An adversarial review of that PR found two real correctness defects in it — the call could reject on a NUL byte, andenvModewas not identical across the two spawn forms — both fixed before it merged. Peer range here moves to>=0.172.1 <0.173.0.Also filed, out of scope here: agent-eval #717,
localCommandRunneris a secondspawnSyncspawn site with none of these bounds; absorbing it needsstdinonBoundedProcessInputfirst.The executor states its own conditions, instead of spelling them for a regex
The first commit restored the three runner-side conditions into the error text
UNRUNNABLE_SIGNATURESwas calibrated on, so the shared runner's fields kept classifying the way the oldexecFileErrortext did. That works and it is the wrong shape — grading would depend on the words a reconstructed error happens to use, and a drift on either side turns an environment failure intocontradictedsilently. The reviewer audit named it; the second commit takes the durable fix instead of leaving it as a follow-up.CheckExecutionnow carrieskilledBySignalandoutputTruncatedbesidetimedOut, andgradeForreads all three before it reads the exit status.UNRUNNABLE_SIGNATURESstays what it was calibrated for — reading a failure out of what the CHECK printed. Nothing is manufactured:stderrcarries the runner's ownrunnerErrortext and nothing else, and the exit status is the one the run produced.This is why the release is a major, 13.0.0. Reading a
CheckExecutionneeds no change; both fields are optional. Producing one does: an executor that kills a check on a caller's signal, or that stops keeping its output, must now say so, orgradeForgrades a fragment as a whole reading.check:version-bumpcalls any exported-shape change breaking, and it is paid for.Behavior is pinned, not adapted
Every existing caller test passes with its fixtures untouched — 58 across
claim-evidence.test.tsandclaim-evidence-intake.test.ts. Grading is unchanged for a check that runs and finishes.Three runner-side conditions are restored into the vocabulary
UNRUNNABLE_SIGNATURESis calibrated on, because none of them is a verdict on the claim and the oldexecFilepath communicated each one asErrortext appended to stderr — the signature list was swept over 272 grade files produced that way:killedBySignal;maxBufferBytes— nowoutputTruncated. The old wrapper killed the process; the shared runner truncates, so grading the kept bytes could reportcontradictedfor an expectation printed in the bytes nobody kept;runnerErrortext, whichUNRUNNABLE_SIGNATURESalready matches onENOENT.The first two are graded from the fields, before the exit status is read, so the expectation comparison is never reached on a run the grader could not observe. That is strictly safer than the old path, which could reach the comparison.
Three new tests, each proven non-vacuous
Every one was run against the deleted runner first:
sleep 60survives,expect(alive).toBe(false)readstrueVerification — the repo's exact CI chain, locally, on the rebased tree
pnpm install --frozen-lockfilepnpm lintpnpm typecheck(src + contracts)pnpm testwithAGENT_KNOWLEDGE_RUN_NETWORK_TESTS=1pnpm buildpnpm verify:packagecheck:version-bumpandcheck:api-surfaceincludedpnpm verify:official-optimizersmainreleased 12.0.1 and then 12.0.2 mid-change; this branch is rebased onto 12.0.2 and re-verified from scratch on the rebased tree each time. 12.0.2 independently moved theagent-evalpeer to>=0.172.1 <0.173.0, so this branch'spackage.jsondiff againstmainis now only the version bump.Known and unchanged from before this PR: a
bash -nparse pass that hits its own deadline still raisesUncheckableClaimErrorsaying the check does not parse. The old wrapper did the same (its timeout also produced a non-zero status), so this is not a regression, but it is a wrong verdict shape and worth a separate fix.The dynamic
import('@tangle-network/agent-eval')is deliberate and preserves the property the old code defended: a consumer that never grades a check still imports this module without loading the runner. Confirmed in the build —dist/inspect-*.jskeeps the dynamic import and the type-only import is erased.Not in this PR
discovery-lab/tools/oracle-jail.mjsis the lab-side copy of the same idea. It is out of scope here; the lab deletes it after the next agent-knowledge release ships this.