Skip to content

quality-gate reports success on review with explicit "two critical" language and "### Critical Issues Found" heading #223

@cbeaulieu-gt

Description

@cbeaulieu-gt

Summary

The claude-pr-review/quality-gate commit-status check in pr-review/action.yml posted state: success with description "No Critical or High-Priority findings" on PR #222 even though the bot's review body contains:

  • A literal markdown heading ### Critical Issues Found 🚨
  • The phrase "two critical inconsistencies"
  • Two distinct issues labeled with sub-headings under the Critical heading
  • An explicit "Recommended action: ... before merge"

The gate's grep-based marker scan apparently doesn't match the heading style + phrasing this review used, so the gate fails open silently — exactly the trap milestone #8 was created to harden against.

Reproducer

  • PR: docs: refresh consumer-facing README for Phase 5+ container-pinned workflows #222
  • Head OID: bed91dda0a2e4c3dad6aa8fe00d968f0eb33a805 (initial commit; review fired against this SHA)
  • Review: github-actions[bot] issue-comment id 4392784955, posted 2026-05-06T22:52:53Z
  • Status payload: gh api repos/glitchwerks/github-actions/commits/bed91dda0a2e4c3dad6aa8fe00d968f0eb33a805/status returns the claude-pr-review/quality-gate context with state: success, description: \"No Critical or High-Priority findings\"

The full review body is preserved on PR #222's conversation tab and contains an unambiguous "two critical issues" callout that the operator caught manually but the gate did not.

Why this matters

The gate is intended as a branch-protection ruleset requirement (#176). A false-success here means that under branch-protection enforcement, this PR would have been mergeable despite a real reviewer-flagged blocker. The whole point of the gate is to make Critical findings load-bearing on merge eligibility — a false-success is a complete failure of the contract, not a partial one.

The two findings on PR #222 happen to be small (missing packages: read lines in two YAML blocks), but the failure mode is independent of severity: any future review that uses similar heading/phrasing would also slip past the gate.

Hypothesis

The marker scan in pr-review/action.yml looks for some specific token set (likely literal strings like BLOCKING, CRITICAL, MAJOR, **Critical**). This review used:

  • ### Critical Issues Found 🚨 (markdown H3 heading with emoji)
  • **two critical inconsistencies** (bold lowercase)
  • #### 1. PR Review quick-start / #### 2. Tag Claude quick-start (sub-headings, no severity tokens)

If the scan is case-sensitive on Critical only when bracketed by specific delimiters (e.g. **Critical** or [CRITICAL]), the lowercase + heading form would slip through.

Relationship to existing issues

Acceptance criteria

  • Add the PR docs: refresh consumer-facing README for Phase 5+ container-pinned workflows #222 review body (or a minimized form preserving the heading + phrasing pattern) as a fixture in the quality-gate's test corpus.
  • Either:
    • (a) Extend the marker scan to recognize the heading-style "### Critical Issues Found" + lowercase "critical" + sub-headed findings pattern AND the existing patterns. Tactical fix; doesn't address the underlying fragility.
    • (b) Implement Adopt structured marker contract for quality-gate (replace grep) #185 (structured marker contract) and instruct the reviewer persona to emit it. Strategic fix; closes the failure class.
  • Either fix passes the new fixture as a failure state, not success.
  • Document the chosen marker contract in pr-review/README.md so future reviewer-persona changes don't drift away from it.

Out of scope

🤖 Generated by Claude Code on behalf of @cbeaulieu-gt

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions