From 70214a283964b6a1c8e68a508b97aca3188857a5 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Thu, 24 Sep 2026 22:20:40 -0400 Subject: [PATCH] ci: comment checks grade branches cut before their scripts existed --- .github/workflows/comments.yml | 28 ++++++++++++-- package.json | 2 +- scripts/comments-workflow.test.mjs | 59 ++++++++++++++++++++++++++++++ 3 files changed, 85 insertions(+), 4 deletions(-) create mode 100644 scripts/comments-workflow.test.mjs diff --git a/.github/workflows/comments.yml b/.github/workflows/comments.yml index f11c35f0a2..5c0a048b90 100644 --- a/.github/workflows/comments.yml +++ b/.github/workflows/comments.yml @@ -19,27 +19,49 @@ jobs: if: ${{ !startsWith(github.head_ref, 'release/') }} runs-on: ubuntu-latest timeout-minutes: 10 + env: + # Every repo script this job runs. They import each other, so all run from one tree. + COMMENT_SCRIPTS: >- + scripts/check-comment-citations.mjs scripts/check-comment-citations.test.mjs + scripts/comment-ratchet.mjs scripts/comment-ratchet.test.mjs steps: - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 with: # The PR's own head, graded against its merge-base: not the merge preview, which would # charge the branch for every comment main changed since it forked. ref: ${{ github.event.pull_request.head.sha }} + path: pr fetch-depth: 0 + # A branch cut before any of these scripts existed runs the base's copies. A branch that has + # them all runs its own, so a PR that fixes a check is graded by the fix. + - name: Pick the checks + run: | + dir=pr + for script in $COMMENT_SCRIPTS; do [ -f "pr/$script" ] || dir=base; done + echo "CHECKS_DIR=$dir" >> "$GITHUB_ENV" + - if: env.CHECKS_DIR == 'base' + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + with: + ref: ${{ github.event.pull_request.base.sha }} + path: base - uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2 - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version: 22 # The commented-out-code rule asks the TypeScript parser, a root dependency. - - run: bash scripts/ci/install-workspace-dependencies.sh --ignore-scripts + - working-directory: ${{ env.CHECKS_DIR }} + run: bash scripts/ci/install-workspace-dependencies.sh --ignore-scripts - name: Test the checker + working-directory: ${{ env.CHECKS_DIR }} run: node --test scripts/check-comment-citations.test.mjs scripts/comment-ratchet.test.mjs - name: Check the comments this diff touched + working-directory: pr env: COMMENT_CHECK_BASE: ${{ github.event.pull_request.base.sha }} - run: node scripts/check-comment-citations.mjs + run: node "../$CHECKS_DIR/scripts/check-comment-citations.mjs" - name: A package file's comment share may not rise if: ${{ !cancelled() }} + working-directory: pr env: COMMENT_CHECK_BASE: ${{ github.event.pull_request.base.sha }} - run: node scripts/comment-ratchet.mjs + run: node "../$CHECKS_DIR/scripts/comment-ratchet.mjs" diff --git a/package.json b/package.json index 2e7fe92a14..d7cb5d63c9 100644 --- a/package.json +++ b/package.json @@ -52,7 +52,7 @@ "player:perf": "bun run --filter @hyperframes/player perf", "format:check": "oxfmt --check .", "knip": "knip", - "test:scripts": "node --import tsx --test scripts/animejs-v4-guidance.test.mjs scripts/check-tracked-artifacts.test.mjs scripts/check-registry-set-delta.test.mjs scripts/check-no-main-deletions.test.mjs scripts/check-pr-captures.test.mjs scripts/check-comment-citations.test.mjs scripts/comment-ratchet.test.mjs scripts/check-docs-snippet-motion.test.mjs scripts/registry-target-paths.test.mjs scripts/check-workspace-contracts.test.mjs scripts/check-media-use-copy-parity.test.mjs scripts/check-svg-sanitize-parity.test.mjs scripts/check-media-use-svg-sanitize-generated.test.mjs scripts/check-package-cycles.test.mjs scripts/check-cli-process-ownership.test.mjs scripts/check-large-files.test.mjs scripts/package-subpaths.test.mjs scripts/validate-release-channel.test.mjs scripts/publish-workflow.test.mjs scripts/pr-edit-concurrency.test.mjs scripts/install-workspace-dependencies.test.mjs scripts/draft-changelog.test.ts scripts/set-version.test.ts scripts/release-prepare.test.ts scripts/cli-options.test.ts scripts/changelog-weekly.test.ts scripts/claude-plugin-compression.test.ts scripts/catalog-payload-assets.test.ts scripts/host-registry-assets.test.ts scripts/catalog-preview-temp.test.ts scripts/catalog-hosted-files.test.ts scripts/player-cdn-pin.test.ts scripts/studio-runtime-smoke.test.mjs scripts/verify-packed-manifests.test.mjs scripts/lint-skills.test.mjs scripts/creator-editing-recipes.test.mjs packages/gcp-cloud-run/check-dockerfile-workspaces.test.mjs packages/core/scripts/writeGeneratedFile.test.ts scripts/catalog-publication.test.mjs scripts/ci/resolve-workflow-pr.test.mjs scripts/check-catalog-source-pr.test.mjs scripts/generate-registry-items.test.ts scripts/catalog-drift.test.ts scripts/catalog-fetch-mirror.test.ts scripts/catalog-script-inlining.test.ts scripts/catalog-detail.test.ts scripts/generate-catalog-pages.test.ts scripts/verify-catalog-payloads.test.ts scripts/registry-skill-files.test.ts scripts/creator-editing-capabilities.test.mjs scripts/generate-catalog-previews.test.ts scripts/registry-primitive-payloads.test.ts registry/components/pan-stations/pan-stations.test.mjs && vitest run scripts/catalog/ scripts/contrastRatchet.test.ts scripts/generate-catalog-payloads.test.ts", + "test:scripts": "node --import tsx --test scripts/animejs-v4-guidance.test.mjs scripts/check-tracked-artifacts.test.mjs scripts/check-registry-set-delta.test.mjs scripts/check-no-main-deletions.test.mjs scripts/check-pr-captures.test.mjs scripts/check-comment-citations.test.mjs scripts/comments-workflow.test.mjs scripts/comment-ratchet.test.mjs scripts/check-docs-snippet-motion.test.mjs scripts/registry-target-paths.test.mjs scripts/check-workspace-contracts.test.mjs scripts/check-media-use-copy-parity.test.mjs scripts/check-svg-sanitize-parity.test.mjs scripts/check-media-use-svg-sanitize-generated.test.mjs scripts/check-package-cycles.test.mjs scripts/check-cli-process-ownership.test.mjs scripts/check-large-files.test.mjs scripts/package-subpaths.test.mjs scripts/validate-release-channel.test.mjs scripts/publish-workflow.test.mjs scripts/pr-edit-concurrency.test.mjs scripts/install-workspace-dependencies.test.mjs scripts/draft-changelog.test.ts scripts/set-version.test.ts scripts/release-prepare.test.ts scripts/cli-options.test.ts scripts/changelog-weekly.test.ts scripts/claude-plugin-compression.test.ts scripts/catalog-payload-assets.test.ts scripts/host-registry-assets.test.ts scripts/catalog-preview-temp.test.ts scripts/catalog-hosted-files.test.ts scripts/player-cdn-pin.test.ts scripts/studio-runtime-smoke.test.mjs scripts/verify-packed-manifests.test.mjs scripts/lint-skills.test.mjs scripts/creator-editing-recipes.test.mjs packages/gcp-cloud-run/check-dockerfile-workspaces.test.mjs packages/core/scripts/writeGeneratedFile.test.ts scripts/catalog-publication.test.mjs scripts/ci/resolve-workflow-pr.test.mjs scripts/check-catalog-source-pr.test.mjs scripts/generate-registry-items.test.ts scripts/catalog-drift.test.ts scripts/catalog-fetch-mirror.test.ts scripts/catalog-script-inlining.test.ts scripts/catalog-detail.test.ts scripts/generate-catalog-pages.test.ts scripts/verify-catalog-payloads.test.ts scripts/registry-skill-files.test.ts scripts/creator-editing-capabilities.test.mjs scripts/generate-catalog-previews.test.ts scripts/registry-primitive-payloads.test.ts registry/components/pan-stations/pan-stations.test.mjs && vitest run scripts/catalog/ scripts/contrastRatchet.test.ts scripts/generate-catalog-payloads.test.ts", "typecheck:scripts": "tsc --noEmit -p scripts/tsconfig.json", "test:skills": "node --test 'skills/**/*.test.mjs' 'packages/cli/src/media-use/**/*.test.mjs'", "generate:previews": "tsx scripts/generate-template-previews.ts", diff --git a/scripts/comments-workflow.test.mjs b/scripts/comments-workflow.test.mjs new file mode 100644 index 0000000000..686bfc4c08 --- /dev/null +++ b/scripts/comments-workflow.test.mjs @@ -0,0 +1,59 @@ +// A branch cut before a Comments script existed has no copy of it; the job must then run every +// script from the base, or it crashes before grading (seen on branches older than #4444, #4445). +import assert from "node:assert/strict"; +import { execFileSync } from "node:child_process"; +import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { test } from "node:test"; +import { parse } from "yaml"; + +const job = parse( + readFileSync(new URL("../.github/workflows/comments.yml", import.meta.url), "utf8"), +).jobs.comments; +const SCRIPTS = job.env.COMMENT_SCRIPTS.trim().split(/\s+/); +const pick = job.steps.find((s) => s.name === "Pick the checks"); +const base = job.steps.find((s) => s.with?.ref === "${{ github.event.pull_request.base.sha }}"); + +function picked(present) { + const workspace = mkdtempSync(path.join(tmpdir(), "comments-workflow-")); + try { + for (const script of present) { + mkdirSync(path.dirname(path.join(workspace, "pr", script)), { recursive: true }); + writeFileSync(path.join(workspace, "pr", script), ""); + } + const envFile = path.join(workspace, "env"); + execFileSync("bash", ["-c", pick.run], { + cwd: workspace, + env: { ...process.env, COMMENT_SCRIPTS: SCRIPTS.join(" "), GITHUB_ENV: envFile }, + }); + return readFileSync(envFile, "utf8").match(/^CHECKS_DIR=(.*)$/m)[1]; + } finally { + rmSync(workspace, { recursive: true, force: true }); + } +} + +test("every repo script the job runs is in COMMENT_SCRIPTS", () => { + const run = job.steps.map((s) => s.run ?? "").join("\n"); + for (const script of run.match(/scripts\/[\w.-]+\.mjs/g)) { + assert.ok(SCRIPTS.includes(script), `${script} is missing from COMMENT_SCRIPTS`); + } +}); + +test("a branch with every script runs its own copies", () => { + assert.equal(picked(SCRIPTS), "pr"); +}); + +test("a branch missing any one script runs all of them from the base", () => { + assert.equal(base.if, `env.CHECKS_DIR == '${base.with.path}'`); + for (const missing of SCRIPTS) { + assert.equal(picked(SCRIPTS.filter((s) => s !== missing)), base.with.path, missing); + } +}); + +test("every step that runs a script runs the picked copy", () => { + for (const step of job.steps.filter((s) => SCRIPTS.some((x) => (s.run ?? "").includes(x)))) { + const where = `${step["working-directory"] ?? ""} ${step.run}`; + assert.match(where, /CHECKS_DIR/, step.name ?? step.run); + } +});