v1.42.1: skip Claude PR review on wizard self-PRs - #253
Merged
Conversation
User call-out: 7 wizard self-PRs (v1.39.0–v1.42.0) shipped to main with the `review` job RED. I'd been treating "API canary firing as expected" as safe-to-merge because `validate` (the only required check) was green. That's wrong. Per CLAUDE.md, CI_CD.md, and the SDLC quality bar: ALL TESTS MUST PASS BEFORE COMMIT. Red CI normalizes red — a real review failure (tomorrow, on a real bug) would look identical to the canary failure and be missed. Fix: pr-review.yml `review` job's `if:` gate now skips when `github.repository == 'BaseInfinity/claude-sdlc-wizard'`. Exact-string match — consumer projects' forks/copies of pr-review.yml are unaffected. The wizard uses Codex (codex exec xhigh) for cross-model review on its own PRs, so Claude PR review is redundant on self-repo. Why API canary stays: the maintainer keeps ANTHROPIC_API_KEY's credit balance dead so unexpected API draws are detected by failed CI. ROADMAP #212 is migrating CI off API entirely (local-Max shepherd). Topping up the balance would defeat the canary signal — instead we kill the redundant job that needs it. Codex round 1: 7/10 NOT CERTIFIED. 2 P1: - R-001: test 1 didn't catch `==` (inverted operator) — would skip consumers and break the workflow's purpose. Added Test 1b negative control; mutation `\!=` → `==` now fails tests 1 + 1b. - R-002: test file untracked. git add. Round 2: 10/10 CERTIFIED. 6 quality tests in tests/test-self-pr-review-skip.sh. Documented in CI_CD.md → "Self-PR Skip on the Wizard Repo".
Caught by tests/test-docs-usability.sh — Latest: 1.42.0 was stale after the v1.42.1 version bump. Doc-only follow-up, no behavior change.
4 tasks
BaseInfinity
added a commit
that referenced
this pull request
Apr 27, 2026
* docs(handoff): document pr_number self-heal opt-in (#209, v1.42.2) ROADMAP #209 closure. The precompact-seam-check.sh hook self-heals on PENDING_* handoffs whose linked PR has merged: when handoff has pr_number and `gh pr view N --json state` returns MERGED, hook treats handoff as implicit CERTIFIED and unblocks /compact silently. Self-heal logic shipped earlier alongside #229 (stale-expire fallback) but was undocumented in the handoff template schemas, leaving the path effectively dead code on fresh installs (consumers had no way to discover the opt-in). Documented pr_number in all 3 handoff templates: - skills/sdlc/SKILL.md (Step 1: Mission-First Handoff) + key-fields prose - CLAUDE_CODE_SDLC_WIZARD.md (Round 1: Initial Review) - CLAUDE_CODE_SDLC_WIZARD.md (cross-model review section) New regression test test_handoff_template_documents_pr_number in tests/test-hooks.sh enforces template/doc parity going forward — a future schema edit that drops pr_number will fail this test. Together with #229 (mtime auto-expire) closes the "stuck PENDING handoff blocks /compact forever" footgun from both directions: PR-linked reviews self-heal on merge (instant), unlinked reviews auto-expire on mtime (14d default). Hit live in this repo 2026-04-19 (PR #205) and 2026-04-26 (PR #253) where handoffs lacked pr_number and fell through to the stale-expire fallback. Tests: 130 hook tests passing (1 new), 10 adjacent test files green. * fix(handoff-test): round-2 — coverage + prose + count (#255) Codex round-1 review (6/10 NOT CERTIFIED) found 3 issues: 1. P1: test_handoff_template_documents_pr_number false-greened on single-schema deletion. Required ANY occurrence per file, but wizard has 2 schemas — removing pr_number from one left the other intact and test passed. Fix: count handoff schemas via `"status": "PENDING_REVIEW"` marker (response.json uses PENDING_RECHECK so no collision). Assert pr_number occurrence count >= handoff count per file. Mutation re-verified: deleting pr_number from any of the 3 schemas now flips test to FAIL with skill-coverage(0/1) or wizard-coverage(1/2). Discovered separate grep -c quirk: `|| echo 0` after grep -c with no matches produces '0\n0' (grep -c always prints a number AND exits 1; || appends another 0). Switched to `|| true` + `${var:-0}` defaults — preserves count, discards failing exit, set -e safe. 2. P2: cross-model schema had pr_number field but no nearby prose explanation. Round 1 schema's inline explanation wasn't mirrored. Fix: added paragraph at CLAUDE_CODE_SDLC_WIZARD.md:3709 explaining when to set pr_number, what precompact-seam-check.sh does with it, when to omit. 3. P2: CHANGELOG overstated hook test count (130 vs actual 129). Fix: corrected to "129 hook tests, 1 new". All 3 mutation paths verified catching: - skill schema delete → FAIL skill-coverage(0/1) - wizard cross-model delete → FAIL wizard-coverage(1/2) - wizard Round 1 delete → FAIL wizard-coverage(1/2) Adjacent regression: 10 test files green (test-hooks 129/0 + 9 others).
BaseInfinity
added a commit
that referenced
this pull request
May 5, 2026
* fix(ci): skip Claude PR review on wizard self-PRs (v1.42.1) User call-out: 7 wizard self-PRs (v1.39.0–v1.42.0) shipped to main with the `review` job RED. I'd been treating "API canary firing as expected" as safe-to-merge because `validate` (the only required check) was green. That's wrong. Per CLAUDE.md, CI_CD.md, and the SDLC quality bar: ALL TESTS MUST PASS BEFORE COMMIT. Red CI normalizes red — a real review failure (tomorrow, on a real bug) would look identical to the canary failure and be missed. Fix: pr-review.yml `review` job's `if:` gate now skips when `github.repository == 'BaseInfinity/claude-sdlc-wizard'`. Exact-string match — consumer projects' forks/copies of pr-review.yml are unaffected. The wizard uses Codex (codex exec xhigh) for cross-model review on its own PRs, so Claude PR review is redundant on self-repo. Why API canary stays: the maintainer keeps ANTHROPIC_API_KEY's credit balance dead so unexpected API draws are detected by failed CI. ROADMAP #212 is migrating CI off API entirely (local-Max shepherd). Topping up the balance would defeat the canary signal — instead we kill the redundant job that needs it. Codex round 1: 7/10 NOT CERTIFIED. 2 P1: - R-001: test 1 didn't catch `==` (inverted operator) — would skip consumers and break the workflow's purpose. Added Test 1b negative control; mutation `\!=` → `==` now fails tests 1 + 1b. - R-002: test file untracked. git add. Round 2: 10/10 CERTIFIED. 6 quality tests in tests/test-self-pr-review-skip.sh. Documented in CI_CD.md → "Self-PR Skip on the Wizard Repo". * fix(docs): bump update-skill example to 1.42.1 (CI usability test) Caught by tests/test-docs-usability.sh — Latest: 1.42.0 was stale after the v1.42.1 version bump. Doc-only follow-up, no behavior change.
BaseInfinity
added a commit
that referenced
this pull request
May 5, 2026
* docs(handoff): document pr_number self-heal opt-in (#209, v1.42.2) ROADMAP #209 closure. The precompact-seam-check.sh hook self-heals on PENDING_* handoffs whose linked PR has merged: when handoff has pr_number and `gh pr view N --json state` returns MERGED, hook treats handoff as implicit CERTIFIED and unblocks /compact silently. Self-heal logic shipped earlier alongside #229 (stale-expire fallback) but was undocumented in the handoff template schemas, leaving the path effectively dead code on fresh installs (consumers had no way to discover the opt-in). Documented pr_number in all 3 handoff templates: - skills/sdlc/SKILL.md (Step 1: Mission-First Handoff) + key-fields prose - CLAUDE_CODE_SDLC_WIZARD.md (Round 1: Initial Review) - CLAUDE_CODE_SDLC_WIZARD.md (cross-model review section) New regression test test_handoff_template_documents_pr_number in tests/test-hooks.sh enforces template/doc parity going forward — a future schema edit that drops pr_number will fail this test. Together with #229 (mtime auto-expire) closes the "stuck PENDING handoff blocks /compact forever" footgun from both directions: PR-linked reviews self-heal on merge (instant), unlinked reviews auto-expire on mtime (14d default). Hit live in this repo 2026-04-19 (PR #205) and 2026-04-26 (PR #253) where handoffs lacked pr_number and fell through to the stale-expire fallback. Tests: 130 hook tests passing (1 new), 10 adjacent test files green. * fix(handoff-test): round-2 — coverage + prose + count (#255) Codex round-1 review (6/10 NOT CERTIFIED) found 3 issues: 1. P1: test_handoff_template_documents_pr_number false-greened on single-schema deletion. Required ANY occurrence per file, but wizard has 2 schemas — removing pr_number from one left the other intact and test passed. Fix: count handoff schemas via `"status": "PENDING_REVIEW"` marker (response.json uses PENDING_RECHECK so no collision). Assert pr_number occurrence count >= handoff count per file. Mutation re-verified: deleting pr_number from any of the 3 schemas now flips test to FAIL with skill-coverage(0/1) or wizard-coverage(1/2). Discovered separate grep -c quirk: `|| echo 0` after grep -c with no matches produces '0\n0' (grep -c always prints a number AND exits 1; || appends another 0). Switched to `|| true` + `${var:-0}` defaults — preserves count, discards failing exit, set -e safe. 2. P2: cross-model schema had pr_number field but no nearby prose explanation. Round 1 schema's inline explanation wasn't mirrored. Fix: added paragraph at CLAUDE_CODE_SDLC_WIZARD.md:3709 explaining when to set pr_number, what precompact-seam-check.sh does with it, when to omit. 3. P2: CHANGELOG overstated hook test count (130 vs actual 129). Fix: corrected to "129 hook tests, 1 new". All 3 mutation paths verified catching: - skill schema delete → FAIL skill-coverage(0/1) - wizard cross-model delete → FAIL wizard-coverage(1/2) - wizard Round 1 delete → FAIL wizard-coverage(1/2) Adjacent regression: 10 test files green (test-hooks 129/0 + 9 others).
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.
Summary
User call-out: 7 wizard self-PRs (v1.39.0–v1.42.0) shipped to main with the
reviewjob RED. I was treating "API canary firing" as safe-to-merge — wrong. Red is red, even if expected.Fix:
pr-review.ymlreview job now skips whengithub.repository == 'BaseInfinity/claude-sdlc-wizard'. Exact-string match — consumer projects unaffected. Wizard uses Codex on self-repo, so Claude PR review is redundant here.Codex Review
Test plan
!=→==fails tests 1 + 1b)