workflows/lint-changed-files: bypass gh PAT validation for ghs_ token format#865
workflows/lint-changed-files: bypass gh PAT validation for ghs_ token format#865d-morrison wants to merge 6 commits into
Conversation
GitHub now hands secrets.GITHUB_TOKEN as a `ghs_*` server-to-server token. gh 1.5.0's `validate_gh_pat()` only accepts 40-hex / `ghp_*` / `github_pat_*` formats, so the `gh::gh()` call that fetches the PR's changed-files list aborts before lintr ever runs and the job fails with "Invalid GitHub PAT format". This affects every PR (recently seen on #706 across multiple consecutive runs). Pass `.token = Sys.getenv("GITHUB_PAT")` explicitly so gh uses the token directly via the Authorization header and skips the validator. The proper fix (r-lib/gh PR #233) is merged into gh main but not in any CRAN release yet, so bumping the renv pin is not yet an option. https://claude.ai/code/session_01PtDgktdMme5SZtze1EGx9U
|
Claude finished @d-morrison's task in 56s —— View job Code Review: workflows/lint-changed-files: bypass gh PAT validation for ghs_ token formatTasks:
SummaryThe fix is correct and well-reasoned. The root cause is accurately diagnosed, and the workaround is the right minimal approach given that Review FindingsFix correctness — ✅ Looks good Passing The Comment verbosity — minor nit The in-code comment is quite long for an R snippet inside a YAML workflow. The project's CLAUDE.md policy is "only add a comment when the WHY is non-obvious" — this qualifies because the version-mismatch context would be genuinely baffling without it. That said, the comment could be condensed without losing meaning: # Bypass gh's validate_gh_pat(): gh 1.5.0 rejects the `ghs_*` server-to-server
# token that Actions now issues. Passing `.token` explicitly skips validation.
# Remove once r-lib/gh#233 lands on CRAN.Not blocking, just a readability suggestion. Pre-existing: no pagination on the files API call — not blocking
No security concerns The Verdict✅ Approve. The fix is minimal, correct, and includes a clear upgrade path (swap for a |
|
Claude finished @d-morrison's task in 53s —— View job Claude Code Review — PR #865 Tasks
Code Review:
|
|
Claude finished @d-morrison's task in 52s —— View job Code Review — PR #865Tasks:
SummaryThe fix is correct, minimal, and well-motivated. Root cause is accurately diagnosed; the workaround is the right call given that Fix correctness — ✅Passing The Comment verbosity — minor nitThe 6-line comment (lines 39–45) is warranted — this is exactly the "workaround for a specific bug" category that CLAUDE.md says merits a comment, and the upgrade path ( # Explicit `.token` bypasses gh 1.5.0's validate_gh_pat(), which rejects the
# `ghs_*` server-to-server token Actions now issues. Remove once
# r-lib/gh#233 lands on CRAN (merged main 2026-05-28, not yet released).Not a blocker — the current comment is fine as-is. Pre-existing: no pagination on files API — out of scopeLines 46–49: Security — no concerns
Verdict✅ Approve. The fix is correct and includes a clear upgrade path. Nothing blocking. |
|
@claude review |
|
Claude finished @d-morrison's task in 1m 6s —— View job Code Review — PR #865Tasks:
SummaryThe fix is correct, minimal, and well-motivated. The root cause is accurately diagnosed; the workaround is appropriate given that Fix correctness — ✅Passing The Comment quality — ✅The 6-line comment (lines 39–45) is warranted — this is exactly the "workaround for a specific bug / hidden constraint" category that CLAUDE.md says merits a comment. The upgrade path ( # Explicit `.token` bypasses gh 1.5.0's validate_gh_pat(), which rejects the
# `ghs_*` server-to-server token Actions now issues. Remove once
# r-lib/gh#233 lands on CRAN (merged main 2026-05-28, not yet released).Not a blocker — current form is fine. Pre-existing: no pagination on files API — out of scopeLines 46–49: Security — no concerns
Verdict✅ Approve. The fix is correct, the comment documents the upgrade path clearly, and the change is exactly as minimal as it should be. |
|
👀 Picked up by workflow run #26916102523. R/Quarto/renv setup runs first (~3-5 min); Claude itself responds after that. |
|
Claude Code CLI (local session) is working on this — paws off until I'm done. |
Tightens the explanatory comment to the reviewer-suggested form while keeping the r-lib/gh#233 upgrade pointer. No logic change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Addressed the two items from the latest review:
@claude review |
|
👀 Picked up by workflow run #26916399055. R/Quarto/renv setup runs first (~3-5 min); Claude itself responds after that. |
|
Claude finished @d-morrison's task in 43s —— View job Code Review — PR #865Tasks:
SummaryThe two items from the previous review have been addressed. The comment is now condensed to 4 lines and matches the reviewer-suggested form closely; the pagination follow-up is tracked in #878. The underlying fix is unchanged and remains correct. Fix correctness — ✅Passing Comment quality — ✅Lines 39–42 are now 4 lines, down from 6. Content matches the reviewer-suggested form: the Pre-existing: no pagination — deferred ✅Tracked in #878 as noted. Out of scope for this PR. Verdict✅ Approve. The fix is correct, the comment is appropriately condensed, and the pagination follow-up is tracked. Nothing blocking. |
|
@claude now that gh 1.6.0 has landed on CRAN, can we simplify this workflow? also, resolve merge conflicts |
|
👀 Picked up by workflow run #26988008542. R/Quarto/renv setup runs first (~3-5 min); Claude itself responds after that. |
|
I've confirmed the blocker: the session is checked out on Here's the exact simplified content the PR branch needs, along with the resolution for the merge conflict: Good news: I can see exactly what the simplified workflow should look like, though I hit a permission wall applying it automatically (the Here's what to do: On - name: Extract and lint files changed by this PR
run: |
files <- gh::gh(
"GET https://api.github.com/repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/files",
.limit = Inf
)
changed_files <- files |>
purrr::keep(~ .$status != "renamed" || .$changes > 0) |>
purrr::map_chr("filename")
changed_files <- setdiff(changed_files, "renv/activate.R")
if (length(changed_files) == 0) {
message("No changed files to lint.")
} else {
all_files <- list.files(recursive = TRUE)
exclusions_list <- as.list(setdiff(all_files, changed_files))
lintr::lint_dir(exclusions = exclusions_list)
}
shell: Rscript {0}
env:
LINTR_ERROR_ON_LINT: trueThe full simplified file (drop the 4-line comment, replace # Workflow derived from https://github.com/r-lib/actions/tree/v2/examples
# Need help debugging build failures? Start at https://github.com/r-lib/actions#where-to-find-help
on:
pull_request:
name: lint-changed-files.yaml
permissions: read-all
jobs:
lint-changed-files:
runs-on: ubuntu-latest
env:
GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
steps:
- uses: actions/checkout@v5
- uses: r-lib/actions/setup-r@v2
with:
r-version: '4.6.0'
use-public-rspm: true
- name: Install system dependencies
run: |
sudo apt-get update
sudo apt-get install -y jags libcurl4-openssl-dev libpng-dev libfontconfig1-dev libjpeg-dev libglpk-dev libharfbuzz-dev libfribidi-dev libfreetype6-dev libtiff5-dev libwebp-dev libnode109
- uses: r-lib/actions/setup-renv@v2
env:
GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
- name: Add lintr options
run: |
cat('\noptions(lintr.linter_file = ".lintr")\n', file = "~/.Rprofile", append = TRUE)
shell: Rscript {0}
- name: Extract and lint files changed by this PR
run: |
files <- gh::gh(
"GET https://api.github.com/repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/files",
.limit = Inf
)
changed_files <- files |>
purrr::keep(~ .$status != "renamed" || .$changes > 0) |>
purrr::map_chr("filename")
changed_files <- setdiff(changed_files, "renv/activate.R")
if (length(changed_files) == 0) {
message("No changed files to lint.")
} else {
all_files <- list.files(recursive = TRUE)
exclusions_list <- as.list(setdiff(all_files, changed_files))
lintr::lint_dir(exclusions = exclusions_list)
}
shell: Rscript {0}
env:
LINTR_ERROR_ON_LINT: trueWhat this does vs. the current PR branch:
Why I couldn't apply this automatically: The — posted by @claude post-step from workflow run |
Resolves merge conflict against main and lands the simplified form @claude identified but couldn't push (the @claude session was checked out on main and its token lacks `workflows: write`). What this does, end-to-end on the `lint-changed-files.yaml` step: - Single R step (revert main's CLI two-step that worked around gh 1.5.0). - Drop the `.token = Sys.getenv("GITHUB_PAT")` bypass — gh 1.6.0's `validate_gh_pat()` accepts the `ghs_*` server-to-server tokens Actions issues, so the default token resolution works without override. - Pass `.limit = Inf` so the gh::gh files-API call follows pagination (closes the concern split out as #878). - Keep the empty-files guard added on main so a PR that only changes non-R/qmd files reports cleanly instead of trying to lint nothing. - Drop the obsolete 4-line comment explaining the bypass. renv.lock: bump gh 1.5.0 → 1.6.0. Mirrors the upstream DESCRIPTION diff: tighter rlang dep (>=1.0.0 → >=1.1.0), Suggests adds vctrs and webfakes (>=1.5.0), new Config/roxygen2/version, and the docs URL drops its `#readme` suffix. https://claude.ai/code/session_01PtDgktdMme5SZtze1EGx9U
|
Claude finished @d-morrison's task in 1m 57s —— View job Code Review — PR #865Tasks:
SummaryThis is the simplification d-morrison requested once Fix correctness — ✅Workflow (
The branch replaces both with a single R step calling
Filter predicate (line 44): purrr::keep(~ .$status != "renamed" || .$changes > 0)The logic is equivalent to the original jq renv.lock: The No leftover workaround artifacts — ✅The step no longer passes Pagination — resolved ✅Issue #878 (silently linting only the first 30 files for large PRs) is resolved by One minor note — not blockingThe empty-files guard added on if (length(changed_files) == 0) {
message("No changed files to lint.")
} else {
...
}This was also included in the simplification (lines 47–53), which is good — it preserves the behavior from main's version and avoids Verdict✅ Approve. The simplification is correct, complete, and exactly as minimal as it should be. No workaround artifacts remain, pagination is fixed, and the |
|
Claude finished @d-morrison's task in 1m 44s —— View job Code Review — PR #865Tasks:
SummaryThis PR has evolved through several iterations. The current state is the clean simplification requested once Workflow — ✅Consolidation from two steps to one:
This branch replaces both with a single R step calling Token lookup — ✅ The job-level Filter predicate — ✅ purrr::keep(~ .$status != "renamed" || .$changes > 0)Correctly mirrors the jq Pagination — ✅ (resolves #878)
Empty-files guard — ✅ The No workaround artifacts — ✅ The 4-line
|
| Field | Before | After |
|---|---|---|
Version |
1.5.0 |
1.6.0 |
URL |
...gh#readme |
...gh (removed #readme) |
rlang requirement |
>= 1.0.0 |
>= 1.1.0 |
Suggests |
— | adds vctrs, webfakes (>= 1.5.0) |
Config/roxygen2/version |
absent | "8.0.0" |
No unrelated packages are touched. The RoxygenNote: "7.3.2.9000" in the entry is the package author's dev-cycle metadata in their DESCRIPTION file; it has no effect on how renv installs the package and is not our concern.
Verdict
✅ Approve. The simplification is correct, complete, and exactly as minimal as it should be. The gh 1.6.0 upgrade makes the workaround unnecessary, the pagination gap is resolved, and no artifacts remain.
Summary
The
lint-changed-filesjob has been failing on every recent PR (observed across three consecutive runs on #706) with:GitHub Actions now hands
secrets.GITHUB_TOKENas aghs_*server-to-server token, whichgh 1.5.0'svalidate_gh_pat()doesn't recognize. The validator aborts before any lint can run —LINTR_ERROR_ON_LINT: truethen halts the script and the job exits 1. No lint findings ever existed; CI just couldn't reach the lint step.Fix
Pass
.token = Sys.getenv("GITHUB_PAT")explicitly togh::gh(). When.token=is supplied,ghuses it directly via the Authorization header and skipsvalidate_gh_pat()entirely. The token authenticates fine against the API — the only issue wasgh's own format check.Why not bump the gh pin
The proper fix is r-lib/gh#233, merged into
ghmainon 2026-05-28. But the latest CRAN/tagged release is still v1.5.0 (May 2025) — the fix has not yet shipped in a CRAN release, so bumping the renv pin isn't an option yet. Once a CRAN release containing #233 lands, swap this workaround for a cleanrenv::install("gh"); renv::snapshot().Test plan
lint-changed-filesjob goes green on this PR (and on every other open PR after merge).https://claude.ai/code/session_01PtDgktdMme5SZtze1EGx9U
Generated by Claude Code