From dc3370af9b2e422d1f7ed28fe3cd2d5fe4d92069 Mon Sep 17 00:00:00 2001 From: Lee Overy Date: Sun, 26 Jul 2026 17:46:34 +0100 Subject: [PATCH] fix(prose-tests): the stop hook records, and names the model that walked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The walker has declared a stop hook for weeks and it has never written a line. Every other event is scoped to its world by finding the world path in the payload; a stop event carries no tool input, so there was nothing to match and the record was silently dropped. Nothing failed — the record was simply short, which is the failure mode this whole subsystem exists to prevent. The stop payload does name the agent's own harness transcript, and that file states the model each message ran on. Resolving the world from the transcript revives the hook and, in the same read, answers a question the verdicts could not previously answer: which model produced this result. That matters because an edited agent definition does not reach a running session until its plugins are reloaded. A walk can therefore run on a model nobody intended, and every verdict in that run is trusted at the wrong one with nothing on the page to show it. The model now travels from the harness record through the asserter into the orchestrator's verdict block, so a stale definition is visible in the output instead of being something to remember. Also hardens the asserter-violation log — it appends to a path whose parent it never created, so a contract breach went unrecorded anywhere the directory was absent — and drops a stale claim in the orchestrator's description that it escalates failures to Opus, which it has not done since both agents moved to the same model. Co-Authored-By: Claude Opus 5 --- .claude/agents/prose-asserter.md | 4 + .claude/agents/prose-orchestrator.md | 3 +- .claude/skills/prose-test/SKILL.md | 10 +- package.json | 2 +- tests/prose/lib/record-action.cjs | 72 ++++++- tests/scripts/test-prose-record-action.cjs | 223 +++++++++++++++++++++ 6 files changed, 299 insertions(+), 15 deletions(-) create mode 100644 tests/scripts/test-prose-record-action.cjs diff --git a/.claude/agents/prose-asserter.md b/.claude/agents/prose-asserter.md index 03dfce5f8..c635f3189 100644 --- a/.claude/agents/prose-asserter.md +++ b/.claude/agents/prose-asserter.md @@ -83,6 +83,10 @@ the actions. Return exactly this and nothing else: +0. **Model** — the model named on the recorded `SubagentStop` line, quoted + as recorded. If no such line exists, say `unrecorded`. A verdict is + only as trustworthy as the model that produced the walk, so it is + stated before anything is judged, never inferred, and never omitted. 1. **Path** — one line per expected step: PASS or FAIL, each PASS quoting the transcript line that shows it, each FAIL stating what is missing or contradicting. diff --git a/.claude/agents/prose-orchestrator.md b/.claude/agents/prose-orchestrator.md index b1db42161..5db9c4c68 100644 --- a/.claude/agents/prose-orchestrator.md +++ b/.claude/agents/prose-orchestrator.md @@ -1,6 +1,6 @@ --- name: prose-orchestrator -description: Runs one prose-test case end to end — builds the world, dispatches the walker, computes the delta, dispatches the asserter, escalates a failure to Opus, destroys the world — and returns just the verdict. Dispatched by the /prose-test skill, one per case. +description: Runs one prose-test case end to end — builds the world, dispatches the walker, computes the delta, dispatches the asserter, reruns a failure from a fresh world to confirm it, destroys the world — and returns just the verdict. Dispatched by the /prose-test skill, one per case. tools: Bash, Read, Agent model: sonnet hooks: @@ -90,6 +90,7 @@ Return exactly this and nothing else: ``` CASE: +MODEL: VERDICT: PASS | FAIL | FLAKY | INVALID | HARNESS ERROR PATH: / steps passed WORLD: PASS | FAIL diff --git a/.claude/skills/prose-test/SKILL.md b/.claude/skills/prose-test/SKILL.md index 4ebab0ca3..54b9f812b 100644 --- a/.claude/skills/prose-test/SKILL.md +++ b/.claude/skills/prose-test/SKILL.md @@ -38,8 +38,14 @@ about an expected result can leak into a later dispatch. ## Step 3: Collate Report a verdict table from what the orchestrators returned: case id, -verdict, path steps passed, world, markers. Quote the evidence line for -every failure. +model, verdict, path steps passed, world, markers. Quote the evidence +line for every failure. + +The model column comes from the harness record, not from any agent's +say-so. An edited agent definition does not reach a running session until +its plugins are reloaded, so a run can silently use the previous model — +if the column disagrees with what the agent definitions declare, say so +and treat every verdict in the run as unproven. A `FLAKY` verdict means a failure did not reproduce on a second run from a fresh world — surface both runs, resolve neither. diff --git a/package.json b/package.json index a2be82ef1..50661ef01 100644 --- a/package.json +++ b/package.json @@ -7,7 +7,7 @@ "build": "node build/knowledge.build.js", "knowledge:repl": "node scripts/knowledge-repl.js", "typecheck": "tsc -p tsconfig.json", - "test": "node --test tests/scripts/test-render.cjs tests/scripts/test-engine-gateway.cjs tests/scripts/test-engine-epic-projections.cjs tests/scripts/test-engine-start-projections.cjs tests/scripts/test-engine-workunit-projections.cjs tests/scripts/test-engine-discussion-map.cjs tests/scripts/test-engine-discovery-map.cjs tests/scripts/test-engine-tasks.cjs tests/scripts/test-engine-render-surfaces.cjs tests/scripts/test-engine-transactions.cjs tests/scripts/test-engine-workunit-create.cjs tests/scripts/test-engine-workunit-pivot.cjs tests/scripts/test-engine-workunit-absorb.cjs tests/scripts/test-engine-workunit-promote.cjs tests/scripts/test-engine-discovery-session.cjs tests/scripts/test-engine-cache.cjs tests/scripts/test-engine-manifest-io.cjs tests/scripts/test-engine-manifest-fields.cjs tests/scripts/test-engine-boot.cjs tests/scripts/test-migration-orchestrator.cjs tests/scripts/test-migration-047.cjs tests/scripts/test-migration-048.cjs tests/scripts/test-migration-049.cjs tests/scripts/test-migration-050.cjs tests/scripts/test-migration-051.cjs tests/scripts/test-migration-052.cjs tests/scripts/test-engine-specification-projections.cjs tests/scripts/test-engine-discovery-projections.cjs tests/scripts/test-gateway-for-start.cjs tests/scripts/test-gateway-for-specification.cjs tests/scripts/test-gateway-for-workflow-continue-epic.cjs tests/scripts/test-gateway-for-workflow-continue-feature.cjs tests/scripts/test-gateway-for-workflow-continue-bugfix.cjs tests/scripts/test-gateway-for-workflow-continue-quickfix.cjs tests/scripts/test-gateway-for-workflow-continue-cross-cutting.cjs tests/scripts/test-gateway-for-discovery-skill.cjs tests/scripts/test-gateway-for-bridge.cjs tests/scripts/test-reads-derivations.cjs tests/scripts/test-ensure-discovery-item.cjs tests/scripts/test-legacy-research-split.cjs tests/scripts/test-knowledge-chunker.cjs tests/scripts/test-knowledge-config.cjs tests/scripts/test-knowledge-discovery.cjs tests/scripts/test-knowledge-embeddings.cjs tests/scripts/test-knowledge-integration.cjs tests/scripts/test-knowledge-openai.cjs tests/scripts/test-knowledge-openai-compatible.cjs tests/scripts/test-knowledge-openai-integration.cjs tests/scripts/test-knowledge-progress.cjs tests/scripts/test-knowledge-rerank.cjs tests/scripts/test-knowledge-retry.cjs tests/scripts/test-knowledge-setup-forms.cjs tests/scripts/test-knowledge-store.cjs tests/scripts/test-conventions-lint.cjs tests/scripts/test-pipeline-simulation.cjs tests/scripts/test-engine-agent-state.cjs tests/scripts/test-prose-cases.cjs tests/scripts/test-prose-snapshots.cjs", + "test": "node --test tests/scripts/test-render.cjs tests/scripts/test-engine-gateway.cjs tests/scripts/test-engine-epic-projections.cjs tests/scripts/test-engine-start-projections.cjs tests/scripts/test-engine-workunit-projections.cjs tests/scripts/test-engine-discussion-map.cjs tests/scripts/test-engine-discovery-map.cjs tests/scripts/test-engine-tasks.cjs tests/scripts/test-engine-render-surfaces.cjs tests/scripts/test-engine-transactions.cjs tests/scripts/test-engine-workunit-create.cjs tests/scripts/test-engine-workunit-pivot.cjs tests/scripts/test-engine-workunit-absorb.cjs tests/scripts/test-engine-workunit-promote.cjs tests/scripts/test-engine-discovery-session.cjs tests/scripts/test-engine-cache.cjs tests/scripts/test-engine-manifest-io.cjs tests/scripts/test-engine-manifest-fields.cjs tests/scripts/test-engine-boot.cjs tests/scripts/test-migration-orchestrator.cjs tests/scripts/test-migration-047.cjs tests/scripts/test-migration-048.cjs tests/scripts/test-migration-049.cjs tests/scripts/test-migration-050.cjs tests/scripts/test-migration-051.cjs tests/scripts/test-migration-052.cjs tests/scripts/test-engine-specification-projections.cjs tests/scripts/test-engine-discovery-projections.cjs tests/scripts/test-gateway-for-start.cjs tests/scripts/test-gateway-for-specification.cjs tests/scripts/test-gateway-for-workflow-continue-epic.cjs tests/scripts/test-gateway-for-workflow-continue-feature.cjs tests/scripts/test-gateway-for-workflow-continue-bugfix.cjs tests/scripts/test-gateway-for-workflow-continue-quickfix.cjs tests/scripts/test-gateway-for-workflow-continue-cross-cutting.cjs tests/scripts/test-gateway-for-discovery-skill.cjs tests/scripts/test-gateway-for-bridge.cjs tests/scripts/test-reads-derivations.cjs tests/scripts/test-ensure-discovery-item.cjs tests/scripts/test-legacy-research-split.cjs tests/scripts/test-knowledge-chunker.cjs tests/scripts/test-knowledge-config.cjs tests/scripts/test-knowledge-discovery.cjs tests/scripts/test-knowledge-embeddings.cjs tests/scripts/test-knowledge-integration.cjs tests/scripts/test-knowledge-openai.cjs tests/scripts/test-knowledge-openai-compatible.cjs tests/scripts/test-knowledge-openai-integration.cjs tests/scripts/test-knowledge-progress.cjs tests/scripts/test-knowledge-rerank.cjs tests/scripts/test-knowledge-retry.cjs tests/scripts/test-knowledge-setup-forms.cjs tests/scripts/test-knowledge-store.cjs tests/scripts/test-conventions-lint.cjs tests/scripts/test-pipeline-simulation.cjs tests/scripts/test-engine-agent-state.cjs tests/scripts/test-prose-record-action.cjs tests/scripts/test-prose-cases.cjs tests/scripts/test-prose-snapshots.cjs", "test:migrations": "for t in tests/scripts/test-migration-*.sh; do bash $t || exit 1; done", "test:cli": "for t in tests/scripts/test-engine-manifest.sh tests/scripts/test-inbox-promotion.sh tests/scripts/test-knowledge-cli.sh tests/scripts/test-knowledge-build.sh; do bash $t || exit 1; done" }, diff --git a/tests/prose/lib/record-action.cjs b/tests/prose/lib/record-action.cjs index 564534206..e228b4064 100644 --- a/tests/prose/lib/record-action.cjs +++ b/tests/prose/lib/record-action.cjs @@ -13,15 +13,24 @@ // // Every event is captured, not just the call: the intent before it // (PreToolUse), the result after it (PostToolUse, with output), the -// failures (PostToolUseFailure), and the finish (Stop). Logs are -// throwaway — they live in the disposable world and die with it — so -// there is no reason to record less than everything. +// failures (PostToolUseFailure), and the finish (SubagentStop, carrying +// the model the walk actually ran on). Logs are throwaway — they live in +// the disposable world and die with it — so there is no reason to record +// less than everything. +// +// The model matters because an edited agent definition does not reach a +// running session until its plugins are reloaded. A walk can therefore +// run on a model nobody intended, and a verdict trusted at the wrong one +// is worse than no verdict. Recording it makes that visible in the +// result rather than something to remember. // // Self-scoping: a prose world lives at a temp path containing // `prose-world-`. The payload is scanned for one, and anything happening -// outside a world is ignored. The asserter is the exception: it is -// contracted to use NO tools, so any tool call it makes is a contract -// violation and lands in a repo-local log instead of a world. +// outside a world is ignored. A stop event names no file, so its world +// and its model both come from the agent transcript the payload points +// at. The asserter is the exception: it is contracted to use NO tools, +// so any tool call it makes is a contract violation and lands in a +// repo-local log instead of a world. // // The log is written to /.walk-actions.log, which collectTree() // excludes, so recording never shows up as a world difference. @@ -56,6 +65,34 @@ function summarise(input) { return flatten(raw, 200); } +/** + * The agent's own harness transcript — written by the runtime, not the + * agent. Authority on the model it ran on, and on which world it walked, + * neither of which a stop payload states. + */ +function fromTranscript(transcriptPath) { + const blank = { world: null, model: '' }; + if (!transcriptPath) return blank; + let raw; + try { + raw = fs.readFileSync(transcriptPath, 'utf8'); + } catch { + return blank; + } + const models = new Set(); + for (const line of raw.split('\n')) { + try { + const model = JSON.parse(line)?.message?.model; + if (model) models.add(model); + } catch { /* a partial line is not worth failing the record over */ } + } + const found = raw.match(WORLD); + return { + world: found ? found[2].replace(/\\+/g, '') : null, + model: [...models].join(',') || '', + }; +} + function main() { const payload = read(); if (!payload) return; @@ -71,19 +108,30 @@ function main() { const projectDir = process.env.CLAUDE_PROJECT_DIR; if (projectDir) { try { - fs.appendFileSync(path.join(projectDir, VIOLATIONS), + const file = path.join(projectDir, VIOLATIONS); + fs.mkdirSync(path.dirname(file), { recursive: true }); + fs.appendFileSync(file, `${agent}\t${event}\t${tool}\t${summarise(payload.tool_input)}\n`); } catch { /* a hook must never break what it observes */ } } return; } + // A stop event carries no tool input, so the world path that scopes + // every other event is absent from it — resolve it from the transcript. + const stop = event === 'Stop' || event === 'SubagentStop'; + const traced = stop ? fromTranscript(payload.agent_transcript_path) : null; const found = JSON.stringify(payload).match(WORLD); - if (!found) return; - const world = found[2].replace(/\\+/g, ''); - if (!fs.existsSync(world)) return; + const world = found ? found[2].replace(/\\+/g, '') : traced && traced.world; + if (!world || !fs.existsSync(world)) return; - const parts = [event, tool, summarise(payload.tool_input).split(world).join('.')]; + const parts = [ + event, + tool, + stop + ? traced.model || 'model-unknown' + : summarise(payload.tool_input).split(world).join('.'), + ]; if (event === 'PostToolUse') { parts.push(payload.tool_output_is_error ? 'ERROR' : 'ok'); @@ -91,6 +139,8 @@ function main() { } else if (event === 'PostToolUseFailure') { parts.push('FAILED'); parts.push(flatten(payload.tool_output ?? payload.error, MAX_OUTPUT)); + } else if (stop) { + parts.push(flatten(payload.last_assistant_message, MAX_OUTPUT).split(world).join('.')); } try { diff --git a/tests/scripts/test-prose-record-action.cjs b/tests/scripts/test-prose-record-action.cjs new file mode 100644 index 000000000..756aef4ee --- /dev/null +++ b/tests/scripts/test-prose-record-action.cjs @@ -0,0 +1,223 @@ +'use strict'; + +// The prose-test recorder is the only witness to what a walker actually +// did — and it went a long stretch silently dropping every stop event, +// because a stop payload names no file and the recorder scopes work to a +// world by finding its path in the payload. Nothing failed; the record +// was simply short. These tests pin the resolution paths so that class of +// silence cannot return. + +const { describe, it, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert'); +const { execFileSync } = require('node:child_process'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); + +const HOOK = path.join(__dirname, '..', 'prose', 'lib', 'record-action.cjs'); +const LOG = '.walk-actions.log'; + +let world; +let projectDir; + +/** Feed the hook a payload exactly as the harness would, on stdin. */ +function fire(payload) { + execFileSync('node', [HOOK], { + input: JSON.stringify(payload), + env: { ...process.env, CLAUDE_PROJECT_DIR: projectDir }, + }); +} + +function logLines() { + const file = path.join(world, LOG); + if (!fs.existsSync(file)) return []; + return fs.readFileSync(file, 'utf8').trim().split('\n').filter(Boolean); +} + +/** A transcript in the shape the runtime writes: one JSON object per line. */ +function writeTranscript(entries) { + const file = path.join(world, 'agent-transcript.jsonl'); + fs.writeFileSync(file, entries.map((e) => JSON.stringify(e)).join('\n')); + return file; +} + +beforeEach(() => { + world = fs.mkdtempSync(path.join(os.tmpdir(), 'prose-world-')); + projectDir = fs.mkdtempSync(path.join(os.tmpdir(), 'prose-project-')); +}); + +afterEach(() => { + fs.rmSync(world, { recursive: true, force: true }); + fs.rmSync(projectDir, { recursive: true, force: true }); +}); + +describe('prose recorder — tool events', () => { + it('records a call scoped to the world named in its input', () => { + fire({ + hook_event_name: 'PreToolUse', + tool_name: 'Bash', + agent_type: 'prose-walker', + tool_input: { command: `cd ${world} && engine boot` }, + }); + const [row] = logLines(); + assert.match(row, /^PreToolUse\tBash\t/); + }); + + it('records the output of a call, so a claim about it can be checked', () => { + fire({ + hook_event_name: 'PostToolUse', + tool_name: 'Bash', + agent_type: 'prose-walker', + tool_input: { command: `cd ${world} && engine view` }, + tool_output: 'MENU: 1. Continue "Pay"', + }); + const [row] = logLines(); + assert.ok(row.includes('\tok\t'), 'a successful call is marked ok'); + assert.ok(row.includes('MENU: 1. Continue "Pay"'), 'output is recorded verbatim'); + }); + + it('marks a failing call so a walk cannot look cleaner than it was', () => { + fire({ + hook_event_name: 'PostToolUse', + tool_name: 'Bash', + agent_type: 'prose-walker', + tool_input: { command: `cd ${world} && engine nope` }, + tool_output: 'unknown command', + tool_output_is_error: true, + }); + assert.ok(logLines()[0].includes('\tERROR\t')); + }); + + it('ignores anything happening outside a world', () => { + fire({ + hook_event_name: 'PreToolUse', + tool_name: 'Bash', + agent_type: 'prose-walker', + tool_input: { command: 'git status' }, + }); + assert.deepEqual(logLines(), []); + }); +}); + +describe('prose recorder — the stop event', () => { + it('resolves the world from the transcript, which the payload omits', () => { + const transcript = writeTranscript([ + { message: { model: 'claude-sonnet-5' }, cwd: world }, + ]); + fire({ + hook_event_name: 'SubagentStop', + agent_type: 'prose-walker', + agent_transcript_path: transcript, + last_assistant_message: 'STOPPED: end of flow', + }); + assert.equal(logLines().length, 1, 'the stop event reaches the world log'); + }); + + it('names the model the walk actually ran on', () => { + const transcript = writeTranscript([ + { message: { model: 'claude-sonnet-5' }, cwd: world }, + { message: { model: 'claude-sonnet-5' } }, + ]); + fire({ + hook_event_name: 'SubagentStop', + agent_type: 'prose-walker', + agent_transcript_path: transcript, + last_assistant_message: 'STOPPED: end of flow', + }); + const [row] = logLines(); + assert.equal(row.split('\t')[2], 'claude-sonnet-5'); + assert.ok(row.includes('STOPPED: end of flow'), 'the final message is recorded too'); + }); + + it('names every model when a walk did not run on just one', () => { + const transcript = writeTranscript([ + { message: { model: 'claude-sonnet-5' }, cwd: world }, + { message: { model: 'claude-opus-5' } }, + ]); + fire({ + hook_event_name: 'SubagentStop', + agent_type: 'prose-walker', + agent_transcript_path: transcript, + }); + assert.equal(logLines()[0].split('\t')[2], 'claude-sonnet-5,claude-opus-5'); + }); + + it('says so rather than guessing when the model cannot be read', () => { + const transcript = writeTranscript([{ cwd: world }]); + fire({ + hook_event_name: 'SubagentStop', + agent_type: 'prose-walker', + agent_transcript_path: transcript, + }); + assert.equal(logLines()[0].split('\t')[2], 'model-unknown'); + }); + + it('survives a partial line, which a live transcript can end on', () => { + const file = path.join(world, 'partial.jsonl'); + fs.writeFileSync( + file, + `${JSON.stringify({ message: { model: 'claude-sonnet-5' }, cwd: world })}\n{ truncated`, + ); + fire({ + hook_event_name: 'SubagentStop', + agent_type: 'prose-walker', + agent_transcript_path: file, + }); + assert.equal(logLines()[0].split('\t')[2], 'claude-sonnet-5'); + }); + + it('stays silent when a stop names no readable transcript', () => { + // A real transcript lives outside the world, so an unreadable one + // leaves nothing anywhere in the payload to scope the record to. + fire({ + hook_event_name: 'SubagentStop', + agent_type: 'prose-walker', + agent_transcript_path: path.join(projectDir, 'absent.jsonl'), + }); + assert.deepEqual(logLines(), []); + }); +}); + +describe('prose recorder — the asserter contract', () => { + it('logs an asserter tool call as a violation, away from any world', () => { + fire({ + hook_event_name: 'PreToolUse', + tool_name: 'Read', + agent_type: 'prose-asserter', + tool_input: { file_path: `${world}/.workflows/manifest.json` }, + }); + const violations = path.join(projectDir, 'tests/prose/.agent-tool-use.log'); + assert.ok(fs.existsSync(violations), 'the breach is recorded'); + assert.ok(fs.readFileSync(violations, 'utf8').includes('prose-asserter')); + assert.deepEqual(logLines(), [], 'and never into the world it was judging'); + }); + + it('does not count an asserter finishing as a breach', () => { + fire({ + hook_event_name: 'SubagentStop', + agent_type: 'prose-asserter', + agent_transcript_path: writeTranscript([{ message: { model: 'claude-opus-5' } }]), + }); + const violations = path.join(projectDir, 'tests/prose/.agent-tool-use.log'); + assert.equal(fs.existsSync(violations), false); + }); +}); + +describe('prose recorder — it never breaks what it observes', () => { + it('exits quietly on a payload that is not JSON', () => { + execFileSync('node', [HOOK], { + input: 'not json at all', + env: { ...process.env, CLAUDE_PROJECT_DIR: projectDir }, + }); + }); + + it('exits quietly when the world named has already been destroyed', () => { + const gone = path.join(os.tmpdir(), 'prose-world-ZZZZZZ'); + fire({ + hook_event_name: 'PreToolUse', + tool_name: 'Bash', + agent_type: 'prose-walker', + tool_input: { command: `cd ${gone} && ls` }, + }); + }); +});