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
Open

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

Changes from 1 commit
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
139 changes: 139 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,139 @@
name: verify api
on:
pull_request:
paths:
- 'src/**'
- 'test/**/ApiApprovalTests*'
- 'test/Sentry.Testing/ApiExtensions.cs'
- '.github/workflows/verify-api.yml'

permissions:
contents: write
pull-requests: write
Comment thread
jamescrosswell marked this conversation as resolved.
Outdated

jobs:
run-api-tests:
name: Run API Approval Tests (${{ matrix.rid }})
runs-on: ${{ matrix.os }}

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"

@cursor cursor Bot May 20, 2026

Copy link
Copy Markdown

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.

@jamescrosswell jamescrosswell May 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

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.


- name: Upload Received API Files
if: ${{ always() }}
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
with:
name: api-verify-received-${{ matrix.rid }}
path: "**/*.received.*"
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.

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
Comment thread
jamescrosswell marked this conversation as resolved.

Comment on lines +99 to +104

@sentry sentry Bot May 20, 2026 •
edited
Loading

Copy link
Copy Markdown

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.

@jamescrosswell jamescrosswell May 20, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in b011745

@jamescrosswell jamescrosswell May 20, 2026

Copy link
Copy Markdown
Collaborator Author

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

# For fork PRs we can't push back to the contributor's branch, so we fail the check
# instead — prompting the contributor to accept the changes locally.
- name: Commit Accepted API Changes
if: steps.detect.outputs.has_changes == 'true'
shell: bash
run: |
if [[ "${{ github.event.pull_request.head.repo.full_name }}" != "${{ github.repository }}" ]]; 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
git config --global user.name 'Sentry Github Bot'
git config --global user.email 'bot+github-bot@sentry.io'
git fetch
git checkout "${GITHUB_HEAD_REF}"
git add -A
git commit -m "Accept API verifier changes"
git push --set-upstream origin "${GITHUB_HEAD_REF}"

- name: Label Public API PR
if: steps.detect.outputs.has_changes == 'true' && github.event.pull_request.head.repo.full_name == github.repository
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 }}"
Loading