Skip to content

ci: Auto-accept API verifier changes in PRs #5228

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jamescrosswell wants to merge 5 commits into
base: main
Choose a base branch
from
+172 −0
Open

ci: Auto-accept API verifier changes in PRs #5228

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
92dab02
ci: Auto-accept API verifier changes in PRs
jamescrosswell May 13, 2026
dc60272
Refactor: Checkout in branch to simplify the logic later
jamescrosswell May 20, 2026
b011745
Update verify-api.yml
jamescrosswell May 20, 2026
698a813
ci: Scope workflow permissions per-job
jamescrosswell May 20, 2026
ed7001e
ci: Remove continue-on-error from artifact download steps
jamescrosswell May 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
172 changes: 172 additions & 0 deletions .github/workflows/verify-api.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,172 @@
name: verify api
on:
pull_request:
paths:
- 'src/**'
- 'test/**/ApiApprovalTests*'
- 'test/Sentry.Testing/ApiExtensions.cs'
- '.github/workflows/verify-api.yml'

jobs:
run-api-tests:
name: Run API Approval Tests (${{ matrix.rid }})
runs-on: ${{ matrix.os }}
# This job builds and runs untrusted PR code — keep the token read-only.
permissions:
contents: read

strategy:
fail-fast: false
matrix:
include:
# macOS covers all non-Windows TFMs (net9.0, net10.0, netstandard, iOS, MacCatalyst, Android)
- os: macos-15
rid: macos
slnf: Sentry-CI-Build-macOS.slnf
# Windows is required to produce the .NET Framework (net48 / Net4_8) verified files
- os: windows-latest
rid: win-x64
slnf: Sentry-CI-Build-Windows.slnf

steps:
- name: Checkout
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
with:
submodules: recursive

- name: Remove unused applications
uses: ./.github/actions/freediskspace

- name: Setup Environment
uses: ./.github/actions/environment

- name: Restore sentry-native cache
id: cache-native
uses: actions/cache/restore@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5
with:
path: src/Sentry/Platforms/Native/sentry-native
key: sentry-native-${{ matrix.rid }}-${{ hashFiles('scripts/build-sentry-native.ps1') }}-${{ hashFiles('.git/modules/modules/sentry-native/HEAD') }}
enableCrossOsArchive: true

- name: Build sentry-native (cache miss)
if: steps.cache-native.outputs.cache-hit != 'true'
shell: pwsh
run: scripts/build-sentry-native.ps1

- name: Build Native Dependencies
uses: ./.github/actions/buildnative

- name: Restore .NET Dependencies
run: |
dotnet workload restore
dotnet restore ${{ matrix.slnf }} --nologo

- name: Build
run: dotnet build ${{ matrix.slnf }} -c Release --no-restore --nologo -v:minimal

# API approval tests fail when the public API surface changes. We swallow the failure
# here and rely on the produced *.received.txt files to detect and accept the change.
- name: Run API Approval Tests
continue-on-error: true
run: dotnet test ${{ matrix.slnf }} -c Release --no-build --nologo --filter "FullyQualifiedName~ApiApprovalTests"
Copy link
Copy Markdown

@cursor cursor Bot May 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

API test failures ignored

Medium Severity

The API approval test step uses continue-on-error, so run-api-tests still succeeds when dotnet test fails without producing *.received.txt. No later step checks that outcome, so the workflow can pass while ApiApprovalTests actually failed for non-snapshot reasons.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit dc60272. Configure here.

Copy link
Copy Markdown
Collaborator Author

@jamescrosswell jamescrosswell May 20, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fine. If that happens we'll know about it from the build workflow that is running concurrently. This workflow isn't intended to replace the full build - it's just a convenience so devs don't have to build the solution and run it on multiple operating systems every time one of the verify API files changes.

We're not actually running the tests to see if they pass or fail here - we run them simply as a way to get the latest verify files.


Check warning on line 72 in .github/workflows/verify-api.yml

View check run for this annotation

@sentry/warden / warden: find-bugs

`continue-on-error: true` makes a total test-run failure indistinguishable from a clean API check 

If `dotnet test` fails for any reason other than snapshot mismatches (e.g. build error in the test project, missing workload, dotnet crash), no `*.received.txt` files are produced. `accept-api-changes` then finds no artifacts, commits nothing, and exits green — silently hiding the fact that the API tests never ran.
- name: Upload Received API Files
if: ${{ always() }}
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
with:
name: api-verify-received-${{ matrix.rid }}
path: "**/*.received.txt"
if-no-files-found: ignore

accept-api-changes:
name: Accept and Commit API Changes
needs: run-api-tests
runs-on: ubuntu-22.04
Comment thread
jamescrosswell marked this conversation as resolved.
if: github.event.pull_request.head.repo.full_name == github.repository
permissions:
contents: write
pull-requests: write

steps:
- name: Checkout
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
with:
ref: ${{ github.event.pull_request.head.ref }}

# When the matrix produces no received files (clean PR), no artifact is uploaded.
# download-artifact's pattern branch tolerates zero matches without erroring, so
# we don't need `continue-on-error` here — that would mask genuine download failures.
- name: Download Received API Files
uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1
with:
pattern: api-verify-received-*
merge-multiple: true
Comment thread
jamescrosswell marked this conversation as resolved.

Comment on lines +99 to +104
Copy link
Copy Markdown

@sentry sentry Bot May 20, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved in b011745

Bug: The accept-api-changes job may fail on PRs with no API changes because the download-artifact step errors when no API change artifacts are found to download.
Severity: MEDIUM

Suggested Fix

Add continue-on-error: true to the Download Received API Files step in the accept-api-changes job. This will prevent the workflow from failing when no API change artifacts are found. Apply the same fix to the equivalent step in the report-fork-api-changes job.

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/verify-api.yml#L94-L99

Potential issue: The `accept-api-changes` job includes a step to upload API verification
files, which uses `if-no-files-found: ignore`. Consequently, for pull requests without
any public API changes, no `api-verify-received-*` artifact is created. A subsequent
step attempts to download artifacts using the pattern `api-verify-received-*`. Since
`actions/download-artifact@v8.0.1` is expected to fail when a pattern matches no
artifacts and there is no `continue-on-error` configured, this job will likely fail on
every clean PR. The `report-fork-api-changes` job has the same vulnerability.

Also affects:

  • .github/workflows/verify-api.yml:148~151

Did we get this right? 👍 / 👎 to inform future reviews.

Copy link
Copy Markdown
Collaborator Author

@jamescrosswell jamescrosswell May 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in b011745

Copy link
Copy Markdown
Collaborator Author

@jamescrosswell jamescrosswell May 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted per #5228 (comment)

Looking at the source for the action it only throws an error if there are no files to download if either isDownloadByIds or isSingleArtifactDownload is true... so we don't need continue on error in the download step.

So the initial review comment was erroneous.

- name: Accept Verifier Changes
shell: pwsh
run: pwsh ./scripts/accept-verifier-changes.ps1
Comment thread
sentry[bot] marked this conversation as resolved.

- name: Detect API Changes
id: detect
shell: bash
run: |
if [[ -z "$(git status --porcelain)" ]]; then
echo "has_changes=false" >> "$GITHUB_OUTPUT"
echo "No API verifier changes detected."
else
echo "has_changes=true" >> "$GITHUB_OUTPUT"
echo "API verifier changes detected:"
git status --short
fi

- name: Commit Accepted API Changes
if: steps.detect.outputs.has_changes == 'true'
shell: bash
run: |
git config --global user.name 'Sentry Github Bot'
git config --global user.email 'bot+github-bot@sentry.io'
git add -A
git commit -m "Accept API verifier changes"
git push

- name: Label Public API PR
if: steps.detect.outputs.has_changes == 'true'
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
run: |
gh label create "public API" --color "0075ca" --description "Modifies the public API surface" --repo "${{ github.repository }}" 2>/dev/null || true
gh pr edit "${{ github.event.pull_request.number }}" --add-label "public API" --repo "${{ github.repository }}"

# Fork PRs can't be auto-committed since the bot can't push to a contributor's repo.
# Fail the check with guidance so the contributor accepts the changes locally.
report-fork-api-changes:
name: Report API Changes (Fork PR)
needs: run-api-tests
runs-on: ubuntu-22.04
if: github.event.pull_request.head.repo.full_name != github.repository
permissions:
contents: read

steps:
- name: Checkout
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2

- name: Download Received API Files
uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1
with:
pattern: api-verify-received-*
merge-multiple: true

- name: Accept Verifier Changes
shell: pwsh
run: pwsh ./scripts/accept-verifier-changes.ps1

- name: Fail If API Changes Detected
shell: bash
run: |
if [[ -n "$(git status --porcelain)" ]]; then
echo "::error::Public API changes detected. Please run the following locally and push the result:"
echo "::error:: dotnet test && pwsh ./scripts/accept-verifier-changes.ps1"
exit 1
fi
echo "No API verifier changes detected."
Loading