From a013ff5a7947b1712377b1da18ca0dd4e7d25906 Mon Sep 17 00:00:00 2001 From: kai392 Date: Thu, 30 Jul 2026 07:55:55 +0800 Subject: [PATCH 1/2] fix(orb): canonicalize issue-body path tokens in checkContentLaneDeliverable (#9667) checkContentLaneDeliverable compares an issue-body path against the spec's entryFilePattern/providerFilePattern, which globToRegExp compiles against a CANONICALIZED (lowercased) path with no `i` flag -- so they only match lowercase input. The changedFiles side already canonicalizes each path before testing; the issue-body side did not. An issue naming the path with any capitalization (`Registry/Subnets/Foo.json`) produced no mentionedPath, so with no issueTitleImpliesEntryPattern the check returned "not-applicable" and the deliverable gate silently did not run -- the exact "test-only PR closes a content issue without delivering the content" gap the function exists to close (classifyRegistryPrScope in the same file already documents this canonicalization hazard). - Canonicalize each token from extractPathTokens before matchesSpec, exactly as the changedFiles side does. `.find` still returns the ORIGINAL token, so the "missing" verdict's mentionedPath stays the path as written in the issue body (rendered verbatim into the public PR comment). Tests: a mixed-case issue path is "missing" (original case preserved in mentionedPath) and "delivered" when the real lowercase file is changed; the all-lowercase behaviour is unchanged; a path matching no spec pattern still returns not-applicable (no over-broadening). The mixed-case cases fail against the current code. Closes #9667 Co-Authored-By: Claude Opus 4.8 --- src/review/content-lane/registry-logic.ts | 7 ++++- test/unit/content-lane-registry-logic.test.ts | 26 +++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/review/content-lane/registry-logic.ts b/src/review/content-lane/registry-logic.ts index 921b018b2..0c3b6b2cd 100644 --- a/src/review/content-lane/registry-logic.ts +++ b/src/review/content-lane/registry-logic.ts @@ -930,7 +930,12 @@ export function checkContentLaneDeliverable( issueTitle?: string, ): ContentLaneDeliverableCheck { const matchesSpec = (candidate: string): boolean => spec.entryFilePattern.test(candidate) || (spec.providerFilePattern?.test(candidate) ?? false); - const mentionedPath = extractPathTokens(issueText).find(matchesSpec); + // #9667: entryFilePattern/providerFilePattern are compiled by globToRegExp against a CANONICALIZED path (no `i` + // flag), so they only match lowercase. Canonicalize each issue-body token before testing it, exactly as the + // changedFiles side already does below — otherwise an issue naming `Registry/Subnets/Foo.json` produces no + // mentionedPath and the deliverable gate silently doesn't run. `.find` still returns the ORIGINAL token, so the + // "missing" verdict's mentionedPath stays what the contributor/maintainer see in the issue body. + const mentionedPath = extractPathTokens(issueText).find((token) => matchesSpec(canonicalize(token))); const titleImplies = !mentionedPath && Boolean(issueTitle && spec.issueTitleImpliesEntryPattern?.test(issueTitle)); if (!mentionedPath && !titleImplies) return { verdict: "not-applicable" }; const delivered = changedFiles.some((file) => matchesSpec(canonicalize(file))); diff --git a/test/unit/content-lane-registry-logic.test.ts b/test/unit/content-lane-registry-logic.test.ts index f017c0798..b8ac3001e 100644 --- a/test/unit/content-lane-registry-logic.test.ts +++ b/test/unit/content-lane-registry-logic.test.ts @@ -676,6 +676,32 @@ describe("checkContentLaneDeliverable (generic, spec-driven — #content-lane-de expect(checkContentLaneDeliverable(entryOnlySpec, issueText, ["tests/foo.test.mjs"]).verdict).toBe("missing"); }); + it("REGRESSION (#9667): is 'missing' for a MIXED-CASE issue-body path, preserving the original case in mentionedPath", () => { + // The spec patterns are compiled against a canonicalized (lowercased) path, so before the fix a mixed-case + // issue path produced no mentionedPath and the gate silently returned not-applicable. + const issueText = "Add the missing surfaces to Registry/Subnets/Foo.json before merging."; + expect(checkContentLaneDeliverable(spec, issueText, ["tests/foo-verify.test.mjs"])).toEqual({ + verdict: "missing", + mentionedPath: "Registry/Subnets/Foo.json", // original case, as the contributor/maintainer see it + }); + }); + + it("REGRESSION (#9667): is 'delivered' for that mixed-case issue body when the PR changes the real lowercase file", () => { + const issueText = "Add the missing surfaces to Registry/Subnets/Foo.json before merging."; + expect(checkContentLaneDeliverable(spec, issueText, ["registry/subnets/foo.json"])).toEqual({ verdict: "delivered" }); + }); + + it("#9667: all-lowercase issue-path behaviour is unchanged (delivered when the file is touched, missing otherwise)", () => { + const issueText = "Add registry/subnets/foo.json."; + expect(checkContentLaneDeliverable(spec, issueText, ["registry/subnets/foo.json"])).toEqual({ verdict: "delivered" }); + expect(checkContentLaneDeliverable(spec, issueText, ["tests/foo.test.mjs"])).toEqual({ verdict: "missing", mentionedPath: "registry/subnets/foo.json" }); + }); + + it("#9667: an issue path matching NO spec pattern still returns not-applicable (the fix does not over-broaden)", () => { + // A mixed-case path that is not a registry entry/provider file must not now spuriously match. + expect(checkContentLaneDeliverable(spec, "See Docs/README.md for details.", ["docs/readme.md"])).toEqual({ verdict: "not-applicable" }); + }); + // #content-lane-deliverable follow-up (metagraphed #7060-class gap): the literal-path scan alone is BLIND // to metagraphed's own ~120 "MCP execute: verify + wire SN*" issues, whose bodies write the registry path // with a generic `` documentation placeholder rather than the real resolved filename — confirmed From 34db85d4e46acdc3c52bdd4568b2dd59d0db01b6 Mon Sep 17 00:00:00 2001 From: kai392 Date: Thu, 30 Jul 2026 08:25:26 +0800 Subject: [PATCH 2/2] ci: wire the coco-dev-versions and import-specifiers drift checks into CI (#9649) Two checks in package.json's `test:ci` had no GitHub Actions job running them, so the two could silently diverge with zero CI signal: - `coco-dev-versions:check` keeps k8s/coco-dev/versions.json and the KBS kustomization in lockstep; its only test exercises the pure core against fabricated inputs and never reads the two real files. - `import-specifiers:check` (#9221) scans src/scripts/test and every packages/* workspace; #9240 and #9249 are two separate drifts that landed after it shipped, precisely because nothing enforced it in CI. - ci.yml: the `changes` job gains a `cocoDev` path filter (k8s/coco-dev/**, scripts/check-coco-dev-versions*.ts) and two named validate-code steps -- `coco-dev-versions:check` gated on cocoDev + the push clause, and `import-specifiers:check` gated on push || backend || mcp || engine || miner || discoveryIndex (every filter covering a scanned root). - check-import-specifiers-script.test.ts: a real-tree regression test that calls findImportSpecifierViolations() with no injected fs and asserts [], matching the real-tree guards on validate-no-hand-written-js and coverage-bolt-on-filenames -- the enforcement path that was missing. actionlint and lint:composite-actions pass on the edited workflow. Closes #9649 Co-Authored-By: Claude Opus 4.8 --- .github/workflows/ci.yml | 18 ++++++++++++++++++ .../check-import-specifiers-script.test.ts | 6 ++++++ 2 files changed, 24 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f122e26c0..d09c9ed01 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -38,6 +38,7 @@ jobs: outputs: backend: ${{ steps.filter.outputs.backend }} backendConfig: ${{ steps.filter.outputs.backendConfig }} + cocoDev: ${{ steps.filter.outputs.cocoDev }} ui: ${{ steps.filter.outputs.ui }} uiContract: ${{ steps.filter.outputs.uiContract }} mcp: ${{ steps.filter.outputs.mcp }} @@ -160,6 +161,11 @@ jobs: observability: - 'grafana/dashboards/**' - 'prometheus/rules/**' + # #9649: the coco-dev version manifest and the KBS kustomization that must stay in lockstep, plus the + # checker itself. Nothing else in the workflow references k8s/, so this is its own narrow filter. + cocoDev: + - 'k8s/coco-dev/**' + - 'scripts/check-coco-dev-versions*.ts' # The UI's own app code -- triggers the FULL toolchain (lint/typecheck/test/build). # A dependency bump (package.json/package-lock.json) stays here too since it can break the UI # build or types in ways only that full toolchain would catch. @@ -406,6 +412,18 @@ jobs: - name: Re-gate sort-key check if: ${{ github.event_name == 'push' || needs.changes.outputs.backend == 'true' }} run: npm run regate-sort-key:check + # #9649: coco-dev-versions:check keeps k8s/coco-dev/versions.json and the KBS kustomization in lockstep, but + # was wired into test:ci only -- no CI job ran it, and nothing in this workflow references k8s otherwise. + # Gated on the new cocoDev path filter plus the standard push clause every drift step above uses. + - name: Coco-dev versions drift check + if: ${{ github.event_name == 'push' || needs.changes.outputs.cocoDev == 'true' }} + run: npm run coco-dev-versions:check + # #9649: import-specifiers:check (#9221) scans import specifiers across src/scripts/test and every + # packages/* workspace (BUNDLER_ROOTS/NODENEXT_ROOTS), but no CI job ran it -- #9240/#9249 both drifted in + # after it shipped. Gate on every filter covering one of those scanned roots. + - 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 070af6b07..d5e3447c2 100644 --- a/test/unit/check-import-specifiers-script.test.ts +++ b/test/unit/check-import-specifiers-script.test.ts @@ -131,4 +131,10 @@ describe("check-import-specifiers script", () => { expect(multi.map((v) => v.file)).toEqual(["src/a.ts", "src/z.ts", "src/z.ts"]); expect(multi.map((v) => v.specifier)).toEqual(["./c.js", "./a.js", "./b.js"]); }); + + it("REGRESSION (#9649): the real repo tree has zero import-specifier violations (the guard was wired into test:ci only, so #9240 and #9249 both reached main while it was local-only)", () => { + // No injected listSourceFiles/readFile: runs the real gate against the real src/scripts/test/packages tree, + // matching the real-tree regression guards on validate-no-hand-written-js and coverage-bolt-on-filenames. + expect(findImportSpecifierViolations()).toEqual([]); + }); });