Skip to content

ci(go-test): skip coverage upload on fork PRs#4255

Merged
reinkrul merged 1 commit into
masterfrom
ci/skip-coverage-upload-fork-prs
May 19, 2026
Merged

ci(go-test): skip coverage upload on fork PRs#4255
reinkrul merged 1 commit into
masterfrom
ci/skip-coverage-upload-fork-prs

Conversation

@reinkrul
Copy link
Copy Markdown
Member

Summary

  • The Go test workflow fails on PRs from forks because GitHub does not grant id-token: write to cross-repository runs, so qlty-action's OIDC upload errors with Unable to get ACTIONS_ID_TOKEN_REQUEST_URL env variable — turning the whole job red for external contributors (e.g. fix(vcr/verifier): handle unparseable issuer DID instead of nil deref #4247, run 25824280714).
  • Gate the upload step on push events and same-repo PRs. Tests still run on fork PRs; coverage continues to upload on push to master / V* and on internal PRs.

Test plan

  • Confirm a fork PR's Go test job now reaches green (the upload step is skipped, not failed).
  • Confirm an internal PR still uploads coverage to Qlty.
  • Confirm push to master still uploads coverage to Qlty.

Assisted by AI

GitHub strips id-token: write from workflow runs triggered by
pull_request from a forked repository, so qlty-action's OIDC upload
fails with "Unable to get ACTIONS_ID_TOKEN_REQUEST_URL env variable"
and the whole job goes red for external contributors.

Gate the upload step on push events and same-repo PRs. Tests still run
on fork PRs; coverage is still uploaded authoritatively on push to
master / V* branches.

Assisted by AI
@qltysh
Copy link
Copy Markdown
Contributor

qltysh Bot commented May 18, 2026

Qlty


Coverage Impact

⬆️ Merging this pull request will increase total coverage on master by 0.01%.

🚦 See full report on Qlty Cloud »

🛟 Help
  • Diff Coverage: Coverage for added or modified lines of code (excludes deleted files). Learn more.

  • Total Coverage: Coverage for the whole repository, calculated as the sum of all File Coverage. Learn more.

  • File Coverage: Covered Lines divided by Covered Lines plus Missed Lines. (Excludes non-executable lines including blank lines and comments.)

    • Indirect Changes: Changes to File Coverage for files that were not modified in this PR. Learn more.

@stevenvegt
Copy link
Copy Markdown
Member

⬆️ Merging this pull request will increase total coverage on master by 0.01%.

NICE!!

Other question: how do we know if the tests succeed? This is to protect our CI secrets etc. from malicious code right? The test don't start anyway and are now blocking. It would be nice if we can somehow trigger them ourselves if we reviewed the code and concluded that it is safe.

@reinkrul
Copy link
Copy Markdown
Member Author

Other question: how do we know if the tests succeed?

Workflow runs from external collaborators need manual approval, so before you hit that button, you should check whether the code doesn't do any nasty extraction of secrets, for instance.

Tests are still required to pass before we can merge.

@reinkrul reinkrul merged commit 33aa993 into master May 19, 2026
12 of 13 checks passed
@reinkrul reinkrul deleted the ci/skip-coverage-upload-fork-prs branch May 19, 2026 11:23
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.

2 participants