Skip to content

Run no review pass at all; /code-review is the whole job - #18

Merged
UnityChaos merged 1 commit into
mainfrom
drop-security-review
Jul 26, 2026
Merged

Run no review pass at all; /code-review is the whole job#18
UnityChaos merged 1 commit into
mainfrom
drop-security-review

Conversation

@UnityChaos

Copy link
Copy Markdown
Member

Follow-up to #17, which had execute and remediation run /security-review then /simplify before handing /code-review --fix to the operator. Two things were wrong with that.

It stalls the stage

/security-review finishes by reporting findings to the user, and reporting to the user ends the turn. The session never reaches the steps after it.

Caught on issue #903 in the run that was meant to validate #17. The execute session emitted a full eight-surface security report — and then sat idle at the prompt for ~1h20m. No /simplify, no Execute.md, no commit, no session result. The conductor's classifier called it STALLED three times running:

Execute stage has no session_result (disk still holds the plan-stage result) and the last turn was a completed security-review report that asks nothing; the session has sat at idle_prompt since 23:13.

A manual nudge restarted it. Unattended it would have idled indefinitely — which is arguably worse than the original bug this all started from, since that one at least completed.

The v1.8.0 text asserted "returning from them is NOT the end of this stage." That is true of /simplify and simply false of /security-review. I carried the sentence over from the old /code-review step and assumed it generalized.

The passes were redundant anyway

/simplify and /security-review are each a subset of what /code-review does. Running them buys a weaker version of the pass that is already being requested one line later — at the cost of the turn that strands the stage.

The fix

Both stages now run nothing and just hand off. That removes the stall by construction rather than reordering around it, and drops ~50 lines of skill text.

The substitution rule gains a third clause: no other slash-command passes in its place — because nothing that might end the turn belongs before steps that must run. That ordering rule is the durable lesson and is recorded in CLAUDE.md, since it will outlive this particular pair of skills.

Unchanged

The operator_request contract, the REVIEW PASS OUTSTANDING -- run: /code-review --fix line, and the session deliberately staying open so the command can land in it. Patch bumps only — issue-workflow 1.8.0 → 1.8.1, deep-review 1.6.0 → 1.6.1 — since no interface changed. conductor #9's send verdict needs no update; it keys on operator_request, which is untouched.

Remaining mentions of /simplify and /security-review in the skills are deliberate: they name them as things not to run, and why.

🤖 Generated with Claude Code

https://claude.ai/code/session_014NubqCohjDPmKaf9usXchc

v1.8.0 had execute and remediation run /security-review then /simplify
before handing /code-review --fix to the operator. Two things were wrong
with that.

It stalls the stage. /security-review finishes by reporting findings to the
user, and reporting to the user ends the turn -- so the session never
reached the following steps. On issue #903 the execute session emitted a
full eight-surface security report and then sat idle at the prompt for
~1h20m: no /simplify, no Execute.md, no commit, no session result. The
conductor's classifier called it STALLED three times running before a
manual nudge restarted it. Unattended, it would have idled indefinitely.
The v1.8.0 skill text asserted "returning from them is NOT the end of this
stage", which is true of /simplify and simply false of /security-review.

The passes were also redundant. /simplify and /security-review are each a
subset of what /code-review does, so running them buys a weaker version of
the pass that is already being requested, at the cost of the turn that
strands the stage.

So both stages now run nothing and just hand off. That removes the stall by
construction rather than by reordering around it, and the substitution rule
gets a third clause: no other slash-command passes in its place, because
nothing that might end the turn belongs before steps that must run. That
ordering rule is the durable lesson and is recorded in CLAUDE.md, since it
will outlive this particular pair of skills.

Unchanged: the operator_request contract, the REVIEW PASS OUTSTANDING line,
and the session staying open so the command can land in it. Patch bumps --
issue-workflow 1.8.0 -> 1.8.1, deep-review 1.6.0 -> 1.6.1 -- as no
interface changed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014NubqCohjDPmKaf9usXchc
@UnityChaos
UnityChaos merged commit f0f0288 into main Jul 26, 2026
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