ci(pr): Explicitly pass copilot token, add debug info for spec review workflow#15795
Conversation
| -p "$retry_prompt" | ||
| else | ||
| copilot --agent "spec-review" \ | ||
| GH_TOKEN="${COPILOT_TOKEN:-$GH_TOKEN}" copilot --agent "spec-review" \ |
There was a problem hiding this comment.
Just making sure -- copilot consumes from GH_TOKEN?
There was a problem hiding this comment.
There was a problem hiding this comment.
Pull request overview
Updates the spec review CI workflow authentication flow by separating standard GitHub API auth from Copilot CLI auth, and adds additional git-clone diagnostics to help troubleshoot guideline-repo fetch failures.
Changes:
- Pass
GITHUB_TOKENviaGH_TOKENand addCOPILOT_TOKENto the spec review workflow step env. - Override
GH_TOKENonly for thecopilotprocess so the Copilot CLI uses the dedicated Copilot credential. - Add clone debug capture (trace/verbose) and print it only on clone failure.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| .github/workflows/spec-review.yml | Adjusts env vars so GH operations use GITHUB_TOKEN while providing COPILOT_TOKEN for the Copilot CLI. |
| .github/workflows/scripts/spec_review.sh | Adds clone failure diagnostics and overrides GH_TOKEN for the Copilot invocation to use COPILOT_TOKEN. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| COPILOT_TOKEN: ${{ secrets.COPILOT_TOKEN }} |
There was a problem hiding this comment.
This step now sets GH_TOKEN to GITHUB_TOKEN, but earlier the workflow's "Test gh auth" step still uses GH_TOKEN: secrets.COPILOT_TOKEN. That means GH_TOKEN refers to different credentials in different steps, which makes troubleshooting harder. Consider standardizing GH_TOKEN to always be GITHUB_TOKEN and only passing the Copilot credential via COPILOT_TOKEN (and update/rename the auth test accordingly).
| GH_TOKEN="${COPILOT_TOKEN:-$GH_TOKEN}" copilot --agent "spec-review" \ | ||
| --model "$MODEL" \ |
There was a problem hiding this comment.
With set -u, GH_TOKEN="${COPILOT_TOKEN:-$GH_TOKEN}" will error with an unbound variable if both COPILOT_TOKEN and GH_TOKEN are unset (common in local runs). Use a nested default like ${COPILOT_TOKEN:-${GH_TOKEN:-}} and add an explicit error if the resulting token is empty, so failures are deterministic and readable.
| clone_log="$(mktemp)" | ||
| if ! GIT_CURL_VERBOSE=1 GIT_TRACE=1 git clone --depth 1 --single-branch "$GIT_SOURCE" "$dest" 2>"$clone_log"; then | ||
| echo "Error: Failed to clone $GIT_SOURCE" >&2 | ||
| cat "$clone_log" >&2 | ||
| rm -f "$clone_log" | ||
| exit 1 | ||
| fi | ||
| rm -f "$clone_log" |
There was a problem hiding this comment.
clone_log is created outside temp_sources_dir and isn't covered by the existing EXIT trap, so it can be left behind if the script is interrupted between mktemp and rm -f. Consider creating it under $temp_sources_dir (or adding it to the trap) to ensure cleanup on signals/exits.
Attempt at fix for auth issues, with additional debug info in case it doesn't work.