feat(trekexecute): every MAJOR answered before merge, fixed runs the fix loop

This commit is contained in:
Kjell Tore Guttormsen 2026-11-02 18:44:42 +01:00
commit 9e7e8d1516
Signed by: ktg
SSH key fingerprint: SHA256:JakMjO6FTBBzN0Bhfj9saOoEjaFxlSdYuZQQpM/lF9Q
15 changed files with 376 additions and 30 deletions

View file

@ -22,13 +22,17 @@
| trekbrief-stats | ts, task, slug, mode, interview_turns, review_iterations, brief_quality, research_topics, auto_research, auto_result, intent_approved, project_dir, order_id, order_claimed_at, order_reason | commands/trekbrief.md (orchestrator-emit Phase 7; order_* added by lib/stats/stats-append.mjs) | trekbrief.md:1141-1165 | profile, phase_models, profile_source | none |
| trekresearch-stats | ts, question, mode, scope, engine, slug, project_dir, brief_path, dimensions, dimensions_baseline, dimensions_baseline_preserved, effort, conv_turns, empty_turns, unique_sources, agents_local, agents_external, gemini_used, confidence, contradictions, open_questions | lib/stats/stats-append.mjs (trekresearch.md Stats tracking) | trekresearch.md:829-862 | profile, phase_models, parallel_agents, external_research_enabled, profile_source | none |
| trekplan-stats | ts, task, mode, slug, brief_path, project_dir, codebase_size, codebase_files, agents_deployed, deep_dives, research_briefs_used, research_scout_used, critic_verdict, guardian_verdict, outcome | lib/stats/stats-append.mjs (trekplan.md Phase 12) | trekplan.md:1307-1335 | profile, phase_models, parallel_agents, profile_source | none |
| trekexecute-stats (Phase 9 record) | ts, plan, plan_type, mode, result, steps_total, steps_passed, steps_failed, steps_skipped, failed_at_step | lib/stats/stats-append.mjs (trekexecute.md Phase 9) | trekexecute.md:2992-3014 | profile, phase_models, profile_source | none |
| trekexecute-stats (Phase 9 record) | ts, plan, plan_type, mode, result, steps_total, steps_passed, steps_failed, steps_skipped, failed_at_step | lib/stats/stats-append.mjs (trekexecute.md Phase 9) | trekexecute.md:3005-3027 | profile, phase_models, profile_source | none |
| trekexecute-stats (autonomy events) | ts, event, known_event, payload | lib/stats/event-emit.mjs `emit()` | event-emit.mjs:78-99 | payload.profile, payload.phase_models, payload.profile_source | none |
| trekexecute-stats (PostToolUse Bash) | ts, session_id, command_excerpt, duration_ms, success | hooks/scripts/post-bash-stats.mjs (Bash PostToolUse) | post-bash-stats.mjs:42-54 | none (hook is plugin-level, not profile-aware) | command_excerpt (CWE-212) |
| trekreview-stats | ts, slug, verdict, counts (BLOCKER/MAJOR/MINOR/SUGGESTION), reviewed_files_count, mode, duration_ms | lib/stats/stats-append.mjs (trekreview.md Phase 8) | trekreview.md:708-758 | profile, phase_models, profile_source | none |
| trekreview-stats | ts, slug, verdict, counts (BLOCKER/MAJOR/MINOR/SUGGESTION), reviewed_files_count, mode, duration_ms | lib/stats/stats-append.mjs (trekreview.md Phase 8) | trekreview.md:711-764 | profile, phase_models, profile_source | none |
| trekcontinue-stats | ts, project, next_session_label, status | lib/stats/stats-append.mjs (trekcontinue.md Phase 5) | trekcontinue.md:304-308 | profile, profile_source | none |
| token-usage | ts, session_id, scope, model, tokens_input, tokens_output, tokens_cache_creation, tokens_cache_read, cost_usd, is_estimate, price_table_version | lib/stats/token-usage.mjs `captureTokenUsage` (via hooks/scripts/otel-export.mjs Stop hook, opt-in `VOYAGE_TOKEN_METER`) | token-usage.mjs:buildRecord | n/a (SKAL-2 schema) | session_id (stripped at export) |
The `main-merge-declined` event's `payload.reason` (the autonomy-events row) is one of the literal words of
`commands/trekexecute.md` Phase 8: `review-block`, `review-stale`, `review-unanswered`, `review-not-measured`,
`run-not-completed`, `no-main-branch`, `hold-for-review`, `merge-conflict`, `merge-refused`.
## Field-allowlist input for Step 11
The OTel exporter (Step 11 `lib/exporters/field-allowlist.mjs`) MUST inline the

View file

@ -13,6 +13,12 @@
// to mark in Phase 5, and the round fields passed to finalize in Phase 7.
// CAP one cap of three rounds: /trekreview and /trekexecute name
// `review-rounds.mjs` and `MAX_REVIEW_ROUNDS`, and no env name.
// FXL the operator answers each counting MAJOR (/trekreview Phase 8.8 and
// its `--answers` mode); /trekexecute enters the fix loop on
// `review-unanswered: fixed-pending`, closes the merge with the decline
// reason `review-unanswered` otherwise, and counts the answers commit;
// every WARN statement says exit 0 is "ALLOW, or WARN with every
// counting MAJOR answered".
// The helpers below are copied, never imported from an earlier prose test.
// Fix the SOURCE a pin guards, not the pin.
@ -21,6 +27,9 @@ import { strict as assert } from 'node:assert';
import { existsSync, readFileSync } from 'node:fs';
import { dirname, join } from 'node:path';
import { fileURLToPath } from 'node:url';
import { spawnSync } from 'node:child_process';
import { parseArgs } from '../../lib/parsers/arg-parser.mjs';
import { childEnv } from '../helpers/child-env.mjs';
const ROOT = join(dirname(fileURLToPath(import.meta.url)), '..', '..');
@ -146,3 +155,143 @@ test('[CAP] /trekreview and /trekexecute both name review-rounds.mjs and MAX_REV
assert.ok(!text.includes('TREKEXECUTE_MAX_REVIEW_ROUNDS'), `${rel} no longer names TREKEXECUTE_MAX_REVIEW_ROUNDS`);
}
});
// --- FXL: every counting MAJOR answered (Step 7) -------------------------------------
const REVIEW_CMD = 'commands/trekreview.md';
const EXEC_CMD = 'commands/trekexecute.md';
const P88 = '## Phase 8.8 — Answers';
const MERGE_GATE = '### Main-merge gate (MAIN_MERGE_GATE — always pauses)';
const PHASE_72 = '## Phase 7.2 — Simplify, review, verdict (P11)';
/** The sentences of `text` that contain `needle`. */
const sentencesWith = (text, needle) => sentences(text).filter((s) => s.includes(needle));
/** The literal decline reasons of the main-merge gate, in backticks. */
function literalReasons() {
const gate = flat(section(read(EXEC_CMD), MERGE_GATE));
const literal = gate.match(/The reason is one of the literal words above \(([^)]*)\)/);
assert.ok(literal, 'the gate lists the literal decline reasons');
return [...literal[1].matchAll(/`([a-z-]+)`/g)].map((m) => m[1]);
}
test('[FXL] /trekreview Phase 8.8 runs review-answers.mjs pending and record, asks at most four per call, one finding per question, with the two options and their consequences', () => {
const cmd = read(REVIEW_CMD);
const p88 = section(cmd, P88);
assert.ok(cmd.indexOf(P88) > cmd.indexOf('## Phase 8.7 — The project\'s memory'), 'Phase 8.8 comes after Phase 8.7');
assert.ok(cmd.indexOf(P88) < cmd.indexOf('## Phase 9 — Present summary'), 'Phase 8.8 comes before Phase 9');
assert.ok(p88.includes('node ${CLAUDE_PLUGIN_ROOT}/lib/review/review-answers.mjs pending --review {project_dir}/review.md --json'),
'Phase 8.8 runs review-answers.mjs pending');
assert.ok(p88.includes('node ${CLAUDE_PLUGIN_ROOT}/lib/review/review-answers.mjs record --review "{project_dir}/review.md" --finding {id} --answer {accepted|fixed} --repo . --round {R} --store {artifact_store} --trailer "Co-Authored-By: Claude {model} <noreply@anthropic.com>"'),
'Phase 8.8 runs review-answers.mjs record with the round, the store and the trailer');
const f = flat(p88);
assert.match(f, /`AskUserQuestion`/, 'Phase 8.8 asks with AskUserQuestion');
assert.match(f, /at most four per call/, 'Phase 8.8 asks at most four per call');
assert.match(f, /one finding per question/, 'Phase 8.8 asks one finding per question');
assert.match(f, /no file or function names/, 'Phase 8.8 asks in plain words with no file or function names');
assert.match(f, /\*\*Fix it\*\*[^.]*plan step is written[^.]*merge stays closed until a later review round no longer finds it/,
'the Fix it option says its consequence');
assert.match(f, /\*\*Accept: <a reason in plain words>\*\*[^.]*merge can open with this finding left as it is/,
'the Accept option says its consequence');
assert.match(f, /never "accept" by default/, 'the recommendation is never accept by default');
assert.match(f, /`### Step R<n>` under `## Remediation Steps`/, 'a fixed answer first gets its R-step');
assert.match(f, /`"committed": false`/, 'a failed commit is said in one line');
assert.match(f, /ask that finding again once/, 'a refusal asks that finding again once');
// Phase 8 says the review-unanswered exit 2 is expected before Phase 8.8.
const p8 = section(cmd, '## Phase 8 — Validate output + stats');
assert.ok(sentencesWith(p8, '`review-unanswered:`').some((s) => /Phase 8\.8/.test(s) && /expected/.test(s)),
'Phase 8 says an exit 2 with review-unanswered: before Phase 8.8 is expected');
});
test('[FXL] /trekexecute: Phase 7.2 enters the loop on review-unanswered: fixed-pending, both decline lists name review-unanswered, and the audit counts the answers commit', () => {
const cmd = read(EXEC_CMD);
const p72 = section(cmd, PHASE_72);
// Control: the pinned gate line of step 4 is unchanged.
assert.ok(p72.includes('node ${CLAUDE_PLUGIN_ROOT}/lib/review/verdict-gate.mjs --json "{project_dir}/review.md" --repo .'),
'Control: Phase 7.2 step 4 keeps its gate line');
assert.ok(sentencesWith(p72, '`review-unanswered: fixed-pending`').some((s) => /enters the loop below/.test(s) && /as a BLOCK does/.test(s)),
'Phase 7.2 enters the loop on review-unanswered: fixed-pending, as a BLOCK does');
assert.ok(sentencesWith(p72, 'Any other `review-unanswered:`').some((s) => /Phase 7\.5/.test(s) && /merge closed/.test(s)),
'any other review-unanswered: goes to Phase 7.5 with the merge closed');
assert.ok(flat(p72).includes('"The review asked for fixes: {N} BLOCKER(s) and {M} MAJOR(s) answered "fixed", added as steps {list}. Approve the plan as it is now?"'),
'the loop\'s step-4 question names the MAJORs answered fixed');
assert.ok(sentencesWith(p72, 'iterations_remaining').some((s) => /WARN or BLOCK/.test(s) && /new round counted against the cap/.test(s)),
'each loop, WARN or BLOCK, takes 1 from iterations_remaining and the next review is a new round against the cap');
const gate = flat(section(cmd, MERGE_GATE));
const closed = gate.match(/\*\*Closed:\*\* emit `main-merge-declined` with the reason ([^.]*)\./);
assert.ok(closed, 'the gate says which reasons a closed lock emits');
assert.ok(closed[1].includes('`review-unanswered`'), `the closed-lock reasons name review-unanswered: ${closed[1]}`);
assert.ok(literalReasons().includes('review-unanswered'), 'the literal decline reasons name review-unanswered');
assert.match(gate, /Exit 2 with a reason that starts `review-unanswered:` is closed with the decline reason `review-unanswered`/,
'the exit table maps a review-unanswered: reason to its decline reason');
assert.match(gate, /Any other exit 2 that does not start `review-stale` or `review-unanswered:` is `review-not-measured`/,
'only the other exit 2 reasons are review-not-measured');
assert.match(gate, /\| 0 \| ALLOW \/ WARN \(every counting MAJOR answered\) \|/, 'the exit-0 row names the answered MAJORs');
// Control: the gate is a lock, not a question.
assert.doesNotMatch(section(cmd, MERGE_GATE), /AskUserQuestion/, 'Control: the main-merge gate asks nothing');
const audit = flat(section(cmd, '## Phase 7.5 — Manifest audit (independent)'));
assert.ok(audit.includes('`docs(voyage): review answers (round {R})`'), 'the audit\'s process commits name the review answers commit');
});
test('[FXL] tests/fixtures/jsonl-schemas.md lists the main-merge-declined reasons, review-unanswered included, after the field table', () => {
const text = read('tests/fixtures/jsonl-schemas.md');
const lines = text.split('\n');
const table = lines.filter((l) => l.startsWith('|'));
assert.ok(!table.some((l) => l.includes('review-unanswered')), 'the field table itself does not hold the reason list');
const hits = sentences(text).filter((s) => s.includes('main-merge-declined') && s.includes('payload.reason'));
assert.equal(hits.length, 1, `one sentence lists the main-merge-declined payload.reason words: ${hits.join(' | ')}`);
for (const word of literalReasons()) {
assert.ok(hits[0].includes(`\`${word}\``), `the reason list names ${word}: ${hits[0]}`);
}
assert.ok(hits[0].includes('`review-unanswered`'), `the reason list names review-unanswered: ${hits[0]}`);
const tableEnd = lines.findLastIndex((l, i) => l.startsWith('|') && i < lines.indexOf('## Field-allowlist input for Step 11'));
const at = lines.findIndex((l) => l.includes('main-merge-declined') && l.includes('payload.reason'));
assert.ok(at > tableEnd, 'the reason list stands after the field table');
});
// The WARN statements the step names, each by its file.
const WARN_FILES = ['CLAUDE.md', 'README.md', 'docs/command-modes.md', 'docs/architecture.md',
'agents/review-orchestrator.md', 'docs/release-runbook.md', REVIEW_CMD];
// "0 = ALLOW/WARN", "0 ALLOW/WARN", "0 = ALLOW or WARN", "0 — ALLOW / WARN": WARN alone exits 0.
const WARN_ALONE = /\b0\s*(?:=|—|-)?\s*ALLOW\s*(?:\/|or)\s*WARN\b/;
test('[FXL] no WARN statement says WARN alone exits 0; each names the answered MAJORs, and the runbook names review-answers.mjs pending', () => {
for (const rel of WARN_FILES) {
const text = read(rel);
const alone = text.split('\n').filter((l) => WARN_ALONE.test(l));
assert.deepEqual(alone, [], `${rel} still says WARN alone exits 0`);
assert.ok(text.includes('WARN with every counting MAJOR answered'), `${rel} says exit 0 is ALLOW, or WARN with every counting MAJOR answered`);
}
const runbook = flat(read('docs/release-runbook.md'));
assert.ok(runbook.includes('node lib/review/review-answers.mjs pending --review <review.md> --repo .'),
'the release runbook names review-answers.mjs pending');
assert.match(runbook, /NOT_READY/, 'the runbook says an unanswered MAJOR reads NOT_READY');
const coord = flat(section(read('agents/review-coordinator.md'), '### Pass 4 — Compute verdict'));
assert.match(coord, /the verdict gate, not the coordinator, decides whether it was answered/,
'the coordinator\'s Pass 4 says the gate decides whether a MAJOR was answered');
});
test('[FXL] the --answers mode: the parser accepts it, /trekreview names it and says it opens no new round, /trekexecute\'s closed lock names it', () => {
const r = parseArgs('--answers --project x', 'trekreview');
assert.equal(r.flags['--answers'], true, `--answers is a trekreview boolean: ${JSON.stringify(r)}`);
assert.deepEqual(r.unknown, [], `--answers is not unknown: ${JSON.stringify(r)}`);
const usage = spawnSync(process.execPath, [join(ROOT, 'lib', 'parsers', 'arg-parser.mjs'), '--command', 'trekreview', '--stdin'],
{ input: '--answers\n', encoding: 'utf8', env: childEnv() });
assert.equal(usage.status, 1, usage.stdout + usage.stderr);
assert.match(usage.stderr, /Usage: \/trekreview --project <dir> [^\n]*\[--answers\]/, `the usage line names --answers: ${usage.stderr}`);
const cmd = read(REVIEW_CMD);
assert.match(cmd.match(/^argument-hint: (.*)$/m)?.[1] ?? '', /\[--answers\]/, 'the argument-hint names --answers');
const p1 = section(cmd, '## Phase 1 — Parse mode and validate input');
const row = p1.split('\n').find((l) => l.startsWith('| `--answers` |'));
assert.ok(row, 'the Phase 1 mode table has an --answers row');
assert.match(row, /Phase 8\.8/, `the --answers row runs Phase 8.8: ${row}`);
assert.match(row, /no new round/, `the --answers row says it opens no new round: ${row}`);
const modes = section(read('docs/command-modes.md'), '## /trekreview modes');
assert.ok(modes.split('\n').some((l) => l.startsWith('| `--answers` |')), 'docs/command-modes.md lists --answers in the /trekreview modes table');
const claudeRow = read('CLAUDE.md').split('\n').find((l) => l.startsWith('| `/trekreview` |'));
assert.ok(claudeRow && claudeRow.includes('--answers'), 'the /trekreview row of CLAUDE.md names --answers');
const gate = flat(section(read(EXEC_CMD), MERGE_GATE));
assert.ok(gate.includes('/trekreview --answers --project {project_dir}'), 'the closed-lock line names /trekreview --answers');
});

View file

@ -504,6 +504,7 @@ test('[ANS] an accepted answer with a reason, bound to this review, exits 0', ()
test('[ANS] a fixed answer keeps exit 2 with fixed-pending', () => {
assert.ok(existsSync(ANSWERS), 'lib/review/review-answers.mjs must exist');
const fx = ansFixture();
fx.write(fx.read().replace('## Remediation Steps\n\nNone.\n', `## Remediation Steps\n\n### Step R1: fix t1\n\nSource finding: ${fid(1)}\n`));
const rec = fx.record({ answer: 'fixed', reason: 'fixed in step R1' });
assert.equal(rec.status, 0, rec.stdout + rec.stderr);
const r = fx.gate();

View file

@ -11,6 +11,9 @@
// CAP one cap of three rounds (`MAX_REVIEW_ROUNDS`).
// CRY an accepted answer is carried to the next round only by a link to the
// same run's earlier archive, checked by the gate.
// FXL (Step 7) a MAJOR answered "fixed" needs its `### Step R<n>` first:
// `record --answer fixed` refuses without it; /trekreview Phase 8.8
// offers only "Fix it" for a finding `pending` marks `security: true`.
// Tests run the CLIs (prepare, finalize, review-answers, validator, gate) in
// throwaway git repos with no hooks; `carry` is also called directly. Each
// tagged test's main assertion is red at the step's start; a case that
@ -908,3 +911,76 @@ test('[CRY] an answer to one carried occurrence, linked from another carried occ
assert.equal(r3.status, 2, r3.stdout);
assert.deepEqual(parse(r3).unanswered, [h2.id], r3.stdout);
});
// ---- [FXL] answers that ask for a fix (Step 7) ----------------------------------------
/** The review with one `### Step R1` for `id` under `## Remediation Steps`, in place of its "None.". */
function withRStep(text, id, title = 'fix the parser') {
const step = `### Step R1: ${title}\n\n- **Files:** \`a.mjs\`\n- **Source finding:** \`${id}\`\n- **Test first:** a test that fails today\n- **Verify:** re-run /trekreview\n- **On failure:** escalate\n`;
assert.ok(text.includes('## Remediation Steps\n\nNone.\n'), 'the review has no R-step yet');
return text.replace('## Remediation Steps\n\nNone.\n', `## Remediation Steps\n\n${step}`);
}
test('[FXL] a MAJOR answered fixed gives a ### Step R<n>: record --answer fixed refuses without the R-step and records with it, and the validator then passes', () => {
const fx = rndRepo();
// A review of HEAD (c2), so the gate decides on the answers, not on staleness.
fx.write(rndReview({ start: fx.start, end: fx.c2, findings: [MAJ_A3] }).text);
fx.out(CCR);
const f1 = fx.finalize(1);
assert.equal(f1.status, 0, f1.stdout + f1.stderr);
const id = jsonOf(fx.read()).findings[0].id;
const logFile = join(fx.repo, '.claude', 'voyage', 'approvals.jsonl');
const record = () => run([ANSWERS, 'record', '--review', fx.reviewPath, '--finding', id, '--answer', 'fixed', '--repo', fx.repo,
'--round', '1', '--store', 'local', '--trailer', TRAILER], { input: 'the parser gets the missing token back' });
const before = fx.read();
const refused = record();
assert.equal(refused.status, 1, `a fixed answer without its R-step is refused: ${refused.stdout}${refused.stderr}`);
assert.match(String(parse(refused).reason), /write its R-step first/, refused.stdout);
assert.equal(fx.read(), before, 'a refused answer writes no row');
assert.ok(!existsSync(logFile), 'a refused answer appends no K2 record');
// An R-step for another finding is not this finding's R-step.
fx.write(withRStep(before, 'e'.repeat(40)));
const other = record();
assert.equal(other.status, 1, `an R-step for another finding does not count: ${other.stdout}${other.stderr}`);
assert.ok(!existsSync(logFile), 'a refused answer appends no K2 record');
// With its R-step: recorded.
fx.write(withRStep(before, id));
const ok = record();
assert.equal(ok.status, 0, `with its R-step the answer is recorded: ${ok.stdout}${ok.stderr}`);
const text = fx.read();
assert.ok(text.includes(`| ${id} | fixed | the parser gets the missing token back | ${parse(ok).record} | - |`), text);
const v = fx.validate();
assert.equal(v.status, 0, `review-validator --strict: ${v.stdout}${v.stderr}`);
// Control: a fixed answer keeps the merge closed as fixed-pending.
const gate = fx.gate();
assert.equal(gate.status, 2, `Control: ${gate.stdout}${gate.stderr}`);
assert.match(String(parse(gate).reason), /^review-unanswered: fixed-pending 1/, `Control: ${gate.stdout}`);
});
test('[FXL] Phase 8.8 offers only Fix it for a finding pending marks security: true; pending returns that flag for a MAJOR-tier key from the security-review engine', () => {
const p88 = (() => {
const cmd = readFileSync(join(ROOT, 'commands', 'trekreview.md'), 'utf8');
const at = cmd.indexOf('## Phase 8.8 — Answers');
assert.ok(at >= 0, '/trekreview has a Phase 8.8 — Answers');
const next = cmd.indexOf('\n## ', at + 4);
return cmd.slice(at, next === -1 ? undefined : next).replace(/\s+/g, ' ');
})();
assert.match(p88, /Never offered for a finding `pending` marks `security: true`/, 'Accept is never offered for a security finding');
assert.match(p88, /That finding gets one option, Fix it, and its question says a security finding cannot be accepted/,
'a security finding gets one option, Fix it, and the question says why');
// Control: pending marks a MAJOR-tier key from the security-review engine security: true, and record refuses to accept it.
const fx = rndRepo();
const SEC = { severity: 'MAJOR', file: 'a.mjs', line: 3, rule_key: 'MISSING_TEST', engine: 'security-review', title: 'Token logged in clear' };
fx.write(rndReview({ start: fx.start, end: fx.c1, findings: [SEC, { ...MAJ_A3, line: 5 }] }).text);
const p = run([ANSWERS, 'pending', '--review', fx.reviewPath, '--repo', fx.repo, '--json']);
assert.equal(p.status, 1, `Control: ${p.stdout}${p.stderr}`);
const pending = parse(p).pending ?? [];
const sec = pending.find((x) => x.line === 3);
const plain = pending.find((x) => x.line === 5);
assert.ok(sec && plain, `Control: both MAJORs are pending: ${p.stdout}`);
assert.equal(sec.security, true, `Control: the security-review MAJOR is marked security: ${p.stdout}`);
assert.equal(plain.security, false, `Control: the other MAJOR is not: ${p.stdout}`);
const acc = run([ANSWERS, 'record', '--review', fx.reviewPath, '--finding', sec.id, '--answer', 'accepted', '--repo', fx.repo,
'--round', '1', '--store', 'local', '--trailer', TRAILER], { input: 'the token is a test token' });
assert.equal(acc.status, 1, `Control: a security finding is never accepted: ${acc.stdout}${acc.stderr}`);
});