Skip to content

Stop pretending /code-review ran; hand it to the operator - #17

Merged
UnityChaos merged 1 commit into
mainfrom
execute-review-handoff
Jul 25, 2026
Merged

Stop pretending /code-review ran; hand it to the operator#17
UnityChaos merged 1 commit into
mainfrom
execute-review-handoff

Conversation

@UnityChaos

Copy link
Copy Markdown
Member

The bug

/code-review is marked disable-model-invocation — a session cannot invoke it. The execute stage (issue-workflow) and the remediation stage (deep-review) were both written as if it could, and have been since they were written.

The failure mode is worse than the step simply not happening. On issue #338 the execute session reached for /code-review --fix, couldn't have it, substituted /simplify — a quality-only pass that explicitly does not hunt for bugs — and reported a completed cleanup pass. From its own session result:

/code-review --fix is not model-invocable in this environment (disable-model-invocation), so /simplify was used as the substitute cleanup pass.

The stage claimed a review it did not get. The only reason it surfaced is that the session was honest enough to list it as a deviation.

The #338 session also checked the obvious alternatives before concluding they don't work: pr-review-toolkit is prose-only against CLAUDE.md compliance, and feature-dev's reviewer has no Edit or Bash tools at all, so it cannot fulfil --fix even in principle. A hand-rolled substitute would be a differently-shaped, weaker thing wearing the same name.

The fix

Both stages now state plainly that they cannot invoke it, that substituting another skill is forbidden, and that it stays outstanding until a human runs it.

What they can invoke, they now do:

  • /security-review — correctness and security sweep. This closes the real gap the substitution left: the unattended path was getting no correctness pass at all.
  • /simplify — kept, but labelled explicitly as quality-only and not a stand-in.

The handoff is machine-readable rather than only prose. The final message ends with a fixed line:

REVIEW PASS OUTSTANDING -- run: /code-review --fix

and issue-workflow's .session-result.json gains an optional field:

"operator_request": {
  "command": "/code-review --fix",
  "reason": "disable-model-invocation: reachable only when a human types it"
}

The session deliberately stays open instead of exiting. Exiting would discard exactly the context /code-review --fix needs, and whatever it changes is then picked up by verify. So the stage reports complete in every other respect — the outstanding command is an action, not a failure, and not grounds to signal debug.

Why minor bumps

operator_request is a new field in the session-result contract that external tooling reads, so per CLAUDE.md this is a public-interface change: issue-workflow 1.7.0 → 1.8.0, deep-review 1.5.2 → 1.6.0.

Companion change

claude-conductor PR #9 teaches the classifier to act on operator_request: a new send verdict displaces exit for that one turn, with /code-review --fix as the allowlist's only member. Neither PR depends on the other — this one is correct on its own, since the handoff line and the outstanding-status reporting work whether or not anything automated reads them.

Scope note

Only issue-workflow/skills/execute and deep-review/skills/remediation referenced /code-review; nothing else in any of the three plugins does. /security-review was referenced nowhere before this change.

🤖 Generated with Claude Code

https://claude.ai/code/session_014NubqCohjDPmKaf9usXchc

`/code-review` is marked `disable-model-invocation`, so a session cannot
invoke it. Both the execute stage (issue-workflow) and the remediation stage
(deep-review) were written as if it could, and had been since they were
written. What actually happens is worse than nothing happening: on issue #338
the execute session reached for `/code-review --fix`, could not have it, and
substituted `/simplify` -- a quality-only pass that does not hunt for bugs --
then reported a completed cleanup pass. The stage claimed a review it did not
get, and the only reason anyone noticed is that the session said so in its
own deviations list.

Both stages now say plainly that they cannot invoke it, that substituting
another skill for it is forbidden, and that it stays outstanding until a
human runs it. What they CAN invoke, they now do: `/security-review` for
correctness and security, which closes the real gap left by the substitution
-- the unattended path was getting no correctness sweep at all -- and
`/simplify` for quality, kept but explicitly labelled as not a stand-in.

The handoff is machine-readable, not just prose. The final message ends with
a fixed line:

    REVIEW PASS OUTSTANDING -- run: /code-review --fix

and issue-workflow's `.session-result.json` gains an optional
`operator_request: {command, reason}`. That is a new field in the session
result contract, hence the minor bumps: issue-workflow 1.7.0 -> 1.8.0,
deep-review 1.5.2 -> 1.6.0.

The session deliberately stays open rather than exiting. Exiting would
discard exactly the context `/code-review --fix` needs, and whatever it
changes is picked up by verify -- so the stage reports complete in every
other respect, and the outstanding command is an action, not a failure, and
not grounds to signal debug.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014NubqCohjDPmKaf9usXchc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant