diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dcd6b0f..3de67b6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -90,8 +90,10 @@ jobs: - run: npm ci # `npm run check` is the same set of checks the ubuntu job runs as named - # steps. None of them shells out, so nothing here needs zsh. - # `bench/run.sh` does, and CI never invokes the runner. + # steps. One of them shells out: `test/run-sh.test.js` drives + # `bench/run.sh` over a stand-in `claude`, so a host with zsh exercises + # the runner's control flow and a host without one skips those two tests. + # Nothing here ever reaches a model. - name: Every check run: npm run check diff --git a/AGENTS.md b/AGENTS.md index fec7ee3..efaf7cd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -847,6 +847,16 @@ Do not read a green pipeline as coverage of this one. `test/gfm-render.test.js` is the only evidence that the grammar admits the prose a reader admits. Read a green catalogue run as evidence about the block path, and not about this one. +- **The runner is driven over a stand-in, and only where zsh is installed.** + `test/run-sh.test.js` runs `bench/run.sh` end to end with a fake `claude` on + `PATH`, which is what a fresh arm directory needed and no fixture-built arm + could give it. It says nothing about the real CLI, its flags, or the + credential the calling shell supplies, and it skips entirely on a host with + no zsh. Read a green run as evidence about the runner's control flow. + It says nothing about DURATION either, and that is the reading this hole + cost. The stand-in answers at once, where a real arm is quiet for hours, so + an arm that had merely not finished was read here as an arm that had died — + and the artifacts of the two are identical while the call is open. - **An editorial stamp is a claim, and the suite checks its form alone.** `test/editorial.test.js` covers the record's table, the day, the digest and both notes. Nothing can tell a row a person wrote after reading from a row an diff --git a/bench/README.md b/bench/README.md index 9e88626..a773b7c 100644 --- a/bench/README.md +++ b/bench/README.md @@ -11,11 +11,20 @@ reproducible and artificial. The field half is real and uncontrolled. Fixed scenarios, fresh context, several runs each, one variable at a time. -`run.sh` needs **zsh** and the `claude` CLI on `PATH`. Neither is a dependency of -this package, and nothing in `npm run check` invokes the runner, so continuous -integration never exercises this half. A Linux container without zsh will fail -every command below before the harness starts. The scorer is plain Node and runs -anywhere. +`run.sh` needs **zsh** and the `claude` CLI on `PATH`. The calling shell also +supplies the credential, because the runner builds no environment and reads no +value: a real arm authenticates by the routes issue #77 ranks, which are +`CLAUDE_CODE_OAUTH_TOKEN` for a subscription ahead of `ANTHROPIC_API_KEY`. None +of that is a dependency of this package. A Linux container without zsh will fail +every command below before the harness starts. `test/run-sh.test.js` drives the +runner over a stand-in `claude` and skips where zsh is absent, so what +continuous integration exercises is the runner's own control flow and never a +model call. The scorer is plain Node and runs anywhere. + +**An arm is slow, and silence is not failure.** The first review arm on issue +#109 took about three hours to collect eight samples, unaudited, and it printed +nothing between them. Read a running arm by the files it has written, and give +a claim that one died the evidence such a claim needs. ``` bench/run.sh control # no guidance at all diff --git a/bench/run.sh b/bench/run.sh index 0773941..c853cd7 100755 --- a/bench/run.sh +++ b/bench/run.sh @@ -134,7 +134,23 @@ trap "$CLEAN_UP" EXIT # old samples and generates only the missing ones, so the cell silently holds # two conditions. The arm directory name is not a fingerprint. Compare against # what is already there and refuse rather than resume. -existing="$(ls "$HERE/out/$ARM"/*.meta 2>/dev/null | head -1)" +# +# The comparison reads whatever sidecars are there, and a FRESH arm has none. +# `ls` over an unmatched glob is how that was asked, and zsh answers NOMATCH: +# the substitution fails and `no matches found` prints, over a run that then +# proceeds correctly with `existing` empty. The first review arm on issue #109 +# printed it and went on to collect all eight of its samples three hours later. +# So the message cost nobody a sample and it cost the operator the run anyway: +# an arm that prints a failure on its first line of work and then says nothing +# for hours cannot be told from a dead one, and it was read as dead. A check +# that passes has to be SILENT as well as harmless, because silence is the only +# thing the next line of output is measured against. +# +# `(N)` is the null-glob qualifier, so no match gives no words. The first +# element is the sidecar to compare against, and an empty array leaves +# `existing` empty, which is the state that skips the block below. +sidecars=("$HERE/out/$ARM"/*.meta(N)) +existing="${sidecars[1]}" if [ -n "$existing" ]; then was_system="$(sed -n 's/.*system_sha=\([^ ]*\).*/\1/p' "$existing")" was_rules="$(sed -n 's/.*rules=\([^ ]*\) .*/\1/p' "$existing")" diff --git a/test/run-sh.test.js b/test/run-sh.test.js new file mode 100644 index 0000000..24252ad --- /dev/null +++ b/test/run-sh.test.js @@ -0,0 +1,111 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { execFile } from 'node:child_process'; +import fs from 'node:fs/promises'; +import os from 'node:os'; +import path from 'node:path'; + +import { NAME } from '../bench/arm-manifest.mjs'; + +/** + * The runner itself, driven end to end over a stand-in `claude`. + * + * Everything else that touches an arm builds one by writing the FILES an arm + * holds — `test/bench-helpers.js` does exactly that, and so the runner that + * produces them had no exercise anywhere. The first real review arm on issue + * #109 then printed `no matches found` on its first line of work, from a + * resume check asking a glob about a fresh directory zsh answers NOMATCH for. + * That arm was healthy and finished every sample hours later. Nobody could say + * so at the time, which is what the hole actually cost: with the runner + * unexercised, a slow run and a dead one produce the same evidence, and the + * benign reading is the one that goes unconsidered. + * + * The stand-in echoes one fixed JSON run, so this costs no model call. It is + * the same trick `bench/review-arms.mjs` gets from an injected `git`. + * + * A host with zsh runs this and a host without one skips it, which is the + * disposition `bench/README.md` already gives every command in that directory. + * zsh is asked for by name through `PATH` rather than at `/bin/zsh`, because + * that path is macOS's and Linux keeps it elsewhere — testing the wrong one + * skips the whole file on a host that could have run it. + */ + +const REPO = path.dirname(import.meta.dirname); +const RUN = path.join(REPO, 'bench', 'run.sh'); + +const STANDIN = `#!/bin/sh +if [ "$1" = "--version" ]; then echo "2.1.220 (Claude Code)"; exit 0; fi +cat <<'JSON' +{"is_error":false,"result":"A stand-in answer.","modelUsage":{"m-1":{"outputTokens":11}}} +JSON +`; + +const hasZsh = await new Promise((resolve) => { + execFile('zsh', ['-c', ':'], (err) => resolve(!err)); +}); + +function runArm(args, binDir) { + return new Promise((resolve) => { + execFile('zsh', [RUN, ...args], { + cwd: REPO, env: { ...process.env, PATH: `${binDir}${path.delimiter}${process.env.PATH}` }, + }, (err, stdout, stderr) => resolve({ code: err ? err.code ?? 1 : 0, stdout, stderr })); + }); +} + +/** + * The arm name comes from the temporary directory, because the arm directory + * does not. + * + * `run.sh` writes under its own `bench/out/`, and that is the operator's tree + * where real samples live. A name written here as a literal is a name this + * test would resume if anything already stood at it, and then delete on the + * way out — two processes sharing one checkout is enough to produce it. The + * name `mkdtemp` gives is unique to this run and is already a name + * `arm-manifest.mjs` accepts. + */ +async function scaffold(t) { + const root = await fs.mkdtemp(path.join(os.tmpdir(), 'sw-run-')); + const arm = path.basename(root); + assert.ok(NAME.test(arm), `${arm} must be a name the arm manifest accepts`); + const armDir = path.join(REPO, 'bench', 'out', arm); + t.after(() => Promise.all([ + fs.rm(root, { recursive: true, force: true }), + fs.rm(armDir, { recursive: true, force: true }), + ])); + const binDir = path.join(root, 'bin'); + const prompts = path.join(root, 'prompts'); + await fs.mkdir(binDir); + await fs.mkdir(prompts); + await fs.writeFile(path.join(binDir, 'claude'), STANDIN, { mode: 0o755 }); + await fs.writeFile(path.join(prompts, 'pr-1-r1.txt'), 'Say something.\n'); + return { arm, armDir, binDir, prompts }; +} + +test('a fresh arm directory runs clean, and writes a sample, a sidecar and a manifest', + { skip: hasZsh ? false : 'zsh is not installed, and bench/run.sh is zsh' }, async (t) => { + const { arm, armDir, binDir, prompts } = await scaffold(t); + const run = await runArm([arm, '--prompts', prompts, '--reps', '2'], binDir); + assert.equal(run.code, 0, run.stderr); + // The line this test exists for. The run carried on correctly under it, + // which is why a passing check has to be silent: the operator has nothing + // but the output to tell a working arm from a stopped one. + assert.ok(!run.stderr.includes('no matches found'), run.stderr); + const held = (await fs.readdir(armDir)).sort(); + assert.deepEqual(held, [ + 'arm-manifest.json', + 'pr-1-r1-1.txt', 'pr-1-r1-1.txt.meta', + 'pr-1-r1-2.txt', 'pr-1-r1-2.txt.meta', + ]); + }); + +test('an arm resumed under a changed configuration is still refused', + { skip: hasZsh ? false : 'zsh is not installed, and bench/run.sh is zsh' }, async (t) => { + // The refusal the glob was asking for. A fresh directory must not reach it, + // and a collected one must, or an arm silently holds two conditions. + const { arm, binDir, prompts } = await scaffold(t); + assert.equal((await runArm([arm, '--prompts', prompts, '--reps', '2'], binDir)).code, 0); + const again = await runArm( + [arm, '--prompts', prompts, '--reps', '2', '--system', 'bench/review-contract.md'], binDir); + assert.equal(again.code, 2); + assert.match(again.stderr, /Use a new arm name/); + });