Skip to content

feat(snapshots): Add download command for baseline snapshots#3310

Merged
NicoHinderling merged 2 commits into
masterfrom
feat/snapshots-download
May 27, 2026
Merged

feat(snapshots): Add download command for baseline snapshots#3310
NicoHinderling merged 2 commits into
masterfrom
feat/snapshots-download

Conversation

@NicoHinderling
Copy link
Copy Markdown
Contributor

@NicoHinderling NicoHinderling commented May 21, 2026

Summary

  • Adds sentry-cli snapshots download command for downloading baseline snapshot images from Sentry's preprod system to a local directory
  • Supports --app-id (resolves latest baseline snapshot) or --snapshot-id (direct artifact ID), with optional --branch filter
  • Downloads a ZIP from the server and extracts images to --output directory (default ./snapshots-base/)
  • Adds two API methods: get_latest_base_snapshot and download_snapshot_zip

Usage

# Download latest baselines for an app
sentry-cli snapshots download --app-id sentry-frontend

# Download from a specific branch
sentry-cli snapshots download --app-id sentry-frontend --branch main

# Download a specific snapshot by ID
sentry-cli snapshots download --snapshot-id 259299

# Full local visual regression workflow
sentry-cli snapshots download --app-id sentry-frontend
pnpm run snapshots
sentry-cli snapshots diff ./snapshots-base .artifacts/snapshots --fail-on-diff

Test plan

  • cargo clippy -- -Dwarnings passes clean
  • cargo fmt --check passes
  • cargo test snapshots — all 4 integration tests pass (including new download help test)
  • Manual test: run sentry-cli snapshots download --app-id sentry-frontend against real Sentry org

🤖 Generated with Claude Code

@NicoHinderling NicoHinderling requested review from a team and szokeasaurusrex as code owners May 21, 2026 19:15
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 21, 2026

Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 0335299

Comment thread src/commands/snapshots/download.rs Outdated
Comment thread src/commands/snapshots/download.rs Outdated
Comment thread src/commands/snapshots/download.rs Outdated
@NicoHinderling NicoHinderling force-pushed the feat/snapshots-download branch from f391865 to affb90b Compare May 21, 2026 20:32
Comment thread src/api/mod.rs
Comment thread src/api/mod.rs
@NicoHinderling NicoHinderling force-pushed the feat/snapshots-download branch from affb90b to 8783c35 Compare May 21, 2026 20:42
Comment thread src/api/mod.rs
@NicoHinderling NicoHinderling force-pushed the feat/snapshots-download branch from 8783c35 to a218cba Compare May 21, 2026 21:00
Copy link
Copy Markdown

@cameroncooke cameroncooke left a comment

Choose a reason for hiding this comment

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

sentry-cli snapshots download returns 404 when using an org auth token

The snapshots download command fails with No baseline snapshot found even when a base snapshot clearly exists in the UI.

Root cause: The endpoint GET /api/0/organizations/{org}/preprodartifacts/snapshots/latest-base/ calls get_filter_params(), which — when no project= query param is supplied — falls back to "projects the caller is a member of." Org auth tokens (sntrys_*) aren't associated with a user, so they have no project membership, and the call short-circuits to NoProjects → 404. The user never sees a distinct error; it looks identical to "snapshot doesn't exist."

Confirmed: Adding &project=<id> to the same request returns the snapshot successfully — org tokens satisfy has_project_access via their scopes, just not the membership path.

Fix options:

  1. CLI side: Add a --project flag to sentry-cli snapshots download and forward it as project=<id>. Matches
    the convention used by build upload and other commands. Smallest change, works in CI.
  2. Server side: When no project= is supplied, fall back to scoping by organization_id only (the endpoint already orders by -date_added and filters by app_id). Risk: app_id collisions across projects within an org become possible.

@cameroncooke
Copy link
Copy Markdown

Just wondering if we can add support to include JSON sidecar files too?

@NicoHinderling
Copy link
Copy Markdown
Contributor Author

-o ./snapshots-base/

thanks for flagging this, i've addressed this now

@NicoHinderling
Copy link
Copy Markdown
Contributor Author

Just wondering if we can add support to include JSON sidecar files too?

I think lets hold off on this for now, since this use case is pretty specific for local diffing

Comment thread src/utils/odiff/binary.rs Outdated
@NicoHinderling NicoHinderling force-pushed the feat/snapshots-download branch from 730c411 to 9ab26e1 Compare May 26, 2026 17:59
Copy link
Copy Markdown
Contributor Author

NicoHinderling commented May 26, 2026

Comment thread src/commands/snapshots/download.rs Outdated
@NicoHinderling NicoHinderling force-pushed the feat/snapshots-download branch from e66e51c to e790a06 Compare May 26, 2026 18:04
@NicoHinderling NicoHinderling force-pushed the feat/snapshots-download branch 4 times, most recently from f6c00bd to a5b4084 Compare May 26, 2026 23:56
Copy link
Copy Markdown
Member

@szokeasaurusrex szokeasaurusrex left a comment

Choose a reason for hiding this comment

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

mostly looks good, but please address the comments about tempfile before merging.

This should also be reviewed by someone from Emerge to make sure that the endpoints and any logic specific to the snapshots features are correct, I mainly checked the general structure.

Comment thread src/commands/snapshots/download.rs Outdated
Comment thread Cargo.toml Outdated
@NicoHinderling NicoHinderling requested a review from a team as a code owner May 27, 2026 14:32
@NicoHinderling NicoHinderling force-pushed the feat/snapshots-diff branch 2 times, most recently from cf94ea1 to a70417b Compare May 27, 2026 14:43
@NicoHinderling NicoHinderling force-pushed the feat/snapshots-download branch from a5b4084 to 97998d3 Compare May 27, 2026 14:43
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 97998d3. Configure here.

Comment thread Cargo.toml Outdated
Copy link
Copy Markdown

@cameroncooke cameroncooke left a comment

Choose a reason for hiding this comment

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

LFG!

Copy link
Copy Markdown
Member

@szokeasaurusrex szokeasaurusrex left a comment

Choose a reason for hiding this comment

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

Looks good, though I only skimmed the changes, as they are mostly related to the Emerge functionality. Please have someone from the Emerge team also review and approve especially the downloading logic before merging

Edit: looks like @cameroncooke already approved 😄

Comment thread tests/integration/_cases/snapshots/snapshots-download-help.trycmd
Comment thread tests/integration/snapshots.rs
@cameroncooke
Copy link
Copy Markdown

cameroncooke commented May 27, 2026

@NicoHinderling this might be an issue:

When --project is omitted (and SENTRY_PROJECT / .sentryclirc don't supply one either), the CLI hits latest-base with
no project filter. The server then:

  1. Searches the entire org for snapshots matching app_id.
  2. Picks the single most recent one (.order_by("-date_added").first()).
  3. Then checks has_project_access on whichever project that snapshot happened to belong to.
  4. If you don't have access to that project → 404, with no fallback to older snapshots in projects you can access.

So you can be perfectly entitled to a baseline snapshot, with one sitting in a project you own, and still get "no
baseline snapshot found" — purely because some other team in your org uploaded a newer one for the same app_id in a project you're not on.

It's also possible that an iOS and Android project exists with same app id, but you only have project access to one of them.

@NicoHinderling NicoHinderling force-pushed the feat/snapshots-download branch from 97998d3 to c71df16 Compare May 27, 2026 15:59
@NicoHinderling
Copy link
Copy Markdown
Contributor Author

4. If you don't have access to that project → 404, with no fallback to older snapshots in projects you can access.

i recall this concern coming up in my own personal reviews and don't think it's that common of a case, so i'm opting to ignore this for now until a user reports hitting it

Base automatically changed from feat/snapshots-diff to master May 27, 2026 16:02
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@NicoHinderling NicoHinderling force-pushed the feat/snapshots-download branch from c71df16 to a33edf2 Compare May 27, 2026 16:03
Removes the `tar` crate from runtime dependencies — it was added but
never used anywhere in the source. Only `zip::ZipArchive` is needed
for snapshot extraction.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@NicoHinderling NicoHinderling merged commit 48c1f24 into master May 27, 2026
29 checks passed
@NicoHinderling NicoHinderling deleted the feat/snapshots-download branch May 27, 2026 16:30
@NicoHinderling
Copy link
Copy Markdown
Contributor Author

  1. If you don't have access to that project → 404, with no fallback to older snapshots in projects you can access.

i recall this concern coming up in my own personal reviews and don't think it's that common of a case, so i'm opting to ignore this for now until a user reports hitting it

just kidding im a great guy and did it after all getsentry/sentry#116319

NicoHinderling added a commit to getsentry/sentry-docs that referenced this pull request May 27, 2026
#17903)

## DESCRIBE YOUR PR

Documents the new `sentry-cli snapshots download` and `sentry-cli
snapshots diff` commands added in
[sentry-cli#3306](getsentry/sentry-cli#3306) and
[sentry-cli#3310](getsentry/sentry-cli#3310).

- New "Local Testing" product docs page covering the workflow: download
baselines → run tests → diff locally
- CLI reference sections for `snapshots download` and `snapshots diff`
with flag tables

## IS YOUR CHANGE URGENT?

Help us prioritize incoming PRs by letting us know when the change needs
to go live.
- [ ] Urgent deadline (GA date, etc.):
- [x] Other deadline:
- [ ] None: Not urgent, can wait up to 1 week+

## SLA

- Teamwork makes the dream work, so please add a reviewer to your PRs.
- Please give the docs team up to 1 week to review your PR unless you've
added an urgent due date to it.
Thanks in advance for your help!

## PRE-MERGE CHECKLIST

*Make sure you've checked the following before merging your changes:*

- [ ] Checked Vercel preview for correctness, including links
- [ ] PR was reviewed and approved by any necessary SMEs (subject matter
experts)
- [ ] PR was reviewed and approved by a member of the [Sentry docs
team](https://github.com/orgs/getsentry/teams/docs)

---------

Co-authored-by: Claude <noreply@anthropic.com>
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.

3 participants