fix(sentry): include Rust source bundles, align release tag, add deploy markers#973
fix(sentry): include Rust source bundles, align release tag, add deploy markers#973CodeGhost21 wants to merge 2 commits into
Conversation
…oy markers
The Sentry release infrastructure was almost complete, but three gaps in
`scripts/upload_sentry_symbols.sh` + `release.yml` made the symbols upload
miss the very thing that made events useful for debugging:
- `sentry-cli upload-dif` ran without `--include-sources`, so Sentry had
the DIFs to symbolicate frame addresses but no `.src.zip` bundle to
render surrounding source. This is the "missing source code" symptom
the issue comment called out for Rust panics.
- The script hard-coded `release_name="openhuman@${version}"` while every
event the frontend + core actually emit reports
`openhuman@<version>+<sha>`. Releases must match exactly, so DIFs were
attaching to a release no event ever referenced.
- No `sentry-cli releases deploys ... new` call, so the release page
never linked deploys per environment (an issue acceptance criterion).
Changes:
- `scripts/upload_sentry_symbols.sh`: honor `SENTRY_RELEASE` from env,
add `--include-sources` to `upload-dif`, and emit a deploy marker when
`SENTRY_ENVIRONMENT` is set. All idempotent on re-run.
- `.github/workflows/release.yml`: pass `SENTRY_RELEASE` (canonical
`openhuman@<version>+<sha>`) and `SENTRY_ENVIRONMENT` (derived from
`inputs.build_target`) to the symbols-upload step.
- `scripts/ci-secrets.example.json`: document the Sentry secret + vars
the workflow already references so a fresh CI setup is reproducible.
- `docs/sentry.md`: document `--include-sources`, the deploy marker, the
new env vars, and add troubleshooting bullets for missing source
context / mismatched release tags.
Smoke-tested locally with a `sentry-cli` stub: canonical release name
threads through, `--include-sources` lands in `upload-dif` args, deploy
marker fires when env is set and is silently skipped otherwise.
Refs tinyhumansai#405.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughCI release workflow now computes a 12-char Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant Script as upload_sentry_symbols.sh
participant CLI as sentry-cli
participant SentryAPI as Sentry API
GH->>GH: prepare-build -> compute `short_sha`
GH->>GH: set env `SENTRY_RELEASE = openhuman@<version>+<short_sha>`
GH->>Script: invoke upload step (env: SENTRY_RELEASE, SENTRY_ENVIRONMENT?)
Script->>CLI: sentry-cli releases new / set-commits (release = SENTRY_RELEASE)
Script->>CLI: sentry-cli upload-dif --include-sources
CLI->>SentryAPI: POST /releases, /releases/{rel}/commits, /projects/.../debug-files
Script->>CLI: sentry-cli releases finalize
alt SENTRY_ENVIRONMENT is set
Script->>CLI: sentry-cli releases deploys new -e SENTRY_ENVIRONMENT
CLI->>SentryAPI: POST /releases/{rel}/deploys
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release.yml:
- Around line 554-559: The SENTRY_RELEASE environment value currently uses the
full 40-char SHA (needs.prepare-build.outputs.sha) which mismatches truncated
12-char SHAs used elsewhere; update the SENTRY_RELEASE assignment so the SHA is
truncated to 12 characters (e.g., replace uses of
needs.prepare-build.outputs.sha with a 12-char slice/substring like
.substring(0,12) or .slice(0,12)) so SENTRY_RELEASE (the SENTRY_RELEASE
variable) matches the frontend and Rust truncation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f7c7b872-ebeb-4d89-b384-8ed47d406a96
📒 Files selected for processing (4)
.github/workflows/release.ymldocs/sentry.mdscripts/ci-secrets.example.jsonscripts/upload_sentry_symbols.sh
config.ts, vite.config.ts, and main.rs all take the first 12 chars of VITE_BUILD_SHA / OPENHUMAN_BUILD_SHA when computing the canonical release tag at runtime, but the workflow was passing the full 40-char SHA into SENTRY_RELEASE for both the Vite/Tauri build step and the Rust symbols upload step. The vite plugin and upload script use SENTRY_RELEASE raw when set, so artifacts were attaching to openhuman@<v>+<40char> while events emitted openhuman@<v>+<12char>. Different releases — the upload half of tinyhumansai#973's parity fix was incomplete. Add a `short_sha` output to prepare-build (sliced to 12 chars in the resolve step) and reference it from both SENTRY_RELEASE constructions. Leaves VITE_BUILD_SHA / OPENHUMAN_BUILD_SHA passing the full SHA so the runtime "always slice to 12" invariant stays a single source of truth. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
graycyrus
left a comment
graycyrus
left a comment
There was a problem hiding this comment.
Three real gaps addressed cleanly — the --include-sources fix and the release-tag alignment are the load-bearing changes and both look correct. A few things worth fixing before merge.
The deploy marker fires once per invocation of upload_sentry_symbols.sh, but the loop in release.yml calls the script twice per matrix target (once for the core deps dir, once for the tauri deps dir). On a 3-platform matrix that's up to 6 deploy records for one release. The idempotency claim in the script comment and doc is also wrong: sentry-cli releases deploys new creates a new entry every call — it does not deduplicate by (release, env). Separately, ci-secrets.example.json now has both the old key (OPENHUMAN_SENTRY_DSN) and the new replacement (OPENHUMAN_CORE_SENTRY_DSN) without any annotation, which will confuse whoever provisions the secrets next.
| # links commits → deploys (issue #405 acceptance criterion). Idempotent | ||
| # for the same (release, env) pair. | ||
| if [[ -n "${SENTRY_ENVIRONMENT:-}" ]]; then | ||
| log_info "Recording deploy marker for environment: ${SENTRY_ENVIRONMENT}" |
There was a problem hiding this comment.
[major] The comment says this is idempotent for the same (release, env) pair, but sentry-cli releases deploys new creates a new deploy record on every call — the Sentry API assigns a unique deploy ID each time and does not skip existing entries for the same env name. More importantly, the calling loop in release.yml runs this script twice per matrix target (once for target/${MATRIX_TARGET}/release/deps, once for app/src-tauri/target/${MATRIX_TARGET}/release/deps), so a 3-platform matrix produces up to 6 deploy entries for one release from a single CI run.
The deploy marker should fire exactly once per release, not once per deps directory. Simplest fix: remove the deploy block from upload_symbols() and move it into the workflow after the loop finishes:
# In the 'run' block of the upload step, after the for-loop:
if [[ -n "${SENTRY_ENVIRONMENT:-}" ]]; then
echo "==> Recording deploy marker for ${SENTRY_ENVIRONMENT}"
sentry-cli releases deploys "${SENTRY_RELEASE}" new \
-e "${SENTRY_ENVIRONMENT}" || echo "[WARN] deploy marker failed (non-fatal)"
fiAnd drop the deploy block from upload_symbols() entirely.
| "VITE_BACKEND_URL": "https://localhost:5005", | ||
| "VITE_SKILLS_GITHUB_REPO": "", | ||
| "OPENHUMAN_SENTRY_DSN": "", | ||
| "VITE_SENTRY_DSN": "", |
There was a problem hiding this comment.
[major] The file now has both OPENHUMAN_SENTRY_DSN (the old key, kept from before this PR) and the new OPENHUMAN_CORE_SENTRY_DSN added below. These refer to the same underlying secret — release.yml at line 479 reads vars.OPENHUMAN_CORE_SENTRY_DSN and passes it as the env var OPENHUMAN_SENTRY_DSN into the cargo build. Having both in the example will cause a fresh provisioner to set both, only one of which matters.
The stale OPENHUMAN_SENTRY_DSN and VITE_SENTRY_DSN entries should be removed (superseded by OPENHUMAN_CORE_SENTRY_DSN and OPENHUMAN_REACT_SENTRY_DSN respectively), or at least annotated with a comment explaining the alias so the duplication is intentional.
|
@CodeGhost21 bro pls reolsve these comments |
Brings forward the load-bearing changes from #973 needed to make the Tauri-shell + core-sidecar Sentry instrumentation actually render source context for Rust panics. Adapted to the per-project upload split introduced earlier in this branch (each project now gets its own step, each step now passes SENTRY_RELEASE). scripts/upload_sentry_symbols.sh - Honor SENTRY_RELEASE from env so DIFs attach to the canonical `openhuman@<version>+<sha>` release name the running binaries emit. Without this, DIFs uploaded to `openhuman@<version>` while events reported `openhuman@<version>+<sha>` — different release, no match. - Add `--include-sources` to `sentry-cli upload-dif` so a `.src.zip` bundle ships alongside the DWARF. Sentry then renders surrounding source lines in Rust stack traces instead of bare `function + 0xNNN`. - Local invocations without SENTRY_RELEASE still work — falls back to the bare-version tag. .github/workflows/release.yml - Pass SENTRY_RELEASE to both per-project debug-symbol upload steps (openhuman-core and openhuman-tauri), matching the value baked into the binaries via OPENHUMAN_BUILD_SHA. Refs #973 (script + workflow env subset; deploy markers, ci-secrets example, and sentry.md doc refresh intentionally not included). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@CodeGhost21 pls resolve the git conflicts |
|
Superseded by #1067 — opening a fresh PR straight off current This branch was originally forked from an older #1067 is built directly on current main and contains only the surgical Sentry changes:
Closing this in favor of #1067. |
|
Tip For best results, initiate chat on the files or code changes.
I'll go ahead and resolve my open comments on this PR since the work is continuing in [resolve] 🧠 Learnings used |
3 participants
Summary
sentry-cli upload-difnow runs with--include-sources, so Rust panics in Sentry render surrounding source lines instead of bare function names — the symptom flagged in [Observability] Configure Sentry release tracking and source maps #405's latest comment.openhuman@<version>+<sha>); previously they landed onopenhuman@<version>, a different release no event ever referenced.release.ymlnow emits asentry-cli releases deploys ... newmarker per environment, satisfying [Observability] Configure Sentry release tracking and source maps #405's "release page links commits/deploys" criterion.scripts/ci-secrets.example.jsonanddocs/sentry.mddocument the secret + vars the workflow already references and the new troubleshooting paths.Problem
Issue #405 already had most of the Sentry release wiring in place (frontend
Sentry.initwith release/environment,@sentry/vite-pluginsource-map upload, Rustsentry::initwith matching release tag, CI secrets threaded through the Tauri build step). Three small but load-bearing gaps were left inscripts/upload_sentry_symbols.sh+release.yml:sentry-cli upload-difwas called without--include-sources. Sentry had the DIFs to symbolicate frame addresses but no.src.zipbundle to render surrounding code — exactly the "missing source code" symptom called out by@Al629176.release_name=\"openhuman@\${version}\", but every event the frontend (SENTRY_RELEASE) and core (build_release_tag()) emit isopenhuman@<version>+<sha>. DIFs were attaching to a different release than events.releases new+set-commits+finalizewere called, but neverreleases deploys ... new, so the Sentry release page never showed env-tagged deploys.Solution
scripts/upload_sentry_symbols.shSENTRY_RELEASEfrom env (falls back toopenhuman@<version>for local invocations so the CLI still works without CI env).--include-sourcesto theupload-difarg array. The CI runs from the workspace checkout, so the source paths embedded in DIFs resolve and.src.zipis bundled correctly.releases finalize, ifSENTRY_ENVIRONMENTis set, runsentry-cli releases deploys \"\$release_name\" new -e \"\$SENTRY_ENVIRONMENT\". Skipped silently when unset..github/workflows/release.yml(the symbols-upload step at L545–570)SENTRY_RELEASE: openhuman@<version>+<sha>(matches the canonical tag the Vite step at L520–522 already uses).SENTRY_ENVIRONMENTderived frominputs.build_target(staging/production) so deploy markers land on the right env.scripts/ci-secrets.example.json— add the Sentry secret + vars the workflow references but the example didn't list (SENTRY_AUTH_TOKEN,SENTRY_ORG,SENTRY_PROJECT,SENTRY_PROJECT_FRONTEND,OPENHUMAN_CORE_SENTRY_DSN,OPENHUMAN_REACT_SENTRY_DSN).docs/sentry.md— new "Rust debug symbols + source context" section documenting the upload-script lifecycle; refreshed CI configuration tables with the new env vars andSENTRY_PROJECTfor Rust DIFs; added verification steps (Rust source context renders, deploy markers visible) and troubleshooting bullets for the failure modes above.The script is idempotent on re-run:
releases newuses|| true,set-commitsuses--ignore-missing, andreleases deploys newdeduplicates per (release, env). Re-running CI on the same SHA is safe.Submission Checklist
sentry-clistub that the script (a) resolvesrelease_namefromSENTRY_RELEASEwhen set and falls back toopenhuman@<version>otherwise, (b) passes--include-sourcestoupload-dif, (c) emits thereleases deploys ... new -e <env>call whenSENTRY_ENVIRONMENTis set and skips it cleanly otherwise. Full end-to-end validation needs a staging release to confirm.src.zipartifacts attach to the canonical release in Sentry — see "Verification runbook" indocs/sentry.md.upload_sentry_symbols.shfor the three changes (release-name fallback,--include-sources, deploy marker), and inrelease.ymlfor the new env vars.Impact
.src.zip. Modest extra upload size (Rust source tree is small in practice) and one extrasentry-cliAPI call per matrix target for the deploy marker.function + 0xNNN. Frontend behavior unchanged. Release pages will show deploy markers per env.SENTRY_RELEASEandSENTRY_ENVIRONMENTare optional).Related
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Chores