Skip to content

claude-blocking-review: workflow-self-modification skip is logged as "Doc-only skip" #86

Description

@twistedmelonman

Summary

When claude-blocking-review.yml skips via the workflow-self-modification priority path, the Check review verdict step logs Doc-only skip — no verdict required. The classification is correct; only the log line is wrong. It sent me diagnosing a "docs-only" misclassification on a PR containing zero docs.

Reproduction

Any PR touching .github/workflows/*.yml. Observed on smartwatermelon/qwen-sidebar#10 (run 30135858086), a 4-file PR with no .md files at all.

Correct annotation from the doc-check step:

PR modifies .github/workflows/ (.github/workflows/claude-code-review.yml) — claude-code-action refuses to run by design. Skipping; real review will run on the next non-workflow PR after merge.

Then from Check review verdict:

Doc-only skip — no verdict required.

The step summary is right (SKIPPED (workflow-self-modification)), so the two surfaces disagree.

Cause

Both skip paths write the same skip=true output, and the downstream consumer assumes only the doc-only path can have set it. At v3.0.0:

# line ~478
# Short-circuit: doc-only diff already wrote its summary in the
# Check for doc-only diff step. Nothing to verify, nothing to block.
if [ "$DOC_SKIP" = "true" ]; then
  echo "Doc-only skip — no verdict required."
  exit 0
fi

The env var name DOC_SKIP encodes the same assumption, as does the comment above it.

Suggested fix

Emit a distinct reason alongside skip, so the log can name the actual path:

# in the workflow-self-modification branch
echo "skip=true"                        >> "$GITHUB_OUTPUT"
echo "skip_reason=workflow-self-mod"    >> "$GITHUB_OUTPUT"

# in the doc-only branch
echo "skip=true"                        >> "$GITHUB_OUTPUT"
echo "skip_reason=doc-only"             >> "$GITHUB_OUTPUT"

then in Check review verdict:

env:
  SKIP: ${{ steps.doc-check.outputs.skip }}
  SKIP_REASON: ${{ steps.doc-check.outputs.skip_reason }}
run: |
  if [ "$SKIP" = "true" ]; then
    echo "Review skipped (${SKIP_REASON}) — no verdict required."
    exit 0
  fi

Renaming DOC_SKIPSKIP and updating the stale comment would keep the next reader from inheriting the same assumption.

Impact

Cosmetic — no gating behavior is affected. The required check reported SUCCESS in 7s and the PR was MERGEABLE/CLEAN. Worth fixing because the misleading line costs real debugging time on exactly the PRs where operators are least sure the gate is behaving: the ones installing or bumping the gate itself. This path fires on every Dependabot pin bump, so it is the most frequently seen skip message in the fleet, not an edge case.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions