Skip to content

feat!: require App tokens, remove deprecated gh_pat fallback#114

Merged
cbeaulieu-gt merged 1 commit intomainfrom
v2/token-cleanup
Apr 8, 2026
Merged

feat!: require App tokens, remove deprecated gh_pat fallback#114
cbeaulieu-gt merged 1 commit intomainfrom
v2/token-cleanup

Conversation

@cbeaulieu-gt
Copy link
Copy Markdown
Member

Summary

This PR completes the v2.0.0 token cleanup by addressing two related issues:

Breaking changes

  • The gh_pat input has been removed from lint-diagnose, lint-failure, and lint-apply composite actions.
  • The github_token input has been removed from apply-fix (it was the deprecated fallback name for that action).
  • The gh_pat secret has been removed from claude-lint-fix.yml and ci-failure.yaml reusable workflow secrets.
  • app_id and app_private_key are now required secrets in claude-lint-fix.yml and ci-failure.yaml.
  • The continue-on-error: true safety net on the Generate App token step in ci-failure.yaml has been removed — if App token generation fails, the workflow now fails immediately with a clear error message.

What changed

Part 1 — Remove gh_pat (#107):

  • lint-diagnose/action.yml — removed gh_pat input and elif fallback in token resolution
  • lint-failure/action.yml — same
  • lint-apply/action.yml — same
  • apply-fix/action.yml — removed deprecated github_token input and elif fallback
  • .github/workflows/claude-lint-fix.yml — removed gh_pat from secrets block and both with: blocks; marked app_id/app_private_key as required
  • .github/workflows/ci-failure.yaml — removed gh_pat from secrets block, removed continue-on-error and elif fallback; marked app_id/app_private_key as required
  • .github/workflows/apply-fix.yml — removed github_token: ${{ secrets.GH_PAT }} from the composite action call

Part 2 — Use App token for github_token (#112):

  • lint-diagnose/action.yml — changed github_token: ${{ github.token }} to ${{ steps.token.outputs.value }} in the claude-code-action invocation
  • lint-failure/action.yml — already using steps.token.outputs.value (no change needed)
  • ci-failure.yaml — already using steps.token.outputs.value (no change needed)
  • pr-review/action.yml — intentionally left unchanged. pr-review is read-only (no git push) and generates no App token. It correctly uses github.token for posting review comments, which is appropriate for a read-only action.

Docs:

  • CLAUDE.md and README.md updated to remove all references to GH_PAT as a fallback, and updated the troubleshooting section to reflect the new hard requirement.

Migration guide for consumers

If you currently pass gh_pat to any action or workflow, you must migrate to app_id + app_private_key:

  1. Create a GitHub App with Contents: read+write and Pull requests: read+write permissions
  2. Install it on your repository
  3. Add APP_ID and APP_PRIVATE_KEY as repository secrets
  4. Remove any gh_pat/GH_PAT secret references from your caller workflows

Part of v2.0.0

This is a breaking change and is intended to ship as part of the v2.0.0 release.

🤖 Generated with Claude Code

BREAKING CHANGE: The gh_pat input has been removed from all actions
and reusable workflows. All write-capable actions now require app_id
and app_private_key to generate GitHub App tokens.

Additionally, all claude-code-action invocations now use the App
token for github_token, ensuring comments post under the App's bot
identity consistently.

closes #107
closes #112

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 8, 2026

Claude finished @cbeaulieu-gt's task in 2m 16s —— View job


PR Review Complete ✅

This PR successfully removes the deprecated gh_pat fallback and enforces App token requirements for v2.0.0. The changes are well-structured and consistently applied across all affected files.


Summary

Approved with one suggested improvement — The breaking changes are appropriate for v2.0.0, security posture is improved, and documentation is thorough. However, there's an input validation inconsistency that should be addressed.


Key Findings

✅ Strengths

  1. Consistent implementation — All four composite actions (apply-fix, lint-apply, lint-diagnose, lint-failure) follow the same token resolution pattern
  2. Security improvement — Removing PAT fallback in favor of short-lived App tokens is the right architectural choice
  3. Clear error messages — New error messages provide actionable guidance: "Set app_id + app_private_key inputs. See README for GitHub App setup instructions."
  4. Better validationlint-diagnose/action.yml:44-48 and lint-failure/action.yml:40-44 now validate PR numbers with regex instead of just checking for emptiness
  5. Documentation completeness — Both CLAUDE.md and README.md are updated, and the PR description includes a clear migration guide
  6. Correct token usage for read-only operationspr-review/action.yml correctly continues using github.token since it doesn't push commits

⚠️ Issue: Input validation inconsistency

Files affected: apply-fix/action.yml, lint-apply/action.yml, lint-diagnose/action.yml, lint-failure/action.yml

Problem: All four composite actions mark app_id and app_private_key as required: false, but the "Resolve write token" step will now fail if they're not provided. This creates confusing UX:

inputs:
  app_id:
    description: 'GitHub App ID for generating a short-lived token'
    required: false  # ← Says optional
  app_private_key:
    description: 'GitHub App private key for generating a short-lived token'
    required: false  # ← Says optional

But the resolution logic now requires them:

if [ -n "${{ steps.app-token.outputs.token }}" ]; then
  echo "value=${{ steps.app-token.outputs.token }}" >> "$GITHUB_OUTPUT"
else
  echo "::error::No authentication token provided..."
  exit 1  # ← Fails if not provided
fi

Impact:

  • Users won't get validation errors at the input declaration level
  • Actions will fail later during token resolution with a runtime error
  • Less clear feedback about what went wrong

Suggested fix:
Mark both inputs as required: true in all four composite actions to match the actual behavior and provide validation errors upfront:

inputs:
  app_id:
    description: 'GitHub App ID for generating a short-lived token'
    required: true  # ← Match actual requirement
  app_private_key:
    description: 'GitHub App private key for generating a short-lived token'
    required: true  # ← Match actual requirement

This aligns with how the reusable workflows (ci-failure.yaml, claude-lint-fix.yml) already mark these secrets as required: true.

Fix this →


Code Quality Review

Dimension Assessment
Code quality ✅ Excellent — consistent patterns, clear naming, proper shell quoting
Security ✅ Improved — short-lived App tokens > long-lived PATs
Performance ✅ No concerns — administrative workflow changes only
Test coverage ⚠️ N/A — no test suite (per CLAUDE.md), changes dogfooded via this PR
Documentation ✅ Comprehensive — CLAUDE.md, README.md, and PR description all updated

Detailed Review by File

Workflows (3 files)

.github/workflows/ci-failure.yaml

  • ✅ Removed gh_pat secret and fallback logic
  • ✅ Marked app_id and app_private_key as required: true
  • ✅ Removed continue-on-error: true from token generation (fail fast is better)
  • ✅ Simplified token resolution to single path

.github/workflows/claude-lint-fix.yml

  • ✅ Consistent with ci-failure.yaml changes
  • ✅ Removed gh_pat from both lint-diagnose and lint-apply calls

.github/workflows/apply-fix.yml

  • ✅ Removed deprecated github_token: ${{ secrets.GH_PAT }} line
Composite Actions (4 files)

apply-fix/action.yml

  • ✅ Removed deprecated github_token input (was the old name for gh_pat in this action)
  • ✅ Cleaner error message
  • ⚠️ app_id/app_private_key should be required: true (see issue above)

lint-apply/action.yml

  • ✅ Removed gh_pat input and fallback logic
  • ⚠️ app_id/app_private_key should be required: true

lint-diagnose/action.yml

  • ✅ Removed gh_pat input and fallback logic
  • ✅ Changed github_token from ${{ github.token }} to ${{ steps.token.outputs.value }} in claude-code-action call (line 114)
  • ✅ Improved PR number validation with regex pattern
  • ⚠️ app_id/app_private_key should be required: true

lint-failure/action.yml

  • ✅ Removed gh_pat input and fallback logic
  • ✅ Improved PR number validation with regex pattern
  • ⚠️ app_id/app_private_key should be required: true
Documentation (2 files)

CLAUDE.md

  • ✅ Updated token selection guidance (line 53)
  • ✅ Removed GH_PAT from required secrets table
  • ✅ Correctly notes that pr-review uses github.token (read-only, no push)

README.md

  • ✅ Removed GH_PAT from all secrets tables
  • ✅ Updated troubleshooting section to remove PAT fallback references
  • ✅ Simplified "What happens when App token generation fails?" answer

Migration Impact

This is a well-justified breaking change for v2.0.0:

Breaking:

  • Consumers using gh_pat must migrate to GitHub App
  • No backward compatibility — immediate action required

Mitigated by:

  • Clear migration guide in PR description
  • Updated documentation with setup instructions
  • Actionable error messages pointing to README

Timing: Appropriate for major version bump (v2.0.0)


Recommendation

Approve and merge after addressing the input validation inconsistency, or merge as-is and address in a follow-up if you want to ship v2.0.0 sooner. The inconsistency is a UX issue (confusing error messages) rather than a functional bug — the actions will still fail appropriately when tokens are missing.


Tasks

  • Retrieve PR diff
  • Review code quality and consistency
  • Check security implications
  • Verify documentation updates
  • Post review feedback

@cbeaulieu-gt cbeaulieu-gt merged commit 96b08b7 into main Apr 8, 2026
2 checks passed
@cbeaulieu-gt cbeaulieu-gt deleted the v2/token-cleanup branch April 8, 2026 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant