fix(code-review): stop the inline pass reading as the session's final output - #1127
Conversation
… output The Output section is phrased for a subagent invocation — "return the findings to the caller". Run inline, which is what happens when the session spawns no subagent, there is no separate caller: the session is the caller, so "return to the caller" collapses into "emit as my message", and a session that has just produced its deliverable ends the turn. The review is never posted and the findings survive only in the session log. Name the inline case in the output contract: the findings are handed to the caller's next step, not to the reader. Add the reciprocal at the call site so step 5 names step 6 as what follows.
tend-agent
left a comment
There was a problem hiding this comment.
The diagnosis holds and the shape is right — the failure is upstream of #1080's step 6 gate, so removing the terminal-reading instruction is the fix rather than another gate, and the reciprocal at the one call site (review is the only invoker) closes it from both ends.
One thing the new paragraph doesn't account for: it keys on inline vs. subagent, but the failure it describes only exists when there is a caller skill to continue into. code-review is also invocable directly — its own frontmatter says "or when asked for a thorough diff review", and codex/agents-tail.md lists it among the skills invocable via $<skill-name> mentions. A session invoked that way runs inline with no caller, and the new bolded lead-in tells it its findings "are not the answer" while the file's opening rule (**Return findings; don't act on them.** No review, comment, commit, or artifact from this skill) forbids the only other terminal action it has. That leaves the direct-invocation mode with nothing it is permitted to do — it either stops anyway, or resolves the contradiction by posting, which the opening rule prohibits.
The body sentence already carries the condition ("in the caller's own session"); it's the skimmable lead that's unqualified. Suggestion inline.
…ness The bolded lead-in read as unconditional for any inline run, but a directly-invoked `code-review` also runs inline with no caller to continue into. Combined with the file's opening "return findings; don't act on them" rule, that left the direct-invocation mode with no permitted way to finish. Key the condition on having a caller and name what the direct mode does.
code-review's Output section is written for a subagent invocation — "return the findings to the caller". Run inline, which is what happens whenever the review session doesn't spawn a subagent, there is no separate caller: the session is the caller, so "return to the caller" collapses into "emit as my message", and a session that has just produced what reads as its deliverable ends the turn. The review never posts and the findings survive only in the session log — the failure reported on #1079, three occurrences in nine days on a downstream adopter, most recently on a substantive diff carrying twoCONFIRMEDfindings.This names the inline case in the output contract — the findings go to the caller's next step, not to the reader — and adds the reciprocal at the call site, so
reviewstep 5 names step 6 as what follows rather than leaving the pass's return as the last instruction in view.Verified:
uv run pytest(882 passed),pre-commiton both changed files. No test pins either file's text.Why this shape rather than another round of "run the pass"
#937 and #1080 established that re-wording a mandate doesn't move stochastic compliance; #1080's gate (step 6 states what the pass returned) is the mechanism that did. This failure sits upstream of that gate — the session ends before reaching step 6, so a precondition at step 6 can't fire. The fix isn't a stronger instruction to continue, it's removing the instruction that reads as terminal: "return to the caller" is accurate for a subagent and literally impossible to satisfy inline, and the session resolves the impossibility by treating its own message as the return.
That also bounds the change: two sentences correcting a contract that is wrong for one of its two execution modes, not a new gate.
Reported occurrences, all downstream: diffplug/dormouse#493 round 5, plus diffplug/dormouse#440 and diffplug/dormouse#443. The first is post-#1080, which is what makes it visible — with the pass promoted to its own numbered step, its output is a step deliverable, so a session mistaking it for the deliverable now stops with a fuller-looking result.