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.
This commit is contained in:
@@ -11,6 +11,8 @@ name: Swedish Accounting Compliance Review
|
|||||||
|
|
||||||
on:
|
on:
|
||||||
workflow_run:
|
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"]
|
workflows: ["Compliance diff"]
|
||||||
types: [completed]
|
types: [completed]
|
||||||
|
|
||||||
@@ -22,6 +24,7 @@ permissions:
|
|||||||
jobs:
|
jobs:
|
||||||
review:
|
review:
|
||||||
runs-on: ubuntu-latest
|
runs-on: ubuntu-latest
|
||||||
|
timeout-minutes: 10
|
||||||
# Only act on PR-triggered diffs that actually produced an artifact.
|
# Only act on PR-triggered diffs that actually produced an artifact.
|
||||||
if: >
|
if: >
|
||||||
github.event.workflow_run.event == 'pull_request' &&
|
github.event.workflow_run.event == 'pull_request' &&
|
||||||
|
|||||||
@@ -43,17 +43,30 @@ function getDiff() {
|
|||||||
// secrets and handed to us as an artifact. We read it as DATA: we never run
|
// 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.
|
// fork code here. See .github/workflows/swedish-compliance-{diff,review}.yml.
|
||||||
const diffFile = process.env.DIFF_FILE;
|
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 raw = readFileSync(diffFile, 'utf8');
|
||||||
const filesFile = process.env.FILES_FILE;
|
const filesFile = process.env.FILES_FILE;
|
||||||
const files =
|
const files =
|
||||||
filesFile && existsSync(filesFile)
|
filesFile && existsSync(filesFile)
|
||||||
? readFileSync(filesFile, 'utf8').trim()
|
? readFileSync(filesFile, 'utf8').trim()
|
||||||
: raw
|
: // Fallback: infer filenames from diff headers. Capture both +++ b/ (added/modified)
|
||||||
.split('\n')
|
// and --- a/ (deleted) so delete-only PRs aren't silently omitted.
|
||||||
.filter((l) => l.startsWith('+++ b/'))
|
Array.from(
|
||||||
.map((l) => l.slice('+++ b/'.length))
|
raw.split('\n').reduce((set, l) => {
|
||||||
.join('\n');
|
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) };
|
return { files, ...truncate(raw) };
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user