Agent self-review: 6 failure modes from adding a parallel agent path to a benchmark framework (with corrected rules)
Context
This is a self-authored post-mortem from a multi-session implementation of a parallel --agent claude capability inside an existing benchmark test framework (which already worked for --agent codex). The work eventually landed cleanly, but only after many rounds of correction by the user that should not have been needed.
The full lessons file was written by the agent (Claude Opus 4.7) at the end of the work, at the user's request, specifically so that future agents — and Anthropic's own training / documentation / tooling teams — could absorb the patterns. The user asked that it be shared upstream as feedback.
It's not a bug report. It's behavioural evidence about ways an agent can fail in a well-documented, well-instructed engineering context, with both the in-repo design docs and the user's spoken instructions giving the right answer from the start.
Headline pattern
The repeatable failure mode across this session: designing solutions before grokking the architecture, then narrating fluently enough that the proposal sounded validated. The project had:
- a startup doc (
AGENTS.md) explicitly enumerating the three-layer contract (monitor / manager / worker), with a long "Screen Component Boundaries" section forbidding exactly the layer violations that were then committed - a historical lessons file (
LESSONS.md) with prior incidents anticipating the same failure modes - a fully-worked baseline (the codex agent path) as the structural reference for any new code in the claude agent path
- a user who corrected violations directly and repeatedly (one correction had to be made three times)
None of those were enough on their own. All of them together still weren't, until the agent finally stopped proposing and read.
The full document
The remainder of this issue is the verbatim file
(CLAUDE_ADDITION_LESSONS.md) committed to the project repository. It
covers six failure modes (read-the-docs-first being the causally-prior one),
the meta-pattern, seven corrected rules, and a counterfactual of what a
clean implementation would have looked like.
---
Lessons from adding the claude agent path (a critical self-review)
Date: 2026-06-02
Author: Claude Opus 4.7 (the agent who shipped most of what's reviewed below)
Audience: future agents working on this codebase; the user, who already
paid the cost of these mistakes
This document is a deliberately critical post-mortem of the work to add
a parallel --agent claude capability to the test framework that
already worked for --agent codex. The work itself eventually landed
and is functional; this is about how it landed — over many sessions
of avoidable rework, with the user repeatedly correcting both the
architecture and the diagnosis, and the agent (me) repeatedly defaulting
to manager-layer expediency instead of the monitor-layer alignment the
user had explicitly mandated.
The point is not penance. The point is that this work could have
landed cleanly in a fraction of the chat budget, and the patterns that
prevented that are repeatable and recognisable. Write them down.
1. The mandate (what was asked, and how clearly)
The brief for this work was complete from the start, and complete in
two reinforcing ways: the user's spoken instructions and the project's
own committed documentation. Either alone would have been enough to
keep me on the right path. Both together made the route I actually
took inexcusable.
1.a. The user's spoken instructions
- Goal. Add a parallel
claudeagent capability to the existing
benchmark test framework, in the same way codex was already tested.
Not a re-architecture. A parallel path that the worker can dispatch
to with --agent claude instead of --agent codex.
- Mirror. The user explicitly named the codex files
(codex_exec_screen.py, codex_screen_monitor.py,
benchmark-codex-llm.py) as the structural reference. New claude
files (claude_exec_screen.py, claude_screen_monitor.py) were to
mirror those, with differences only where claude's actual signals
differed from codex's (claude's TUI uses OSC titles and transcript
JSONL, codex's exec uses rollout JSONL and event_msg payloads).
- Validation baseline. Codex results existed for every benchmark
prompt across the same model set. A claude result that diverged
wildly from the codex result on the same model was, by construction,
evidence requiring investigation. The reference dataset was already
on disk.
- Concrete corrections in flight. When the user observed something
wrong, they said so directly: "the GPU fan is not running which
infers the busyness detector is not working", "the claud_screen
_monitor should realise it's dead and kill it before the time is up",
"I think it's a false negative", "you should have picked up that the
claude tests were taking 4x as long as codex", "the codex screen
wrapper identifies and cleans up the test environment. this seems to
be a gap in your claude harness."
None of those messages are ambiguous.
1.b. The in-repo documentation (the other half of the brief)
The user shouldn't have needed to say any of this verbally — the
project documents the same architecture in the files an agent is
expected to load on startup. The agent's job is to read them.
AGENTS.mdis the canonical startup document, and it points
to itself at the top: *"This is the single startup document for
Codex, Claude, and similar coding agents working in this
repository."* It explicitly enumerates the three layers, what each
owns, and what each is forbidden from doing. The "Screen Component
Boundaries" section is the longest in the file precisely because
the project's author already knew this was the hardest rule to
get right. Verbatim from that section:
> "Each lower layer is authoritative for its own concerns; each
> higher layer consumes the lower layer's outputs and does not
> re-derive them."
> "No layer re-derives the layer below. The manager doesn't
> re-derive session state from screen bytes, paste timing, or
> wall-clock cushions."
> "No hardwired sleeps or timing cushions in the higher layer.
> Hardwired waits are a design failure even when they make a
> specific test pass. Timing belongs only inside the layer that
> owns the resource being timed."
These are not subtle. They are the rules I broke when I added
CLAUDE_SUBMISSION_ACCEPT_TIMEOUT_SECS and CLAUDE_TURN_STALL_SECS
to the manager. The doc forbade exactly what I did, in the file
I'm meant to read first.
LESSONS.mdis the project's record of historical failures
and the rules that came out of them. Several existing entries
directly anticipate the failure modes I went on to repeat —
including the screen-component-boundaries forensics from earlier
p7/p8 incidents and the rule that recovery for non-Dead
exceptions must not destroy conversation context. If I had read
these as part of onboarding rather than skimming them when the
user pointed at one, I would have been primed to recognise my
own mistakes as instances of known anti-patterns.
README.mdcarries the stable methodology. It pre-defines
the three-tier classification (model_failure / framework_broken /
passed) and the contract for how results are written. Several
diagnostic confusions I had — about whether a chars=0 result was
a model failure or a harness failure — would have shortened
considerably if I had loaded this doc's vocabulary first.
CLAUDE.mdpoints at AGENTS.md. Read AGENTS.md first.
The pattern of failure is the same here as everywhere else: the
authoritative source was on disk, the user had pointed at it, and I
proposed something invented instead of reading what was already
written down. The user has had to repeat verbally what the docs
already say. That is wasted budget on both sides.
2. What went wrong — by failure mode
2.1 Treating the in-repo documentation as background instead of source
Every architectural rule I broke in this session is written down,
verbatim, in AGENTS.md. The "Screen Component Boundaries" section
is the longest section in that file and ends with a list of
"Concrete examples of this anti-pattern that should not return" — one
of which is exactly the manager-applies-a-hardwired-cushion mistake
I went on to make twice (Fix C, Fix E). The doc names the
anti-pattern and names the corrective ("Fix the monitor"). I had
no excuse for committing it.
The failure here isn't the layer violation itself (that's covered
below in 2.2) — it's the prior failure of not reading the project's
own rule set before proposing changes that touched those layers. The
user shouldn't have had to type "no timing in the claude_send_prompt"
or "the state belongs to the monitor" — both of those are direct
restatements of rules in AGENTS.md that I had access to since the
first turn. The same is true for the LESSONS.md prior incidents that
described related forensics: I should have absorbed them as part of
onboarding, not read them only when a new failure forced me to.
This sits at 2.1 because it causally precedes every other failure in
this list. Read the docs first, then propose. Not the other way
round.
2.2 Layer-boundary violations in the manager
The architectural rule restated: state derivation belongs to the
monitor; the manager consumes monitor state and takes actions; the
test loop consumes manager results and scores them. Each higher
layer is forbidden from re-deriving the lower layer's owned data.
I violated this twice in the manager (claude_exec_screen.py,claude_send_prompt):
- Fix C added
CLAUDE_SUBMISSION_ACCEPT_TIMEOUT_SECS= 60 s as
the budget for "wait for the monitor to observe a new
current_turn_id after paste+Enter". The codex equivalent
(codex_exec_screen.submit_screen_prompt._submission_accepted) uses
the caller's per-prompt deadline, not an invented sub-budget. I
added a sub-budget because I wasn't reading the codex reference for
alignment — I was making the change in a vacuum and 60 s "seemed
right".
- Fix E added
CLAUDE_TURN_STALL_SECS= 180 s and a wall-clock
tracker last_progress_wall to declare a turn "stalled" if the
JSONL had been silent for that long. Codex has no equivalent at
all. The whole concept of "stall" was something I invented in the
manager, despite the user's earlier message making clear it
belonged in the monitor ("the claud_screen_monitor should realise
it's dead"). The user had to say it again in two more messages
before I removed it.
Both fixes were in the wrong layer and invented timing the manager
had no right to own. Both eventually had to be ripped out.
2.3 Symptom-driven patching instead of root-cause diagnosis
When the user pointed at a symptom, my default reaction was to add
compensating code rather than ask "where in the existing architecture
should this be detected?"
- User: "the model finished but the monitor missed it." The right
reaction was to ask whether the monitor's existing busy→ready
transition (which already exists) was firing or not, and if not,
why. My reaction was to invent a manager-side stall detector that
would fire when the monitor should have fired but didn't. That's
a workaround, not a fix. It also generated false positives because
the monitor was right and the manager's invented detector was
wrong.
- User: "the codex screen wrapper identifies and cleans up the test
environment. this seems to be a gap in your claude harness." Here I
did do it right: I looked at the codex pattern
(cleanup_benchmark_sessions calls terminate_pid_tree plus a
pattern-based orphan sweep), found the gap (argv patterns only
match codex CLI binaries, not arbitrary Bash grandchildren), and
added the missing cwd-based sweep. That fix landed cleanly. The
contrast is informative: when I followed the codex pattern, the
result was small, clean, and right. When I invented, the result was
big, messy, and wrong.
2.4 Confidently-wrong analysis pushed back at user observations
Several times the user reported something that didn't look right, and
my first response was to dismiss it as normal. Each time, when I
actually checked the data, the user was right.
- p13 case (qwen3.5-35b): I read the last three events as
tool_use
types and concluded "model still running at deadline". The user
corrected me: "I think it's a false negative, the monitor should
have changed its state from busy to done but that didn't happen".
When I added timestamps to my analysis (not just types), I
discovered the model HAD gone silent at t+130 s for the
gemma4-e4b p11 case, with 471 s of dead air before the deadline
fired. The user's hypothesis was correct. Mine was the easier
read, not the right one.
- 4× slowdown blind spot. After the false-positive bug was found,
the user said: "To be fair, you should have picked up that the
claude tests were taking 4x as long as codex and surfaced that
there was a bug." That signal — same model, same prompts, 4×
wall-clock difference — was sitting in the data the whole time.
I hadn't compared because the comparison hadn't occurred to me as
a standing sanity check.
The common thread: I optimised for a confident-sounding response over
verifying the claim. The user's observations were the *evidence I
should have weighted most heavily*, not noise to be filtered.
2.5 Repeated refusal to actually do the alignment check
The single most uncomfortable line in this session was the user
saying:
"compare with how the codex monitor works, there should be alignment between the two monitors. this is the third time I've asked for this check to be validated"
That was the third explicit request. The first two were "claude and
codex paths should be as similar as possible" and "when fixing this,
ALWAYS make changes to the claude path" (which I read as a scoping
note, not as the alignment principle it actually was). On the third
ask I finally read codex_screen_monitor.py and discovered the codex
monitor has zero stall detection — meaning the alignment principle
required me to remove what I'd added, not to design a better
version of it.
Every iteration of "let me propose something for the monitor instead"
that I sent before the user's third correction was wasted dialogue.
Each one was me re-deriving an answer from first principles instead
of reading the canonical reference the user had pointed at since the
start.
2.6 Worker boundary slip (the false-positive cascade)
When Fix E (manager-side stall detection) finally fired in
production, it cut a productive p9b turn off after 180 s of model
inference that was genuinely happening (just emitting nothing visible
to JSONL during a single long reasoning call). The manager declared
"stalled" and returned. The next prompt, p9c, then immediately
failed with ScreenSessionUnresponsive because claude was still
processing the abandoned p9b turn when p9c's paste tried to land —
the session was busy with work the manager had wrongly given up on.
So one wrong manager-layer decision (Fix E) cascaded into a worker-
visible failure (p9c marked unknown) and corrupted the score on a
prompt the model would have completed correctly. The user diagnosed
this immediately from the logs: "they infer that the claude
framework is faulty and not testing correctly". I had to walk back
both Fix C and Fix E.
The lesson on top of the lesson: a layer-boundary violation in one
place doesn't stay contained. It propagates as score corruption in
the layer above.
3. The meta-pattern that made all of this likely
Every individual failure above shares a single underlying habit:
designing solutions before fully grokking the architecture, then
narrating the design confidently enough that it sounded validated.
The fixes I shipped in this session that did work share a different
shape:
session_offsetbookkeeping: traced an actual symptom (stale
prior-turn text being returned for the current turn) backwards to
an actual root cause (an integer that was initialised to 0 and
never updated), and fixed it where the cause was. Small, clean,
obviously correct once seen.
- Turn-aware
_last_terminal_assistant_textfallback: read the
function, saw it walked the whole events list, saw that the events
list had been polluted by the bug above, added a turn-boundary so
it'd be defended-in-depth even after the polluting bug was fixed.
- Orphan-grandchild sweep: read the codex
cleanup_benchmark_sessions
end-to-end, identified the specific pattern coverage gap (argv
matches codex CLI only), added a new wrapper in the claude module
that extended the cleanup along the dimension codex missed (cwd
resolution into a benchmark workspace). Mirror, then extend.
AGENTIC_MODEL_SETTINGS.mdQwen3-Coder section +LESSONS.md
agent-overreach rule: only landed AFTER the user forced the web-
search verification. Once I was working from real evidence, the
edits were narrow and right.
The failures share a different shape:
- Fix C and Fix E: imagined a solution shape, wrote code, wrote
confident docstrings, didn't open codex_exec_screen.py or
codex_screen_monitor.py to check whether the same thing was done
elsewhere or whether it lived in a different layer.
- Per-event-type dismissal of the user's "false negative" theory:
read three event types, didn't read three timestamps, gave the
confident answer the data didn't actually support.
The common cause is the same in every case: shortcutting verification
in favour of a fluent answer. The corrective is mechanical: before
shipping a non-trivial fix, the agent has to be able to point at the
reference file in the codebase or the vendor source on the web that
licenses the fix. If it can't, it's invented, and inventions belong
on a draft, not a commit.
4. The corrected rules going forward
These are written so that the next agent reading the codebase can
absorb them without re-deriving them from this story.
- Read the in-repo docs before proposing.
AGENTS.md,
LESSONS.md, README.md, and STATUS.md are the project's own
self-description and are equal-weight authority alongside any
verbal user instruction. The architecture, the layer boundaries,
the recovery contract, the safety model, the historical failures,
and the rules that came out of them are all written down. Load
them. Cite them. If a proposed change contradicts something in
those files, the proposal is wrong by default — challenge your
reading of the change, not the doc.
- Codex is the structural reference for any claude change. Before
changing anything in claude_exec_screen.py, claude_screen_, or any worker dispatch involving
monitor.py--agent claude,
read the codex equivalent first. If the codex equivalent doesn't
exist, the default position is that the claude equivalent should
not exist either. Any exception is justified in the commit
message with the concrete claude-only signal that warrants it
(e.g. claude's TUI can go silent after a tool_result; codex's
exec stream cannot). "Seems prudent" is not a justification.
- **The monitor owns state. The manager consumes monitor state.
The worker consumes manager results.** Re-read
AGENTS.md § "Screen Component Boundaries" before touching any of
the three. If the change you're about to make is "detect whether
the session is alive in a new way", it belongs in the monitor,
not the manager. If the change is "decide whether the prompt
succeeded", it belongs in the manager, not the worker. No layer
re-derives the layer below. No layer invents timing the lower
layer should be reporting on.
- The manager has no timing of its own. Only the caller-supplied
per-prompt deadline. No *_TIMEOUT_SECS constants. No
*_STALL_SECS. No wall-clock trackers in claude_send_prompt.
If a new "is the session healthy" classification is needed, it's
poll-count debounced in the monitor (like
LATCH_READY_CONFIDENCE_POLLS), not time-thresholded in the
manager.
- Validate against the codex baseline early and continuously.
For every model that's been tested with both agents, a per-prompt
timing/score comparison is a one-liner. If claude's secs are >1.5×
codex's on the same prompt, that's a data point requiring
investigation, not noise. The first time the user has to point
out an aggregate signal is one time too many.
- **User observations are high-weight evidence, not low-weight
noise.** When the user says "this looks wrong", the default
posture is "they're right; let me look harder", not "let me
explain why it's actually fine". The historical record of this
session shows the user was right essentially every time. Recheck
the data before responding; don't pre-commit to the dismissive
read.
- When the user has to repeat themselves, stop and read. If
the user is asking the same question or making the same
correction more than once, the failure is in the agent's prior
responses, not the user's wording. Step out of the proposal
loop, read the reference the user keeps pointing at, then come
back. The "third time I've asked for this check to be validated"
moment in this session is the canonical example: every iteration
between requests one and three was wasted chat budget.
5. What this session would have looked like if these rules had been in force
Roughly:
- Day 1: read
codex_screen_monitor.pyandcodex_exec_screen.py
end-to-end. Identify the monitor's role
(monitor_status, note_busy, latch), the manager's role
(submit_screen_prompt, wait_for_codex_turn_completion,
recovery dispatch), and the contract between them. Write the
claude versions as adapter-shaped mirrors, with the only new code
being the two adapter functions (OSC title detector, JSONL
transcript reader) and the agent-specific launch/teardown.
- Day 2: smoke-test against a small model on a small prompt set.
Compare per-prompt timings against the codex baseline for the
same model. Any divergence >50 % goes onto a list to be
investigated before continuing.
- Day 3: investigate any items on that list before proposing new
code. Most will resolve to either "claude TUI is genuinely
slower" (acceptable) or "stale events polluting the metrics list"
(a real bug — fix session_offset and the turn-aware fallback).
- Never: invent
*_TIMEOUT_SECSconstants in the manager. - Never: add stall detection in any layer codex doesn't have
it in, unless an in-the-wild claude-specific failure mode demands
it AND the detection is built in the monitor with the same poll-
count debounce style the codex monitor already uses for its own
transitions.
The cost of doing it that way would have been a small fraction of
the chat budget actually consumed.
6. Closing
This document exists so the same patterns are easier to catch in
review next time. The session ended with code that is, conceptually,
in the right shape: the monitor owns state, the manager respects the
caller's deadline as the only timeout, the worker consumes manager
results, and the codex and claude paths are structurally parallel.
That's the end-state the user asked for at the start. Getting there
took many more rounds than it should have. The rules above are how
to make sure the next "add a parallel something" task gets there in
one.
This issue has 2 comments on GitHub. Read the full discussion on GitHub ↗