fix(execute): the helper and the executor give red proof no list
Operator decisions 30 and 31 (14.10); review findings 11ee78c5 and d01df508. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
73e53f3134
commit
a70f082f6e
6 changed files with 224 additions and 43 deletions
|
|
@ -17,6 +17,17 @@
|
|||
# 6. lib/verification/red-proof.mjs with --plan/--step and a --tests that
|
||||
# differs from the plan's list (exit 2, tests-mismatch).
|
||||
# 7. artifact-commit with an empty-model trailer (exit 1, HEAD unchanged).
|
||||
# 8. plan-validator on a 1.9 plan whose Files entry wraps onto a line that
|
||||
# opens with a backticked flag (exit 0, no PLAN_STEP_PATH_NOT_A_PATH).
|
||||
# 9. plan-validator on a 1.9 plan whose Test first File line wraps onto a
|
||||
# line that opens with the source path (exit 0, no PLAN_STEP_TESTS_ONLY).
|
||||
# 10. red-proof --red-only with --plan/--step and no --tests in a second temp
|
||||
# repo (exit 0; its JSON tests field is the plan's one test file).
|
||||
# 11. the same with a plan path that does not exist (exit 2, plan-unreadable).
|
||||
# 12. the same as 10 in text form with a --tests that differs (exit 2, the
|
||||
# line " plan_tests: tests/parser.test.mjs").
|
||||
# 13. red-proof --red-only on a 1.8 plan whose File line names two tests,
|
||||
# each with a (new) note (exit 0; tests is both, in order; decision 31).
|
||||
# Prints "e2e: ok" and exits 0, or names the failed check and exits 1.
|
||||
# bash 3.2 compatible, ASCII only.
|
||||
|
||||
|
|
@ -157,5 +168,94 @@ rc=$?
|
|||
after="$(git -C "$REPO" rev-parse HEAD)"
|
||||
[ "$before" = "$after" ] || fail "artifact-commit with an empty-model trailer: HEAD moved from $before to $after"
|
||||
|
||||
# ---- 8. to 13. decisions 29 to 31: wrapped 1.9 lines, red proof without --tests ----
|
||||
|
||||
node --input-type=module -e '
|
||||
import { writeFileSync } from "node:fs";
|
||||
import { buildPlan } from "./tests/helpers/plan-v18-builder.mjs";
|
||||
const dir = process.argv[1];
|
||||
const e2e = { endToEnd: "bash tests/e2e.sh" };
|
||||
writeFileSync(dir + "/p8.md", buildPlan({ version: "1.9", steps: [{
|
||||
filesRaw: "`src/a.mjs`\n `--strict` flag added",
|
||||
}, e2e] }));
|
||||
writeFileSync(dir + "/p9.md", buildPlan({ version: "1.9", steps: [{
|
||||
file: "src/parser.mjs", test: "tests/parser.test.mjs",
|
||||
filesRaw: "`src/parser.mjs`, `tests/parser.test.mjs`",
|
||||
testFirstRaw: " - File: `tests/parser.test.mjs`\n `src/parser.mjs` is the module it drives",
|
||||
}, e2e] }));
|
||||
writeFileSync(dir + "/p13.md", buildPlan({ steps: [{
|
||||
file: "src/parser.mjs",
|
||||
filesRaw: "`src/parser.mjs`, `tests/a.test.mjs`, `tests/b.test.mjs`",
|
||||
testFirstRaw: " - File: `tests/a.test.mjs` (new), `tests/b.test.mjs` (new)",
|
||||
}, e2e] }));
|
||||
' "$TMP" || fail "could not write the plans for checks 8 to 13"
|
||||
|
||||
out="$(node lib/validators/plan-validator.mjs --strict --json "$TMP/p8.md" 2>&1)"
|
||||
rc=$?
|
||||
[ "$rc" -eq 0 ] || fail "8: 1.9 plan with a wrapped Files entry: plan-validator exit $rc, expected 0: $out"
|
||||
if echo "$out" | grep -q 'PLAN_STEP_PATH_NOT_A_PATH'; then
|
||||
fail "8: 1.9 plan with a wrapped Files entry: PLAN_STEP_PATH_NOT_A_PATH in: $out"
|
||||
fi
|
||||
|
||||
out="$(node lib/validators/plan-validator.mjs --strict --json "$TMP/p9.md" 2>&1)"
|
||||
rc=$?
|
||||
[ "$rc" -eq 0 ] || fail "9: 1.9 plan with a wrapped Test first File line: plan-validator exit $rc, expected 0: $out"
|
||||
if echo "$out" | grep -q 'PLAN_STEP_TESTS_ONLY'; then
|
||||
fail "9: 1.9 plan with a wrapped Test first File line: PLAN_STEP_TESTS_ONLY in: $out"
|
||||
fi
|
||||
|
||||
R="$TMP/repo-red"
|
||||
mkdir -p "$R/src" "$R/tests" || fail "mkdir $R"
|
||||
git -C "$R" init -q || fail "git init (second repo)"
|
||||
git -C "$R" config user.name 'E2E Test'
|
||||
git -C "$R" config user.email 'test@example.invalid'
|
||||
git -C "$R" config commit.gpgsign false
|
||||
git -C "$R" config core.hooksPath "$TMP/hooks-none"
|
||||
echo 'x' > "$R/README.md"
|
||||
git -C "$R" add README.md && git -C "$R" commit -q -m 'init' || fail "initial commit in the second temp repo"
|
||||
echo 'export const parse = (s) => Number(s);' > "$R/src/parser.mjs"
|
||||
for t in parser a b; do
|
||||
cat > "$R/tests/$t.test.mjs" <<'EOT'
|
||||
import { test } from 'node:test';
|
||||
import { strict as assert } from 'node:assert';
|
||||
import { parse } from '../src/parser.mjs';
|
||||
test('parse reads a number', () => { assert.equal(parse('2'), 2); });
|
||||
EOT
|
||||
done
|
||||
|
||||
# tests_equal <json file> <expected JSON array>: exit 0 when the JSON's tests deep-equals it.
|
||||
tests_equal() {
|
||||
node -e '
|
||||
const assert = require("node:assert");
|
||||
const fs = require("node:fs");
|
||||
const r = JSON.parse(fs.readFileSync(process.argv[1], "utf8"));
|
||||
try { assert.deepStrictEqual(r.tests, JSON.parse(process.argv[2])); } catch (e) { process.exit(1); }
|
||||
' "$1" "$2"
|
||||
}
|
||||
|
||||
node lib/verification/red-proof.mjs --red-only --cwd "$R" --base HEAD --cmd 'node --test tests/parser.test.mjs' --plan "$TMP/p9.md" --step 1 --json >"$TMP/rp10.json" 2>"$TMP/rp10.err"
|
||||
rc=$?
|
||||
[ "$rc" -eq 0 ] || fail "10: red-proof with --plan/--step and no --tests: exit $rc, expected 0: $(cat "$TMP/rp10.json" "$TMP/rp10.err")"
|
||||
tests_equal "$TMP/rp10.json" '["tests/parser.test.mjs"]' \
|
||||
|| fail "10: red-proof tests is not [\"tests/parser.test.mjs\"]: $(cat "$TMP/rp10.json")"
|
||||
|
||||
out="$(node lib/verification/red-proof.mjs --red-only --cwd "$R" --base HEAD --cmd 'node --test tests/parser.test.mjs' --plan "$TMP/nope.md" --step 1 --json 2>&1)"
|
||||
rc=$?
|
||||
[ "$rc" -eq 2 ] || fail "11: red-proof with a plan that does not exist: exit $rc, expected 2: $out"
|
||||
echo "$out" | grep -q 'plan-unreadable' \
|
||||
|| fail "11: red-proof with a plan that does not exist: no plan-unreadable in: $out"
|
||||
|
||||
out="$(node lib/verification/red-proof.mjs --red-only --cwd "$R" --base HEAD --cmd 'node --test tests/parser.test.mjs' --tests tests/a.test.mjs --plan "$TMP/p9.md" --step 1 2>&1)"
|
||||
rc=$?
|
||||
[ "$rc" -eq 2 ] || fail "12: red-proof with a --tests that differs (text): exit $rc, expected 2: $out"
|
||||
echo "$out" | grep -Fqx ' plan_tests: tests/parser.test.mjs' \
|
||||
|| fail "12: red-proof with a --tests that differs (text): no line ' plan_tests: tests/parser.test.mjs' in: $out"
|
||||
|
||||
node lib/verification/red-proof.mjs --red-only --cwd "$R" --base HEAD --cmd 'node --test tests/a.test.mjs tests/b.test.mjs' --plan "$TMP/p13.md" --step 1 --json >"$TMP/rp13.json" 2>"$TMP/rp13.err"
|
||||
rc=$?
|
||||
[ "$rc" -eq 0 ] || fail "13: red-proof on a 1.8 plan with two (new) tests: exit $rc, expected 0: $(cat "$TMP/rp13.json" "$TMP/rp13.err")"
|
||||
tests_equal "$TMP/rp13.json" '["tests/a.test.mjs","tests/b.test.mjs"]' \
|
||||
|| fail "13: red-proof tests is not the two test paths in order: $(cat "$TMP/rp13.json")"
|
||||
|
||||
echo 'e2e: ok'
|
||||
exit 0
|
||||
|
|
|
|||
|
|
@ -191,10 +191,12 @@ test('review: no sixth round, attempts never reset, a step at 5 is never built a
|
|||
assert.match(c, /A step at `fix_rounds` 5 is never built again/);
|
||||
});
|
||||
|
||||
test('review: the helper appends its report, and passes tests as one comma-separated list', () => {
|
||||
test('review: the helper appends its report, and gives red proof no --tests', () => {
|
||||
const agent = read(AGENT);
|
||||
assert.match(agent, /APPEND it to `report_path`/);
|
||||
assert.match(agent, /ONE comma-separated argument/);
|
||||
const gate = agent.split('\n').find((l) => l.includes('red-proof.mjs --red-only'));
|
||||
assert.ok(gate, 'the helper has its red-proof --red-only line');
|
||||
assert.ok(!gate.includes('--tests'), `the helper's red-proof line passes --tests: ${gate}`);
|
||||
assert.match(agent, /A step with no `Test first:`/);
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -63,7 +63,7 @@ test('1b: plan-critic has it as a finding under Verification gaps, and still 12
|
|||
|
||||
test('1c: the executor runs red-proof with --plan/--step and sends a tests_only not-red-at-base up to the plan', () => {
|
||||
const ex = read('commands/trekexecute.md');
|
||||
assert.match(ex, /red-proof\.mjs --base \{start_sha\} --cmd "\{Verify command\}" --tests [^\n]*--plan "\{plan_path\}" --step \{N\}[^\n]*--json/);
|
||||
assert.match(ex, /red-proof\.mjs --base \{start_sha\} --cmd "\{Verify command\}" --plan "\{plan_path\}" --step \{N\}[^\n]*--json/);
|
||||
assert.match(ex, /`not-red-at-base`, `tests_only: true`[^\n]*straight up to the plan \(Sub-step F, `fix_rounds = 4`, stop class 4\)/);
|
||||
// The helper's gate, as the executor describes it, carries the same flags.
|
||||
assert.match(ex, /red-proof\.mjs --red-only --base \{start_sha\} [^\n]*--plan "\{plan_path\}" --step \{N\}/);
|
||||
|
|
|
|||
|
|
@ -251,10 +251,15 @@ test('decisions 28 and 31: at 1.9 the two-test line with notes still gives only
|
|||
|
||||
// ---- one definition in the prose (plan step 11, review cf241526) -----------
|
||||
// The executor and the step helper define the step's test files once, as
|
||||
// decision 20 reads them; every other place points at that definition. Red
|
||||
// proof refuses a --tests that differs from the plan (`tests-mismatch`) and
|
||||
// prints the plan's list (`plan_tests`): the executor and the helper run it
|
||||
// once more with that list, and the lock follows the list red proof accepted.
|
||||
// decisions 20 and 31 read them; every other place points at that definition.
|
||||
// Operator decision 30 (14.10): red proof reads that list from the plan, so
|
||||
// neither the helper nor the executor passes --tests; it prints the list it
|
||||
// ran (`tests`), and the lock's --files is that printed list. A plan or step
|
||||
// red proof cannot read is `plan-unreadable`: the helper stops before it
|
||||
// builds, the executor corrects its own call and restarts the step once with
|
||||
// a fresh build helper (`build (restart)`), outside the fix rounds. A --tests
|
||||
// that differs is still refused (`tests-mismatch`, the plan's list in
|
||||
// `plan_tests`), but the documented call never gives one.
|
||||
|
||||
const EXEC = ['commands', 'trekexecute.md'];
|
||||
const HELPER = ['agents', 'step-implementer.md'];
|
||||
|
|
@ -276,33 +281,101 @@ test('prose: the decision-20 definition stands exactly once in each file and kee
|
|||
}
|
||||
});
|
||||
|
||||
test('prose: the executor definition names tests-mismatch and plan_tests and keeps its line shape', () => {
|
||||
test('prose: the executor definition names decisions 30 and 31, tests-mismatch, plan_tests and plan-unreadable, and keeps its line shape', () => {
|
||||
const lines = readRel(EXEC).split('\n');
|
||||
const i = lines.findIndex((l) => l.startsWith("**The step's test files** are"));
|
||||
assert.ok(i >= 0, 'the definition line is there');
|
||||
const def = lines[i];
|
||||
assert.ok(def.includes('operator decision 20'), 'the definition line carries decision 20');
|
||||
assert.ok(def.includes('operator decision 30'), 'the definition line carries decision 30');
|
||||
assert.ok(def.includes('`tests-mismatch`'), 'the definition names tests-mismatch');
|
||||
assert.ok(def.includes('`plan_tests`'), 'the definition names plan_tests');
|
||||
assert.ok(def.includes('`--files` is the list red proof accepted'), 'the definition says the lock follows the accepted list');
|
||||
assert.ok(def.includes('`plan-unreadable`'), 'the definition names plan-unreadable');
|
||||
assert.ok(def.includes('`--files` is the list red proof printed as `tests`'), 'the definition says the lock follows the printed list');
|
||||
assert.ok(def.endsWith(' The'), `the definition line still ends with " The": ${def.slice(-40)}`);
|
||||
assert.ok(lines[i + 1].startsWith('plan template lists them'), `the next line: ${lines[i + 1]}`);
|
||||
});
|
||||
|
||||
test('prose: the executor exit-2 row names tests-mismatch and the rerun with plan_tests', () => {
|
||||
const row = readRel(EXEC).split('\n').find((l) => l.startsWith('| 2 | `"not-run"`'));
|
||||
assert.ok(row, 'the exit-2 row is there');
|
||||
assert.ok(row.includes('`tests-mismatch`'), 'the exit-2 row names tests-mismatch');
|
||||
assert.ok(row.includes('`plan_tests`'), 'the exit-2 row names plan_tests');
|
||||
test('prose: both definitions name decision 31 and read past (new), and, plus or + below 1.9', () => {
|
||||
for (const rel of [EXEC, HELPER]) {
|
||||
const text = readRel(rel).replace(/\n\s*/g, ' ');
|
||||
const at20 = text.search(/operator decision 20(?!\d)/);
|
||||
const head = text.lastIndexOf('every file on the `Test first:` `File:` line', at20);
|
||||
assert.ok(at20 > 0 && head >= 0, `${rel.join('/')}: the definition is there`);
|
||||
const def = text.slice(head, at20);
|
||||
assert.ok(def.includes('operator decision 31'), `${rel.join('/')}: the definition names operator decision 31`);
|
||||
assert.ok(def.includes('`plus`'), `${rel.join('/')}: the definition names plus`);
|
||||
assert.ok(def.includes('`(new)`'), `${rel.join('/')}: the definition names (new)`);
|
||||
}
|
||||
});
|
||||
|
||||
test('prose: the helper reruns once with plan_tests on tests-mismatch, before the 1.8 branches, and locks the accepted list', () => {
|
||||
test('prose: the executor exit-2 row names plan-unreadable first, the one restart outside the fix rounds, and tests-mismatch with plan_tests', () => {
|
||||
const row = readRel(EXEC).split('\n').find((l) => l.startsWith('| 2 | `"not-run"`'));
|
||||
assert.ok(row, 'the exit-2 row is there');
|
||||
for (const s of ['`plan-unreadable`', '`tests-mismatch`', '`plan_tests`', 'fresh build helper', 'outside the fix rounds',
|
||||
'absolute path', 'reruns only its own red proof', '`build (restart)`', '`cost_if_wrong`', '`attempts`',
|
||||
'`iterations_remaining`', '`error`', '`--resume`']) {
|
||||
assert.ok(row.includes(s), `the exit-2 row names ${s}`);
|
||||
}
|
||||
assert.match(row, /decision[- ]log\s+entry/, 'the exit-2 row records a decision-log entry');
|
||||
});
|
||||
|
||||
test('prose: a second plan-unreadable goes up to the plan (stop class 4 row, Back to planning)', () => {
|
||||
const text = readRel(EXEC);
|
||||
const row4 = text.split('\n').find((l) => l.startsWith('| 4 | the plan |'));
|
||||
assert.ok(row4, 'the stop-class-4 row is there');
|
||||
assert.ok(row4.includes('`plan-unreadable`'), 'the stop-class-4 row names plan-unreadable');
|
||||
const b = text.indexOf('**Back to planning**');
|
||||
assert.ok(b > 0, 'the Back to planning paragraph is there');
|
||||
const end = text.indexOf('\n\n', b);
|
||||
const para = text.slice(b, end < 0 ? undefined : end);
|
||||
assert.ok(para.includes('`plan-unreadable`'), 'the Back to planning paragraph names plan-unreadable');
|
||||
});
|
||||
|
||||
test('prose: the exit-1 otherwise row no longer counts tests_only: null', () => {
|
||||
const row = readRel(EXEC).split('\n').find((l) => l.startsWith('| 1, `not-red-at-base`, otherwise'));
|
||||
assert.ok(row, 'the exit-1 otherwise row is there');
|
||||
assert.ok(!row.includes('tests_only: null'), 'the exit-1 otherwise row no longer says tests_only: null');
|
||||
});
|
||||
|
||||
test('prose: no red-proof call line in either file passes --tests (operator decision 30)', () => {
|
||||
for (const rel of [EXEC, HELPER]) {
|
||||
const calls = readRel(rel).split('\n').filter((l) => l.includes('red-proof.mjs --red-only') || l.includes('red-proof.mjs --base'));
|
||||
assert.ok(calls.length > 0, `${rel.join('/')}: a red-proof call line is there`);
|
||||
for (const l of calls) assert.ok(!l.includes('--tests'), `${rel.join('/')}: the call passes --tests: ${l}`);
|
||||
}
|
||||
});
|
||||
|
||||
test('prose: the helper stops on plan-unreadable before the 1.8 branches and locks the printed list', () => {
|
||||
const text = readRel(HELPER);
|
||||
const mm = text.match(/tests-mismatch[^.]*once\s+more[^.]*plan_tests/);
|
||||
assert.ok(mm, 'the helper runs the gate once more with plan_tests on tests-mismatch');
|
||||
assert.match(text, /whatever\s+the\s+plan_version/);
|
||||
const pu = text.match(/plan-unreadable[^.]*do not build/);
|
||||
assert.ok(pu, 'the helper does not build on plan-unreadable');
|
||||
const below = text.indexOf('Below 1.8');
|
||||
assert.ok(below > 0, 'the helper still has its Below 1.8 branch');
|
||||
assert.ok(mm.index < below, 'the mismatch sentence stands before "Below 1.8"');
|
||||
assert.match(text, /`--files` is the list red proof accepted/);
|
||||
assert.ok(pu.index < below, 'the plan-unreadable sentence stands before "Below 1.8"');
|
||||
assert.ok(text.includes('`--files` is the list red proof printed'), 'the lock follows the printed list');
|
||||
assert.ok(text.includes('copy that line into the report'), 'the helper copies the tests line into the report');
|
||||
});
|
||||
|
||||
test('prose: the round label build (restart) is named by the executor prompt, the report heading and the helper build mode', () => {
|
||||
const ex = readRel(EXEC);
|
||||
assert.ok(ex.split('\n').some((l) => l.includes('(`build`, `build (restart)`, or `fix round R`)')), 'the Sub-step C prompt line names build (restart)');
|
||||
const h = readRel(HELPER);
|
||||
assert.ok(h.split('\n').includes('## Step {N} - {build | build (restart) | fix round R}'), 'the helper report heading names build (restart)');
|
||||
const a = h.indexOf('## Mode `build`');
|
||||
const z = h.indexOf('## Mode `fix`');
|
||||
assert.ok(a >= 0 && z > a, 'the helper has both modes');
|
||||
assert.match(h.slice(a, z), /build \(restart\)[^.]*one\s+copy\s+of\s+each\s+test\s+case/);
|
||||
});
|
||||
|
||||
test('prose: the old rerun wording is gone from both files', () => {
|
||||
const ex = readRel(EXEC);
|
||||
for (const s of ['runs it once more with the list it printed', 'reruns once with the list red proof printed', 'the list red proof accepted']) {
|
||||
assert.ok(!ex.includes(s), `commands/trekexecute.md still says "${s}"`);
|
||||
}
|
||||
const h = readRel(HELPER);
|
||||
assert.doesNotMatch(h, /once\s+more[^.]*plan_tests/);
|
||||
for (const s of ['after a rerun, `plan_tests`', 'the list red proof accepted']) {
|
||||
assert.ok(!h.includes(s), `agents/step-implementer.md still says "${s}"`);
|
||||
}
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue