fix(plan): every exploration agent runs; the brief is reviewed again only when it changed
voyage-lekerepo trial (2026-10-06) point 14, operator decision 2026-10-08
(«alle agenter, ny briefsjekk bare ved endring»):
- trekplan: the "Adaptive" hard rule (a 10-file project does not need 7
agents) contradicted Phase 5 ("All agents run for all codebase sizes");
it is gone, in line with quality before cost.
- brief-review-stamp.mjs: /trekbrief stamps the brief body when its
brief-reviewer gate passes; /trekplan Phase 4b skips its own review only
while the stamp is current. Stale (edited) or absent (another producer,
or a gate that did not pass) reviews as before. Additive optional
frontmatter fields, documented in Handover 1.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
393b8e5a0a
commit
1b5218e8f4
6 changed files with 231 additions and 4 deletions
|
|
@ -715,7 +715,13 @@ stumble.)
|
|||
1. Move `brief.md.draft` → `brief.md` (atomic rename).
|
||||
2. Delete the draft file if rename is not possible on the OS; write
|
||||
`brief.md` fresh.
|
||||
3. Break the loop and proceed to Step 4g.
|
||||
3. Stamp the reviewed body, so `/trekplan` does not run the same review on
|
||||
the same text again (operator decision 2026-10-08):
|
||||
`node ${CLAUDE_PLUGIN_ROOT}/lib/validators/brief-review-stamp.mjs --stamp "{PROJECT_DIR}/brief.md"`.
|
||||
Any later edit to the body makes the stamp stale and `/trekplan` reviews
|
||||
again. Exit 1 (not stamped) is said in one line; it only means the review
|
||||
runs twice. Only a passed gate is stamped.
|
||||
4. Break the loop and proceed to Step 4g.
|
||||
|
||||
**If gate fails AND iteration count < the class cap:**
|
||||
1. Identify the weakest dimension (lowest score; tie broken by priority:
|
||||
|
|
|
|||
|
|
@ -429,6 +429,19 @@ Codebase: {N} source files ({scale}). Deploying exploration agents.
|
|||
|
||||
## Phase 4b — Brief review
|
||||
|
||||
First check whether `/trekbrief` already reviewed exactly this text:
|
||||
|
||||
```bash
|
||||
node ${CLAUDE_PLUGIN_ROOT}/lib/validators/brief-review-stamp.mjs --check "{brief_path}"
|
||||
```
|
||||
|
||||
Exit 0 (`current`): the body is unchanged since `/trekbrief`'s brief-reviewer
|
||||
gate passed. Skip the review, say so in one line ("Brief review: skipped —
|
||||
unchanged since /trekbrief's review"), and continue to Phase 5. Exit 1
|
||||
(`stale`: edited after that review; `absent`: no stamp — a gate that did not
|
||||
pass, or a brief from another producer) or 2 (could not run): review it as
|
||||
below (operator decision 2026-10-08).
|
||||
|
||||
Launch the **brief-reviewer** agent:
|
||||
Prompt: "Review this task brief for quality: {brief_path}. Check completeness,
|
||||
consistency, testability, scope clarity, and research-plan validity."
|
||||
|
|
@ -1264,7 +1277,5 @@ Standard and low effort: do NOT emit the section at all.
|
|||
- **Honesty**: If exploration reveals the task is trivial (single file, obvious
|
||||
change), say so. Do not inflate the plan to justify the process. Suggest
|
||||
the user just implements it directly.
|
||||
- **Adaptive**: Never spawn more agents than the codebase warrants. A 10-file
|
||||
project does not need 7 exploration agents. Scale down.
|
||||
- **Research transparency**: Always distinguish codebase-derived decisions from
|
||||
research-derived decisions in the plan.
|
||||
|
|
|
|||
|
|
@ -76,6 +76,7 @@ Every validator exposes a CLI: `node lib/validators/<name>.mjs --json <path>` re
|
|||
| `framing` | string | **required at ≥ 2.2** (v5.5) | `preserve \| refine \| replace \| new-direction` | How this brief relates to prior operator intent. Enum-checked on any version when present; missing → `BRIEF_MISSING_FRAMING` at `brief_version ≥ 2.2`. Layer 1 of the framing-alignment defense. |
|
||||
| `intent_approved_hash` | string | optional in the schema; **required by `/trekplan`** (veikart steg 1) | `"sha256:<64 hex>"` | Hash over the normalized `## Intent` + `## Goal`. Written only by `lib/validators/intent-approval.mjs --stamp`, which `/trekbrief` runs on the operator's explicit "Approve" (Phase 4h, or `--approve <project-dir \| brief-file>`). |
|
||||
| `intent_approved_at` | string | optional | ISO-8601 | When the stamp was written. Informational; the gate reads the hash. |
|
||||
| `brief_reviewed_hash` / `brief_reviewed_at` | string | optional, additive (no `brief_version` bump) | `"sha256:<64 hex>"` / ISO-8601 | Hash over the brief's body (after the frontmatter), written by `lib/validators/brief-review-stamp.mjs --stamp` when `/trekbrief`'s brief-reviewer gate passes. `/trekplan` Phase 4b skips its own brief review only while the hash is current; absent (any other producer) or stale means it reviews. A trace, not a signature. |
|
||||
| `ceremony` | string | optional (K1) | `small \| bounded \| architectural` | The brief's size class: sets `/trekbrief`'s question budget and review cap, and what downstream phases scale to. **Absent (or unreadable) = `architectural`**, the heaviest, so another producer's brief is never treated lighter by accident. Enum-checked when present (`BRIEF_INVALID_CEREMONY`). Read it through `briefCeremony(fm)` in `brief-validator.mjs`. |
|
||||
| `artifact_store` | string | optional | `git \| local` | Whether the brief and plan are committed to the target repo (`git`) or kept out of git through `.git/info/exclude` (`local`). The approval log is committed either way. **Absent = `local`**: nothing is published without the operator's yes. Enum-checked when present (`BRIEF_INVALID_ARTIFACT_STORE`). |
|
||||
|
||||
|
|
|
|||
|
|
@ -9,7 +9,7 @@ Imported from `CLAUDE.md` via pointer.
|
|||
- `lib/util/{frontmatter,result,atomic-write,autonomy-gate}.mjs` — shared YAML-frontmatter parser + Result helpers + `atomicWriteJson(path, obj)` for tmp+rename writes + autonomy-gate state machine (v3.4.0)
|
||||
- `lib/parsers/{plan-schema,manifest-yaml,project-discovery,arg-parser,bash-normalize,jaccard,finding-id}.mjs` — pure parsers (no I/O), unit-tested. `manifest-yaml` extended in v3.4.0 with additive `skip_commit_check` + `memory_write` flags (forward-compat: unknown keys ignored)
|
||||
- `lib/review/{rule-catalogue,plan-review-dedup}.mjs` — version-pinned rule catalogue (14 keys) + Phase 9 inline dedup helpers (v3.4.0)
|
||||
- `lib/review/{verdict-gate,engine-findings,recurrence}.mjs` — the review gate (K7: 0 ALLOW/WARN · 1 BLOCK · 2 never green), Claude Code `/code-review` + `/security-review` findings → reviewer payload, recurring finding classes → operator-approved project-memory lines (review_version 1.1); `lib/review/fill-finding-ids.mjs` computes the coordinator's `@@ID_<n>@@` placeholders over `(file, line, rule_key)`
|
||||
- `lib/review/{verdict-gate,engine-findings,recurrence}.mjs` — the review gate (K7: 0 ALLOW/WARN · 1 BLOCK · 2 never green), Claude Code `/code-review` + `/security-review` findings → reviewer payload, recurring finding classes → operator-approved project-memory lines (review_version 1.1); `lib/review/fill-finding-ids.mjs` computes the coordinator's `@@ID_<n>@@` placeholders over `(file, line, rule_key)`; `lib/validators/brief-review-stamp.mjs` lets `/trekplan` skip a brief review `/trekbrief` already ran on the same body
|
||||
- `lib/stats/event-emit.mjs` — single-source stats event emitter for autonomy-gate transitions and main-merge-gate (v3.4.0)
|
||||
- `lib/validators/{brief,research,plan,progress,session-state}-validator.mjs` — schema validators with CLI shims (`node lib/validators/X.mjs --json <path>`)
|
||||
- `lib/validators/architecture-discovery.mjs` — drift-WARN external-contract discovery for `architecture/overview.md`
|
||||
|
|
|
|||
117
lib/validators/brief-review-stamp.mjs
Normal file
117
lib/validators/brief-review-stamp.mjs
Normal file
|
|
@ -0,0 +1,117 @@
|
|||
// lib/validators/brief-review-stamp.mjs
|
||||
// The brief-review stamp: /trekplan reviews a brief again only when it changed.
|
||||
//
|
||||
// Operator decision 2026-10-08 (voyage-lekerepo trial, point 14): /trekplan
|
||||
// Phase 4b re-ran brief-reviewer right after /trekbrief had run it on the same
|
||||
// text. Now /trekbrief, when its brief-reviewer gate passes, stamps the brief's
|
||||
// BODY (everything after the frontmatter) into two optional frontmatter fields:
|
||||
//
|
||||
// brief_reviewed_hash: "sha256:<64 hex>"
|
||||
// brief_reviewed_at: "<ISO-8601>"
|
||||
//
|
||||
// /trekplan Phase 4b runs `--check`: `current` skips the second review,
|
||||
// `stale` (the body changed after the review) or `absent` (no stamp: a gate
|
||||
// that did not pass, or a brief from another producer) runs it as before.
|
||||
// Frontmatter is outside the hash, so the intent approval stamp written after
|
||||
// the review keeps it current; CRLF and trailing whitespace do not count.
|
||||
//
|
||||
// WHAT THE STAMP DOES NOT PROVE: the same trust class as the intent marker
|
||||
// (lib/validators/intent-approval.mjs) — any session that can edit the brief
|
||||
// can write it. It is a trace that the review ran against this body, never a
|
||||
// signature; the worst a forged stamp does is skip one duplicate review.
|
||||
//
|
||||
// CLI:
|
||||
// node lib/validators/brief-review-stamp.mjs --stamp <brief.md> exit 0 stamped, 1 refused
|
||||
// node lib/validators/brief-review-stamp.mjs --check [--json] <brief.md>
|
||||
// exit 0 = current (skip the review); 1 = stale or absent (review); 2 = usage / unreadable
|
||||
|
||||
import { createHash } from 'node:crypto';
|
||||
import { readFileSync, realpathSync, renameSync, writeFileSync } from 'node:fs';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
|
||||
export const REVIEWED_HASH_FIELD = 'brief_reviewed_hash';
|
||||
export const REVIEWED_AT_FIELD = 'brief_reviewed_at';
|
||||
|
||||
const FRONTMATTER = /^(?---\r?\n)([\s\S]*?)(\r?\n---)(?:\r?\n|$)/;
|
||||
|
||||
function split(text) {
|
||||
const m = FRONTMATTER.exec(String(text ?? ''));
|
||||
if (!m) return null;
|
||||
return { m, front: m[2], body: String(text).slice(m.index + m[0].length) };
|
||||
}
|
||||
|
||||
/** sha256 over the body: CRLF -> LF, trailing whitespace per line and at the end dropped. */
|
||||
export function computeBodyHash(body) {
|
||||
const norm = String(body ?? '').replace(/\r\n?/g, '\n').split('\n').map((l) => l.replace(/\s+$/, '')).join('\n').replace(/\n+$/, '');
|
||||
return `sha256:${createHash('sha256').update(norm, 'utf8').digest('hex')}`;
|
||||
}
|
||||
|
||||
function fieldValue(front, name) {
|
||||
const line = front.split(/\r?\n/).find((l) => l.startsWith(`${name}:`));
|
||||
if (!line) return null;
|
||||
return line.slice(name.length + 1).trim().replace(/^["']|["']$/g, '');
|
||||
}
|
||||
|
||||
/** @returns {{ stamped: boolean, text: string, hash?: string, reason?: string }} */
|
||||
export function stampBriefReview(text, now = new Date()) {
|
||||
const parts = split(text);
|
||||
if (!parts) return { stamped: false, text, reason: 'no frontmatter block' };
|
||||
const hash = computeBodyHash(parts.body);
|
||||
const kept = parts.front.split(/\r?\n/).filter((l) =>
|
||||
!l.startsWith(`${REVIEWED_HASH_FIELD}:`) && !l.startsWith(`${REVIEWED_AT_FIELD}:`));
|
||||
kept.push(`${REVIEWED_HASH_FIELD}: "${hash}"`, `${REVIEWED_AT_FIELD}: "${now.toISOString()}"`);
|
||||
const { m } = parts;
|
||||
const out = m[1] + kept.join('\n') + m[3] + '\n' + parts.body;
|
||||
return { stamped: true, text: out, hash };
|
||||
}
|
||||
|
||||
/** @returns {{ state: 'current'|'stale'|'absent', stamped: string|null, actual: string|null }} */
|
||||
export function checkBriefReview(text) {
|
||||
const parts = split(text);
|
||||
if (!parts) return { state: 'absent', stamped: null, actual: null };
|
||||
const stamped = fieldValue(parts.front, REVIEWED_HASH_FIELD);
|
||||
const actual = computeBodyHash(parts.body);
|
||||
if (!stamped) return { state: 'absent', stamped: null, actual };
|
||||
return { state: stamped === actual ? 'current' : 'stale', stamped, actual };
|
||||
}
|
||||
|
||||
function isMain() {
|
||||
try {
|
||||
return realpathSync(process.argv[1]) === realpathSync(fileURLToPath(import.meta.url));
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
if (isMain()) {
|
||||
const args = process.argv.slice(2);
|
||||
const mode = args.includes('--stamp') ? 'stamp' : args.includes('--check') ? 'check' : null;
|
||||
const path = args.find((a) => !a.startsWith('--'));
|
||||
if (!mode || !path) {
|
||||
process.stderr.write('Usage: brief-review-stamp.mjs --stamp <brief.md> | --check [--json] <brief.md>\n');
|
||||
process.exit(2);
|
||||
}
|
||||
let text;
|
||||
try {
|
||||
text = readFileSync(path, 'utf-8');
|
||||
} catch (e) {
|
||||
process.stderr.write(`brief-review-stamp: cannot read ${path}: ${e.message}\n`);
|
||||
process.exit(2);
|
||||
}
|
||||
if (mode === 'stamp') {
|
||||
const r = stampBriefReview(text);
|
||||
if (!r.stamped) {
|
||||
process.stderr.write(`brief-review-stamp: not stamped: ${r.reason}\n`);
|
||||
process.exit(1);
|
||||
}
|
||||
const tmp = `${path}.tmp-${process.pid}`;
|
||||
writeFileSync(tmp, r.text);
|
||||
renameSync(tmp, path);
|
||||
process.stdout.write(`brief-review-stamp: stamped ${path} (${r.hash})\n`);
|
||||
process.exit(0);
|
||||
}
|
||||
const r = checkBriefReview(text);
|
||||
if (args.includes('--json')) process.stdout.write(JSON.stringify(r) + '\n');
|
||||
else process.stdout.write(`brief-review-stamp: ${r.state} ${path}\n`);
|
||||
process.exit(r.state === 'current' ? 0 : 1);
|
||||
}
|
||||
92
tests/validators/brief-review-stamp.test.mjs
Normal file
92
tests/validators/brief-review-stamp.test.mjs
Normal file
|
|
@ -0,0 +1,92 @@
|
|||
// tests/validators/brief-review-stamp.test.mjs
|
||||
// Operator decision 2026-10-08 (voyage-lekerepo trial point 14): /trekplan
|
||||
// Phase 4b re-ran brief-reviewer right after /trekbrief had run it on the same
|
||||
// text. Now /trekbrief stamps the body it reviewed (on a passed gate), and
|
||||
// /trekplan reviews again only when the stamp is absent or the body changed —
|
||||
// a brief from any other producer has no stamp and is always reviewed.
|
||||
// Same decision: every exploration agent runs at every codebase size; the
|
||||
// contradicting "Adaptive" rule is gone.
|
||||
|
||||
import { test } from 'node:test';
|
||||
import { strict as assert } from 'node:assert';
|
||||
import { spawnSync } from 'node:child_process';
|
||||
import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
|
||||
import { dirname, join } from 'node:path';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
import { stampBriefReview, checkBriefReview } from '../../lib/validators/brief-review-stamp.mjs';
|
||||
import { stampIntentApproval } from '../../lib/validators/intent-approval.mjs';
|
||||
|
||||
const HERE = dirname(fileURLToPath(import.meta.url));
|
||||
const ROOT = join(HERE, '..', '..');
|
||||
const CLI = join(ROOT, 'lib', 'validators', 'brief-review-stamp.mjs');
|
||||
|
||||
const BRIEF = [
|
||||
'---', 'type: trekbrief', 'brief_version: "2.2"', 'task: t', 'slug: s', '---', '',
|
||||
'## TL;DR', '', 'A toy.', '', '## Intent', '', 'Why it matters.', '', '## Goal', '', 'What.', '',
|
||||
'## Success Criteria', '', '- `npm test` exits 0.', '',
|
||||
].join('\n');
|
||||
|
||||
test('stamp, then check: current', () => {
|
||||
const s = stampBriefReview(BRIEF);
|
||||
assert.equal(s.stamped, true);
|
||||
assert.match(s.text, /^brief_reviewed_hash: "sha256:[0-9a-f]{64}"$/m);
|
||||
assert.equal(checkBriefReview(s.text).state, 'current');
|
||||
});
|
||||
|
||||
test('no stamp: absent (another producer\'s brief is always reviewed)', () => {
|
||||
assert.equal(checkBriefReview(BRIEF).state, 'absent');
|
||||
});
|
||||
|
||||
test('a changed body after the stamp: stale', () => {
|
||||
const s = stampBriefReview(BRIEF).text;
|
||||
assert.equal(checkBriefReview(s.replace('Why it matters.', 'Why it matters a lot.')).state, 'stale');
|
||||
assert.equal(checkBriefReview(s + '\n## Non-Goals\n\n- none\n').state, 'stale');
|
||||
});
|
||||
|
||||
test('frontmatter changes (the intent approval stamp) and CRLF keep the review current', () => {
|
||||
const s = stampBriefReview(BRIEF).text;
|
||||
const approved = stampIntentApproval(s).text;
|
||||
assert.notEqual(approved, s);
|
||||
assert.equal(checkBriefReview(approved).state, 'current');
|
||||
assert.equal(checkBriefReview(s.replace(/\n/g, '\r\n')).state, 'current');
|
||||
});
|
||||
|
||||
test('re-stamping replaces the old stamp, never adds a second', () => {
|
||||
const twice = stampBriefReview(stampBriefReview(BRIEF).text).text;
|
||||
assert.equal(twice.match(/^brief_reviewed_hash:/gm).length, 1);
|
||||
});
|
||||
|
||||
test('CLI: --stamp writes the file, --check exits 0 current / 1 stale or absent / 2 usage', () => {
|
||||
const dir = mkdtempSync(join(tmpdir(), 'voyage-brief-stamp-'));
|
||||
try {
|
||||
const p = join(dir, 'brief.md');
|
||||
writeFileSync(p, BRIEF);
|
||||
const run = (...a) => spawnSync(process.execPath, [CLI, ...a], { encoding: 'utf8' });
|
||||
assert.equal(run('--check', p).status, 1);
|
||||
assert.equal(run('--stamp', p).status, 0);
|
||||
assert.equal(run('--check', p).status, 0);
|
||||
writeFileSync(p, readFileSync(p, 'utf8').replace('What.', 'What else.'));
|
||||
const c = run('--check', '--json', p);
|
||||
assert.equal(c.status, 1);
|
||||
assert.equal(JSON.parse(c.stdout).state, 'stale');
|
||||
assert.equal(run('--check').status, 2);
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('prose: /trekbrief stamps after a passed gate; /trekplan Phase 4b checks before it reviews again', () => {
|
||||
const brief = readFileSync(join(ROOT, 'commands', 'trekbrief.md'), 'utf8');
|
||||
const plan = readFileSync(join(ROOT, 'commands', 'trekplan.md'), 'utf8');
|
||||
assert.match(brief, /brief-review-stamp\.mjs --stamp/);
|
||||
const p4b = plan.slice(plan.indexOf('## Phase 4b'), plan.indexOf('## Phase 5'));
|
||||
assert.match(p4b, /brief-review-stamp\.mjs --check/);
|
||||
});
|
||||
|
||||
test('prose: every exploration agent runs at every codebase size; no "Adaptive" rule contradicts it', () => {
|
||||
const plan = readFileSync(join(ROOT, 'commands', 'trekplan.md'), 'utf8');
|
||||
assert.match(plan, /All agents run for all codebase sizes/);
|
||||
assert.doesNotMatch(plan, /\*\*Adaptive\*\*/);
|
||||
assert.doesNotMatch(plan, /does not need 7 exploration agents/);
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue