From fb0d33c490c6cb83e0942a59f925ac5fb7ac8b6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Wed, 23 Sep 2026 14:56:59 -0400 Subject: [PATCH] fix(ci): scope catalog dispatch and fail closed on detection errors (#4347) * ci: scope catalog dispatch and fail closed on detection errors * fix(ci): distinguish full catalog validation dispatches --- .github/workflows/ci.yml | 57 ++++++++++++------ .github/workflows/regression.yml | 31 +++++++--- scripts/catalog-publication.test.mjs | 86 +++++++++++++++++++++++++++- scripts/publish-catalog.mjs | 16 +++++- 4 files changed, 159 insertions(+), 31 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ec2304bcfe..56e382fd9e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,4 +1,5 @@ name: CI +run-name: ${{ github.event_name == 'workflow_dispatch' && format('Checks (catalog_publish={0})', inputs.catalog_publish) || '' }} permissions: contents: read @@ -11,6 +12,11 @@ env: on: workflow_dispatch: + inputs: + catalog_publish: + description: "Run required checks for a generated catalog publication." + type: boolean + default: false pull_request: # `edited` is required so the workflow re-fires when a PR's base ref is # set back to `main` after a Graphite stack restack momentarily flips @@ -57,7 +63,7 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 2 outputs: - code: ${{ github.event_name == 'workflow_dispatch' || steps.filter.outputs.code == 'true' }} + code: ${{ (github.event_name == 'workflow_dispatch' && inputs.catalog_publish) && 'true' || steps.filter.outputs.code }} docs_catalog: ${{ steps.filter.outputs.docs_catalog }} cli: ${{ steps.filter.outputs.cli }} skills: ${{ steps.filter.outputs.skills }} @@ -1066,36 +1072,51 @@ jobs: if: github.event_name == 'pull_request' || github.event_name == 'workflow_dispatch' runs-on: ubuntu-latest timeout-minutes: 5 + env: + PR_TITLE_TYPES: | + feat + fix + docs + style + refactor + perf + test + build + ci + chore + revert steps: - - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - - id: pr + - if: github.event_name == 'pull_request' + uses: amannn/action-semantic-pull-request@e32d7e603df1aa1ba07e981f2a23455dee596825 # v5 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + with: + types: ${{ env.PR_TITLE_TYPES }} + - if: github.event_name == 'workflow_dispatch' + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + with: + persist-credentials: false + - if: github.event_name == 'workflow_dispatch' + id: pr run: node scripts/ci/resolve-workflow-pr.mjs env: GH_TOKEN: ${{ github.token }} - - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + - if: github.event_name == 'workflow_dispatch' + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 with: repository: amannn/action-semantic-pull-request ref: e32d7e603df1aa1ba07e981f2a23455dee596825 # v5 path: .semantic-pr-action persist-credentials: false - - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 + - if: github.event_name == 'workflow_dispatch' + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: 22 - - name: Validate PR title + - if: github.event_name == 'workflow_dispatch' + name: Validate PR title run: GITHUB_EVENT_PATH="$PR_EVENT_PATH" node .semantic-pr-action/dist/index.js env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} PR_EVENT_PATH: ${{ steps.pr.outputs.event_path }} INPUT_GITHUBBASEURL: ${{ github.api_url }} - INPUT_TYPES: | - feat - fix - docs - style - refactor - perf - test - build - ci - chore - revert + INPUT_TYPES: ${{ env.PR_TITLE_TYPES }} diff --git a/.github/workflows/regression.yml b/.github/workflows/regression.yml index bda7ece6b7..306c591818 100644 --- a/.github/workflows/regression.yml +++ b/.github/workflows/regression.yml @@ -1,4 +1,5 @@ name: regression +run-name: ${{ github.event_name == 'workflow_dispatch' && format('Checks (catalog_publish={0})', inputs.catalog_publish) || '' }} permissions: contents: read @@ -16,6 +17,11 @@ concurrency: on: workflow_dispatch: + inputs: + catalog_publish: + description: "Run required checks for a generated catalog publication." + type: boolean + default: false pull_request: # Re-run when Graphite changes only the PR base for the same head SHA. types: [opened, synchronize, reopened, edited] @@ -29,7 +35,7 @@ jobs: runs-on: ubuntu-latest timeout-minutes: 2 outputs: - code: ${{ github.event_name == 'workflow_dispatch' || steps.filter.outputs.code == 'true' }} + code: ${{ (github.event_name == 'workflow_dispatch' && inputs.catalog_publish) && 'true' || steps.filter.outputs.code }} matrix: ${{ steps.shards.outputs.matrix }} steps: # Force git-based change detection instead of the pull_request REST API. @@ -146,12 +152,23 @@ jobs: if: always() steps: - name: Check results + env: + CHANGES_RESULT: ${{ needs.changes.result }} + CODE_CHANGED: ${{ needs.changes.outputs.code }} + SHARDS_RESULT: ${{ needs.regression-shards.result }} run: | - if [ "${{ needs.changes.outputs.code }}" != "true" ]; then - echo "No code changes — skipping regression (auto-pass)" - exit 0 - fi - if [ "${{ needs.regression-shards.result }}" != "success" ]; then - echo "One or more regression shards failed" + if [ "$CHANGES_RESULT" != "success" ]; then + echo "::error::Change detection did not succeed." exit 1 fi + case "$CODE_CHANGED" in + false) echo "No regression code changes." ;; + true) + if [ "$SHARDS_RESULT" != "success" ]; then + echo "::error::Regression shards did not succeed." + exit 1 + fi + echo "All regression shards succeeded." + ;; + *) echo "::error::Change detection returned no valid decision."; exit 1 ;; + esac diff --git a/scripts/catalog-publication.test.mjs b/scripts/catalog-publication.test.mjs index 78b1fc6d04..92abc36032 100644 --- a/scripts/catalog-publication.test.mjs +++ b/scripts/catalog-publication.test.mjs @@ -1,9 +1,10 @@ import assert from "node:assert/strict"; -import { execFileSync } from "node:child_process"; +import { execFileSync, spawnSync } from "node:child_process"; import { mkdirSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import { test } from "node:test"; +import { parse } from "yaml"; import { catalogChanges, commitBatches } from "./publish-catalog.mjs"; import { GENERATED_CATALOG_PATHS } from "./catalog-generated-paths.mjs"; import { committedCatalogOutputs } from "./check-catalog-source-pr.mjs"; @@ -227,7 +228,16 @@ test("publication dispatches every required workflow after updating its branch a dispatches.map((call) => call.endpoint.split("/").at(-2)), ["ci.yml", "regression.yml", "windows-render.yml", "pr-captures.yml", "codeql.yml"], ); - for (const call of dispatches) assert.deepEqual(call.body, { ref: "bot/catalog-publish" }); + assert.deepEqual( + dispatches.map((call) => call.body), + [ + { ref: "bot/catalog-publish", inputs: { catalog_publish: true } }, + { ref: "bot/catalog-publish", inputs: { catalog_publish: true } }, + { ref: "bot/catalog-publish" }, + { ref: "bot/catalog-publish" }, + { ref: "bot/catalog-publish" }, + ], + ); assert.ok( calls.indexOf(dispatches[0]) > calls.findIndex((call) => call.endpoint.endsWith("/pulls/42") && call.method === "PATCH"), @@ -237,14 +247,84 @@ test("publication dispatches every required workflow after updating its branch a test("publication leaves existing successful workflows at the same head alone", (t) => { const fixture = publicationFixture(t, true, { PUBLISH_SUCCESS: "1", - WORKFLOW_RUNS: JSON.stringify([{ status: "completed", conclusion: "success" }]), + WORKFLOW_RUNS: JSON.stringify([ + { status: "completed", conclusion: "success", display_title: "Ordinary dispatch" }, + { + status: "completed", + conclusion: "success", + display_title: "Checks (catalog_publish=true)", + }, + ]), }); fixture.run(); assert.equal(fixture.calls().filter((call) => call.endpoint.endsWith("/dispatches")).length, 0); }); +test("ordinary dispatch success cannot suppress full publication checks", (t) => { + const fixture = publicationFixture(t, true, { + PUBLISH_SUCCESS: "1", + WORKFLOW_RUNS: JSON.stringify([ + { status: "completed", conclusion: "success", display_title: "Ordinary dispatch" }, + ]), + }); + fixture.run(); + assert.deepEqual( + fixture + .calls() + .filter((call) => call.endpoint.endsWith("/dispatches")) + .map((call) => call.endpoint.split("/").at(-2)), + ["ci.yml", "regression.yml"], + ); +}); + test("dispatch rejection fails publication and still cleans up the staging branch", (t) => { const fixture = publicationFixture(t, true, { PUBLISH_SUCCESS: "1", DISPATCH_FAIL: "1" }); assert.throws(fixture.run); assert.equal(fixture.calls().at(-1).method, "DELETE"); }); + +const regressionWorkflow = parse( + readFileSync(new URL("../.github/workflows/regression.yml", import.meta.url), "utf8"), +); +const summary = regressionWorkflow.jobs.regression.steps.find( + (step) => step.name === "Check results", +); + +function runSummary(changes, code, shards) { + return spawnSync("bash", ["-e", "-c", summary.run], { + encoding: "utf8", + env: { ...process.env, CHANGES_RESULT: changes, CODE_CHANGED: code, SHARDS_RESULT: shards }, + timeout: 5000, + }); +} + +test("regression fails when change detection did not finish successfully", () => { + for (const state of ["failure", "cancelled", "skipped"]) { + const result = runSummary(state, "false", "skipped"); + assert.equal(result.status, 1, result.stdout + result.stderr); + } +}); + +test("regression accepts an explicit successful no-change decision", () => { + const result = runSummary("success", "false", "skipped"); + assert.equal(result.status, 0, result.stdout + result.stderr); +}); + +test("regression rejects missing or malformed change decisions", () => { + for (const decision of ["", "unknown"]) { + const result = runSummary("success", decision, "success"); + assert.equal(result.status, 1, result.stdout + result.stderr); + } +}); + +test("regression code changes require every shard to succeed", () => { + for (const state of ["failure", "cancelled", "skipped"]) { + const result = runSummary("success", "true", state); + assert.equal(result.status, 1, result.stdout + result.stderr); + } +}); + +test("regression accepts completed shards after successful change detection", () => { + const result = runSummary("success", "true", "success"); + assert.equal(result.status, 0, result.stdout + result.stderr); +}); diff --git a/scripts/publish-catalog.mjs b/scripts/publish-catalog.mjs index c76259fbb8..3c73e5e97b 100644 --- a/scripts/publish-catalog.mjs +++ b/scripts/publish-catalog.mjs @@ -191,6 +191,12 @@ function checkAlreadyScheduled(run) { return run.status !== "completed" || run.conclusion === "success"; } +function publishCheckRequest(workflow) { + if (workflow === "ci.yml" || workflow === "regression.yml") + return { ref: BRANCH, inputs: { catalog_publish: true } }; + return { ref: BRANCH }; +} + function dispatchPublishChecks(repository) { const head = commitOid( api(`repos/${repository}/git/ref/heads/${BRANCH}`, "GET", undefined, ".object.sha"), @@ -203,11 +209,15 @@ function dispatchPublishChecks(repository) { "codeql.yml", ]) { const endpoint = `repos/${repository}/actions/workflows/${workflow}`; + const request = publishCheckRequest(workflow); const runs = JSON.parse( - api(`${endpoint}/runs?event=workflow_dispatch&head_sha=${head}&per_page=1`), + api(`${endpoint}/runs?event=workflow_dispatch&head_sha=${head}&per_page=100`), ); - if (checkAlreadyScheduled(runs.workflow_runs[0])) continue; - api(`${endpoint}/dispatches`, "POST", { ref: BRANCH }); + const latest = runs.workflow_runs.find( + (run) => !request.inputs || run.display_title === "Checks (catalog_publish=true)", + ); + if (checkAlreadyScheduled(latest)) continue; + api(`${endpoint}/dispatches`, "POST", request); } }