DEV Community

Andre Burgstahler
Andre Burgstahler

Posted on Originally published at Medium

Define Done as a Command

#ai #productivity #programming #codereview

Every plan should carry the exact command that proves the work is finished, a dependency allowlist that CI enforces, and a reviewer that sees only the plan and the diff.

Abstract illustration: several thin paths of nodes converge from the left into a narrow glowing amber gate, with a calm solid block beyond it and a small observer window watching from above
Many paths in. One gate out.

TL;DR

  • "It compiles" and "tests pass" aren't finish lines when the agent wrote the tests. Put a finish-line command in the plan before any code: one command that exits 0 only when the behaviour works.
  • New dependencies are decisions. Keep an allowlist file and a CI step that fails when the manifest gains a package that isn't on it.
  • The reviewer should see the plan and the diff, and nothing the builder said about its own work.

Two threads this week described the same failure from opposite ends.

In r/ClaudeCode, someone wrote up six months of their plan, code and review loop: a team of four, a TypeScript backend and a React app, around 600 PRs since April. The "what hurts" list is the one to read. Letting the agent pick packages: "we ended up with 3 date libraries." Treating "it compiles" as the finish line: "Our worst bug this year passed tsc, lint and the tests the agent wrote for it." And on review: "I started with Claude reviewing its own PR in the same session and it mostly agreed with itself."

In r/ChatGPTPro, a Codex user described their loop: a GPT chat for architecture that also writes the prompt for Codex, a new Codex session to implement, then another Codex session to review "adversarially." The goal is to take an architectural hypothesis "from paper to a working implementation and produce evidence to validate it."

Their actual question was whether Codex is worse than Claude for this. The first reply was "For this kind of workflow, i doubt you would notice much of a difference," and I agree. Both loops fail in the same places, and none of those places is the model. They fail where nobody defined what finished looks like.

Evidence is the right word. This article is about making "done" something that produces evidence, not something the agent declares.

Three ways "done" goes wrong

The agent grades its own homework. The bug that passed tsc, lint and the agent's own tests is the classic case. Another r/ClaudeCode post, about the bug that got past a Claude Code plan and its tests, shows why. The plan told yt-dlp to write files with a .part suffix, which yt-dlp treats as its own temporary suffix and strips. Every finished download looked too small and got marked failed. "Tests written from that wrong assumption passed fine. It only showed up when the code ran." The tests and the code came from the same mistaken plan, so they agreed with each other.

The agent picks the dependency. Three date libraries isn't a model failure. Nothing in the loop said which library to use, so each session picked whichever one came to mind, and nothing checked.

The reviewer shares the builder's context. A reviewer in the same session has read the plan, the reasoning, the dead ends and the builder's own summary of what it did. It's reviewing the story, not the code. A fresh session is better, and the Codex user is right to open one. But if the review prompt includes the builder's account of its work, it inherits the same framing.

The common thread: the check came from the same place as the work, so it had the same blind spots.

The rest of this article is three gates, one for each failure, that sit between "the agent says done" and done.

Pipeline diagram. PLAN.md, with goal, file-level steps, a finish-line command, anti-goals and no new dependencies, feeds a fresh build session. The build then has to pass three gates: Gate A, the finish-line command exits 0, enforced by a Stop hook or /goal; Gate B, the manifest only has packages on deps.allow, enforced in CI; Gate C, a reviewer that sees only the plan, the diff and CI output. Any failure goes back to the builder as its next instruction. When all pass, merge.
Three gates the agent can't talk past.

Finish-line commands

The fix for the first failure is boring and very effective. Before any code is written, the plan names one command that proves the work is done. Not "add tests." A specific command, with its expected result, that exits non-zero until the behaviour actually works.

A finish line the agent can't declare, only pass.

Anthropic's best-practices page for Claude Code opens with this advice: "Give Claude a check it can run." It also says to have Claude "show evidence rather than asserting success: the test output, the command it ran and what it returned." OpenAI's Codex prompting guide lists the same ingredient: a useful prompt "says how to verify the change," and its example asks Codex to run checks and "Report the commands and results."

What makes a good finish line:

Task Weak finish line Finish-line command
Session expiry "tests pass" npm run e2e -- session-expiry (3 named cases, written before the code)
Download feature "downloads work" scripts/smoke-download.sh runs the real downloader on a fixture and checks file size
Bug fix "fixed" the reproduction from the issue, as a script that exits 1 today
Refactor "no behaviour change" full suite plus git diff --stat limited to the listed files

Three rules I use:

  1. Write it before the code, in the plan. If the agent writes the check after the code, it tests what it built rather than what you asked for.
  2. Exercise the behaviour from outside. A real request, a real file, the real CLI. The yt-dlp bug would have failed a smoke test that downloaded one song and checked the file on disk.
  3. Make it cheap enough to run every time. If it takes ten minutes, the agent will find reasons not to run it.

Then make running it non-optional. An instruction alone isn't enough. In a r/codex thread about AGENTS.md rules being ignored, one commenter said their instruction to always run the local pipeline before reporting a change as done was honoured "only 7 out of 10 times."

Claude Code's docs describe an escalation that fits well. Ask in the prompt. Set the check as a /goal condition, where a separate evaluator checks after every turn whether it holds. Or use a Stop hook, a script that "blocks the turn from ending until it passes." One caveat about /goal is in its own docs: the evaluator "doesn't run commands or read files independently," so the condition has to be something the agent's output demonstrates, like a test run in the transcript. Codex has a /goal command too.

The hook version works in both tools. Claude Code and Codex both support a Stop event where exit code 2 sends the agent back to work with your stderr as the reason:

#!/usr/bin/env bash
# .claude/hooks/finish-line.sh (also usable from .codex/hooks.json)
ok() { echo '{}'; exit 0; }       # Codex wants JSON on exit 0
input=$(cat)
[ "$(echo "$input" | jq -r '.stop_hook_active')" = "true" ] && ok
git diff --quiet HEAD && ok        # nothing changed, nothing to prove
cmd=$(sed -n 's/^Finish line: `\(.*\)`.*/\1/p' PLAN.md | head -1)
[ -z "$cmd" ] && { echo "PLAN.md has no 'Finish line:' command." >&2; exit 2; }
if ! out=$(bash -c "$cmd" 2>&1); then
  echo "Finish line failed: $cmd" >&2
  echo "$out" | tail -20 >&2
  exit 2
fi
ok
Enter fullscreen mode Exit fullscreen mode

The stop_hook_active check stops it from looping forever, and Claude Code also caps consecutive continuations at eight. In Codex, review and trust the hook in /hooks before it runs.

A dependency allowlist, enforced in CI

The second failure needs a different gate, because "don't add dependencies" is exactly the kind of rule that holds most of the time.

The commenter who answered the date-library post suggested "a short allowed dependencies list" plus "a CI check that fails when package.json gains something not on it." That's what I do, and it takes about fifteen lines:

# deps.allow  (one package per line; a change here needs a human)
react
react-dom
date-fns
zod
Enter fullscreen mode Exit fullscreen mode 
# .github/workflows/deps.yml
name: deps-allowlist
on: pull_request
jobs:
  check:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
      - run: |
          jq -r '(.dependencies // {}) + (.devDependencies // {}) | keys[]' \
            package.json | sort > /tmp/used
          grep -v '^#' deps.allow | sort > /tmp/allowed
          extra=$(comm -23 /tmp/used /tmp/allowed)
          [ -z "$extra" ] || { echo "Not on deps.allow: $extra"; exit 1; }
Enter fullscreen mode Exit fullscreen mode

Flow diagram. A pull request changes package.json. The CI step compares dependency names against deps.allow. If every package is listed, the check passes. If not, the check fails with the package name, and the only way forward is a one-line change to deps.allow that a human approves, ideally with an ADR.
A new dependency stops being a side effect and becomes a decision.

Put one line in AGENTS.md so the agent knows the rule exists before CI tells it: "Never add a package that isn't in deps.allow. If you need one, stop and propose it in the plan." The CI check is what makes it true. An allowlist change is a tiny diff a human can review in seconds, and if the choice matters it deserves an ADR. For the three-date-libraries case, you can go one step further with ESLint's built-in no-restricted-imports rule and ban the libraries you've decided against.

Reviewer isolation

The third gate is the reviewer, and the important decision is what it doesn't see.

Two panels. The reviewer sees: the plan file (goal, finish-line command, anti-goals, allowed dependencies), the diff against the base branch, and the finish-line output from CI. The reviewer does not see: the builder's conversation, its reasoning and dead ends, its summary of what it did, or the planner chat's prose about intent.
The reviewer judges the result against the plan, not the builder's account of it.

The Claude Code best-practices page describes the benefit: "A reviewer running in a fresh subagent context sees only the diff and the criteria you give it, not the reasoning that produced the change, so it evaluates the result on its own terms." The same page adds a warning I agree with: a reviewer asked to find gaps will usually report some, so tell it to flag only gaps that affect correctness or the stated requirements.

In Claude Code, a subagent runs in its own context window, and its tools field limits what it can do:

---
name: plan-reviewer
description: Reviews a branch against PLAN.md. Use before merging.
tools: Read, Grep, Glob, Bash
---
Read PLAN.md. Run `git diff main...HEAD`. Do not read HANDOFF.md
or any session notes. Report only:
1. Plan steps or anti-goals the diff violates, with file:line.
2. Whether the finish-line command exists, ran, and passed.
3. Packages added outside deps.allow.
Ignore style. If nothing applies, say so in one line.
Enter fullscreen mode Exit fullscreen mode

In Codex, codex review --base main runs a review non-interactively in a separate process (CLI reference), and /review does it inside a session. The review_model setting in config.toml picks a different model for reviews if you want one. For the GPT-plans, Codex-builds, Codex-reviews loop, the change is small: give the review session the plan file and the diff, and keep the builder's summary and the planner's prompt out of it.

One caution from experience. Per-task reviews like this don't replace one review of the whole branch before merge. In an earlier r/ClaudeCode thread, a commenter found that a final whole-branch review still caught issues the per-task reviews had missed, all of them interactions between tasks.

The plan template

All three gates fit in one file. This is what I ask the planning session to produce, and what the builder and reviewer both read:

# PLAN: <ticket>
## Goal
What and why, in two lines.

## Done when
Finish line: `npm run e2e -- session-expiry`
Expected: 3 passed (expired, refreshed, revoked). Exits 0.
Written before code: yes

## Steps (file level)
- src/auth/session.ts: add expiresAt check
- update the 3 callers: <paths>

## Anti-goals (failure even if the code works)
- A refreshed session ever outlives the refresh token

## Dependencies
New: none. (If needed: name, why, and stop for approval.)

## Review
Reviewer gets: this file, `git diff main...HEAD`, CI output.
Enter fullscreen mode Exit fullscreen mode

The "file level" steps come straight from the r/ClaudeCode post, whose author says "'Change X in the auth service' is useless" while "add an expiresAt check in src/auth/session.ts and update the 3 callers" is a plan. The anti-goals section comes from the yt-dlp post, whose author wrote down "things that count as failure even if the code works." Both are things a finish-line command can test.

Start with one gate

If you only add one of these this week, add the finish line. It costs one line in the plan and changes what the agent aims at. The allowlist is next: fifteen lines of CI that you write once and never think about again. The isolated reviewer comes third, because it pays off most once the first two exist. A reviewer that can check "did the finish-line command run and pass?" has a much easier job than one asked to judge whether the code is good.

What changes

The r/ClaudeCode author ended with a hot take: "The agent isn't the bottleneck anymore, the guardrails are." I'd put it more narrowly. The bottleneck is the word "done." As long as the agent gets to say it, you have to check every claim yourself.

Write the command first. Let CI hold the dependency list. Give the reviewer the plan and the diff and nothing else. Then "done" means something you can check without trusting anyone, including the agent.

Sources

If this was useful, subscribe to Agents in Practice for one practical note on AI coding agents a week. Helping your team adopt Claude Code or Codex safely and without the surprise bill? I consult on exactly that: andre-burgstahler-9n5.pages.dev.

This article was written with AI assistance and reviewed by the author.

Top comments (1)

Collapse
 
lin7c profile image
xcl •

The dependency allowlist gate raises a lockfile question: the CI step described compares names in the manifest, but a change that only touches the lockfile can swap a transitive dependency, or move a resolved version, without any new name appearing in package.json. Does the gate also need to diff resolved versions in the lockfile, or is the argument that direct names are the decision point and transitive changes belong to a separate audit step? Keeping those separate feels important, because conflating them would make the allowlist fail on routine security patches as well.