A subagent pipeline that reviews its own work

I built a Go project this week — agentop, a top-style terminal dashboard for Claude Code sessions — but I didn't write most of it myself. I ran a pipeline: one agent implements a task, a second agent reviews the diff before the next task begins, and I sit in the middle as the controller, wiring context in and out.

The interesting part isn't that it worked. It's what the reviewer caught — bugs that would have shipped if the implementer's own "all tests pass" had been the final word. This post is about those bugs, and the structure that surfaced them.

The shape of the pipeline

The project was planned up front as 16 bite-sized, mostly-independent tasks, each with a written spec, a test-first contract, and explicit interfaces (types and signatures) that later tasks depend on. Then, per task:

1. Extract the task brief to a file. The implementer subagent gets its one task — not the whole plan, not the session history. Just its requirements, the interfaces it touches, and the global constraints.
2. Implement, test, commit. The implementer follows TDD: failing test first, then code, then a green run, then a conventional commit. It writes a report to a file and returns only a status line.
3. Package the diff and hand it to a reviewer. A different agent — a more capable model than the implementer — reads the brief, the report, and the exact diff. It renders two verdicts: spec compliance (did it build what was asked, nothing more) and code quality.
4. Fix loop. Any Critical/Important finding goes back to the implementer as a fix, test-first, re-reviewed. Only then does the task close.

Everything moves as files, not pasted text — briefs, reports, diffs. The controller's context stays lean enough to coordinate all 16 tasks without drowning in the code it never needs to read. Progress is tracked in a ledger file, not just conversation memory, so a compaction or a crash never re-runs a finished task.

Two deliberate choices made this cheap:

• Model routing. Mechanical, fully-specified tasks went to a fast, cheap model. Reviews — where judgment matters — went to the most capable one. Turn count, not token price, is what actually costs you; a clear spec turns implementation into transcription.
• Fresh context per task. No task inherits another's history. The controller constructs exactly what each subagent needs. This is the whole point of subagents: you curate their inputs precisely so they stay focused and succeed.

The bugs the reviewer caught

Every one of these passed the implementer's own test suite. Every one was caught by the adversarial review pass, before the next task built on top of it.

A data race hiding behind a passing test. The transcript watcher populated a map of file tailers in its constructor, then read and wrote that map from a goroutine. Single-call, it's fine — and the test called it once, so it was green. But nothing stopped a second Watch() on the same instance, which would have two goroutines racing the same map. The reviewer flagged it as a reasoning-level race the test could never surface (the race detector doesn't run on this Windows/386 toolchain). Fix: make Watch() single-use with an atomic compare-and-swap, plus a test for the second-call error.

A path check that trusted string prefixes. A risk detector flags writes outside the session's working directory. It compared paths with a plain prefix match — so a write to /w/demo2/evil.go counted as "inside" /w/demo, because the string starts the same way. Sibling directories that share a prefix slipped straight through. Fix: require the target to equal the base or start with base + "/", with a regression test for exactly that sibling case.

A budget alert that fired once and never again. The daily-spend detector had a dayAlerted flag it set on the first breach — and never reset. A dashboard left running overnight would go silent after day one. The per-task test only checked a single day, so it was green. Fix: reset the flag on calendar-day rollover, with a test that asserts day 1 alerts, the same day stays quiet, and the next day re-arms.

Code that edits your live editor config — and quietly broke it. The hook-install command merges an entry into Claude Code's settings.json. Two findings here, both on the highest-blast-radius file in the project:

• The "is this our hook?" check matched any command containing the substring agentop. A foreign hook whose command merely mentioned the word — mytool --ignore agentop — would be classified as ours and silently deleted on uninstall. Fix: match precisely (the command must end in hook and its executable token must actually be the binary), plus a test that a foreign mention survives.
• Install ignored the error from writing its backup file, and uninstall took no backup at all — a destructive edit to a live config with no recovery copy. Fix: a backupOnce helper that propagates errors and refuses to mutate if the backup failed.

A kill command that could kill the wrong thing. The dashboard can terminate a runaway agent's process. You press K, the target is chosen, you confirm with y. But between K and y, a new transcript event could arrive and reorder the session list (it's sorted by most-recent activity). The pid was pinned at K-time; the session to mark as ended was re-derived from the cursor at y-time. So the tool could terminate session A's process while marking session B as dead, and show B's name next to A's pid. The implementer's report had literally claimed "no TOCTOU gap." There was one. Fix: pin the target session ID and project at K-time and use them everywhere; a regression test arms the kill, fires a reordering event, confirms, and asserts the originally-targeted session is the one ended.

The headline feature, silently dead on Windows. The final whole-branch review found the real blocker. The installed hook command was built as exePath + " hook" — unquoted. On a machine where the binary lives in C:\Program Files\..., the shell parses C:\Program as the executable and the hook never spawns. The product's entire "pause a runaway agent" capability would fail to fire, with no error surfaced, for every Windows user who installed to a path with a space. go install and /usr/local/bin are space-free, so every per-task test missed it. Fix: quote the path when it contains whitespace.

Why the review pass is non-negotiable

Notice the pattern: every one of these was green. The implementer wrote the test, the test passed, the implementer self-reviewed and reported done. If "tests pass" were the bar, all of it ships.

The reviewer's edge isn't that it's smarter in the abstract. It's that it has a different job. The implementer is trying to make the task pass; it's motivated to stop when the bar it set is met. The reviewer is trying to break the result, with fresh eyes, the spec in hand, and no attachment to the code. Splitting those two motivations across two agents is the whole trick. A single agent grading its own homework rationalizes; two agents with opposing incentives don't.

The final whole-branch pass matters for a different reason: cross-cutting properties no single task owns. Is the fail-open guarantee actually wired end to end? Does the "honest numbers" rule (unknown pricing renders ~?, never a guess) survive from the parser all the way to the status bar? Is there a real dev machine's home path sitting in a committed test fixture, about to go public? Per-task reviews can't see any of that. The whole-branch review re-verified the scary things — the concurrency contract, the process-kill blast radius — and cleared the release after exactly one fix.

What I'd tell you before you try it

• Write the plan so tasks are transcription, not invention. The cheaper your implementer, the more the spec has to carry. Exact values, exact signatures, test cases — in the brief, verbatim.
• Don't let the reviewer grade the tests it was handed. Give it the diff and the spec; let it hunt. Never tell it what not to flag — if you're pre-judging a finding as a false positive, you've already lost the second pair of eyes.
• Move artifacts as files. Briefs, reports, diffs. Anything you paste into a prompt lives in your context forever and gets re-read every turn.
• Keep a ledger on disk. Conversation memory doesn't survive compaction; a completed task re-run from scratch is the most expensive failure there is.
• Adjudicate plan-vs-review conflicts yourself. When a reviewer flags something the plan explicitly accepted as a scope cut, that's your call, not the fix agent's. I made those calls in the middle — and wrote them down.

agentop itself is a small tool: it tails Claude Code's local transcripts, shows each session's spend, tool calls, and risk, and can pause or kill a runaway one — all offline. But the way it got built is the part I'll reuse. The next time someone tells you an agent "finished and all tests pass," ask what a second agent, trying to break it, would say.