From b5f568004f13b8dba96d8eb5ec20b14bbbb0ed33 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jonas=20Flod=C3=A9n?= <jonas@floden.nu>
Date: Mon, 6 Jul 2026 09:29:01 +0200
Subject: [PATCH] fix(ci): fail loud when the compliance diff artifact is
 missing (#832)

Follow-up to #830: the review script now throws if DIFF_FILE is set but the artifact file is missing, instead of silently reviewing a base-vs-base diff and posting a misleading 'No diff detected' comment. The fallback filename parser also captures deleted files, and the Bedrock job gets a 10-minute timeout.
---
 .../workflows/swedish-compliance-review.yml   |  3 +++
 scripts/swedish-compliance-review.mjs         | 25 ++++++++++++++-----
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/.github/workflows/swedish-compliance-review.yml b/.github/workflows/swedish-compliance-review.yml
index 7c530c61..27ca3d06 100644
--- a/.github/workflows/swedish-compliance-review.yml
+++ b/.github/workflows/swedish-compliance-review.yml
@@ -11,6 +11,8 @@ name: Swedish Accounting Compliance Review
 
 on:
   workflow_run:
+    # Must match the `name:` field in swedish-compliance-diff.yml exactly.
+    # A rename there silently stops this trigger from firing on all subsequent PRs.
     workflows: ["Compliance diff"]
     types: [completed]
 
@@ -22,6 +24,7 @@ permissions:
 jobs:
   review:
     runs-on: ubuntu-latest
+    timeout-minutes: 10
     # Only act on PR-triggered diffs that actually produced an artifact.
     if: >
       github.event.workflow_run.event == 'pull_request' &&
diff --git a/scripts/swedish-compliance-review.mjs b/scripts/swedish-compliance-review.mjs
index 682998bc..550a29cd 100644
--- a/scripts/swedish-compliance-review.mjs
+++ b/scripts/swedish-compliance-review.mjs
@@ -43,17 +43,30 @@ function getDiff() {
   // secrets and handed to us as an artifact. We read it as DATA: we never run
   // fork code here. See .github/workflows/swedish-compliance-{diff,review}.yml.
   const diffFile = process.env.DIFF_FILE;
-  if (diffFile && existsSync(diffFile)) {
+  if (diffFile) {
+    // Fail loud: if DIFF_FILE is set but missing, the artifact download failed.
+    // Falling through to the legacy git path would produce an empty diff (stage-2
+    // checkout is the base repo HEAD, not the PR head) and post "No diff detected"
+    // as a misleading green signal.
+    if (!existsSync(diffFile)) {
+      throw new Error(
+        `DIFF_FILE is set to "${diffFile}" but the file does not exist: artifact download likely failed`,
+      );
+    }
     const raw = readFileSync(diffFile, 'utf8');
     const filesFile = process.env.FILES_FILE;
     const files =
       filesFile && existsSync(filesFile)
         ? readFileSync(filesFile, 'utf8').trim()
-        : raw
-            .split('\n')
-            .filter((l) => l.startsWith('+++ b/'))
-            .map((l) => l.slice('+++ b/'.length))
-            .join('\n');
+        : // Fallback: infer filenames from diff headers. Capture both +++ b/ (added/modified)
+          // and --- a/ (deleted) so delete-only PRs aren't silently omitted.
+          Array.from(
+            raw.split('\n').reduce((set, l) => {
+              if (l.startsWith('+++ b/')) set.add(l.slice('+++ b/'.length));
+              else if (l.startsWith('--- a/')) set.add(l.slice('--- a/'.length));
+              return set;
+            }, new Set()),
+          ).join('\n');
     return { files, ...truncate(raw) };
   }