From c2bc9b999414142a9d79f7c3468dde7bfb72e369 Mon Sep 17 00:00:00 2001 From: Claude Auto-Fix Date: Sat, 18 Jul 2026 11:33:12 -0400 Subject: [PATCH 1/2] chore: sunset automated Claude PR-review gate All 4 known consumers have migrated to CodeRabbit, so the automated review gate that ran on every PR is no longer needed. Removes the pr-review/ composite action, the claude-pr-review.yml reusable workflow, its example caller, and the now-dead marker-emission-aggregate.yml (which only aggregated commit statuses posted by pr-review/action.yml). Fixes the entangled digest-pin rewrite tooling in runtime-build.yml/runtime-rollback.yml (5->4 workflows, 7->6 occurrences) and the test.yml marker-corpus job that would otherwise reference deleted files. Updates docs/comments accordingly. The separate interactive `@claude review` tag-comment path (claude-command-router -> claude-runtime-review overlay -> claude-tag-respond.yml) is untouched and remains fully functional. Closes #309 Co-Authored-By: Claude Sonnet 5 --- .github/workflows/claude-pr-review.yml | 70 -- .../workflows/marker-emission-aggregate.yml | 198 ----- .github/workflows/overlay-smoke.yml | 4 +- .github/workflows/runtime-build.yml | 13 +- .github/workflows/runtime-rollback.yml | 19 +- .github/workflows/test.yml | 12 - CLAUDE.md | 6 +- README.md | 39 +- docs/consumer-onboarding.md | 4 +- examples/README.md | 1 - examples/claude-pr-review.yml | 27 - pr-review/README.md | 95 --- pr-review/action.yml | 714 ------------------ pr-review/lib/parse-marker.sh | 154 ---- pr-review/lib/severity-regex.sh | 7 - .../tests/marker-cases/clean-review.expected | 1 - pr-review/tests/marker-cases/clean-review.md | 31 - .../marker-cases/marker-malformed.expected | 1 - .../tests/marker-cases/marker-malformed.md | 32 - .../marker-cases/marker-missing.expected | 1 - .../tests/marker-cases/marker-missing.md | 24 - .../marker-cases/mixed-severity.expected | 1 - .../tests/marker-cases/mixed-severity.md | 71 -- .../pr-222-critical-issues-found.expected | 1 - .../pr-222-critical-issues-found.md | 119 --- .../zero-count-low-tally.expected | 1 - .../marker-cases/zero-count-low-tally.md | 45 -- .../zero-count-medium-tally.expected | 1 - .../marker-cases/zero-count-medium-tally.md | 40 - .../zero-count-tally-lines.expected | 1 - .../marker-cases/zero-count-tally-lines.md | 47 -- pr-review/tests/run-marker-cases.sh | 125 --- runtime/base/Dockerfile | 2 +- runtime/scripts/smoke-test.sh | 5 +- 34 files changed, 25 insertions(+), 1887 deletions(-) delete mode 100644 .github/workflows/claude-pr-review.yml delete mode 100644 .github/workflows/marker-emission-aggregate.yml delete mode 100644 examples/claude-pr-review.yml delete mode 100644 pr-review/README.md delete mode 100644 pr-review/action.yml delete mode 100644 pr-review/lib/parse-marker.sh delete mode 100644 pr-review/lib/severity-regex.sh delete mode 100644 pr-review/tests/marker-cases/clean-review.expected delete mode 100644 pr-review/tests/marker-cases/clean-review.md delete mode 100644 pr-review/tests/marker-cases/marker-malformed.expected delete mode 100644 pr-review/tests/marker-cases/marker-malformed.md delete mode 100644 pr-review/tests/marker-cases/marker-missing.expected delete mode 100644 pr-review/tests/marker-cases/marker-missing.md delete mode 100644 pr-review/tests/marker-cases/mixed-severity.expected delete mode 100644 pr-review/tests/marker-cases/mixed-severity.md delete mode 100644 pr-review/tests/marker-cases/pr-222-critical-issues-found.expected delete mode 100644 pr-review/tests/marker-cases/pr-222-critical-issues-found.md delete mode 100644 pr-review/tests/marker-cases/zero-count-low-tally.expected delete mode 100644 pr-review/tests/marker-cases/zero-count-low-tally.md delete mode 100644 pr-review/tests/marker-cases/zero-count-medium-tally.expected delete mode 100644 pr-review/tests/marker-cases/zero-count-medium-tally.md delete mode 100644 pr-review/tests/marker-cases/zero-count-tally-lines.expected delete mode 100644 pr-review/tests/marker-cases/zero-count-tally-lines.md delete mode 100644 pr-review/tests/run-marker-cases.sh diff --git a/.github/workflows/claude-pr-review.yml b/.github/workflows/claude-pr-review.yml deleted file mode 100644 index 2fa2606..0000000 --- a/.github/workflows/claude-pr-review.yml +++ /dev/null @@ -1,70 +0,0 @@ -name: Claude PR Review - -on: - # Fires directly in this repo for dogfood PR review. - # pull_request_target runs in the base repo's context (with secrets) even for fork PRs, - # which prevents auth rejection from claude-code-action. Safe here because no fork code - # is checked out or executed — Claude only reads the diff via `gh pr diff`. - pull_request_target: - types: [opened, synchronize, reopened, ready_for_review] - # Called by external consumers via `uses: glitchwerks/github-actions/.github/workflows/claude-pr-review.yml@v2`. - workflow_call: - secrets: - claude_code_oauth_token: - required: true - app_id: - required: true - app_private_key: - required: true - inputs: - authorized_users: - description: 'Comma-separated list of GitHub usernames that bypass the association check (useful when org membership is private). See #253.' - type: string - required: false - default: '' - max_turns: - description: 'Maximum number of Claude turns' - type: string - required: false - default: '30' - model: - description: 'Claude model to use' - type: string - required: false - default: 'claude-sonnet-4-5' - -# Required for the pull_request_target trigger in this repo. -# External consumers using workflow_call must declare pull-requests: write in their own caller workflow. -permissions: - contents: read - packages: read - pull-requests: write - statuses: write - -jobs: - review: - # Skip while the PR is in draft. The pull_request_target event types include - # ready_for_review so a single review fires when the user marks the PR ready. - # Issue #174 — avoids redundant API spend + duplicate-finding noise during - # iterative draft work (cf. PR #171 which accumulated 14 review runs as a draft). - if: github.event.pull_request.draft == false - runs-on: ubuntu-latest - # Phase 5 (#188): pin the review overlay image by SHA256 digest. - # The composite action's bash steps and the embedded `claude-code-action@v1` - # invocation all run inside this container. The image bakes in Claude CLI - # at $PATH_TO_CLAUDE_CODE_EXECUTABLE plus the review-specific agent set - # (different-eyes guarantee per spec §3.1, §10.2). - # Digest source: STAGE 3 build run 25405636887 (post-#200 rebuild, 3bb6a22) — base image bakes safe.directory '*' (#199, #197). - container: ghcr.io/glitchwerks/claude-runtime-review@sha256:3796f8e59c740b5cf826ef812edb1fa2620326b776a288519a65eb894f104c60 - concurrency: - group: claude-pr-review-${{ github.repository }}-${{ github.event.pull_request.number }} - cancel-in-progress: true - steps: - - uses: glitchwerks/github-actions/pr-review@v2 - with: - claude_code_oauth_token: ${{ secrets.claude_code_oauth_token || secrets.CLAUDE_CODE_OAUTH_TOKEN }} - app_id: ${{ secrets.app_id || secrets.APP_ID }} - app_private_key: ${{ secrets.app_private_key || secrets.APP_PRIVATE_KEY }} - authorized_users: ${{ inputs.authorized_users || '' }} - model: ${{ inputs.model || 'claude-sonnet-4-5' }} - max_turns: ${{ inputs.max_turns || '30' }} diff --git a/.github/workflows/marker-emission-aggregate.yml b/.github/workflows/marker-emission-aggregate.yml deleted file mode 100644 index 7a53789..0000000 --- a/.github/workflows/marker-emission-aggregate.yml +++ /dev/null @@ -1,198 +0,0 @@ -name: marker-emission-aggregate - -# Weekly aggregator for claude-pr-review/quality-gate-shadow commit statuses. -# -# Queries the last 7 days of merged PRs, checks each PR's head SHA for -# quality-gate-shadow statuses, and counts outcomes (agree, disagree, -# marker_missing, marker_invalid). Surfaces marker_missing events as a -# deduped GitHub issue so the Phase 3 cutover criterion can be tracked. -# -# This workflow is OBSERVABILITY ONLY — it always exits 0. The issue it -# may open is the signal, not the workflow run result. -# -# Cutover criterion (locked decision #5, plan §Phase 2): -# ≥20 reviews across ≥5 PRs, zero disagreement, zero marker_missing. -# -# Refs: #185, #183, #223 - -on: - schedule: - # Every Monday at 07:00 UTC - - cron: '0 7 * * MON' - workflow_dispatch: - -permissions: - contents: read - issues: write - -jobs: - aggregate: - name: Aggregate shadow-gate outcomes (last 7 days) - runs-on: ubuntu-latest - timeout-minutes: 10 - steps: - - uses: actions/checkout@v6 - with: - fetch-depth: 1 - - - name: Aggregate quality-gate-shadow outcomes - env: - GH_TOKEN: ${{ github.token }} - REPO: ${{ github.repository }} - run: | - # shellcheck disable=SC2016 - set -uo pipefail - - SINCE=$(date -u -d '7 days ago' +%Y-%m-%dT%H:%M:%SZ 2>/dev/null \ - || date -u -v-7d +%Y-%m-%dT%H:%M:%SZ) - echo "Aggregating shadow-gate outcomes since $SINCE" - - # Fetch recently merged PRs (last 7 days, up to 100) - # shellcheck disable=SC2016 - MERGED_PRS=$(gh api "repos/$REPO/pulls?state=closed&sort=updated&direction=desc&per_page=100" \ - --jq --arg since "$SINCE" \ - '[.[] | select(.merged_at != null and .merged_at >= $since) | {number: .number, head_sha: .head.sha, title: .title}]' \ - 2>/dev/null || echo "[]") - - PR_COUNT=$(printf '%s' "$MERGED_PRS" | jq 'length') - echo "Found $PR_COUNT merged PRs in the last 7 days" - - # Counters - total=0 - agree_clean=0 - agree_blocking=0 - disagree_count=0 - marker_missing_count=0 - marker_invalid_count=0 - missing_pr_numbers="" - - # For each merged PR, query commit statuses for quality-gate-shadow - while IFS= read -r pr_obj; do - pr_number=$(printf '%s' "$pr_obj" | jq -r '.number') - head_sha=$(printf '%s' "$pr_obj" | jq -r '.head_sha') - - # Fetch statuses for the head SHA, filter to our context - shadow_status=$(gh api "repos/$REPO/commits/$head_sha/statuses" \ - --jq '[.[] | select(.context == "claude-pr-review/quality-gate-shadow")] | sort_by(.updated_at) | last // empty' \ - 2>/dev/null || echo "") - - if [ -z "$shadow_status" ]; then - # No shadow status on this PR's head SHA — skip (may have been reviewed - # before Phase 2 landed, or was too small/skipped) - continue - fi - - total=$((total + 1)) - desc=$(printf '%s' "$shadow_status" | jq -r '.description // ""') - - case "$desc" in - agree:clean*) - agree_clean=$((agree_clean + 1)) - ;; - agree:blocking*) - agree_blocking=$((agree_blocking + 1)) - ;; - disagree:*) - disagree_count=$((disagree_count + 1)) - ;; - marker_missing*) - marker_missing_count=$((marker_missing_count + 1)) - missing_pr_numbers="$missing_pr_numbers $pr_number" - ;; - marker_invalid*) - marker_invalid_count=$((marker_invalid_count + 1)) - ;; - *) - # Unexpected description — count as marker_invalid for safety - marker_invalid_count=$((marker_invalid_count + 1)) - ;; - esac - done < <(printf '%s' "$MERGED_PRS" | jq -c '.[]') - - echo "Aggregation complete: total=$total agree_clean=$agree_clean agree_blocking=$agree_blocking disagree=$disagree_count marker_missing=$marker_missing_count marker_invalid=$marker_invalid_count" - - # Export for subsequent steps - { - echo "TOTAL=$total" - echo "AGREE_CLEAN=$agree_clean" - echo "AGREE_BLOCKING=$agree_blocking" - echo "DISAGREE=$disagree_count" - echo "MARKER_MISSING=$marker_missing_count" - echo "MARKER_INVALID=$marker_invalid_count" - # Trim leading space - echo "MISSING_PRS=${missing_pr_numbers# }" - echo "WEEK_OF=$(date -u +%Y-%m-%d)" - } >> "$GITHUB_ENV" - - - name: Write step summary - run: | - # shellcheck disable=SC2016 - { - echo "## Quality-gate shadow — weekly aggregate" - echo "" - printf '**Week of:** %s \n' "$WEEK_OF" - printf '**PRs with shadow status:** %d\n' "$TOTAL" - echo "" - echo "| Outcome | Count |" - echo "|---------|-------|" - printf '| `agree:clean` | %d |\n' "$AGREE_CLEAN" - printf '| `agree:blocking` | %d |\n' "$AGREE_BLOCKING" - printf '| `disagree:*` | %d |\n' "$DISAGREE" - printf '| `marker_missing` | %d |\n' "$MARKER_MISSING" - printf '| `marker_invalid` | %d |\n' "$MARKER_INVALID" - echo "" - if [ "$MARKER_MISSING" -gt 0 ]; then - echo "> **marker_missing PRs:** $MISSING_PRS" - echo "> These PRs had a quality-gate-shadow \`error\` status due to a missing marker block." - echo "> See issue \`Quality-gate shadow: marker_missing events detected\` for tracking." - fi - echo "" - echo "### Phase 3 cutover criterion" - echo "Locked decision \#5: ≥20 reviews across ≥5 PRs, zero disagreement, zero marker_missing." - printf 'Current: %d reviews, disagree=%d, marker_missing=%d\n' "$TOTAL" "$DISAGREE" "$MARKER_MISSING" - } >> "$GITHUB_STEP_SUMMARY" - - - name: Open or update deduped issue for marker_missing events - if: env.MARKER_MISSING != '0' - env: - GH_TOKEN: ${{ github.token }} - REPO: ${{ github.repository }} - run: | - # shellcheck disable=SC2016 - set -uo pipefail - - ISSUE_TITLE="Quality-gate shadow: marker_missing events detected" - - # Search for an existing open issue with this exact title - # shellcheck disable=SC2016 - EXISTING=$(gh issue list \ - --repo "$REPO" \ - --state open \ - --search "\"$ISSUE_TITLE\" in:title" \ - --json number,title \ - --jq '[.[] | select(.title == "'"$ISSUE_TITLE"'")] | first | .number // empty' \ - 2>/dev/null || echo "") - - # Build body via printf to avoid YAML-parser confusion from bare - # "**key:** value" patterns at column 0 inside a block scalar. - # shellcheck disable=SC2016 - BODY_TEXT=$(printf \ - '**Week of:** %s\n\n**marker_missing count:** %s\n\n**Affected PR numbers:** %s\n\n**Context:** The `claude-pr-review/quality-gate-shadow` advisory status reported `marker_missing`\nfor %s PR(s) this week. This means those PRs were reviewed by a persona that did not\nemit the `' \ - "$CRITICAL" "$HIGH" "$MEDIUM" "$LOW") - - NEW_BODY=$(printf '%s\n\n%s' "$BODY" "$MARKER") - - gh api "repos/$REPO/issues/comments/$COMMENT_ID" \ - -X PATCH \ - -f body="$NEW_BODY" - echo "Marker synthesized and appended: critical=$CRITICAL high=$HIGH medium=$MEDIUM low=$LOW" - - - name: Quality gate — structured marker (advisory shadow) - if: steps.authz.outputs.skip != 'true' && steps.size-check.outputs.skip != 'true' && steps.claude-review.outcome == 'success' - shell: bash - env: - GH_TOKEN: ${{ github.token }} - GH_REPOSITORY: ${{ github.repository }} - PR_NUMBER: ${{ github.event.pull_request.number }} - HEAD_SHA: ${{ github.event.pull_request.head.sha }} - # Path to the sourceable marker parser — resolved relative to the action's - # directory at runtime. The $GITHUB_ACTION_PATH variable is set by the - # Actions runner to the directory containing this action.yml file. - PARSE_MARKER_SH: ${{ github.action_path }}/lib/parse-marker.sh - SEVERITY_REGEX_SH: ${{ github.action_path }}/lib/severity-regex.sh - # SYNTHESIS_COMMENT_ID is written to $GITHUB_ENV by the synthesis step above - # when it successfully selects and patches a comment. When set, this step - # fetches that specific comment ID directly to avoid re-selecting and - # potentially hitting API replication lag on the just-patched body. - # shellcheck disable=SC2016 - SYNTHESIS_COMMENT_ID: ${{ env.SYNTHESIS_COMMENT_ID }} - REVIEW_START_TIME: ${{ env.REVIEW_START_TIME }} - run: | - set -euo pipefail - REPO="$GH_REPOSITORY" - # shellcheck source=lib/severity-regex.sh - # shellcheck disable=SC1090 - source "$SEVERITY_REGEX_SH" - - # --- Fetch the review comment body. ------------------------------------------ - # If the synthesis step pinned a comment ID to $GITHUB_ENV, use it directly - # (avoids replication-lag race on the just-PATCHed body). Otherwise fall back - # to the sort_by(updated_at)|last selector, gated by REVIEW_START_TIME so a - # foreign bot's stale comment can't win the sort under endswith("[bot]") - # (#250) — mirrors the authoritative gate's selector. - if [ -n "${SYNTHESIS_COMMENT_ID:-}" ]; then - BODY=$(gh api "repos/$REPO/issues/comments/$SYNTHESIS_COMMENT_ID" \ - --jq '.body // ""' 2>/dev/null || echo "") - echo "Shadow gate: fetching pinned comment id=$SYNTHESIS_COMMENT_ID" - else - # IMPORTANT (#184 fix): sort_by(.updated_at) | last picks the most-recently- - # edited bot comment; the API returns created_at ASC by default and does not - # honour sort/direction params. - BODY=$(gh api "repos/$REPO/issues/$PR_NUMBER/comments?per_page=100" \ - --jq "map(select((.user.login | endswith(\"[bot]\")) and .updated_at >= \"$REVIEW_START_TIME\")) | sort_by(.updated_at) | last | .body // \"\"" \ - 2>/dev/null || echo "") - fi - - # Skip entirely when there is no bot comment (review did not complete). - if [ -z "$BODY" ]; then - echo "Shadow gate: no [bot] sticky comment found — skipping" - exit 0 - fi - - # --- Prose-regex result (sourced from lib/severity-regex.sh) ---------------- - # Regex sourced above; $SEVERITY_BLOCKER_RE is the combined blocker class. - # Pre-filter drops resolution-narration lines so resolved findings - # on incremental reviews don't inflate the prose hit count. Expanded - # in #257 to also cover **FIXED**/**ADDRESSED**/**RESOLVED** suffixes - # (bold markdown). Anchored to end-of-line (\s*$) so only the persona's - # terminal suffix is suppressed, not mid-line occurrences (#258). Third - # branch (issue #269): drop persona-emitted summary-tally lines of the form - # `- 🔴 Critical (BLOCKING): 0` — the parenthesized severity-class marker - # `(BLOCKING|MAJOR|MEDIUM|LOW)` is the structural signature. Legitimate - # per-finding callouts like `- 🟢 Medium: 1` don't carry the parenthesized - # class. Reproducer: glitchwerks/claude-configs#564. Issue: #269. - # Must stay in sync with the authoritative gate step and the synthesis - # pre-filter (#248, #257, #258, #269). - BLOCKER_HITS=$(printf '%s' "$BODY" | grep -vE '✅|\*\*(FIXED|ADDRESSED|RESOLVED)\*\*\s*$|\((BLOCKING|MAJOR|MEDIUM|LOW)\):\s*[0-9]+\s*$' | grep -E -c "$SEVERITY_BLOCKER_RE" || true) - if [ "$BLOCKER_HITS" -gt "0" ]; then - PROSE_RESULT="blocking" - else - PROSE_RESULT="clean" - fi - echo "Shadow gate — prose-regex result: $PROSE_RESULT ($BLOCKER_HITS hit(s))" - - # --- Marker-based result ---------------------------------------------------- - # Source the parse-marker helper; function parse_marker is now available. - # shellcheck disable=SC1090 - source "$PARSE_MARKER_SH" - - MARKER_OUTPUT=$(printf '%s' "$BODY" | parse_marker) - MARKER_OUTCOME=$(printf '%s' "$MARKER_OUTPUT" | cut -d'|' -f1) - echo "Shadow gate — marker parser output: $MARKER_OUTPUT" - - # --- Compute status state and description ----------------------------------- - case "$MARKER_OUTCOME" in - marker_missing) - SHADOW_STATE="error" - SHADOW_DESC="marker_missing" - AGREE_LABEL="marker_missing" - ;; - marker_invalid) - SHADOW_STATE="error" - SHADOW_DESC="marker_invalid" - AGREE_LABEL="marker_invalid" - ;; - clean) - SHADOW_STATE="success" - if [ "$PROSE_RESULT" = "clean" ]; then - AGREE_LABEL="agree:clean" - else - AGREE_LABEL="disagree:prose=blocking/marker=clean" - fi - SHADOW_DESC="$AGREE_LABEL" - ;; - blocking) - SHADOW_STATE="failure" - if [ "$PROSE_RESULT" = "blocking" ]; then - AGREE_LABEL="agree:blocking" - else - AGREE_LABEL="disagree:prose=clean/marker=blocking" - fi - SHADOW_DESC="$AGREE_LABEL" - ;; - *) - SHADOW_STATE="error" - SHADOW_DESC="marker_invalid: unexpected outcome token" - AGREE_LABEL="marker_invalid" - ;; - esac - - # --- Post advisory commit status ------------------------------------------- - # CRITICAL: this status is ADVISORY ONLY. It must NOT be added to branch - # protection. The authoritative gate is claude-pr-review/quality-gate. - gh api "repos/$REPO/statuses/$HEAD_SHA" \ - -f state="$SHADOW_STATE" \ - -f context="claude-pr-review/quality-gate-shadow" \ - -f description="$SHADOW_DESC" - echo "Posted status: context=claude-pr-review/quality-gate-shadow state=$SHADOW_STATE desc=$SHADOW_DESC" - - # --- Step summary ----------------------------------------------------------- - { - echo "## Quality-gate shadow summary" - echo "" - echo "| Signal | Result |" - echo "|--------|--------|" - printf '| Prose-regex | `%s` (%d hit(s)) |\n' "$PROSE_RESULT" "$BLOCKER_HITS" - printf '| Structured marker | `%s` |\n' "$MARKER_OUTCOME" - printf '| Agreement label | `%s` |\n' "$AGREE_LABEL" - echo "" - echo "> Shadow status \`claude-pr-review/quality-gate-shadow\` is **advisory only**." - echo "> The authoritative gate is \`claude-pr-review/quality-gate\`." - } >> "$GITHUB_STEP_SUMMARY" - - - name: Post incomplete-review warning - if: failure() && steps.authz.outputs.skip != 'true' && steps.size-check.outputs.skip != 'true' - shell: bash - env: - GH_TOKEN: ${{ github.token }} - PR_NUMBER: ${{ github.event.pull_request.number }} - EFFECTIVE_MAX_TURNS: ${{ env.EFFECTIVE_MAX_TURNS }} - FILES: ${{ env.PR_FILES }} - REVIEW_START_TIME: ${{ env.REVIEW_START_TIME }} - run: | - # Skip the warning if Claude already posted a review comment during this run. - # Claude posts its summary as a regular PR comment (via use_sticky_comment), not a - # formal review submission, so we check comments rather than reviews. - # Filter by author (any [bot]) and timestamp to avoid false positives - # from human comments or comments posted by a prior run on the same PR. - COMMENT_COUNT=$(gh pr view "$PR_NUMBER" --json comments \ - | jq --arg since "$REVIEW_START_TIME" \ - '[.comments[] | select(.author.login | endswith("[bot]")) | select(.createdAt > $since)] | length' \ - 2>/dev/null || echo "0") - - if [ "$COMMENT_COUNT" -gt "0" ]; then - echo "Claude posted a review comment during this run before hitting the turn limit — skipping incomplete-review warning" - exit 0 - fi - - gh pr comment "$PR_NUMBER" --body "$(cat <) -# # or via stdin: -# outcome=$(echo "$BODY" | parse_marker) -# -# Output format (stdout, one line): -# outcome|critical|high|medium|low -# -# Outcome values: -# marker_missing — sentinel not found in body (counts are all 0) -# marker_invalid — sentinel found but JSON malformed or schema check fails -# clean — schema valid AND critical==0 AND high==0 -# blocking — schema valid AND (critical>0 OR high>0) -# -# For marker_missing and marker_invalid, counts are emitted as 0. -# -# Design notes: -# - TOP-LEVEL scope sets NO flags so callers' shells are unaffected. -# - The function uses `local -` to scope its own `set -uo pipefail`. -# - Accepts body as a file path argument OR on stdin (if no arg given). -# - Anchors on the exact sentinel line `. - local sentinel=" --------------- - # Strategy: use awk to capture lines from the sentinel through the first - # closing --> that follows it. - local raw_json - raw_json=$(printf '%s\n' "$body_input" | awk ' - BEGIN { capturing=0; buf="" } - //) { - # End of block — stop capturing (do not include the --> line) - capturing=0 - exit - } - buf = buf $0 "\n" - } - END { printf "%s", buf } - ') - - if [ -z "$raw_json" ]; then - printf 'marker_invalid|0|0|0|0\n' - return 0 - fi - - # --- Step 3: Validate JSON is parseable ------------------------------------ - if ! printf '%s\n' "$raw_json" | jq . >/dev/null 2>&1; then - printf 'marker_invalid|0|0|0|0\n' - return 0 - fi - - # --- Step 4: Schema check --------------------------------------------------- - # Required: schemaVersion (integer), findings.{critical,high,medium,low} (integers >=0) - # Forbidden: any extra top-level keys outside schemaVersion and findings. - - # Check schemaVersion is present and is a number - local schema_version - schema_version=$(printf '%s\n' "$raw_json" | jq -r '.schemaVersion // "MISSING"' 2>/dev/null) - if [ "$schema_version" = "MISSING" ]; then - printf 'marker_invalid|0|0|0|0\n' - return 0 - fi - if ! printf '%s\n' "$raw_json" | jq -e '.schemaVersion | type == "number"' >/dev/null 2>&1; then - printf 'marker_invalid|0|0|0|0\n' - return 0 - fi - - # Check findings object is present - if ! printf '%s\n' "$raw_json" | jq -e '.findings | type == "object"' >/dev/null 2>&1; then - printf 'marker_invalid|0|0|0|0\n' - return 0 - fi - - # Check all four findings fields exist and are non-negative integers - local field - for field in critical high medium low; do - local val - val=$(printf '%s\n' "$raw_json" | jq -r ".findings.${field} // \"MISSING\"" 2>/dev/null) - if [ "$val" = "MISSING" ]; then - printf 'marker_invalid|0|0|0|0\n' - return 0 - fi - # Must be a number (not a string) - if ! printf '%s\n' "$raw_json" | jq -e ".findings.${field} | type == \"number\"" >/dev/null 2>&1; then - printf 'marker_invalid|0|0|0|0\n' - return 0 - fi - # Must be >= 0 - if ! printf '%s\n' "$raw_json" | jq -e ".findings.${field} >= 0" >/dev/null 2>&1; then - printf 'marker_invalid|0|0|0|0\n' - return 0 - fi - done - - # Check no extra top-level keys outside schemaVersion and findings - local extra_keys - extra_keys=$(printf '%s\n' "$raw_json" | jq -r 'keys[] | select(. != "schemaVersion" and . != "findings")' 2>/dev/null) - if [ -n "$extra_keys" ]; then - printf 'marker_invalid|0|0|0|0\n' - return 0 - fi - - # --- Step 5: Extract counts and derive verdict ----------------------------- - local critical high medium low - critical=$(printf '%s\n' "$raw_json" | jq -r '.findings.critical') - high=$(printf '%s\n' "$raw_json" | jq -r '.findings.high') - medium=$(printf '%s\n' "$raw_json" | jq -r '.findings.medium') - low=$(printf '%s\n' "$raw_json" | jq -r '.findings.low') - - # Verdict rule (locked decision #3): blocking if critical>0 OR high>0 - if [ "$critical" -gt 0 ] || [ "$high" -gt 0 ]; then - printf 'blocking|%s|%s|%s|%s\n' "$critical" "$high" "$medium" "$low" - else - printf 'clean|%s|%s|%s|%s\n' "$critical" "$high" "$medium" "$low" - fi - - return 0 -} diff --git a/pr-review/lib/severity-regex.sh b/pr-review/lib/severity-regex.sh deleted file mode 100644 index 8a0bf25..0000000 --- a/pr-review/lib/severity-regex.sh +++ /dev/null @@ -1,7 +0,0 @@ -# severity-regex.sh — source this file; do not execute directly. -# Provides SEVERITY_BLOCKER_RE (combined) and per-bucket patterns. -SEVERITY_BLOCKER_RE='🔴 Critical|Critical \(BLOCKING\)|🟡 High-Priority|\*\*MAJOR\*\*|\*\*BLOCKING\*\*' -SEVERITY_CRITICAL_RE='🔴 Critical|Critical \(BLOCKING\)|\*\*BLOCKING\*\*' -SEVERITY_HIGH_RE='🟡 High-Priority|\*\*MAJOR\*\*' -SEVERITY_MEDIUM_RE='🟢 Medium' -SEVERITY_LOW_RE='\bNit\b' diff --git a/pr-review/tests/marker-cases/clean-review.expected b/pr-review/tests/marker-cases/clean-review.expected deleted file mode 100644 index c30f157..0000000 --- a/pr-review/tests/marker-cases/clean-review.expected +++ /dev/null @@ -1 +0,0 @@ -clean|0|0|0|0 diff --git a/pr-review/tests/marker-cases/clean-review.md b/pr-review/tests/marker-cases/clean-review.md deleted file mode 100644 index ad5d501..0000000 --- a/pr-review/tests/marker-cases/clean-review.md +++ /dev/null @@ -1,31 +0,0 @@ -**Claude finished @cbeaulieu-gt's task in 45s** —— [View job](https://github.com/glitchwerks/github-actions/actions/runs/99999000001) - ---- -### PR Review Complete - -I've reviewed this pull request and found no significant issues. The changes are well-structured, -follow existing conventions, and the test coverage is appropriate for the scope of the change. - -### What's Working Well ✅ - -1. **Code quality** — naming is clear, logic is straightforward, no unnecessary duplication -2. **Test coverage** — new code paths are covered with appropriate unit tests -3. **Documentation** — public functions are documented with accurate docstrings - -### No Issues Found - -This PR is clean and ready to merge. No Critical, High-Priority, Medium, or Nit findings. - -Verdict: APPROVE - - diff --git a/pr-review/tests/marker-cases/marker-malformed.expected b/pr-review/tests/marker-cases/marker-malformed.expected deleted file mode 100644 index e704fbd..0000000 --- a/pr-review/tests/marker-cases/marker-malformed.expected +++ /dev/null @@ -1 +0,0 @@ -marker_invalid diff --git a/pr-review/tests/marker-cases/marker-malformed.md b/pr-review/tests/marker-cases/marker-malformed.md deleted file mode 100644 index f2913ba..0000000 --- a/pr-review/tests/marker-cases/marker-malformed.md +++ /dev/null @@ -1,32 +0,0 @@ -**Claude finished @cbeaulieu-gt's task in 50s** —— [View job](https://github.com/glitchwerks/github-actions/actions/runs/99999000004) - ---- -### PR Review Complete - -Minor issues found. See findings below. - -### Findings - -#### 🟢 Medium - -**Variable naming inconsistency** — some variables use camelCase while others use -snake_case. Standardise to snake_case per the project's Python style guide. - ---- - -Verdict: APPROVE - - - - diff --git a/pr-review/tests/marker-cases/marker-missing.expected b/pr-review/tests/marker-cases/marker-missing.expected deleted file mode 100644 index eea18fa..0000000 --- a/pr-review/tests/marker-cases/marker-missing.expected +++ /dev/null @@ -1 +0,0 @@ -marker_missing diff --git a/pr-review/tests/marker-cases/marker-missing.md b/pr-review/tests/marker-cases/marker-missing.md deleted file mode 100644 index a14ef84..0000000 --- a/pr-review/tests/marker-cases/marker-missing.md +++ /dev/null @@ -1,24 +0,0 @@ -**Claude finished @cbeaulieu-gt's task in 55s** —— [View job](https://github.com/glitchwerks/github-actions/actions/runs/99999000003) - ---- -### PR Review Complete - -I've reviewed this pull request. The changes look correct and the implementation follows -the existing patterns in the codebase. - -### What's Working Well ✅ - -1. **Logic** — the algorithm is correct and handles edge cases appropriately -2. **Style** — consistent with the surrounding code -3. **Tests** — adequate coverage for the new behaviour - -### Summary - -No significant issues found. Ready to merge. - -**Recommendation:** Merge when CI passes. - ---- -(This is a pre-Phase-1 era review body — it pre-dates the structured marker -contract and therefore contains no HTML-comment marker block. The shadow gate -should report marker_missing for this body.) diff --git a/pr-review/tests/marker-cases/mixed-severity.expected b/pr-review/tests/marker-cases/mixed-severity.expected deleted file mode 100644 index 5e2edea..0000000 --- a/pr-review/tests/marker-cases/mixed-severity.expected +++ /dev/null @@ -1 +0,0 @@ -blocking|1|1|1|1 diff --git a/pr-review/tests/marker-cases/mixed-severity.md b/pr-review/tests/marker-cases/mixed-severity.md deleted file mode 100644 index 652eae6..0000000 --- a/pr-review/tests/marker-cases/mixed-severity.md +++ /dev/null @@ -1,71 +0,0 @@ -**Claude finished @cbeaulieu-gt's task in 1m 30s** —— [View job](https://github.com/glitchwerks/github-actions/actions/runs/99999000002) - ---- -### PR Review Complete - -I've reviewed this pull request and found findings at multiple severity levels. - -### Findings - -#### 🔴 Critical (BLOCKING) - -**Unvalidated input passed directly to shell command** - -The `run_command` function at line 42 constructs a shell command by concatenating -user-supplied input without sanitisation. An attacker who controls the input can -inject arbitrary shell commands. - -```python -# current — dangerous -subprocess.run(f"git checkout {branch_name}", shell=True) -``` - -Fix: use the list form and disable `shell=True`: - -```python -subprocess.run(["git", "checkout", branch_name], check=True) -``` - -#### 🟡 High-Priority (MAJOR) - -**Missing error handling on network call** - -`fetch_data()` at line 87 makes an HTTP request with no timeout and no exception -handling. A hung upstream service will block the workflow indefinitely. - -Add a timeout and wrap in a try/except: - -```python -response = requests.get(url, timeout=10) -response.raise_for_status() -``` - -#### 🟢 Medium - -**Magic number should be a named constant** - -Line 114 uses the literal `3600` for a cache TTL. Define a module-level constant -`CACHE_TTL_SECONDS = 3600` to make the intent clear and the value easy to change. - -#### Nit - -**Redundant `else` after `return`** - -Line 201 has `else:` after a `return` in the preceding `if` block. The `else` -is unnecessary and can be removed to reduce nesting. - ---- - -Verdict: BLOCK - - diff --git a/pr-review/tests/marker-cases/pr-222-critical-issues-found.expected b/pr-review/tests/marker-cases/pr-222-critical-issues-found.expected deleted file mode 100644 index eea18fa..0000000 --- a/pr-review/tests/marker-cases/pr-222-critical-issues-found.expected +++ /dev/null @@ -1 +0,0 @@ -marker_missing diff --git a/pr-review/tests/marker-cases/pr-222-critical-issues-found.md b/pr-review/tests/marker-cases/pr-222-critical-issues-found.md deleted file mode 100644 index e1847c7..0000000 --- a/pr-review/tests/marker-cases/pr-222-critical-issues-found.md +++ /dev/null @@ -1,119 +0,0 @@ -**Claude finished @cbeaulieu-gt's task in 2m 16s** —— [View job](https://github.com/glitchwerks/github-actions/actions/runs/25465608989) - ---- -### PR Review Complete - -I've reviewed PR #222 which refreshes the consumer-facing README for Phase 5+ container-pinned workflows. The PR successfully addresses most of the stated objectives, but I found **two critical inconsistencies** in the quick-start examples that need fixing before merge. - ---- - -### What's Working Well ✅ - -**Accurate updates (6/8 changes):** - -1. **Intro refresh** (line 3) — capability list is comprehensive and accurate -2. **Authentication clarification** (line 5) — clearly explains the two-tier auth model (OAuth token + App credentials) -3. **Actions table** (line 14) — `apply-fix` row added -4. **Permissions Reference** (lines 22, 28-32) — excellent addition of "Container pull permission" blockquote, all rows correctly show `packages: read`, includes #192 citation -5. **CI Failure quick-start** (lines 310-322) — `claude-ci-failure.yml@v2` + `packages: read` ✅ -6. **Lint Fix quick-start** (lines 359-383) — `claude-lint-failure.yml@v2` + `packages: read` ✅ -7. **Apply Fix manual trigger** (line 430) — `claude-apply-fix.yml` ✅ -8. **GHCR section** (lines 229-259) — well-structured, explains both Internal visibility and per-repo grant patterns, clear explanation of why `packages: read` is mandatory - -The new GHCR section is particularly well-written. The distinction between "necessary condition" (visibility setting) and "also necessary" (`packages: read` permission) is clear and will prevent the #192 trap. - ---- - -### Critical Issues Found 🚨 - -The permissions table (lines 28-29) correctly lists `packages: read` for **all** container-pinned workflows, but two quick-start examples are missing it: - -#### 1. PR Review quick-start (lines 52-54) - -**Current:** -```yaml -permissions: - contents: read - pull-requests: write -``` - -**Should be (per permissions table line 28):** -```yaml -permissions: - contents: read - pull-requests: write - packages: read -``` - -#### 2. Tag Claude quick-start (lines 85-88) - -**Current:** -```yaml -permissions: - contents: write - issues: write - pull-requests: write -``` - -**Should be (per permissions table line 29):** -```yaml -permissions: - contents: write - issues: write - pull-requests: write - packages: read -``` - -**Impact:** Consumers who copy-paste these examples will hit the `manifest unknown` error described in #192 and the new GHCR section. Since these are the **first two quick-start examples** most consumers encounter, this undermines the entire PR's goal of preventing the #192 trap. - -**Why this matters:** The PR correctly updates the CI Failure, Lint Fix, and Apply Fix examples (changes #5, #6, #7), but misses the two most commonly used workflows. This creates an inconsistency where the permissions table says one thing but the examples show another. - ---- - -### Recommendations - -1. **Before merge:** Add `packages: read` to: - - PR Review example at line 54 (after `pull-requests: write`) - - Tag Claude example at line 88 (after `pull-requests: write`) - -2. **Optional improvement:** Consider adding a brief inline comment in the quick-start examples explaining why `packages: read` is required, e.g.: - ```yaml - permissions: - contents: read - pull-requests: write - packages: read # Required for GHCR overlay image pull (see "GHCR package access" section) - ``` - This would make the examples more self-documenting, but the current permissions table blockquote (line 22) already explains it well. - -3. **Validation:** After fixing, verify all 5 quick-start examples match their corresponding permissions table rows. - ---- - -### Test Plan Status - -From the PR body test plan: -- ✅ All 4 quick-start YAML examples reference Phase 5 workflow names (verified) -- ✅ Permissions table lists `packages: read` for every container-pinned row (verified) -- ✅ New GHCR-access section present (verified — lines 229-259) -- ✅ Commit on correct branch (verified — `issue-219-readme-onboarding`) -- ⏸️ CI lint workflow — cannot verify without pushing -- ⏸️ Reviewer eyes on GHCR section — **this review confirms the click-paths and technical accuracy are correct** - ---- - -### Summary - -Strong PR that addresses the Phase 5 documentation gap and the #192 trap. The permissions table, GHCR section, and 3/5 quick-start examples are excellent. The two missing `packages: read` entries in PR Review and Tag Claude examples need fixing before merge to ensure consistency and prevent the exact error this PR aims to prevent. - -**Recommended action:** Add `packages: read` to lines 54 and 88, then merge. - ---- - -### Tasks -- [x] Retrieve PR diff -- [x] Read README.md to understand full context -- [x] Review workflow name updates -- [x] Verify `packages: read` additions -- [x] Check GHCR access documentation -- [x] Evaluate quick-start examples -- [x] Post comprehensive review diff --git a/pr-review/tests/marker-cases/zero-count-low-tally.expected b/pr-review/tests/marker-cases/zero-count-low-tally.expected deleted file mode 100644 index a2e6877..0000000 --- a/pr-review/tests/marker-cases/zero-count-low-tally.expected +++ /dev/null @@ -1 +0,0 @@ -clean|0|0|1|0 diff --git a/pr-review/tests/marker-cases/zero-count-low-tally.md b/pr-review/tests/marker-cases/zero-count-low-tally.md deleted file mode 100644 index 32c08dd..0000000 --- a/pr-review/tests/marker-cases/zero-count-low-tally.md +++ /dev/null @@ -1,45 +0,0 @@ -**Claude finished @cbeaulieu-gt's task in 37s** —— [View job](https://github.com/glitchwerks/github-actions/actions/runs/99999000567) - ---- -### PR Review Complete - -I've reviewed this pull request and found no blocking issues. - -### Findings - -**Findings:** -- 🔴 Critical (BLOCKING): 0 -- 🟡 High-Priority (MAJOR): 0 -- 🟢 Medium (MEDIUM): 1 -- Nit (LOW): 0 - -The `Nit (LOW): 0` line above is a persona-emitted summary-tally line. It carries -the parenthesized `(LOW)` marker and MUST be dropped by the pre-filter regex -`\((BLOCKING|MAJOR|MEDIUM|LOW)\):\s*[0-9]+\s*$` regardless of count. The plain -`- 🟢 Medium: 1` line below is a legitimate per-finding tally entry (no -parenthesized class) and MUST be kept. - -- 🟢 Medium: 1 - -#### 🟢 Medium - -**Magic number should be a named constant** - -Line 88 uses the literal `86400` for a TTL. Define a module-level constant to make -the intent clear. - ---- - -Verdict: APPROVE (no blocking issues found) - - diff --git a/pr-review/tests/marker-cases/zero-count-medium-tally.expected b/pr-review/tests/marker-cases/zero-count-medium-tally.expected deleted file mode 100644 index 373738b..0000000 --- a/pr-review/tests/marker-cases/zero-count-medium-tally.expected +++ /dev/null @@ -1 +0,0 @@ -clean|0|0|0|1 diff --git a/pr-review/tests/marker-cases/zero-count-medium-tally.md b/pr-review/tests/marker-cases/zero-count-medium-tally.md deleted file mode 100644 index 2f35908..0000000 --- a/pr-review/tests/marker-cases/zero-count-medium-tally.md +++ /dev/null @@ -1,40 +0,0 @@ -**Claude finished @cbeaulieu-gt's task in 41s** —— [View job](https://github.com/glitchwerks/github-actions/actions/runs/99999000566) - ---- -### PR Review Complete - -I've reviewed this pull request and found no blocking issues. - -### Findings - -**Findings:** -- 🔴 Critical (BLOCKING): 0 -- 🟡 High-Priority (MAJOR): 0 -- 🟢 Medium (MEDIUM): 0 -- Nit: 1 - -The `🟢 Medium (MEDIUM): 0` line above is a persona-emitted summary-tally line. -It carries the parenthesized `(MEDIUM)` marker and MUST be dropped by the -pre-filter regex `\((BLOCKING|MAJOR|MEDIUM|LOW)\):\s*[0-9]+\s*$` regardless of -count. The plain `- Nit: 1` line is a legitimate per-finding tally entry (no -parenthesized class) and MUST be kept. - -#### Nit - -**Trailing whitespace** at line 23. - ---- - -Verdict: APPROVE (no blocking issues found) - - diff --git a/pr-review/tests/marker-cases/zero-count-tally-lines.expected b/pr-review/tests/marker-cases/zero-count-tally-lines.expected deleted file mode 100644 index 2516dfd..0000000 --- a/pr-review/tests/marker-cases/zero-count-tally-lines.expected +++ /dev/null @@ -1 +0,0 @@ -clean|0|0|1|2 diff --git a/pr-review/tests/marker-cases/zero-count-tally-lines.md b/pr-review/tests/marker-cases/zero-count-tally-lines.md deleted file mode 100644 index e880813..0000000 --- a/pr-review/tests/marker-cases/zero-count-tally-lines.md +++ /dev/null @@ -1,47 +0,0 @@ -**Claude finished @cbeaulieu-gt's task in 52s** —— [View job](https://github.com/glitchwerks/github-actions/actions/runs/99999000564) - ---- -### PR Review Complete - -I've reviewed this pull request and found no blocking issues. - -### Findings - -**Findings:** -- 🔴 Critical (BLOCKING): 0 -- 🟡 High-Priority (MAJOR): 0 -- 🔴 Critical (BLOCKING): 5 -- 🟢 Medium: 1 -- Nit: 2 - -The three lines above the finding-tally entries are persona-emitted summary-tally -lines (#564 reproducer). They carry the parenthesized severity-class marker and -MUST be dropped by the pre-filter regardless of count (zero or non-zero). -The `- 🟢 Medium: 1` and `- Nit: 2` lines are legitimate per-finding tally -entries — no parenthesized class — and MUST be kept. - -#### 🟢 Medium - -**Minor style inconsistency** — variable naming convention differs from the rest of the module. - -#### Nit - -**Redundant blank line** at line 42. - -**Trailing whitespace** at line 87. - ---- - -Verdict: APPROVE (no blocking issues found) - - diff --git a/pr-review/tests/run-marker-cases.sh b/pr-review/tests/run-marker-cases.sh deleted file mode 100644 index dea7b16..0000000 --- a/pr-review/tests/run-marker-cases.sh +++ /dev/null @@ -1,125 +0,0 @@ -#!/usr/bin/env bash -# run-marker-cases.sh — fixture-driven test runner for pr-review/lib/parse-marker.sh. -# -# Usage: bash run-marker-cases.sh -# -# Sources pr-review/lib/parse-marker.sh, iterates every .md fixture in -# pr-review/tests/marker-cases/, reads its sibling .expected sidecar, runs -# parse_marker against the fixture, and compares actual vs expected outcome. -# Exits 0 if all cases pass, 1 if any fail. -# -# Expected sidecar format (.expected file, one line): -# marker_missing — for marker_missing outcome (counts not checked) -# marker_invalid — for marker_invalid outcome (counts not checked) -# clean|0|0|0|0 — full wire format for clean/blocking outcomes -# blocking|1|1|1|1 — full wire format -# -# For marker_missing and marker_invalid, only the outcome token is compared; -# the count fields (which are 0 placeholders) are ignored. This matches the -# parse-marker.sh contract where counts are not meaningful in error states. -# -# Failures ACCUMULATE — every mismatched case is reported before exit 1. -# Set is `set -uo pipefail`, NOT `-e`, to allow accumulation. - -set -uo pipefail - -# Resolve paths via the script's location so this works from any cwd. -SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -PR_REVIEW_DIR="$(cd "$SCRIPT_DIR/.." && pwd)" -PARSE_SH="$PR_REVIEW_DIR/lib/parse-marker.sh" -FIXTURES_DIR="$SCRIPT_DIR/marker-cases" - -# --- Defensive startup checks ------------------------------------------------- - -if ! command -v jq >/dev/null 2>&1; then - echo "ERROR jq_not_found: jq is required (preinstalled on ubuntu-latest)" >&2 - exit 1 -fi - -if [ ! -f "$PARSE_SH" ]; then - echo "ERROR parse_sh_missing: $PARSE_SH" >&2 - exit 1 -fi - -if [ ! -d "$FIXTURES_DIR" ]; then - echo "ERROR fixtures_dir_missing: $FIXTURES_DIR" >&2 - exit 1 -fi - -# Minimum fixture count check. -fixture_count=$(find "$FIXTURES_DIR" -maxdepth 1 -name '*.md' | wc -l) -if [ "$fixture_count" -lt 5 ]; then - echo "ERROR fixtures_too_few: found $fixture_count .md fixtures in $FIXTURES_DIR, want >= 5" >&2 - exit 1 -fi - -# --- Source parser ------------------------------------------------------------ - -# parse-marker.sh's TOP-LEVEL scope sets no flags (design invariant). -# The function `parse_marker` uses `local -` to scope its own flags. -# shellcheck disable=SC1090 -source "$PARSE_SH" - -if ! declare -F parse_marker >/dev/null; then - echo "ERROR parse_marker_undefined: sourcing $PARSE_SH did not define parse_marker" >&2 - exit 1 -fi - -# --- Iterate fixtures --------------------------------------------------------- - -errs=0 -total=0 - -for fixture in "$FIXTURES_DIR"/*.md; do - total=$((total + 1)) - name=$(basename "$fixture" .md) - expected_file="${fixture%.md}.expected" - - if [ ! -f "$expected_file" ]; then - errs=$((errs + 1)) - printf 'FAIL: %s\n missing sidecar: %s\n' "$name" "$expected_file" >&2 - continue - fi - - # Read expected outcome (trim trailing newline/whitespace) - expected=$(tr -d '[:space:]' < "$expected_file") - - # Run parser against the fixture file - actual=$(parse_marker "$fixture") - rc=$? - if [ "$rc" != "0" ]; then - errs=$((errs + 1)) - printf 'FAIL: %s\n parse_marker exited rc=%s (expected 0)\n' "$name" "$rc" >&2 - continue - fi - - # For marker_missing and marker_invalid, compare only the outcome token - # (the count fields are 0 placeholders and not meaningful in error states). - actual_outcome=$(printf '%s' "$actual" | cut -d'|' -f1) - expected_outcome=$(printf '%s' "$expected" | cut -d'|' -f1) - - if [ "$expected_outcome" = "marker_missing" ] || [ "$expected_outcome" = "marker_invalid" ]; then - # Outcome-only comparison - if [ "$actual_outcome" != "$expected_outcome" ]; then - errs=$((errs + 1)) - printf 'FAIL: %s\n expected outcome: [%s]\n got: [%s]\n full output: [%s]\n' \ - "$name" "$expected_outcome" "$actual_outcome" "$actual" >&2 - fi - else - # Full wire-format comparison (outcome|critical|high|medium|low) - if [ "$actual" != "$expected" ]; then - errs=$((errs + 1)) - printf 'FAIL: %s\n expected: [%s]\n got: [%s]\n' "$name" "$expected" "$actual" >&2 - fi - fi -done - -# --- Summary ------------------------------------------------------------------ - -passed=$((total - errs)) -printf 'summary: %d/%d passed\n' "$passed" "$total" - -if [ "$errs" -gt 0 ]; then - exit 1 -fi -exit 0 diff --git a/runtime/base/Dockerfile b/runtime/base/Dockerfile index 36cf70b..af803ed 100644 --- a/runtime/base/Dockerfile +++ b/runtime/base/Dockerfile @@ -65,7 +65,7 @@ RUN apt-get update \ RUN git config --system --add safe.directory '*' # Install GitHub CLI (`gh`) from the official cli.github.com apt repo. -# Required by composite actions in this repo (pr-review, apply-fix, lint-failure, +# Required by composite actions in this repo (apply-fix, lint-failure, # ci-failure, tag-claude) — they shell out to `gh` for PR diff fetching, status # creation, comment posting, and dedup. claude-code-action@v1 also exposes gh # subcommands to the AI via --allowedTools (Bash(gh pr diff:*), Bash(gh pr review:*), diff --git a/runtime/scripts/smoke-test.sh b/runtime/scripts/smoke-test.sh index 5540bca..9cbfee8 100755 --- a/runtime/scripts/smoke-test.sh +++ b/runtime/scripts/smoke-test.sh @@ -124,8 +124,9 @@ fi echo "smoke-test: persona structural check OK (agents=$agent_count, skills=$skill_count, plugins=$plugin_count, all required files present)" # ---- (a.3) Reachability probe: overlay tree reachable via $HOME discovery ---- -# Validates that the cp -r step in pr-review/action.yml (issue #259) makes -# baked agents discoverable when GHA overrides HOME to a fresh directory. +# Validates that the `cp -r $HOME/.claude` install pattern used by composite +# actions inside the overlay image makes baked agents discoverable when GHA +# overrides HOME to a fresh directory. echo "smoke-test: reachability probe..." if ! docker run --rm --user "$SMOKE_UID" \ -e HOME=/tmp/fake-home \ From e8950097a58d7fc04b143d30e930b1025c705552 Mon Sep 17 00:00:00 2001 From: Claude Auto-Fix Date: Sat, 18 Jul 2026 12:24:35 -0400 Subject: [PATCH 2/2] fix: address CodeRabbit and Claude review findings on pr-review sunset PR - overlay-smoke.yml: scope the digest-lookup grep to claude-*.yml only (matches this repo's documented convention) and fail loudly with a clear error when multiple distinct digest pins are found for the same overlay, instead of silently picking one via `sort -u | head -n1`. - docs/consumer-onboarding.md: scope the Step 5 smoke checklist to the event-driven templates and add a separate manual-dispatch verification step for claude-apply-fix.yml, which cannot produce a PR-timeline comment. - README.md: drop the three remaining pr-review references in the "Migrating from v1 to v2" section now that pr-review/claude-pr-review.yml no longer exist. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/overlay-smoke.yml | 20 +++++++++++++------- README.md | 6 +++--- docs/consumer-onboarding.md | 17 +++++++++++++++++ 3 files changed, 33 insertions(+), 10 deletions(-) diff --git a/.github/workflows/overlay-smoke.yml b/.github/workflows/overlay-smoke.yml index 1bb514b..e047f3d 100644 --- a/.github/workflows/overlay-smoke.yml +++ b/.github/workflows/overlay-smoke.yml @@ -42,18 +42,24 @@ jobs: OVERLAY: ${{ matrix.overlay }} run: | set -euo pipefail - # Grep all workflow files for lines referencing this overlay image and + # Grep claude-*.yml workflow files for lines referencing this overlay image and # extract the @sha256: suffix. The authoritative pins live in # claude-apply-fix.yml (fix) and claude-tag-respond.yml (review / fix / # explain — all three in its case block). - IMG=$(grep -rh "claude-runtime-${OVERLAY}@sha256:" .github/workflows/ \ - | grep -oE 'ghcr\.io/glitchwerks/claude-runtime-[^@]+@sha256:[a-f0-9]+' \ - | sort -u \ - | head -n1) - if [ -z "$IMG" ]; then - echo "::error::No digest found for overlay '${OVERLAY}' in .github/workflows/" + IMAGES=$(grep -h "claude-runtime-${OVERLAY}@sha256:" .github/workflows/claude-*.yml \ + | grep -oE "ghcr\.io/glitchwerks/claude-runtime-${OVERLAY}@sha256:[a-f0-9]+" \ + | sort -u || true) + if [ -z "$IMAGES" ]; then + echo "::error::No digest found for overlay '${OVERLAY}' in .github/workflows/claude-*.yml" exit 1 fi + IMAGE_COUNT=$(printf '%s\n' "$IMAGES" | wc -l) + if [ "$IMAGE_COUNT" -gt 1 ]; then + CONFLICTS=$(printf '%s\n' "$IMAGES" | paste -sd ',' - | sed 's/,/, /g') + echo "::error::Conflicting digest pins found for overlay '${OVERLAY}': ${CONFLICTS}" + exit 1 + fi + IMG="$IMAGES" echo "image=${IMG}" >> "$GITHUB_OUTPUT" echo "Resolved: ${IMG}" diff --git a/README.md b/README.md index a5e7e1c..4decc0e 100644 --- a/README.md +++ b/README.md @@ -122,15 +122,15 @@ If you need more control (e.g., embed the review step inside a larger job), use ### Breaking changes - **All `uses:` references must change from `@v1` to `@v2`.** Every workflow or composite action call that pins to `@v1` must be updated. -- **`gh_pat` input has been removed.** The deprecated `gh_pat` input is no longer accepted by any action. `app_id` + `app_private_key` are now required for all actions (`tag-claude`, `lint-failure`, `ci-failure`, `apply-fix`, `pr-review`). +- **`gh_pat` input has been removed.** The deprecated `gh_pat` input is no longer accepted by any action. `app_id` + `app_private_key` are now required for all actions (`tag-claude`, `lint-failure`, `ci-failure`, `apply-fix`). - **`tag-claude` no longer falls back to `github.token`.** An App token is mandatory — omitting `app_id` and `app_private_key` will cause the action to fail at the token resolution step. -- **All actions now post under the GitHub App's bot identity** (e.g., `my-app[bot]`) rather than under `github-actions[bot]` or a PAT's user identity. This includes `pr-review`, which previously used `github.token` (issue #250). +- **All actions now post under the GitHub App's bot identity** (e.g., `my-app[bot]`) rather than under `github-actions[bot]` or a PAT's user identity. ### Migration steps 1. Update every `uses: glitchwerks/github-actions/...@v1` line (and `uses: .github/workflows/...@v1`) to `@v2`. 2. If you were passing `gh_pat`, create a GitHub App and add `APP_ID` + `APP_PRIVATE_KEY` as repository secrets. See the GitHub App setup section for instructions. -3. For `tag-claude` and `pr-review` consumers: ensure your caller workflow passes `app_id` and `app_private_key` secrets. +3. For `tag-claude` consumers: ensure your caller workflow passes `app_id` and `app_private_key` secrets. ### Before and after diff --git a/docs/consumer-onboarding.md b/docs/consumer-onboarding.md index 303ce58..55a146f 100644 --- a/docs/consumer-onboarding.md +++ b/docs/consumer-onboarding.md @@ -139,6 +139,10 @@ Every template's `permissions:` block includes `packages: read`. This is require ## Step 5 — First-run smoke checklist +### Event-driven templates + +Use this checklist for `claude-tag-respond.yml`, `claude-lint-failure.yml`, and `claude-ci-failure.yml`. It does not apply to the manually dispatched `claude-apply-fix.yml` template. + After committing the workflow file to the consumer repo's default branch (or a PR): 1. **Open a tiny test PR.** A one-line README change is sufficient. Mark it "ready for review" (not draft). @@ -148,6 +152,19 @@ After committing the workflow file to the consumer repo's default branch (or a P If no comment appears after 5 minutes, proceed to [Step 6](#step-6--common-footguns). +### Apply Fix (manual dispatch) + +After committing `claude-apply-fix.yml` to the consumer repo's default branch, dispatch it manually for a test PR: + +```bash +gh workflow run claude-apply-fix.yml \ + -f pr_number=42 \ + -f fix_description="Fix missing null check in auth handler" \ + -f fix_diff="$(cat my.patch)" +``` + +Check the Actions tab for a successful run, then verify that the applied commit appears on the PR branch. This workflow does not post a PR-timeline comment in response to a PR event. + --- ## Step 6 — Common footguns