Conversation
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨Cli
Other
Bug Fixes 🐛Upgrade
Other
Documentation 📚
Internal Changes 🔧Ci
Other
Other🤖 This preview updates automatically when you update the PR. |
Codecov Results 📊✅ Patch coverage is 100.00%. Project has 3871 uncovered lines. Files with missing lines (67)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
+ Coverage 69.85% 69.85% —%
==========================================
Files 105 105 —
Lines 12841 12841 —
Branches 0 0 —
==========================================
+ Hits 8970 8970 —
- Misses 3871 3871 —
- Partials 0 0 —Generated by Codecov Action |
|
This will not work as when you auto commit, it will be done through |
|
You can add a pre-commit hook if you prefer |
BYK
left a comment
BYK
left a comment
There was a problem hiding this comment.
Would not work as expected so blocking
|
We should do this, which will then work: https://github.com/getsentry/sentry/blob/ff5444fbcdca906c03c53346242297120142269b/.github/workflows/pre-commit.yml#L34-L54 |
betegon
force-pushed
the
chore/auto-commit-skill-md
branch
from
bb1d891 to
75ea808
Compare
Switch from getsentry/action-github-app-token with SENTRY_INTERNAL_APP to actions/create-github-app-token with SENTRY_RELEASE_BOT, matching the token flow used in release.yml.
BYK
left a comment
BYK
left a comment
There was a problem hiding this comment.
Token flow now matches release.yml (actions/create-github-app-token with SENTRY_RELEASE_BOT). Auto-commit verified working — CI auto-committed regenerated SKILL.md successfully. Test change reverted.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| git config user.name "github-actions[bot]" | ||
| git config user.email "github-actions[bot]@users.noreply.github.com" | ||
| git add plugins/sentry-cli/skills/sentry-cli/SKILL.md | ||
| git commit -m "chore: regenerate SKILL.md" | ||
| git push |
There was a problem hiding this comment.
Bug: The check-skill job attempts to git push to fork repositories when SKILL.md is stale, but the SENTRY_RELEASE_BOT token likely lacks permissions, causing CI to fail.
Severity: HIGH
Suggested Fix
Modify the workflow to avoid attempting a git push when the trigger is a pull request from a fork. This can be achieved by adding a condition to the push step, such as if: github.repository == 'getsentry/cli', to ensure it only runs on the main repository and not on forks.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: .github/workflows/ci.yml#L75-L79
Potential issue: In the `check-skill` job, when a pull request from a fork has a stale
`SKILL.md` file, the workflow attempts to automatically commit and push a fix. The
checkout step correctly uses `github.head_ref`, targeting the branch in the fork.
However, the subsequent `git push` uses a token generated for the `SENTRY_RELEASE_BOT`
GitHub App. This app's token is unlikely to have write permissions to the contributor's
fork repository. As a result, the push operation will fail with a permission error,
causing the entire CI job to fail for external contributors.
Did we get this right? 👍 / 👎 to inform future reviews.
## Summary Fixes a bug introduced in #224 where the `check-skill` CI job would fail for fork PRs. ## Problem The auto-commit flow for stale SKILL.md uses a GitHub App token (`SENTRY_RELEASE_BOT`) to push commits back to the branch. This fails for fork PRs because: 1. **Secrets are unavailable** — GitHub does not expose repo secrets to `pull_request` workflows triggered from forks (security policy) 2. **No push access** — Even if the token were available, the GitHub App is installed on `getsentry/cli`, not the contributor's fork ## Fix - **Skip the token step** for fork PRs (conditional on `github.event.pull_request.head.repo.full_name == github.repository`) - **Fall back to `github.token`** for checkout when the app token isn't available - **Auto-commit** only when the app token was successfully obtained (same-repo PRs and push events) - **Fail with an actionable error** for fork PRs, asking the contributor to run `bun run generate:skill` locally
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| uses: actions/create-github-app-token@v2.2.1 | ||
| with: | ||
| app-id: ${{ vars.SENTRY_RELEASE_BOT_CLIENT_ID }} | ||
| private-key: ${{ secrets.SENTRY_RELEASE_BOT_PRIVATE_KEY }} |
There was a problem hiding this comment.
Token step breaks check-skill job for fork PRs
High Severity
The Get auth token step runs unconditionally as the first step of check-skill, but secrets.SENTRY_RELEASE_BOT_PRIVATE_KEY is not available for fork PRs (GitHub withholds secrets from fork pull_request workflows). This causes actions/create-github-app-token to fail immediately, breaking the entire job — even when SKILL.md is perfectly up to date. Previously, the job needed no secrets and would pass cleanly for fork PRs with a current SKILL.md. The failure cascades through ci-status, blocking the whole CI pipeline. The old code explicitly handled forks via IS_FORK.
Additional Locations (1)
2 participants
Summary
When
check-skilldetects SKILL.md is out of date, auto-commit the regenerated file instead of failing CI with instructions.check-skill.tsalready runsgenerate-skill.tsinternally and leaves the updated file on disk — so we just need togit add,commit, andpushwhen the check fails.Changes
Replaced the "Output regeneration instructions" step (which printed manual fix commands and failed the build) with a step that commits and pushes the regenerated SKILL.md using the
github-actions[bot]identity.