fix(ci): claude-review.yml cites the wrong pr-review phase numbers (bot cannot review this PR, needs a human) - #436
Merged
Conversation
The CI adaptations told the reviewer to skip "Phase 6 (dotnet build / test)" and
"Phase 7 posting". Both numbers were off by one against the skill they point at,
.claude/skills/pr-review/SKILL.md:
:182 ## Phase 6, Verify findings (adversarial)
:225 ## Phase 7, Validate (dotnet build / dotnet test at :229-232)
:240 ## Phase 8, Report (the Post to GitHub step at :306)
Followed literally, CI skipped the adversarial skeptic pass, the one thing that
decides which findings survive, and ran the dotnet validation it had been told to
skip and that Build / Unit Tests / SonarCloud already cover. The exact inverse of
the intent. The skill's own caller-context note at :326-327 already said the right
thing ("In CI also skip Phase 7 (Validate) ... The Phase 6 adversarial pass still
runs"), so the workflow was contradicting the document it invokes.
Each instruction now carries the number AND the phase name, and an explicit line
tells CI to RUN Phase 6, so a future phase insert cannot silently invert this again.
Also removes the file's seven banned em dashes and its dash-baseline entry, since
the gate reads the whole file on any PR that touches it.
Split out of #435 on purpose: claude-code-action refuses to run on a PR whose
claude-review.yml differs from the default branch, so any PR carrying this file
gets no automated review. Keeping it alone means the other five fixes in #435 stay
reviewable.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018M2vH4pDvNnxUPecWWvwBc
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The defect
The
## CI adaptationsblock told the reviewer to skip "Phase 6 (dotnet build / test)" and "Phase 7 posting". Both numbers are off by one against the skill they point at.Counted in
.claude/skills/pr-review/SKILL.md, the document this workflow invokes::182## Phase 6 - Verify findings (adversarial):225## Phase 7 - Validatedotnet build/dotnet test, the block at:229-232:240## Phase 8 - Report### Post to GitHubstep is at:306So, followed literally, CI skipped the adversarial verification pass and ran the dotnet validation it had been told to skip, which Build / Unit Tests / SonarCloud already cover as separate required checks. The exact inverse of the intent: the expensive redundant step ran, and the step that stops a false Critical from blocking a clean PR did not.
The skill's own caller-context note at
:326-327already said the right thing:The workflow was contradicting the document it invokes.
The fix
Each instruction now carries the phase number and name, so a future phase insert cannot silently invert it again:
Skip the skill's Phase 7, "Validate" (dotnet build / test)Skip the "Post to GitHub" step of the skill's Phase 8, "Report", with a note that the skill still writes the report file that Step 2 postsRUN the skill's Phase 6, "Verify findings (adversarial)" in full, saying why, and noting that only its cross-model second opinion degrades in CI (the skill already handles that asUNAVAILABLE)Also in this diff
The file's seven banned em dashes are removed and its
tools/dash-baseline.jsonentry dropped, because the Dash Ban gate reads the whole file on any PR that touches it.Dash Banpasses on this branch independently (--filesclean,--check-baselineclean), as it does on #435 with the baseline entry left intact there.The
anthropics/claude-code-actionSHA is currentmain's (e0cf66d, from #432); this PR does not change it.Provenance
Recorded as item 2 of "Open follow-ups carried out of Phase 6" in
orbit-ui-mobile/REBUILD.mdsection 14.