fix(rees): gate the expected-commit check on SENTRY_REQUIRE_COMMITS#3431
Conversation
validateSentryRelease's "release commits do not include expected commit" check read only config.expectedCommitSha, never config.requireCommits, so it fired regardless of strict mode. upload-sourcemaps.ts always passes SENTRY_COMMIT_SHA (the deploy's actual git SHA, not itself a strictness signal), so this check was effectively always enforced even when strict was explicitly off, causing recurring release-validation failures in non-strict deploys (Sentry GITTENSORY-X).
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
|
Tip 🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩 ✅ Gittensory review result - approve/merge recommendedReview updated: 2026-07-05 07:27:52 UTC
✅ Suggested Action - Approve/Merge
Review summary Nits — 4 non-blocking
Review context
Contributor next steps
Signal definitions
🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.
|
…dation The prior fix only gated the failure conditions on requireCommits, leaving the outer block's fetch to /commits/ conditioned on `requireCommits || expectedCommitSha`. Since expectedCommitSha is essentially always set (upload-sourcemaps.ts always passes the deploy's actual git SHA), a non-strict deploy still depended on the commits endpoint succeeding even though nothing would ever fail from its result. Gate the fetch on requireCommits alone.
Summary
Error: Sentry release validation failed (1): ...["release commits do not include expected commit ..."], culpritrunReleaseValidation(upload-sourcemaps).review-enrichment/scripts/validate-sentry-release.mjs'svalidateSentryReleasehas two commit-related checks gated onconfig.requireCommits(mirroringSENTRY_REQUIRE_COMMITS/thestrictdeploy flag): "release has no associated commits" correctly checksconfig.requireCommits, but the sibling "release commits do not include expected commit" check only testedconfig.expectedCommitSha— neverconfig.requireCommits. Sinceupload-sourcemaps.tsalways passesSENTRY_COMMIT_SHA(the deploy's actual git SHA — not itself a strictness signal) regardless of thestrictflag,expectedCommitShais essentially always set, so this specific check fired unconditionally even on a deploy that explicitly requested non-strict validation (SENTRY_REQUIRE_COMMITS=false), causing spurious release-validation failures.config.requireCommits &&guard already used by the sibling check, so both commit-related checks consistently respect strict mode.Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Validation
git diff --checknpm run typecheck(repo root, clean)node --test --experimental-strip-types test/sentry-release-validation.test.ts(run fromreview-enrichment/) — 5/5 passing, including the new regression testnpm run test:workers/npm run build:mcp/npm run test:mcp-pack/npm run ui:openapi:check/npm run ui:build— not run individually this PR; this change is entirely withinreview-enrichment/, a separately-deployed sub-package with its own test runner, and doesn't touch the Worker/MCP/UI/OpenAPI surface.SENTRY_REQUIRE_COMMITS=false), which now resolves successfully instead of throwing; the existing strict-mode test (mismatched commit SHA with defaultrequireCommits: true) is untouched and still correctly rejects.Safety
UI Evidencesection below. — N/A, no visible UI change.Notes
safeCodeSpanTypeError, and codex hang-detection issues found in the same pass.