open/voyage
1
0
Fork 0
Code Issues Releases 13 Activity

fix(approval): approval commits carry the caller's Co-Authored-By trailer

Browse source 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Kjell Tore Guttormsen 2026-10-12 20:17:22 +02:00
commit acd0e3dd47
Signed by: ktg
SSH key fingerprint: SHA256:JakMjO6FTBBzN0Bhfj9saOoEjaFxlSdYuZQQpM/lF9Q
8 changed files with 337 additions and 23 deletions
Download patch file Download diff file Expand all files Collapse all files

108
tests/e2e-files-trial4.sh Normal file
View file

@ -0,0 +1,108 @@
#!/usr/bin/env bash
# tests/e2e-files-trial4.sh
# End to end for the trial-4 plan (step 13): the three CLIs run the way a
# command runs them.
# 1. lib/validators/plan-validator.mjs --strict --json on three plans built
# with tests/helpers/plan-v18-builder.mjs: a valid 1.9 plan (exit 0), a
# 1.9 plan with an unticked Files entry (exit 1, PLAN_STEP_PATH_NOT_FIRST),
# a 1.8 plan with an unticked path (exit 0, PLAN_STEP_PATH_UNTICKED).
# 2. lib/util/artifact-commit.mjs in a temp git repo: with a trailer (exit 0,
# the trailer is the message's last line), without (exit 1, HEAD unchanged).
# 3. lib/review/verdict-gate.mjs on the legal (exit 0) and the illegal
# (exit 2) brief-conflict review fixtures.
# Prints "e2e: ok" and exits 0, or names the failed check and exits 1.
# bash 3.2 compatible, ASCII only.
set -u
cd "$(dirname "$0")/.." || { echo "e2e: FAIL - cannot cd to the repo root" >&2; exit 1; }
ROOT="$(pwd)"
unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE
TMP="$(mktemp -d "${TMPDIR:-/tmp}/e2e-files-trial4.XXXXXX")" || { echo "e2e: FAIL - mktemp" >&2; exit 1; }
trap 'rm -rf "$TMP"' EXIT
fail() {
echo "e2e: FAIL - $1" >&2
exit 1
}
TRAILER='Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>'
# ---- 1. the plan validator on three plans ----
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 + "/plan-1-9-valid.md", buildPlan({ version: "1.9" }));
writeFileSync(dir + "/plan-1-9-unticked.md",
buildPlan({ version: "1.9", steps: [{ filesRaw: "src/mod1.mjs - the parser" }, e2e] }));
writeFileSync(dir + "/plan-1-8-unticked.md",
buildPlan({ steps: [{ filesRaw: "`tests/mod1.test.mjs`, src/mod1.mjs" }, e2e] }));
' "$TMP" || fail "could not write the three plans with tests/helpers/plan-v18-builder.mjs"
out="$(node lib/validators/plan-validator.mjs --strict --json "$TMP/plan-1-9-valid.md" 2>&1)"
rc=$?
[ "$rc" -eq 0 ] || fail "valid 1.9 plan: plan-validator exit $rc, expected 0: $out"
out="$(node lib/validators/plan-validator.mjs --strict --json "$TMP/plan-1-9-unticked.md" 2>&1)"
rc=$?
[ "$rc" -eq 1 ] || fail "1.9 plan with an unticked Files entry: plan-validator exit $rc, expected 1: $out"
echo "$out" | grep -q 'PLAN_STEP_PATH_NOT_FIRST' \
|| fail "1.9 plan with an unticked Files entry: no PLAN_STEP_PATH_NOT_FIRST in: $out"
out="$(node lib/validators/plan-validator.mjs --strict --json "$TMP/plan-1-8-unticked.md" 2>&1)"
rc=$?
[ "$rc" -eq 0 ] || fail "1.8 plan with an unticked path: plan-validator exit $rc, expected 0: $out"
echo "$out" | grep -q 'PLAN_STEP_PATH_UNTICKED' \
|| fail "1.8 plan with an unticked path: no PLAN_STEP_PATH_UNTICKED in: $out"
# ---- 2. the approval commit in a temp git repo ----
REPO="$TMP/repo"
mkdir -p "$REPO" "$TMP/hooks-none" || fail "mkdir $REPO"
git -C "$REPO" init -q || fail "git init"
git -C "$REPO" config user.name 'E2E Test'
git -C "$REPO" config user.email 'test@example.invalid'
git -C "$REPO" config commit.gpgsign false
git -C "$REPO" config core.hooksPath "$TMP/hooks-none"
echo 'x' > "$REPO/README.md"
git -C "$REPO" add README.md && git -C "$REPO" commit -q -m 'init' || fail "initial commit in the temp repo"
echo '# brief' > "$REPO/brief.md"
H1='cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc'
H2='dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd'
(cd "$REPO" && node "$ROOT/lib/util/approval-log.mjs" append --kind intent --subject brief.md --sha256 "$H1" >/dev/null) \
|| fail "approval-log append (first record)"
[ -f "$REPO/.claude/voyage/approvals.jsonl" ] || fail "no .claude/voyage/approvals.jsonl in the temp repo"
out="$(node lib/util/artifact-commit.mjs --repo "$REPO" --store local --message 'docs(voyage): approve intent e2e' --trailer "$TRAILER" 2>&1)"
rc=$?
[ "$rc" -eq 0 ] || fail "artifact-commit with a trailer: exit $rc, expected 0: $out"
last="$(git -C "$REPO" log -1 --format=%B | sed '/^$/d' | tail -n 1)"
[ "$last" = "$TRAILER" ] || fail "artifact-commit with a trailer: the last message line is '$last', expected '$TRAILER'"
(cd "$REPO" && node "$ROOT/lib/util/approval-log.mjs" append --kind intent --subject brief.md --sha256 "$H2" >/dev/null) \
|| fail "approval-log append (second record)"
before="$(git -C "$REPO" rev-parse HEAD)"
out="$(node lib/util/artifact-commit.mjs --repo "$REPO" --store local --message 'docs(voyage): approve intent e2e' 2>&1)"
rc=$?
[ "$rc" -eq 1 ] || fail "artifact-commit without a trailer: exit $rc, expected 1: $out"
after="$(git -C "$REPO" rev-parse HEAD)"
[ "$before" = "$after" ] || fail "artifact-commit without a trailer: HEAD moved from $before to $after"
# ---- 3. the verdict gate on the two brief-conflict fixtures ----
out="$(node lib/review/verdict-gate.mjs tests/fixtures/review-brief-conflict-legal.md 2>&1)"
rc=$?
[ "$rc" -eq 0 ] || fail "verdict-gate on review-brief-conflict-legal.md: exit $rc, expected 0: $out"
out="$(node lib/review/verdict-gate.mjs tests/fixtures/review-brief-conflict-illegal.md 2>&1)"
rc=$?
[ "$rc" -eq 2 ] || fail "verdict-gate on review-brief-conflict-illegal.md: exit $rc, expected 2: $out"
echo 'e2e: ok'
exit 0

176
tests/lib/artifact-commit-trailer.test.mjs Normal file
View file

@ -0,0 +1,176 @@
// tests/lib/artifact-commit-trailer.test.mjs
// Approval commits carry the caller's `Co-Authored-By:` trailer (plan step 13).
// The CLI takes `--trailer "<line>"`; a missing, multi-line or malformed one
// (an unfilled `{model}`, no `<email>`, another key) or one holding
// `Claude-Session:` is refused with exit 1 before any git call, so "nothing to
// commit" cannot mask it. A usage error (`--store`) stays exit 2 and comes
// first. Every call in commands/*.md passes the trailer, and none carries a
// `Claude-Session:` line.
import { test } from 'node:test';
import { strict as assert } from 'node:assert';
import { spawnSync } from 'node:child_process';
import { mkdirSync, mkdtempSync, readFileSync, readdirSync, realpathSync, rmSync, writeFileSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { dirname, join } from 'node:path';
import { fileURLToPath } from 'node:url';
import { childEnv } from '../helpers/child-env.mjs';
import { commitArtifacts } from '../../lib/util/artifact-commit.mjs';
const HERE = dirname(fileURLToPath(import.meta.url));
const ROOT = join(HERE, '..', '..');
const CLI = join(ROOT, 'lib', 'util', 'artifact-commit.mjs');
const LOG = join(ROOT, 'lib', 'util', 'approval-log.mjs');
const H1 = 'c'.repeat(64);
const TRAILER = 'Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>';
function git(cwd, ...args) {
return spawnSync('git', args, { cwd, encoding: 'utf8', env: childEnv() });
}
// Same shape as the repo() factory in tests/lib/artifact-commit.test.mjs.
function repo({ withLog = true } = {}) {
const d = mkdtempSync(join(tmpdir(), 'artifact-trailer-'));
git(d, 'init', '-q');
git(d, 'config', 'user.name', 'Test');
git(d, 'config', 'user.email', 'test@example.invalid');
git(d, 'config', 'commit.gpgsign', 'false');
mkdirSync(join(d, '.hooks-none'));
git(d, 'config', 'core.hooksPath', '.hooks-none');
writeFileSync(join(d, 'README.md'), 'x\n');
git(d, 'add', 'README.md');
git(d, 'commit', '-q', '-m', 'init');
const proj = join(d, '.claude', 'projects', '2026-10-12-trailer');
mkdirSync(proj, { recursive: true });
writeFileSync(join(proj, 'brief.md'), '# brief\n');
if (withLog) {
spawnSync(process.execPath, [LOG, 'append', '--kind', 'intent', '--subject', join(proj, 'brief.md'), '--sha256', H1],
{ cwd: d, env: childEnv() });
}
return { d, brief: join(proj, 'brief.md') };
}
function run(args, cwd) {
return spawnSync(process.execPath, [CLI, ...args], { cwd, encoding: 'utf8', env: childEnv() });
}
const head = (d) => git(d, 'rev-parse', 'HEAD').stdout.trim();
test('a passed trailer ends the commit message and git parses it as a trailer', () => {
const { d, brief } = repo();
try {
const r = run(['--store', 'local', '--artifact', brief, '--message', 'docs(voyage): approve intent trailer',
'--trailer', TRAILER], d);
assert.equal(r.status, 0, r.stdout + r.stderr);
assert.equal(JSON.parse(r.stdout).committed, true);
const body = git(d, 'log', '-1', '--format=%B').stdout.trimEnd().split('\n');
assert.equal(body[0], 'docs(voyage): approve intent trailer');
assert.equal(body[body.length - 1], TRAILER, 'the trailer is the last line of the message');
const parsed = git(d, 'log', '-1', '--format=%(trailers:key=Co-Authored-By,valueonly)').stdout.trim();
assert.equal(parsed, 'Claude Opus 5.5 <noreply@anthropic.com>');
} finally { rmSync(d, { recursive: true, force: true }); }
});
const REFUSED = [
['a two-line value', `${TRAILER}\nCo-Authored-By: Other <other@example.invalid>`],
['a value holding Claude-Session:', 'Co-Authored-By: Claude-Session: https://example.invalid/x <noreply@anthropic.com>'],
['a value without <email>', 'Co-Authored-By: Claude Opus 5.5'],
['a non-Co-Authored-By key', 'Signed-off-by: Claude Opus 5.5 <noreply@anthropic.com>'],
['the unfilled {model} placeholder', 'Co-Authored-By: Claude {model} <noreply@anthropic.com>'],
];
for (const [name, value] of REFUSED) {
test(`refused with exit 1, nothing committed: ${name}`, () => {
const { d, brief } = repo();
try {
const before = head(d);
const r = run(['--store', 'local', '--artifact', brief, '--message', 'x', '--trailer', value], d);
assert.equal(r.status, 1, r.stdout + r.stderr);
const out = JSON.parse(r.stdout);
assert.equal(out.committed, false);
assert.match(out.reason, /trailer/i);
assert.equal(head(d), before, 'HEAD unchanged');
} finally { rmSync(d, { recursive: true, force: true }); }
});
}
test('a call without a trailer exits 1 with HEAD unchanged', () => {
const { d, brief } = repo();
try {
const before = head(d);
const r = run(['--store', 'local', '--artifact', brief, '--message', 'x'], d);
assert.equal(r.status, 1, r.stdout + r.stderr);
assert.match(JSON.parse(r.stdout).reason, /--trailer/);
assert.equal(head(d), before, 'HEAD unchanged');
assert.equal(git(d, 'diff', '--cached', '--name-only').stdout.trim(), '', 'nothing staged either');
} finally { rmSync(d, { recursive: true, force: true }); }
});
test('the trailer check comes before any git call: "nothing to commit" does not mask a missing trailer', () => {
const { d, brief } = repo({ withLog: false });
try {
const r = run(['--store', 'local', '--artifact', brief, '--message', 'x'], d);
assert.equal(r.status, 1, r.stdout + r.stderr);
assert.match(JSON.parse(r.stdout).reason, /--trailer/);
} finally { rmSync(d, { recursive: true, force: true }); }
});
test('commitArtifacts takes the trailer too, and refuses without it', () => {
const { d, brief } = repo();
try {
const repoDir = realpathSync(d);
const before = head(d);
const no = commitArtifacts({ repo: repoDir, store: 'local', artifacts: [brief], message: 'x' });
assert.equal(no.exit, 1, JSON.stringify(no));
assert.equal(head(d), before);
const yes = commitArtifacts({ repo: repoDir, store: 'local', artifacts: [brief], message: 'x', trailer: TRAILER });
assert.equal(yes.exit, 0, JSON.stringify(yes));
assert.equal(yes.committed, true);
assert.equal(git(d, 'log', '-1', '--format=%(trailers:key=Co-Authored-By,valueonly)').stdout.trim(),
'Claude Opus 5.5 <noreply@anthropic.com>');
} finally { rmSync(d, { recursive: true, force: true }); }
});
test('usage before trailer: a missing --store is still exit 2, with or without a trailer', () => {
const { d, brief } = repo();
try {
assert.equal(run(['--artifact', brief, '--message', 'x'], d).status, 2);
assert.equal(run(['--artifact', brief, '--message', 'x', '--trailer', TRAILER], d).status, 2);
assert.equal(run(['--store', 'cloud', '--artifact', brief, '--message', 'x', '--trailer', 'bad'], d).status, 2);
} finally { rmSync(d, { recursive: true, force: true }); }
});
// ---- the calls in commands/*.md ----
const CMD_DIR = join(ROOT, 'commands');
// A call is `artifact-commit.mjs` followed by its flags, up to the end of the
// line or the closing backtick of an inline call. A prose mention without
// flags is not a call.
const CALL = /artifact-commit\.mjs[ \t]+--[^`\n]*/g;
function calls() {
const out = [];
for (const f of readdirSync(CMD_DIR).filter((n) => n.endsWith('.md')).sort()) {
const text = readFileSync(join(CMD_DIR, f), 'utf8');
for (const m of text.matchAll(CALL)) out.push({ file: f, call: m[0], index: m.index, text });
}
return out;
}
test('every artifact-commit.mjs call in commands/*.md (exactly 3) passes --trailer, and none holds Claude-Session', () => {
const found = calls();
assert.deepEqual(found.map((c) => c.file), ['trekbrief.md', 'trekexecute.md', 'trekplan.md'],
found.map((c) => `${c.file}: ${c.call}`).join('\n'));
for (const c of found) {
assert.match(c.call, /--trailer "Co-Authored-By: Claude \{model\} <noreply@anthropic\.com>"/, `${c.file}: ${c.call}`);
assert.doesNotMatch(c.call, /Claude-Session/, `${c.file}: ${c.call}`);
}
});
test('commands/trekexecute.md says what to do when the approval commit exits 1 or 2', () => {
const c = calls().find((x) => x.file === 'trekexecute.md');
assert.ok(c, 'the inline call in /trekexecute is found');
const after = c.text.slice(c.index, c.index + 800).replace(/\s+/g, ' ');
assert.ok(after.includes('On exit 1 or 2, print the `reason` in one line and continue: the approval is logged but not committed.'),
after);
});

20
tests/lib/artifact-commit.test.mjs
View file

@ -16,6 +16,8 @@ const ROOT = join(HERE, '..', '..');
const CLI = join(ROOT, 'lib', 'util', 'artifact-commit.mjs');
const LOG = join(ROOT, 'lib', 'util', 'approval-log.mjs');
const H1 = 'c'.repeat(64);
// Every call passes a valid trailer (the CLI refuses a call without one).
const TRAILER = ['--trailer', 'Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>'];
function git(cwd, ...args) {
return spawnSync('git', args, { cwd, encoding: 'utf8' });
@ -50,7 +52,7 @@ test('store git: the brief and the log land in one commit, and nothing else the
try {
writeFileSync(join(d, 'unrelated.txt'), 'staged by the operator\n');
git(d, 'add', 'unrelated.txt');
const r = run(['--store', 'git', '--artifact', brief, '--message', 'docs(voyage): approve intent toggle'], d);
const r = run(['--store', 'git', '--artifact', brief, '--message', 'docs(voyage): approve intent toggle', ...TRAILER], d);
assert.equal(r.status, 0, r.stdout + r.stderr);
assert.equal(JSON.parse(r.stdout).committed, true);
assert.deepEqual(committedFiles(d).sort(),
@ -63,7 +65,7 @@ test('store git: the brief and the log land in one commit, and nothing else the
test('store local: only the log is committed, and the project folder is kept out of git locally', () => {
const { d, brief } = repo();
try {
const r = run(['--store', 'local', '--artifact', brief, '--message', 'docs(voyage): approve intent toggle'], d);
const r = run(['--store', 'local', '--artifact', brief, '--message', 'docs(voyage): approve intent toggle', ...TRAILER], d);
assert.equal(r.status, 0, r.stdout + r.stderr);
assert.deepEqual(committedFiles(d), ['.claude/voyage/approvals.jsonl']);
const exclude = readFileSync(join(d, '.git', 'info', 'exclude'), 'utf8');
@ -71,7 +73,7 @@ test('store local: only the log is committed, and the project folder is kept out
assert.equal(git(d, 'status', '--porcelain', '--', '.claude/projects').stdout.trim(), '',
'the local-only brief does not show as untracked');
// idempotent: a second run adds no second exclude line
run(['--store', 'local', '--artifact', brief, '--message', 'again'], d);
run(['--store', 'local', '--artifact', brief, '--message', 'again', ...TRAILER], d);
const again = readFileSync(join(d, '.git', 'info', 'exclude'), 'utf8');
assert.equal(again.split('\n').filter((l) => l.includes('2026-10-06-toggle')).length, 1);
} finally { rmSync(d, { recursive: true, force: true }); }
@ -80,8 +82,8 @@ test('store local: only the log is committed, and the project folder is kept out
test('nothing new to commit is exit 0 with committed:false (a re-run is not an error)', () => {
const { d, brief } = repo();
try {
assert.equal(run(['--store', 'git', '--artifact', brief, '--message', 'one'], d).status, 0);
const r = run(['--store', 'git', '--artifact', brief, '--message', 'two'], d);
assert.equal(run(['--store', 'git', '--artifact', brief, '--message', 'one', ...TRAILER], d).status, 0);
const r = run(['--store', 'git', '--artifact', brief, '--message', 'two', ...TRAILER], d);
assert.equal(r.status, 0, r.stdout + r.stderr);
assert.equal(JSON.parse(r.stdout).committed, false);
} finally { rmSync(d, { recursive: true, force: true }); }
@ -91,7 +93,7 @@ test('a path the repo ignores is not forced in: exit 1 with the reason', () => {
const { d, brief } = repo();
try {
writeFileSync(join(d, '.gitignore'), '.claude/\n');
const r = run(['--store', 'git', '--artifact', brief, '--message', 'x'], d);
const r = run(['--store', 'git', '--artifact', brief, '--message', 'x', ...TRAILER], d);
assert.equal(r.status, 1, r.stdout + r.stderr);
assert.match(JSON.parse(r.stdout).reason, /ignored/i);
} finally { rmSync(d, { recursive: true, force: true }); }
@ -101,7 +103,7 @@ test('not a git repository: exit 2 (could not run), never a silent pass', () =>
const d = mkdtempSync(join(tmpdir(), 'artifact-commit-nogit-'));
try {
writeFileSync(join(d, 'brief.md'), 'x');
const r = run(['--store', 'git', '--artifact', join(d, 'brief.md'), '--message', 'x'], d);
const r = run(['--store', 'git', '--artifact', join(d, 'brief.md'), '--message', 'x', ...TRAILER], d);
assert.equal(r.status, 2, r.stdout + r.stderr);
} finally { rmSync(d, { recursive: true, force: true }); }
});
@ -109,7 +111,7 @@ test('not a git repository: exit 2 (could not run), never a silent pass', () =>
test('a missing or unknown --store is a usage error (exit 2)', () => {
const { d, brief } = repo();
try {
assert.equal(run(['--artifact', brief, '--message', 'x'], d).status, 2);
assert.equal(run(['--store', 'cloud', '--artifact', brief, '--message', 'x'], d).status, 2);
assert.equal(run(['--artifact', brief, '--message', 'x', ...TRAILER], d).status, 2);
assert.equal(run(['--store', 'cloud', '--artifact', brief, '--message', 'x', ...TRAILER], d).status, 2);
} finally { rmSync(d, { recursive: true, force: true }); }
});