Skip to content

Commit 5fdb1c3

Browse files
kotakanbeclaude
andcommitted
fix: address Copilot review on PR #376 (round 7)
- claude.yml workflow-file pre-flight: switch from `gh pr diff --name-only` to `gh pr view --json files`. The diff form reports only the rename destination, so a rename OUT of `.github/workflows/**` (e.g. `.github/workflows/ci.yml -> docs/ci.yml`) would leave `touches_workflows=0` even though Phase B push would still 403 on the source path. The view form returns both `path` (added/modified/deleted/destination) and `previousPath` (source side of renames); we now check either side. Also defensive case on the count: non-numeric / blank ⇒ fail-CLOSED to 1. - claude.yml "Resolve PR head ref name": stderr no longer merged into `head_ref`. The previous `2>&1` (added in R3 to get error context on failure) also captured non-fatal `gh` warnings on success, which would write a polluted value like `warning ... branch-name` to `GITHUB_OUTPUT`. Phase B then used that as the push target via `git push origin HEAD:$BRANCH` and the run would fail. New form: capture stderr to a temp file, only emit `GITHUB_OUTPUT` from stdout on success, surface stderr only in the warning path. Verification: GOWORK=off go build/vet ./... — green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent a129dea commit 5fdb1c3

1 file changed

Lines changed: 35 additions & 10 deletions

File tree

.github/workflows/claude.yml

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -75,16 +75,34 @@ jobs:
7575
# active marker in place. Refuse upfront with a clear comment
7676
# so the maintainer knows to fix workflow PRs locally via
7777
# `/review-until-clean` (which uses their own gh auth).
78-
# Fail-CLOSED on `gh pr diff` failure: a transient API / auth
79-
# error must NOT silently bypass the workflow-file guard
80-
# (would let Phase B push hit 403 mid-loop and spin until
81-
# timeout). Treat fetch failure as "could be a workflow-file
82-
# PR" and decline cleanly with a guidance comment.
83-
if ! diff_files=$(gh pr diff "$PR_NUMBER" --repo "$GH_REPO" --name-only 2>&1); then
84-
echo "::warning::PR #$PR_NUMBER: gh pr diff failed ($diff_files) — cannot verify workflow-file status. Failing closed."
78+
# Fail-CLOSED on `gh pr view --json files` failure: a transient
79+
# API / auth error must NOT silently bypass the workflow-file
80+
# guard (would let Phase B push hit 403 mid-loop and spin
81+
# until timeout). Treat fetch failure as "could be a
82+
# workflow-file PR" and decline cleanly with a guidance
83+
# comment.
84+
#
85+
# Use `gh pr view --json files` (returns both `path` and
86+
# `previousPath`) instead of `gh pr diff --name-only` (which
87+
# only reports the rename destination). Renames OUT of
88+
# `.github/workflows/**` would otherwise miss this guard
89+
# even though `git push` would still 403 on the source path.
90+
err_file=$(mktemp)
91+
if ! files_json=$(gh pr view "$PR_NUMBER" --repo "$GH_REPO" --json files 2>"$err_file"); then
92+
err=$(cat "$err_file")
93+
echo "::warning::PR #$PR_NUMBER: gh pr view --json files failed: $err — cannot verify workflow-file status. Failing closed."
94+
rm -f "$err_file"
8595
touches_workflows=1
8696
else
87-
touches_workflows=$(printf '%s\n' "$diff_files" | grep -c '^\.github/workflows/' || true)
97+
rm -f "$err_file"
98+
# `.path` covers added/modified/deleted; `.previousPath`
99+
# covers source side of renames. Match on either.
100+
touches_workflows=$(printf '%s' "$files_json" | jq '[.files[]
101+
| (.path // "") , (.previousPath // "")
102+
| select(startswith(".github/workflows/"))] | length')
103+
case "$touches_workflows" in
104+
''|*[!0-9]*) touches_workflows=1 ;;
105+
esac
88106
fi
89107
if [[ "$touches_workflows" -gt 0 ]]; then
90108
echo "::warning::PR #$PR_NUMBER touches .github/workflows/** — CI claude task cannot push under GH_ACTIONS_TOKEN scope. Posting a guidance comment and exiting cleanly so cron does not retry indefinitely."
@@ -153,12 +171,19 @@ jobs:
153171
# of failure for every @claude run. On failure, leave
154172
# `head_ref` unset; the skill will fall back to its in-skill
155173
# PR detection.
174+
# IMPORTANT: capture stderr separately so non-fatal `gh`
175+
# warnings don't pollute the `head_ref` value (a polluted
176+
# value would propagate into the skill's `git push origin
177+
# HEAD:$BRANCH` and turn a healthy run into an invalid push).
156178
if [[ -n "$PR_NUMBER" ]]; then
157-
if head_ref=$(gh pr view "$PR_NUMBER" --repo "$GH_REPO" --json headRefName --jq .headRefName 2>&1); then
179+
err_file=$(mktemp)
180+
if head_ref=$(gh pr view "$PR_NUMBER" --repo "$GH_REPO" --json headRefName --jq .headRefName 2>"$err_file"); then
158181
echo "head_ref=$head_ref" >> "$GITHUB_OUTPUT"
159182
else
160-
echo "::warning::Resolve PR head ref name: gh pr view failed (PR_NUMBER=$PR_NUMBER): $head_ref — skill will derive from PR_NUMBER directly"
183+
err_msg=$(cat "$err_file")
184+
echo "::warning::Resolve PR head ref name: gh pr view failed (PR_NUMBER=$PR_NUMBER): $err_msg — skill will derive from PR_NUMBER directly"
161185
fi
186+
rm -f "$err_file"
162187
fi
163188
164189
- uses: anthropics/claude-code-action@1c8b699d43e9bfed42b48ef15da85d89bab70960 # v1.0.94

0 commit comments

Comments
 (0)