fix(ci): non-blocking cosmetic steps + flaky test guidance

[BaseInfinity]

Summary

• Fix flaky CI: continue-on-error: true on "Build quick check comment message" and "Comment quick check results on PR" steps — these are cosmetic (PR comments), not quality gates. The intermittent npm ENOENT error from runner environment pollution no longer blocks the build.
• Quality gate preserved: "Fail on regression" step has NO continue-on-error — it remains the actual gatekeeper based on steps.compare.outputs.pass.
• Flaky test guidance: Added "Flaky Test Prevention" section to wizard and skill — 7 principles (investigate every failure, stress-test new tests, quarantine only if actively fixing, track rates, etc.)
• 3 new tests (42-44): Verify continue-on-error configuration is correct on cosmetic steps and absent on the quality gate.

Test plan

• ./tests/test-workflow-triggers.sh — 44/44 pass (including 3 new tests)
• ./tests/test-analysis-schema.sh — 8/8 pass
• ./tests/test-version-logic.sh — 6/6 pass
• CI validates workflow YAML and runs e2e-quick-check
• If flaky npm ENOENT occurs, verify it no longer fails the job

[github-actions]

Claude finished @BaseInfinity's task in 1m 42s —— View job

---

PR Code Review

Summary

This PR adds continue-on-error: true to two cosmetic CI steps (PR comment generation and posting) to prevent flaky npm errors from blocking builds, while preserving the actual quality gate. It also adds comprehensive "Flaky Test Prevention" guidance to both the wizard and skill documents.

Recommendation: APPROVE

Findings

Critical (must fix)

None - all changes are sound and well-tested.

Suggestions (nice to have)

1. Consider documenting the npm ENOENT root cause

.github/workflows/ci.yml:550 and .github/workflows/ci.yml:729 - The comments say "Comment is cosmetic — don't fail the job" which is accurate, but it might be worth adding a brief note about the specific issue being worked around (intermittent npm ENOENT from runner environment pollution) for future maintainers.

For example:

continue-on-error: true   # Cosmetic step - npm ENOENT flakiness shouldn't block build

This makes it clear it's not just "nice to have" but "working around known CI flakiness."

2. Test coverage could verify step ordering

tests/test-workflow-triggers.sh:760-813 - The tests correctly verify that cosmetic steps have continue-on-error and the quality gate doesn't. However, they don't verify that the quality gate (Fail on regression) runs after the cosmetic steps. If the ordering changed (quality gate before comments), the protection would still work but the failure message would be less useful.

Not critical since the quality gate uses steps.compare.outputs.pass which is set earlier, so ordering doesn't affect correctness - just observability.

3. Flaky guidance is excellent but could mention retry strategies

CLAUDE_CODE_SDLC_WIZARD.md:1684-1689 - The "When the Bug Is in CI Infrastructure" section is great. One additional point: when using continue-on-error: true, consider adding automatic retries for the cosmetic steps (e.g., using uses: nick-invision/retry@v2) to maximize the chance of getting the PR comment posted despite CI flakiness.

This is mentioned implicitly ("retry-and-pray" is criticized in the skill), but explicit guidance on when retries are appropriate (cosmetic steps) vs when they hide problems (quality gates) could be valuable.

Strengths

1. Perfect separation of concerns - The quality gate (Fail on regression) correctly has NO continue-on-error, while only the cosmetic steps (comment building and posting) are non-blocking. This is exactly the right pattern.

2. Test coverage is thorough - Tests 42-44 verify the exact configuration that matters: cosmetic steps are resilient, quality gates are strict. The tests use grep -A 2 to verify the property is on the right step, not just somewhere in the file.

3. Documentation is comprehensive - The flaky test guidance in both CLAUDE_CODE_SDLC_WIZARD.md and .claude/skills/sdlc/SKILL.md provides 7 concrete principles with clear rationale. The "3 layers" framework (test code / app code / environment) is particularly useful.

4. SDLC compliance is excellent - This PR demonstrates the exact workflow it teaches: flaky CI was investigated (not ignored), root cause was identified (CI infrastructure, not test/app code), and the fix preserves quality gates while making cosmetic steps resilient.

5. E2E coverage is appropriate - Workflow changes are covered by the new unit tests (42-44). No E2E scenario is needed because this is purely defensive (making the build more resilient to existing flakiness, not changing SDLC behavior).

6. Bash style is clean - The test functions follow repo conventions: descriptive names, clear failure messages, proper quoting, and grep patterns that match the exact structure.

Files Reviewed

• .github/workflows/ci.yml (lines 548-742)
• CLAUDE_CODE_SDLC_WIZARD.md (lines 1661-1689)
• .claude/skills/sdlc/SKILL.md (lines 122-129)
• tests/test-workflow-triggers.sh (lines 754-813)

---

Overall: This is a high-quality fix that follows all SDLC principles. The change is minimal, well-tested, and clearly documented. The flaky test guidance is valuable and will help other projects. No blocking issues - ready to merge once CI passes.

[github-actions]

Review completed. Check the workflow logs for details.

[github-actions]

CI Auto-Fix: Attempt 1/3

Detail	Value
Source	PR Code Review (review-findings)
Branch	fix/flaky-ci-and-flaky-test-guidance
Trigger Run	21935220918
Status	Fix pushed, CI re-triggered

Waiting for CI to re-run. If all checks pass and review approves, this loop is complete.

---

Auto-fix powered by Claude Code. Max 3 attempts.