diff --git a/.claude/.claude-plugin/plugin.json b/.claude/.claude-plugin/plugin.json index 10d8549..3461bb4 100644 --- a/.claude/.claude-plugin/plugin.json +++ b/.claude/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "orchestrai", "displayName": "OrchestrAI", - "version": "2.9.1", + "version": "2.10.0", "description": "Orchestrator team: the role agents, the tm- operational skills, and the review workflows.", "author": { "name": "Thomas Mueller" diff --git a/.claude/adapters/prompts/reviewer.md b/.claude/adapters/prompts/reviewer.md index a5d12ec..0898aef 100644 --- a/.claude/adapters/prompts/reviewer.md +++ b/.claude/adapters/prompts/reviewer.md @@ -23,6 +23,18 @@ first (could 200 lines be 50?), surgical changes, goal-driven execution. Match against the AGENTS.md code style and writing style sections. A weakened or deleted test is always a blocking finding. +## Severity floor + +A finding that matches any of these conditions is must-fix, whatever your +overall read of the change. The floor sets severity, not truth: a finding that +is false on the facts is still dismissed, with the reason. + +1. A test deleted, skipped or weakened, without the PR body saying why. +2. `--no-verify`, or any other bypassed git hook. +3. A new dependency with no justification in the PR body. +4. A CI job with no `timeout-minutes`, or a workflow with no `concurrency` group carrying `cancel-in-progress: true`. +5. A change touching the full stack, shipped without e2e. + ## Report contract End with exactly this structure: diff --git a/.claude/agents/reviewer.md b/.claude/agents/reviewer.md index 4b5be8e..2daa24c 100644 --- a/.claude/agents/reviewer.md +++ b/.claude/agents/reviewer.md @@ -36,6 +36,18 @@ first (could 200 lines be 50?), surgical changes, goal-driven execution. Match against the CLAUDE.md code style and writing style sections. A weakened or deleted test is always a blocking finding. +## Severity floor + +A finding that matches any of these conditions is must-fix, whatever your +overall read of the change. The floor sets severity, not truth: a finding that +is false on the facts is still dismissed, with the reason. + +1. A test deleted, skipped or weakened, without the PR body saying why. +2. `--no-verify`, or any other bypassed git hook. +3. A new dependency with no justification in the PR body. +4. A CI job with no `timeout-minutes`, or a workflow with no `concurrency` group carrying `cancel-in-progress: true`. +5. A change touching the full stack, shipped without e2e. + ## Lean track: check suite On a lean track dispatch (the caller says "lean track" in the input), there is diff --git a/.claude/workflows/__tests__/severity-floor.test.mjs b/.claude/workflows/__tests__/severity-floor.test.mjs new file mode 100644 index 0000000..5008ca8 --- /dev/null +++ b/.claude/workflows/__tests__/severity-floor.test.mjs @@ -0,0 +1,117 @@ +/** + * Severity floor test (issue #407). + * + * Five objectively checkable conditions force a finding to must-fix. The + * list is prompt text copied into several surfaces, so this test pins that + * every surface carries all five conditions and that a drifted edit to one + * copy fails here. Whitespace is normalized on both sides so markdown line + * wrapping does not matter. + */ + +import { test, describe } from 'node:test' +import assert from 'node:assert/strict' +import { readFileSync } from 'node:fs' +import { fileURLToPath } from 'node:url' +import { join, dirname } from 'node:path' +import { createContext, runInContext } from 'node:vm' + +const __dir = dirname(fileURLToPath(import.meta.url)) +const workflowsDir = join(__dir, '..') +const repoRoot = join(__dir, '..', '..', '..') + +const FLOOR_CONDITIONS = [ + 'A test deleted, skipped or weakened, without the PR body saying why.', + '`--no-verify`, or any other bypassed git hook.', + 'A new dependency with no justification in the PR body.', + 'A CI job with no `timeout-minutes`, or a workflow with no `concurrency` group carrying `cancel-in-progress: true`.', + 'A change touching the full stack, shipped without e2e.', +] + +const NO_DOWNGRADE = 'cannot be downgraded' + +const norm = (s) => s.replace(/\s+/g, ' ') + +// Same brace-match plus node:vm approach as prompts-sync.test.mjs. +function parsePromptsFromJs(src) { + const startIdx = src.indexOf('const PROMPTS = {') + let pos = src.indexOf('{', startIdx) + let depth = 0 + let started = false + while (pos < src.length) { + if (src[pos] === '{') { depth++; started = true } + if (src[pos] === '}') depth-- + pos++ + if (started && depth === 0) break + } + const ctx = createContext({}) + runInContext(src.slice(startIdx, pos), ctx) + return runInContext('PROMPTS', ctx) +} + +function loadJs(name) { + return parsePromptsFromJs(readFileSync(join(workflowsDir, name), 'utf8')) +} +function loadJson(name) { + return JSON.parse(readFileSync(join(workflowsDir, 'prompts', name), 'utf8')) +} + +// Every template where a severity gets set. verify and scout set none. +const WORKFLOWS = [ + { name: 'tm-review-changes', keys: ['review', 'consolidate'] }, + { name: 'tm-review-codebase', keys: ['area_review', 'architecture_review', 'consolidate'] }, +] + +function assertHasFloor(text, label) { + const t = norm(text) + for (const cond of FLOOR_CONDITIONS) { + assert.ok(t.includes(norm(cond)), `${label} is missing floor condition: ${cond}`) + } +} + +describe('severity floor in workflow prompts', () => { + for (const { name, keys } of WORKFLOWS) { + const js = loadJs(`${name}.js`) + const json = loadJson(`${name}.prompts.json`) + for (const key of keys) { + test(`${name}.js PROMPTS.${key} carries all five conditions`, () => { + assertHasFloor(js[key], `${name}.js PROMPTS.${key}`) + }) + test(`${name}.prompts.json ${key} carries all five conditions`, () => { + assertHasFloor(json[key], `${name}.prompts.json ${key}`) + }) + } + test(`${name}.js consolidate says a floor finding ${NO_DOWNGRADE}`, () => { + assert.ok(norm(js.consolidate).includes(NO_DOWNGRADE)) + }) + test(`${name}.prompts.json consolidate says a floor finding ${NO_DOWNGRADE}`, () => { + assert.ok(norm(json.consolidate).includes(NO_DOWNGRADE)) + }) + } + + test('tm-review-changes verify prompt is unchanged (floor sets severity, not truth)', () => { + const js = loadJs('tm-review-changes.js') + assert.ok(!norm(js.verify).includes(FLOOR_CONDITIONS[0])) + }) +}) + +describe('severity floor in reviewer role surfaces', () => { + const read = (rel) => readFileSync(join(repoRoot, rel), 'utf8') + + test('.claude/agents/reviewer.md carries all five conditions', () => { + assertHasFloor(read('.claude/agents/reviewer.md'), 'agents/reviewer.md') + }) + + test('.claude/adapters/prompts/reviewer.md carries all five conditions', () => { + assertHasFloor(read('.claude/adapters/prompts/reviewer.md'), 'adapters/prompts/reviewer.md') + }) + + test('role-contracts.md reviewer section carries all five conditions', () => { + const doc = read('docs/architecture/role-contracts.md') + const start = doc.search(/^## reviewer\s*$/m) + assert.ok(start !== -1, 'no ## reviewer section') + const rest = doc.slice(start + 1) + const next = rest.search(/^## /m) + const section = next === -1 ? rest : rest.slice(0, next) + assertHasFloor(section, 'role-contracts.md reviewer section') + }) +}) diff --git a/.claude/workflows/prompts/tm-review-changes.prompts.json b/.claude/workflows/prompts/tm-review-changes.prompts.json index 7d3898e..c7b8b15 100644 --- a/.claude/workflows/prompts/tm-review-changes.prompts.json +++ b/.claude/workflows/prompts/tm-review-changes.prompts.json @@ -1,5 +1,5 @@ { - "review": "You review one dimension of a code change and report findings only; you never edit.\n\nDimension: {{brief}}\n\n{{diffHint}}\n\nReport every finding with file, line, severity (must-fix | should-fix | nit), the problem, and the required fix. If the dimension is clean, return an empty findings array. Stay strictly within your dimension.", + "review": "You review one dimension of a code change and report findings only; you never edit.\n\nDimension: {{brief}}\n\n{{diffHint}}\n\nReport every finding with file, line, severity (must-fix | should-fix | nit), the problem, and the required fix. If the dimension is clean, return an empty findings array. Stay strictly within your dimension.\n\nSeverity floor: if a finding you report matches one of these conditions, its severity is must-fix, whatever your overall read. 1. A test deleted, skipped or weakened, without the PR body saying why. 2. `--no-verify`, or any other bypassed git hook. 3. A new dependency with no justification in the PR body. 4. A CI job with no `timeout-minutes`, or a workflow with no `concurrency` group carrying `cancel-in-progress: true`. 5. A change touching the full stack, shipped without e2e.", "verify": "You are an adversarial verifier. A reviewer reported the finding below as must-fix. Start from the position that it is wrong or stale. It survives only if you can reproduce it against the current tree: open the file at the cited line, read the surrounding code, and show the problem is real. \"Cannot reproduce\", \"already fixed\" and \"the evidence does not hold\" all mean confirmed: false. You report only; you never edit.\n\n{{diffHint}}\n\nFinding (JSON):\n{{finding}}\n\nReturn confirmed (true only if you reproduced the problem) and a note of one or two sentences naming what you checked and what you found.", - "consolidate": "You are the senior reviewer. {{coveredCount}} parallel reviewers produced the findings below.{{coverageNote}} {{diffHint}}\n\nEvery must-fix finding went through an adversarial verification pass against the current tree, so must-fix findings arrive in three groups. Confirmed: a verifier reproduced the finding; keep it as must-fix unless the diff shows otherwise. Refuted: a verifier could not reproduce it; do not report it as must-fix and do not repeat it in any field, because the script lists refuted findings in the report. Unverified: no verifier checked it (cap reached or the verifier returned nothing); judge each against the actual diff yourself. For the confirmed, unverified and raw findings: verify against the actual diff, drop false positives and anything out of scope, merge duplicates, and set a final severity. You may add a finding only if it is a clear must-fix the reviewers missed. Only must-fix findings block: verdict is changes-requested if any remain, approve otherwise. Record every dropped finding under dismissed with the reason.\n\nConfirmed must-fix findings (JSON):\n{{confirmedFindings}}\n\nRefuted must-fix findings (JSON, with the verifier note):\n{{refutedFindings}}\n\nUnverified must-fix findings (JSON):\n{{unverifiedFindings}}\n\nRaw should-fix and nit findings (JSON):\n{{rawFindings}}" + "consolidate": "You are the senior reviewer. {{coveredCount}} parallel reviewers produced the findings below.{{coverageNote}} {{diffHint}}\n\nEvery must-fix finding went through an adversarial verification pass against the current tree, so must-fix findings arrive in three groups. Confirmed: a verifier reproduced the finding; keep it as must-fix unless the diff shows otherwise. Refuted: a verifier could not reproduce it; do not report it as must-fix and do not repeat it in any field, because the script lists refuted findings in the report. Unverified: no verifier checked it (cap reached or the verifier returned nothing); judge each against the actual diff yourself. For the confirmed, unverified and raw findings: verify against the actual diff, drop false positives and anything out of scope, merge duplicates, and set a final severity. You may add a finding only if it is a clear must-fix the reviewers missed. Severity floor: any finding that matches one of these conditions is must-fix, whatever your overall read. 1. A test deleted, skipped or weakened, without the PR body saying why. 2. `--no-verify`, or any other bypassed git hook. 3. A new dependency with no justification in the PR body. 4. A CI job with no `timeout-minutes`, or a workflow with no `concurrency` group carrying `cancel-in-progress: true`. 5. A change touching the full stack, shipped without e2e. A floor finding cannot be downgraded to should-fix or nit, but you may still dismiss it if it is false on the facts, with the reason under dismissed. Only must-fix findings block: verdict is changes-requested if any remain, approve otherwise. Record every dropped finding under dismissed with the reason.\n\nConfirmed must-fix findings (JSON):\n{{confirmedFindings}}\n\nRefuted must-fix findings (JSON, with the verifier note):\n{{refutedFindings}}\n\nUnverified must-fix findings (JSON):\n{{unverifiedFindings}}\n\nRaw should-fix and nit findings (JSON):\n{{rawFindings}}" } diff --git a/.claude/workflows/prompts/tm-review-codebase.prompts.json b/.claude/workflows/prompts/tm-review-codebase.prompts.json index dfb757b..8606e6b 100644 --- a/.claude/workflows/prompts/tm-review-codebase.prompts.json +++ b/.claude/workflows/prompts/tm-review-codebase.prompts.json @@ -1,6 +1,6 @@ { "scout": "You map a repository into coherent review areas. You do not review code in this step.\n\n{{scope}}\n\nFirst gauge the repo's size (for example `git ls-files -- {{root}} | wc -l`). Then split the files into N coherent areas, where an area is a set of files that belong together (a module, package, or directory subtree) and is small enough to read in one pass. Size N to the repo: make one area per top-level module or per a few thousand lines of related code, using as few areas as cover it well. Do NOT split finer just to use the budget; only a genuinely large codebase should approach {{maxAreas}} areas. Return at most {{maxAreas}} areas, ranked by importance (size and how central they are to the system). If the repo is larger than {{maxAreas}} areas can cover at a readable size, return the {{maxAreas}} most important and put every path you cannot fit in \"dropped\" so it is reported, not lost. Return areas (name, paths, why) and dropped.", - "area_review": "You review one area of a codebase and report findings only. You never edit.\n\nArea: {{areaName}}\nPaths: {{areaPaths}}\n\nRead these files in full, with surrounding context where needed. {{dimensions}}\n\nReport every finding with area (\"{{areaName}}\"), dimension, file, line, severity (must-fix | should-fix | nit), the problem, and the required fix. If the area is clean, return an empty findings array. Stay within your area.", - "architecture_review": "You audit a repository's structure and report findings only. You never edit. Use dimension \"architecture\".\n\n{{scope}}\n\nRead the directory layout, module boundaries, imports, and dependency manifests. Read signatures and imports rather than full file bodies, so you can hold the whole tree in view. The area map is:\n{{repoMap}}\n\nFlag: module boundaries and layering that have drifted, the same logic duplicated across modules, dead or orphaned code, dependency health (unused, outdated, risky), test-coverage gaps at the suite level, and doc drift between README/CLAUDE.md claims and the actual repo state (commands that no longer exist, a described layout that does not match the real one, stale status claims). Report each finding with area (the module name or \"repo\"), dimension (\"architecture\"), file, line or \"n/a\", severity, the problem, and the fix.", - "consolidate": "You are the senior reviewer consolidating a full-codebase review. The workers below produced the raw findings.{{coverageNote}}\n\nVerify each finding against the actual code, drop false positives and anything out of scope, merge duplicates (including the same problem found in two areas), and set a final severity. You may add a finding only if it is a clear must-fix the workers missed. Only must-fix findings block: verdict is changes-requested if any remain, approve otherwise. Record every dropped finding under dismissed with the reason.\n\nThen write the report file. Run `date +%F` for today's date, make the docs/reviews/ directory if it does not exist, and write docs/reviews/-codebase-review.md with: the verdict and summary first; then a one-line health impression scored 0-10 and up to three named top risks; then, if coverage is partial, a prominent \"Coverage: PARTIAL\" callout immediately after that opening block that states how many paths were not reviewed and the suggested next action; then the findings as severity sections (must-fix, then should-fix, then nit), each organized by area; then a final \"Coverage\" section listing the areas reviewed, the paths not covered, the workers that failed, and (if partial) the suggested next action. Set reportPath to the file you wrote.\n\nReturn the structured summary. Set coverage.areasReviewed to {{reviewedAreas}}, coverage.areasDropped to {{scoutDropped}}, coverage.workersFailed to {{workersFailed}}, coverage.ceilingReached to {{ceilingReached}}{{suggestedNextActionClause}}.\n\nRaw findings (JSON):\n{{rawFindings}}" + "area_review": "You review one area of a codebase and report findings only. You never edit.\n\nArea: {{areaName}}\nPaths: {{areaPaths}}\n\nRead these files in full, with surrounding context where needed. {{dimensions}}\n\nReport every finding with area (\"{{areaName}}\"), dimension, file, line, severity (must-fix | should-fix | nit), the problem, and the required fix. If the area is clean, return an empty findings array. Stay within your area.\n\nSeverity floor: if a finding you report matches one of these conditions, its severity is must-fix, whatever your overall read. 1. A test deleted, skipped or weakened, without the PR body saying why. 2. `--no-verify`, or any other bypassed git hook. 3. A new dependency with no justification in the PR body. 4. A CI job with no `timeout-minutes`, or a workflow with no `concurrency` group carrying `cancel-in-progress: true`. 5. A change touching the full stack, shipped without e2e. A whole-repo review has no diff and no PR body, so apply only the conditions the current tree can show.", + "architecture_review": "You audit a repository's structure and report findings only. You never edit. Use dimension \"architecture\".\n\n{{scope}}\n\nRead the directory layout, module boundaries, imports, and dependency manifests. Read signatures and imports rather than full file bodies, so you can hold the whole tree in view. The area map is:\n{{repoMap}}\n\nFlag: module boundaries and layering that have drifted, the same logic duplicated across modules, dead or orphaned code, dependency health (unused, outdated, risky), test-coverage gaps at the suite level, and doc drift between README/CLAUDE.md claims and the actual repo state (commands that no longer exist, a described layout that does not match the real one, stale status claims). Report each finding with area (the module name or \"repo\"), dimension (\"architecture\"), file, line or \"n/a\", severity, the problem, and the fix.\n\nSeverity floor: if a finding you report matches one of these conditions, its severity is must-fix, whatever your overall read. 1. A test deleted, skipped or weakened, without the PR body saying why. 2. `--no-verify`, or any other bypassed git hook. 3. A new dependency with no justification in the PR body. 4. A CI job with no `timeout-minutes`, or a workflow with no `concurrency` group carrying `cancel-in-progress: true`. 5. A change touching the full stack, shipped without e2e. A whole-repo review has no diff and no PR body, so apply only the conditions the current tree can show.", + "consolidate": "You are the senior reviewer consolidating a full-codebase review. The workers below produced the raw findings.{{coverageNote}}\n\nVerify each finding against the actual code, drop false positives and anything out of scope, merge duplicates (including the same problem found in two areas), and set a final severity. You may add a finding only if it is a clear must-fix the workers missed. Severity floor: any finding that matches one of these conditions is must-fix, whatever your overall read. 1. A test deleted, skipped or weakened, without the PR body saying why. 2. `--no-verify`, or any other bypassed git hook. 3. A new dependency with no justification in the PR body. 4. A CI job with no `timeout-minutes`, or a workflow with no `concurrency` group carrying `cancel-in-progress: true`. 5. A change touching the full stack, shipped without e2e. A whole-repo review has no diff and no PR body, so apply only the conditions the current tree can show. A floor finding cannot be downgraded to should-fix or nit, but you may still dismiss it if it is false on the facts, with the reason under dismissed. Only must-fix findings block: verdict is changes-requested if any remain, approve otherwise. Record every dropped finding under dismissed with the reason.\n\nThen write the report file. Run `date +%F` for today's date, make the docs/reviews/ directory if it does not exist, and write docs/reviews/-codebase-review.md with: the verdict and summary first; then a one-line health impression scored 0-10 and up to three named top risks; then, if coverage is partial, a prominent \"Coverage: PARTIAL\" callout immediately after that opening block that states how many paths were not reviewed and the suggested next action; then the findings as severity sections (must-fix, then should-fix, then nit), each organized by area; then a final \"Coverage\" section listing the areas reviewed, the paths not covered, the workers that failed, and (if partial) the suggested next action. Set reportPath to the file you wrote.\n\nReturn the structured summary. Set coverage.areasReviewed to {{reviewedAreas}}, coverage.areasDropped to {{scoutDropped}}, coverage.workersFailed to {{workersFailed}}, coverage.ceilingReached to {{ceilingReached}}{{suggestedNextActionClause}}.\n\nRaw findings (JSON):\n{{rawFindings}}" } diff --git a/.claude/workflows/tm-review-changes.js b/.claude/workflows/tm-review-changes.js index c120978..c9e083f 100644 --- a/.claude/workflows/tm-review-changes.js +++ b/.claude/workflows/tm-review-changes.js @@ -292,11 +292,11 @@ const diffHint = // runtime-assembled values (coverageNote, rawFindings) are plugged in there. const PROMPTS = { review: - 'You review one dimension of a code change and report findings only; you never edit.\n\nDimension: {{brief}}\n\n{{diffHint}}\n\nReport every finding with file, line, severity (must-fix | should-fix | nit), the problem, and the required fix. If the dimension is clean, return an empty findings array. Stay strictly within your dimension.', + 'You review one dimension of a code change and report findings only; you never edit.\n\nDimension: {{brief}}\n\n{{diffHint}}\n\nReport every finding with file, line, severity (must-fix | should-fix | nit), the problem, and the required fix. If the dimension is clean, return an empty findings array. Stay strictly within your dimension.\n\nSeverity floor: if a finding you report matches one of these conditions, its severity is must-fix, whatever your overall read. 1. A test deleted, skipped or weakened, without the PR body saying why. 2. `--no-verify`, or any other bypassed git hook. 3. A new dependency with no justification in the PR body. 4. A CI job with no `timeout-minutes`, or a workflow with no `concurrency` group carrying `cancel-in-progress: true`. 5. A change touching the full stack, shipped without e2e.', verify: 'You are an adversarial verifier. A reviewer reported the finding below as must-fix. Start from the position that it is wrong or stale. It survives only if you can reproduce it against the current tree: open the file at the cited line, read the surrounding code, and show the problem is real. "Cannot reproduce", "already fixed" and "the evidence does not hold" all mean confirmed: false. You report only; you never edit.\n\n{{diffHint}}\n\nFinding (JSON):\n{{finding}}\n\nReturn confirmed (true only if you reproduced the problem) and a note of one or two sentences naming what you checked and what you found.', consolidate: - 'You are the senior reviewer. {{coveredCount}} parallel reviewers produced the findings below.{{coverageNote}} {{diffHint}}\n\nEvery must-fix finding went through an adversarial verification pass against the current tree, so must-fix findings arrive in three groups. Confirmed: a verifier reproduced the finding; keep it as must-fix unless the diff shows otherwise. Refuted: a verifier could not reproduce it; do not report it as must-fix and do not repeat it in any field, because the script lists refuted findings in the report. Unverified: no verifier checked it (cap reached or the verifier returned nothing); judge each against the actual diff yourself. For the confirmed, unverified and raw findings: verify against the actual diff, drop false positives and anything out of scope, merge duplicates, and set a final severity. You may add a finding only if it is a clear must-fix the reviewers missed. Only must-fix findings block: verdict is changes-requested if any remain, approve otherwise. Record every dropped finding under dismissed with the reason.\n\nConfirmed must-fix findings (JSON):\n{{confirmedFindings}}\n\nRefuted must-fix findings (JSON, with the verifier note):\n{{refutedFindings}}\n\nUnverified must-fix findings (JSON):\n{{unverifiedFindings}}\n\nRaw should-fix and nit findings (JSON):\n{{rawFindings}}', + 'You are the senior reviewer. {{coveredCount}} parallel reviewers produced the findings below.{{coverageNote}} {{diffHint}}\n\nEvery must-fix finding went through an adversarial verification pass against the current tree, so must-fix findings arrive in three groups. Confirmed: a verifier reproduced the finding; keep it as must-fix unless the diff shows otherwise. Refuted: a verifier could not reproduce it; do not report it as must-fix and do not repeat it in any field, because the script lists refuted findings in the report. Unverified: no verifier checked it (cap reached or the verifier returned nothing); judge each against the actual diff yourself. For the confirmed, unverified and raw findings: verify against the actual diff, drop false positives and anything out of scope, merge duplicates, and set a final severity. You may add a finding only if it is a clear must-fix the reviewers missed. Severity floor: any finding that matches one of these conditions is must-fix, whatever your overall read. 1. A test deleted, skipped or weakened, without the PR body saying why. 2. `--no-verify`, or any other bypassed git hook. 3. A new dependency with no justification in the PR body. 4. A CI job with no `timeout-minutes`, or a workflow with no `concurrency` group carrying `cancel-in-progress: true`. 5. A change touching the full stack, shipped without e2e. A floor finding cannot be downgraded to should-fix or nit, but you may still dismiss it if it is false on the facts, with the reason under dismissed. Only must-fix findings block: verdict is changes-requested if any remain, approve otherwise. Record every dropped finding under dismissed with the reason.\n\nConfirmed must-fix findings (JSON):\n{{confirmedFindings}}\n\nRefuted must-fix findings (JSON, with the verifier note):\n{{refutedFindings}}\n\nUnverified must-fix findings (JSON):\n{{unverifiedFindings}}\n\nRaw should-fix and nit findings (JSON):\n{{rawFindings}}', } // Replace {{slot}} markers with vals[slot]; throw on unknown slot. diff --git a/.claude/workflows/tm-review-codebase.js b/.claude/workflows/tm-review-codebase.js index ceb74c4..f5e8ee4 100644 --- a/.claude/workflows/tm-review-codebase.js +++ b/.claude/workflows/tm-review-codebase.js @@ -270,11 +270,11 @@ const PROMPTS = { scout: 'You map a repository into coherent review areas. You do not review code in this step.\n\n{{scope}}\n\nFirst gauge the repo\'s size (for example `git ls-files -- {{root}} | wc -l`). Then split the files into N coherent areas, where an area is a set of files that belong together (a module, package, or directory subtree) and is small enough to read in one pass. Size N to the repo: make one area per top-level module or per a few thousand lines of related code, using as few areas as cover it well. Do NOT split finer just to use the budget; only a genuinely large codebase should approach {{maxAreas}} areas. Return at most {{maxAreas}} areas, ranked by importance (size and how central they are to the system). If the repo is larger than {{maxAreas}} areas can cover at a readable size, return the {{maxAreas}} most important and put every path you cannot fit in "dropped" so it is reported, not lost. Return areas (name, paths, why) and dropped.', area_review: - 'You review one area of a codebase and report findings only. You never edit.\n\nArea: {{areaName}}\nPaths: {{areaPaths}}\n\nRead these files in full, with surrounding context where needed. {{dimensions}}\n\nReport every finding with area ("{{areaName}}"), dimension, file, line, severity (must-fix | should-fix | nit), the problem, and the required fix. If the area is clean, return an empty findings array. Stay within your area.', + 'You review one area of a codebase and report findings only. You never edit.\n\nArea: {{areaName}}\nPaths: {{areaPaths}}\n\nRead these files in full, with surrounding context where needed. {{dimensions}}\n\nReport every finding with area ("{{areaName}}"), dimension, file, line, severity (must-fix | should-fix | nit), the problem, and the required fix. If the area is clean, return an empty findings array. Stay within your area.\n\nSeverity floor: if a finding you report matches one of these conditions, its severity is must-fix, whatever your overall read. 1. A test deleted, skipped or weakened, without the PR body saying why. 2. `--no-verify`, or any other bypassed git hook. 3. A new dependency with no justification in the PR body. 4. A CI job with no `timeout-minutes`, or a workflow with no `concurrency` group carrying `cancel-in-progress: true`. 5. A change touching the full stack, shipped without e2e. A whole-repo review has no diff and no PR body, so apply only the conditions the current tree can show.', architecture_review: - 'You audit a repository\'s structure and report findings only. You never edit. Use dimension "architecture".\n\n{{scope}}\n\nRead the directory layout, module boundaries, imports, and dependency manifests. Read signatures and imports rather than full file bodies, so you can hold the whole tree in view. The area map is:\n{{repoMap}}\n\nFlag: module boundaries and layering that have drifted, the same logic duplicated across modules, dead or orphaned code, dependency health (unused, outdated, risky), test-coverage gaps at the suite level, and doc drift between README/CLAUDE.md claims and the actual repo state (commands that no longer exist, a described layout that does not match the real one, stale status claims). Report each finding with area (the module name or "repo"), dimension ("architecture"), file, line or "n/a", severity, the problem, and the fix.', + 'You audit a repository\'s structure and report findings only. You never edit. Use dimension "architecture".\n\n{{scope}}\n\nRead the directory layout, module boundaries, imports, and dependency manifests. Read signatures and imports rather than full file bodies, so you can hold the whole tree in view. The area map is:\n{{repoMap}}\n\nFlag: module boundaries and layering that have drifted, the same logic duplicated across modules, dead or orphaned code, dependency health (unused, outdated, risky), test-coverage gaps at the suite level, and doc drift between README/CLAUDE.md claims and the actual repo state (commands that no longer exist, a described layout that does not match the real one, stale status claims). Report each finding with area (the module name or "repo"), dimension ("architecture"), file, line or "n/a", severity, the problem, and the fix.\n\nSeverity floor: if a finding you report matches one of these conditions, its severity is must-fix, whatever your overall read. 1. A test deleted, skipped or weakened, without the PR body saying why. 2. `--no-verify`, or any other bypassed git hook. 3. A new dependency with no justification in the PR body. 4. A CI job with no `timeout-minutes`, or a workflow with no `concurrency` group carrying `cancel-in-progress: true`. 5. A change touching the full stack, shipped without e2e. A whole-repo review has no diff and no PR body, so apply only the conditions the current tree can show.', consolidate: - 'You are the senior reviewer consolidating a full-codebase review. The workers below produced the raw findings.{{coverageNote}}\n\nVerify each finding against the actual code, drop false positives and anything out of scope, merge duplicates (including the same problem found in two areas), and set a final severity. You may add a finding only if it is a clear must-fix the workers missed. Only must-fix findings block: verdict is changes-requested if any remain, approve otherwise. Record every dropped finding under dismissed with the reason.\n\nThen write the report file. Run `date +%F` for today\'s date, make the docs/reviews/ directory if it does not exist, and write docs/reviews/-codebase-review.md with: the verdict and summary first; then a one-line health impression scored 0-10 and up to three named top risks; then, if coverage is partial, a prominent "Coverage: PARTIAL" callout immediately after that opening block that states how many paths were not reviewed and the suggested next action; then the findings as severity sections (must-fix, then should-fix, then nit), each organized by area; then a final "Coverage" section listing the areas reviewed, the paths not covered, the workers that failed, and (if partial) the suggested next action. Set reportPath to the file you wrote.\n\nReturn the structured summary. Set coverage.areasReviewed to {{reviewedAreas}}, coverage.areasDropped to {{scoutDropped}}, coverage.workersFailed to {{workersFailed}}, coverage.ceilingReached to {{ceilingReached}}{{suggestedNextActionClause}}.\n\nRaw findings (JSON):\n{{rawFindings}}', + 'You are the senior reviewer consolidating a full-codebase review. The workers below produced the raw findings.{{coverageNote}}\n\nVerify each finding against the actual code, drop false positives and anything out of scope, merge duplicates (including the same problem found in two areas), and set a final severity. You may add a finding only if it is a clear must-fix the workers missed. Severity floor: any finding that matches one of these conditions is must-fix, whatever your overall read. 1. A test deleted, skipped or weakened, without the PR body saying why. 2. `--no-verify`, or any other bypassed git hook. 3. A new dependency with no justification in the PR body. 4. A CI job with no `timeout-minutes`, or a workflow with no `concurrency` group carrying `cancel-in-progress: true`. 5. A change touching the full stack, shipped without e2e. A whole-repo review has no diff and no PR body, so apply only the conditions the current tree can show. A floor finding cannot be downgraded to should-fix or nit, but you may still dismiss it if it is false on the facts, with the reason under dismissed. Only must-fix findings block: verdict is changes-requested if any remain, approve otherwise. Record every dropped finding under dismissed with the reason.\n\nThen write the report file. Run `date +%F` for today\'s date, make the docs/reviews/ directory if it does not exist, and write docs/reviews/-codebase-review.md with: the verdict and summary first; then a one-line health impression scored 0-10 and up to three named top risks; then, if coverage is partial, a prominent "Coverage: PARTIAL" callout immediately after that opening block that states how many paths were not reviewed and the suggested next action; then the findings as severity sections (must-fix, then should-fix, then nit), each organized by area; then a final "Coverage" section listing the areas reviewed, the paths not covered, the workers that failed, and (if partial) the suggested next action. Set reportPath to the file you wrote.\n\nReturn the structured summary. Set coverage.areasReviewed to {{reviewedAreas}}, coverage.areasDropped to {{scoutDropped}}, coverage.workersFailed to {{workersFailed}}, coverage.ceilingReached to {{ceilingReached}}{{suggestedNextActionClause}}.\n\nRaw findings (JSON):\n{{rawFindings}}', } // Replace {{slot}} markers with vals[slot]; throw on unknown slot. diff --git a/docs/architecture/role-contracts.md b/docs/architecture/role-contracts.md index 36ca8a1..09dbdf7 100644 --- a/docs/architecture/role-contracts.md +++ b/docs/architecture/role-contracts.md @@ -175,6 +175,17 @@ then the principles: simplicity first (could 200 lines be 50?), surgical changes, goal-driven execution. A weakened or deleted test is always a blocking finding. +Severity floor: a finding that matches any of these conditions is +must-fix, whatever the reviewer's overall read of the change. The floor +sets severity, not truth: a finding that is false on the facts is still +dismissed, with the reason. + +1. A test deleted, skipped or weakened, without the PR body saying why. +2. `--no-verify`, or any other bypassed git hook. +3. A new dependency with no justification in the PR body. +4. A CI job with no `timeout-minutes`, or a workflow with no `concurrency` group carrying `cancel-in-progress: true`. +5. A change touching the full stack, shipped without e2e. + On a lean track dispatch, there is no tester stage and no sub-plan: the issue body and its `Track: lean` comment are the spec for pass 1. Check out the branch and run the full check suite as a substitute verification