FE-813: Cook evaluator runs the verification tests instead of an LLM verdict

[kostandinang]

What

Makes brunch cook's per-slice done trustworthy. The evaluator was an LLM judging criterion prose with read,write,edit,bash — it could fix code mid-evaluation and rubber-stamp done (collapsing the write-tests → write-code → evaluate loop) and pass orphan output.

• evaluate-done now runs the verification targets (BunTestRunner) and gates done on real pass/fail — requires ≥1 target and every target passing. It no longer calls pi.
• Per-action tool scoping (toolsForAction): the evaluator can't mutate the sandbox; code-producing actions keep write tools.
• Dropped the now-dead evaluator.md prompt; hardened the writer prompts.

Limit

done now requires real test execution, not integration. Targets are agent-authored against the emitter's synthesized paths, so a weak/isolated target can still pass — guaranteeing a wired feature needs the emitter to emit integration-demanding targets (the FE-800 integration-blind-verification follow-on).

Verify

pi-actions.test.ts + test-runner.test.ts + npm run check pass. Two full-suite failures (build-boundary.test.ts, app.test.ts) are a fe-800-base failure and a parallel-run flake — not this diff (see #167).

🤖 Generated with Claude Code

[kostandinang]

• FE-819: Orchestrator <> Petrinaut integration improvements #176
• FE-815: Orchestrator demo fixtures: parallel, diamond, halt #174
• FE-813: Cook evaluator runs the verification tests instead of an LLM verdict #170 👈 (View in Graphite)
• FE-800: Spec to orchestrator plan emitter — generate plan.yaml from a completed specification #167 : 1 other dependent PR (#168 )
• FE-764: Brunch<> Petrinaut stream — ephemeral orchestrator hosted SSE server #165
• FE-784: Petrinaut export: colour-fold per-slice subnet #160
• FE-763: Petrinaut event stream — initial markings + transition firings #158
• FE-762: Petrinaut-format JSON export of the compiled net #157
• FE-761: Petri-net semantic alignment for Petrinaut visualization #156
• FE-755: Cook codebase mode for brownfield brunch #155
• FE-747: Declarative output routing for branching transitions #154
• FE-745: Epic-scoped sandbox merge for verify-epic #152
• FE-743: Petri parallel execution with resource pools and per-slice sandboxes #149
• FE-738: Petri semantic lanes with two-lane subnet and engine factory #148
• FE-730: Greenfield orchestrator POC with pi-agent dispatch #143
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[cursor]

PR Summary

Medium Risk
Changes core cook completion semantics and test execution paths used by every orchestrator run; mitigated by new unit/integration tests and unchanged engine contract fakes, but real bun subprocess behavior can still vary by environment.

Overview
Cook harness fidelity (FE-813): Per-slice done is no longer an LLM judgment. evaluate-done runs every slice verification target via bun test (evaluateVerificationTargets: ≥1 target, all must pass) and drops the evaluator.md / pi path entirely.

Tool scoping: toolsForAction limits evaluate-done to read-only pi tools so evaluation cannot mutate the sandbox; write-capable actions keep full tools.

Net + runner: run-tests handlers now execute all slice verification targets (not just the first). BunTestRunner uses spawnSync with argv (no shell) and merges stdout+stderr so failure diagnostics survive in reports.

Prompts & spec: test-writer / code-writer prompts stress tests as the oracle; D161-K and I126-K codify read-only, execution-derived completion. PLAN marks the brownfield TDD-collapse follow-on resolved.

Tests: New pi-actions.test.ts and test-runner.test.ts; client build-boundary timeout bumped to 120s for parallel vitest flake.

^{Reviewed by Cursor Bugbot for commit 162fc50. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 85753f7. Configure here.}

[lunelson]

This closes a real hole. An evaluator that held write tools and judged criterion prose could fix code mid-evaluation and rubber-stamp done, collapsing the whole test → code → evaluate loop; gating done on actual bun test pass/fail instead is a solid correctness win. Nice touches: evaluateVerificationTargets being pure with an injected runner, requiring at least one target, and the test that pins shell-metacharacter path safety.

A few things above the diff:

1. The gate is only as strong as the targets. Trust moves from "an LLM judges prose" to "agent-authored tests pass" — better, but still self-graded, since the same agent writes the code and its tests, and a weak or isolated target (a component tested in a vacuum) can still go green. You call this out as the integration-demanding-targets follow-on; is that tracked somewhere durable? Without it, "trustworthy done" reads as fully delivered when it's really the first of two halves.

2. Tool scoping fails open. toolsForAction hands back full write tools for everything except the hard-coded evaluate-done. For a boundary whose entire purpose is restriction, that's allow-by-default — a new action is write-capable unless someone remembers to lock it down. Would deny-by-default (explicit per-action grants) be a more durable shape here?

3. Self-reported non-green verify. The PR lands with two full-suite failures attributed to base + flake, and it also bumps the timeout in build-boundary.test.ts (the flaky one). CI is green, so this is just a confidence check: is build-boundary actually stable on this branch now, and is the app.test.ts / base failure tracked against #167 so it doesn't quietly become "expected red"?

Approving — none of these block.

The base branch was changed.