fix(rees): gate the expected-commit check on SENTRY_REQUIRE_COMMITS

review-enrichment/scripts/validate-sentry-release.mjs

@@ -197,7 +197,13 @@ export async function validateSentryRelease(env = process.env, fetchImpl = globa
   }
 
   let commits = [];
-  if (config.requireCommits || config.expectedCommitSha) {
+  // Gated on requireCommits ALONE (not `|| config.expectedCommitSha`): upload-sourcemaps.ts always passes
+  // SENTRY_COMMIT_SHA (the deploy's actual git SHA, not itself a strictness signal), so expectedCommitSha is
+  // essentially always set -- fetching here whenever it was merely present, independent of requireCommits, meant
+  // a non-strict deploy still depended on the /commits/ endpoint being reachable (sentryJson throws on any non-OK
+  // response) even though the checks that consume the result are now all requireCommits-gated below. Skipping the
+  // fetch entirely in non-strict mode is the only way "non-strict" actually means "commits don't matter."
+  if (config.requireCommits) {
     commits = asArray(
       await sentryJson(
         config,
@@ -211,7 +217,7 @@ export async function validateSentryRelease(env = process.env, fetchImpl = globa
       ...commitIdsFrom(release),
       ...commits.flatMap((commit) => commitIdsFrom(commit)),
     ];
-    if (config.requireCommits && commitCount <= 0 && commitIds.length === 0) {
+    if (commitCount <= 0 && commitIds.length === 0) {
       failures.push("release has no associated commits");
     }
     if (config.expectedCommitSha && !commitMatches(config.expectedCommitSha, commitIds)) {

review-enrichment/test/sentry-release-validation.test.ts

@@ -119,6 +119,57 @@ test("validateSentryRelease rejects a release missing the expected commit", asyn
   );
 });
 
+test("REGRESSION: validateSentryRelease does NOT enforce the expected-commit match when SENTRY_REQUIRE_COMMITS=false", async () => {
+  // Same mismatched-commit fixture as the strict-mode test above ("def456" vs the expected "abc123"), but with
+  // requireCommits off (upload-sourcemaps.ts's non-strict deploy path: SENTRY_REQUIRE_COMMITS: fields.strict ?
+  // "true" : "false"). SENTRY_COMMIT_SHA is still passed unconditionally (it's the deploy's actual git SHA, not
+  // itself a strictness signal), so expectedCommitSha stays set -- the bug this guards was that the match check
+  // read only `config.expectedCommitSha`, never `config.requireCommits`, so it fired regardless of strict mode.
+  const calls: string[] = [];
+  const fetchImpl = async (input: string | URL | Request): Promise<Response> => {
+    const path = new URL(String(input)).pathname;
+    calls.push(path);
+    if (path.endsWith("/commits/")) return response([{ id: "def456" }]);
+    if (path.endsWith("/deploys/")) return response([{ name: "deploy-1", environment: "production" }]);
+    return response({
+      version: "gittensory-rees@abc123",
+      dateReleased: "2026-06-29T00:00:00Z",
+      commitCount: 1,
+      deployCount: 1,
+      projects: [{ slug: "gittensory" }],
+    });
+  };
+
+  const result = await validateSentryRelease(validationEnv({ SENTRY_REQUIRE_COMMITS: "false" }), fetchImpl);
+  assert.equal(result.release, "gittensory-rees@abc123");
+  // Non-strict mode must not even CALL the commits endpoint -- confirmed below with a second regression pinning
+  // this exact call-skip, since a real Sentry API hiccup on /commits/ would otherwise still fail a non-strict
+  // deploy (sentryJson throws on any non-OK response) even though no commit check would run on the result.
+  assert.equal(calls.includes("/api/0/organizations/jsonbored/releases/gittensory-rees%40abc123/commits/"), false);
+});
+
+test("REGRESSION: validateSentryRelease never calls the commits endpoint at all when SENTRY_REQUIRE_COMMITS=false, even if that endpoint is unhealthy", async () => {
+  // The specific gap the AI reviewer caught on the first pass of this fix: gating only the FAILURE pushes on
+  // requireCommits, while leaving the fetch itself conditioned on `requireCommits || expectedCommitSha`, meant a
+  // non-strict deploy still depended on /commits/ succeeding even though nothing would ever fail from its result.
+  // Prove it directly: the endpoint returns a hard 500, and non-strict validation must still succeed.
+  const fetchImpl = async (input: string | URL | Request): Promise<Response> => {
+    const path = new URL(String(input)).pathname;
+    if (path.endsWith("/commits/")) return response({ detail: "internal error" }, 500);
+    if (path.endsWith("/deploys/")) return response([{ name: "deploy-1", environment: "production" }]);
+    return response({
+      version: "gittensory-rees@abc123",
+      dateReleased: "2026-06-29T00:00:00Z",
+      commitCount: 1,
+      deployCount: 1,
+      projects: [{ slug: "gittensory" }],
+    });
+  };
+
+  const result = await validateSentryRelease(validationEnv({ SENTRY_REQUIRE_COMMITS: "false" }), fetchImpl);
+  assert.equal(result.release, "gittensory-rees@abc123");
+});
+
 test("validateSentryRelease rejects a release missing the required deploy", async () => {
   const fetchImpl = async (input: string | URL | Request): Promise<Response> => {
     const path = new URL(String(input)).pathname;