From 5fb9c248f3823a039bb4b5c365a3976217cea96c Mon Sep 17 00:00:00 2001 From: bitfathers94 <237535319+bitfathers94@users.noreply.github.com> Date: Wed, 29 Jul 2026 20:41:56 +0000 Subject: [PATCH] ci: run the two test:ci-only drift checks in the workflow coco-dev-versions:check and import-specifiers:check were the last two entries in package.json's test:ci aggregate with no GitHub Actions job running them, so a version-pin drift or a per-zone import drift could reach main with zero CI signal (the import guard already had that happen twice after it shipped). Add a cocoDev path filter (k8s/coco-dev/** plus the checker itself) and two named validate-code steps, each gated the same way as the surrounding drift steps. cocoDev is also added to the validate-code job-level if so a PR touching only k8s/coco-dev/** still schedules the job and reaches the new step's own gate; import-specifiers rides filters already in that if. Also add a real-tree regression assertion so the import-specifier guard is enforced by the vitest suite, not the workflow step alone. --- .github/workflows/ci.yml | 31 +++++++- .../check-import-specifiers-script.test.ts | 9 +++ .../ci-testci-only-drift-checks-wired.test.ts | 79 +++++++++++++++++++ 3 files changed, 118 insertions(+), 1 deletion(-) create mode 100644 test/unit/ci-testci-only-drift-checks-wired.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f122e26c0d..f63d250b1e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -49,6 +49,7 @@ jobs: rees: ${{ steps.filter.outputs.rees }} controlPlane: ${{ steps.filter.outputs.controlPlane }} observability: ${{ steps.filter.outputs.observability }} + cocoDev: ${{ steps.filter.outputs.cocoDev }} steps: # Debounce rapid re-pushes to an open PR: every job below (validate-code, validate-tests) # has `needs: changes` in its dependency chain, so none of @@ -256,6 +257,14 @@ jobs: - 'packages/loopover-miner/**' - 'test/unit/check-miner-package.test.ts' - 'test/unit/miner-calibration-types.test.ts' + # The two files coco-dev-versions:check cross-checks, plus the checker itself. Nothing else in the + # repo references k8s/coco-dev/**, so without this filter a PR editing ONLY one of the two version + # files (or the reverse) would match no other filter, skip validate-code entirely, and never reach + # the "Coco-dev versions drift check" step's own step-level gate -- the exact drift the step exists + # to catch. Feeds validate-code's job-level `if:` (which lists cocoDev) as well as the step below. + cocoDev: + - 'k8s/coco-dev/**' + - 'scripts/check-coco-dev-versions*.ts' # Diff-scoped security gate, folded in from the old standalone `security` job (2026-07-24, one fewer # runner slot on every PR at zero added wall-clock -- the API-based lockfile diff takes seconds and @@ -282,7 +291,7 @@ jobs: needs: changes # draft != true also gates push runs correctly: github.event.pull_request is unset there, so the # property access evaluates to null, and null != true is true. - if: ${{ github.event_name == 'push' || (github.event.pull_request.draft != true && (needs.changes.outputs.backend == 'true' || needs.changes.outputs.mcp == 'true' || needs.changes.outputs.engine == 'true' || needs.changes.outputs.discoveryIndex == 'true' || needs.changes.outputs.miner == 'true' || needs.changes.outputs.rees == 'true' || needs.changes.outputs.controlPlane == 'true' || needs.changes.outputs.ui == 'true' || needs.changes.outputs.observability == 'true')) }} + if: ${{ github.event_name == 'push' || (github.event.pull_request.draft != true && (needs.changes.outputs.backend == 'true' || needs.changes.outputs.mcp == 'true' || needs.changes.outputs.engine == 'true' || needs.changes.outputs.discoveryIndex == 'true' || needs.changes.outputs.miner == 'true' || needs.changes.outputs.rees == 'true' || needs.changes.outputs.controlPlane == 'true' || needs.changes.outputs.ui == 'true' || needs.changes.outputs.observability == 'true' || needs.changes.outputs.cocoDev == 'true')) }} runs-on: ubuntu-latest timeout-minutes: 45 env: @@ -406,6 +415,26 @@ jobs: - name: Re-gate sort-key check if: ${{ github.event_name == 'push' || needs.changes.outputs.backend == 'true' }} run: npm run regate-sort-key:check + # #9213: scripts/check-coco-dev-versions.ts cross-checks the version pin in k8s/coco-dev/versions.json + # against k8s/coco-dev/kbs/base/kustomization.yaml, so a bump made in only one of the two files fails CI + # instead of silently shipping a KBS deploy that doesn't match its own recorded version manifest. Same + # local-only-until-now gap as the drift checks above: it was wired only into `npm run test:ci`, and + # nothing under .github/workflows/ referenced k8s at all, so nothing here ever ran it. Gated on the new + # `cocoDev` filter (which the validate-code job-level `if:` above also lists, so a k8s/coco-dev-only PR + # still schedules this job) plus the standard push clause. + - name: Coco-dev versions drift check + if: ${{ github.event_name == 'push' || needs.changes.outputs.cocoDev == 'true' }} + run: npm run coco-dev-versions:check + # #9221: scripts/check-import-specifiers.ts enforces the per-zone relative-import extension convention + # (bundler zones src/scripts/test resolve extensionless; NodeNext packages/* require `.js`). Same + # local-only-until-now gap as the drift checks above -- it ran only in `npm run test:ci`, so #9240 and + # #9249 were two separate drifts that reached main after #9221 shipped the guard. The checker scans src, + # scripts, test (BUNDLER_ROOTS) and packages (NODENEXT_ROOTS), so it's gated on every filter that covers + # one of those roots: backend (src/scripts/test) plus each packages/* workspace (mcp/engine/miner/ + # discoveryIndex), each of which is already in validate-code's job-level `if:` above. + - name: Import-specifiers drift check + if: ${{ github.event_name == 'push' || needs.changes.outputs.backend == 'true' || needs.changes.outputs.mcp == 'true' || needs.changes.outputs.engine == 'true' || needs.changes.outputs.miner == 'true' || needs.changes.outputs.discoveryIndex == 'true' }} + run: npm run import-specifiers:check # #9563: every webhook-owned handler must decide about #9312's redelivery guard out loud. Three had # silently skipped it -- one writing duplicate permanent suppression rows, two spending a second paid diff --git a/test/unit/check-import-specifiers-script.test.ts b/test/unit/check-import-specifiers-script.test.ts index 070af6b078..282cf950e5 100644 --- a/test/unit/check-import-specifiers-script.test.ts +++ b/test/unit/check-import-specifiers-script.test.ts @@ -116,6 +116,15 @@ describe("check-import-specifiers script", () => { expect(violations).toEqual([]); }); + // Most important regression test in this file: the real repo, scanned with NO injected + // listSourceFiles/readFile, must have zero import-specifier violations. Until #9649 this guard was enforced + // only by `npm run test:ci` locally and by no GitHub Actions job, so #9240 and #9249 were two separate + // drifts that reached main after #9221 shipped the checker. If this fails, a new relative import has drifted + // from its zone's convention -- fix the specifier, don't weaken this check. + it("the real tree has zero import-specifier violations (regression guard, #9240/#9249)", () => { + expect(findImportSpecifierViolations()).toEqual([]); + }); + it("sorts violations by file then specifier, and a missing root yields nothing (real listSourceFiles catch arm)", () => { const violations = findImportSpecifierViolations({ bundlerRoots: ["definitely-not-a-real-directory-xyz"], nodeNextRoots: [] }); expect(violations).toEqual([]); diff --git a/test/unit/ci-testci-only-drift-checks-wired.test.ts b/test/unit/ci-testci-only-drift-checks-wired.test.ts new file mode 100644 index 0000000000..106f6da1d2 --- /dev/null +++ b/test/unit/ci-testci-only-drift-checks-wired.test.ts @@ -0,0 +1,79 @@ +import { readFileSync } from "node:fs"; +import { parse } from "yaml"; +import { describe, expect, it } from "vitest"; + +function readYaml(path: string): Record { + return record(parse(readFileSync(path, "utf8")), path); +} + +function record(value: unknown, label: string): Record { + if (!value || typeof value !== "object" || Array.isArray(value)) { + throw new Error(`${label} must be an object`); + } + return value as Record; +} + +function recordArray(value: unknown, label: string): Array> { + if (!Array.isArray(value)) throw new Error(`${label} must be an array`); + return value.map((entry, index) => record(entry, `${label}[${index}]`)); +} + +// #9649: coco-dev-versions:check and import-specifiers:check were the last two entries in package.json's +// `test:ci` aggregate with no enforcement path in .github/workflows/ci.yml at all -- only local discipline ran +// them, so a version-pin drift (the coco check) or a per-zone import drift (#9240/#9249, the import check) +// could reach main with zero CI signal. This mirrors ci-generated-artifact-drift-checks.test.ts's assertion +// shape for those two checks' own wiring into the validate-code job. +describe("test:ci-only drift checks are wired into the validate-code job", () => { + const workflow = readYaml(".github/workflows/ci.yml"); + const changes = record(record(workflow.jobs, "workflow.jobs").changes, "jobs.changes"); + const validateCode = record(record(workflow.jobs, "workflow.jobs")["validate-code"], "jobs.validate-code"); + const steps = recordArray(validateCode.steps, "jobs.validate-code.steps"); + + const stepFor = (name: string) => { + const step = steps.find((entry) => entry.name === name); + expect(step, `step "${name}" must exist`).toBeDefined(); + return step!; + }; + + it("both checks are still part of package.json's test:ci aggregate", () => { + const pkg = record(JSON.parse(readFileSync("package.json", "utf8")), "package.json"); + const scripts = record(pkg.scripts, "package.json.scripts"); + expect(String(scripts["test:ci"])).toContain("npm run coco-dev-versions:check"); + expect(String(scripts["test:ci"])).toContain("npm run import-specifiers:check"); + }); + + it("the changes job declares a cocoDev filter output covering the two version files and the checker", () => { + const outputs = record(changes.outputs, "jobs.changes.outputs"); + expect(String(outputs.cocoDev)).toBe("${{ steps.filter.outputs.cocoDev }}"); + + // The filters: block is a literal YAML string passed to dorny/paths-filter, so assert against the raw text. + const ci = readFileSync(".github/workflows/ci.yml", "utf8"); + expect(ci).toMatch(/cocoDev:\s*\n\s*- 'k8s\/coco-dev\/\*\*'\s*\n\s*- 'scripts\/check-coco-dev-versions\*\.ts'/); + }); + + it("validate-code's job-level if includes cocoDev, so a k8s/coco-dev-only PR still schedules the job", () => { + // The exact gap that would otherwise skip the whole job -- and with it the step-level gate below -- for a + // PR whose only change matches cocoDev and no other filter. + expect(String(validateCode.if)).toContain("needs.changes.outputs.cocoDev == 'true'"); + }); + + it("runs coco-dev-versions:check, gated on the cocoDev filter plus the push clause", () => { + const step = stepFor("Coco-dev versions drift check"); + expect(String(step.run)).toBe("npm run coco-dev-versions:check"); + const condition = String(step.if); + expect(condition).toContain("github.event_name == 'push'"); + expect(condition).toContain("needs.changes.outputs.cocoDev == 'true'"); + }); + + it("runs import-specifiers:check, gated on backend plus every packages/* workspace filter it scans", () => { + const step = stepFor("Import-specifiers drift check"); + expect(String(step.run)).toBe("npm run import-specifiers:check"); + const condition = String(step.if); + // The checker scans src/scripts/test (backend) and packages (mcp/engine/miner/discoveryIndex) -- each root + // must have a filter that re-triggers it, and each is already in validate-code's job-level if above. + for (const output of ["backend", "mcp", "engine", "miner", "discoveryIndex"]) { + expect(condition, `must gate on ${output}`).toContain(`needs.changes.outputs.${output} == 'true'`); + } + expect(condition).toContain("github.event_name == 'push'"); + }); +});