From 7eb1cbe3f1983b2f1003e98667bcb4b3f3df242c Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Wed, 15 Jul 2026 12:26:19 +0000 Subject: [PATCH 1/9] refactor(checks): migrate repository check suite to .mts Checks importing first-party CJS source use dynamic import so the ESM entrypoints resolve named exports at runtime under tsx. Signed-off-by: Tinson Lai Signed-off-by: Apurv Kumaria --- .../SKILL.md | 2 +- .github/workflows/pr.yaml | 2 +- package.json | 8 +++---- ...dependency-pins.ts => dependency-pins.mts} | 0 ...ntial-env.ts => direct-credential-env.mts} | 4 ++-- ...e2e-mock-parity.ts => e2e-mock-parity.mts} | 0 ...dary.ts => hermes-light-skin-boundary.mts} | 4 +++- ...ndaries.ts => layer-import-boundaries.mts} | 0 ...pin.ts => local-credential-helper-pin.mts} | 0 ...erage-ignore.ts => no-coverage-ignore.mts} | 0 ...st-imports.ts => no-test-dist-imports.mts} | 0 ...-e2e.ts => no-unit-blocks-in-live-e2e.mts} | 0 ....ts => openshell-policy-mutation-read.mts} | 0 scripts/checks/{run.ts => run.mts} | 24 +++++++++---------- ...dget.ts => test-create-require-budget.mts} | 0 ...st-title-style.ts => test-title-style.mts} | 0 ...-overlap.ts => vitest-project-overlap.mts} | 0 src/lib/messaging/manifest/types.test.ts | 2 +- src/lib/security/credential-env.ts | 2 +- test/checks-runner.test.ts | 12 +++++----- test/dependency-pins-check.test.ts | 2 +- test/e2e-mock-parity.test.ts | 2 +- test/layer-import-boundaries.test.ts | 2 +- test/local-credential-helper-pin.test.ts | 2 +- test/no-coverage-ignore.test.ts | 2 +- test/no-direct-credential-env.test.ts | 4 ++-- test/no-unit-blocks-in-live-e2e.test.ts | 5 +++- test/policy-mutation-read-discovery.test.ts | 2 +- test/pr-workflow-contract.test.ts | 1 - test/test-boundary-guards.test.ts | 4 ++-- test/test-create-require-budget.test.ts | 2 +- test/test-title-style.test.ts | 2 +- 32 files changed, 47 insertions(+), 43 deletions(-) rename scripts/checks/{dependency-pins.ts => dependency-pins.mts} (100%) rename scripts/checks/{direct-credential-env.ts => direct-credential-env.mts} (98%) rename scripts/checks/{e2e-mock-parity.ts => e2e-mock-parity.mts} (100%) rename scripts/checks/{hermes-light-skin-boundary.ts => hermes-light-skin-boundary.mts} (91%) rename scripts/checks/{layer-import-boundaries.ts => layer-import-boundaries.mts} (100%) rename scripts/checks/{local-credential-helper-pin.ts => local-credential-helper-pin.mts} (100%) rename scripts/checks/{no-coverage-ignore.ts => no-coverage-ignore.mts} (100%) rename scripts/checks/{no-test-dist-imports.ts => no-test-dist-imports.mts} (100%) rename scripts/checks/{no-unit-blocks-in-live-e2e.ts => no-unit-blocks-in-live-e2e.mts} (100%) rename scripts/checks/{openshell-policy-mutation-read.ts => openshell-policy-mutation-read.mts} (100%) rename scripts/checks/{run.ts => run.mts} (83%) rename scripts/checks/{test-create-require-budget.ts => test-create-require-budget.mts} (100%) rename scripts/checks/{test-title-style.ts => test-title-style.mts} (100%) rename scripts/checks/{vitest-project-overlap.ts => vitest-project-overlap.mts} (100%) diff --git a/.agents/skills/nemoclaw-contributor-update-dependencies/SKILL.md b/.agents/skills/nemoclaw-contributor-update-dependencies/SKILL.md index 64686c63d10..469089ea499 100644 --- a/.agents/skills/nemoclaw-contributor-update-dependencies/SKILL.md +++ b/.agents/skills/nemoclaw-contributor-update-dependencies/SKILL.md @@ -436,7 +436,7 @@ Use these NemoClaw precedents for durable evidence shape, not as inherited concl - `docs/security/openclaw-2026.6.10-dependency-review.md` and `test/openclaw-dependency-review.test.ts` for a tracked dependency review with contract tests; - `docs/security/openshell-0.0.72-compatibility-review.mdx` for a runtime compatibility boundary; -- `scripts/checks/dependency-pins.ts` and `test/dependency-pins-check.test.ts` for selector +- `scripts/checks/dependency-pins.mts` and `test/dependency-pins-check.test.ts` for selector coherence; and - `scripts/check-installer-hash.sh` and `test/installer-hash-check.test.ts` for independently trusted release manifests and consumed artifacts. diff --git a/.github/workflows/pr.yaml b/.github/workflows/pr.yaml index 0d5fdd12135..13bf648ca08 100644 --- a/.github/workflows/pr.yaml +++ b/.github/workflows/pr.yaml @@ -340,7 +340,7 @@ jobs: # covered without executing a mutable replacement action. - name: Validate changed live E2E mock parity (bootstrap) if: ${{ steps.trusted-shard-capabilities.outputs.e2e-support != 'true' && matrix.shard == 1 }} - run: npx tsx scripts/checks/e2e-mock-parity.ts --base HEAD^1 --head HEAD^2 + run: npx tsx scripts/checks/e2e-mock-parity.mts --base HEAD^1 --head HEAD^2 - name: Run E2E support shard (bootstrap) if: ${{ steps.trusted-shard-capabilities.outputs.e2e-support != 'true' }} diff --git a/package.json b/package.json index 707a0ef64d1..8542c2acd40 100644 --- a/package.json +++ b/package.json @@ -35,13 +35,13 @@ "test:coverage:cli": "npm run clean:cli && npm run build:cli && tsx scripts/check-dist-sourcemaps.mts dist && vitest run --project cli --project integration --coverage --coverage.reporter=text-summary --coverage.reporter=json-summary --coverage.reportsDirectory=coverage/cli --coverage.include=\"bin/**/*.js\" --coverage.include=\"src/**/*.ts\" --coverage.exclude=\"test/**/*.js\" --coverage.exclude=\"test/**/*.ts\" && tsx scripts/check-coverage-ratchet.mts coverage/cli/coverage-summary.json ci/coverage-threshold-cli.json \"CLI coverage\"", "test:coverage:plugin": "vitest run --project plugin --coverage --coverage.reporter=text-summary --coverage.reporter=json-summary --coverage.reportsDirectory=coverage/plugin --coverage.include=\"nemoclaw/src/**/*.ts\" --coverage.include=\"nemoclaw/src/**/*.cts\" --coverage.exclude=\"**/*.test.ts\" && tsx scripts/check-coverage-ratchet.mts coverage/plugin/coverage-summary.json ci/coverage-threshold-plugin.json \"Plugin coverage\"", "test:live-e2e": "npm run clean:cli && npm run build:cli && NEMOCLAW_RUN_LIVE_E2E=1 vitest run --project e2e-live", - "test:imports:check": "tsx scripts/checks/no-test-dist-imports.ts", - "test:projects:check": "tsx scripts/checks/vitest-project-overlap.ts", - "test:titles:check": "tsx scripts/checks/test-title-style.ts", + "test:imports:check": "tsx scripts/checks/no-test-dist-imports.mts", + "test:projects:check": "tsx scripts/checks/vitest-project-overlap.mts", + "test:titles:check": "tsx scripts/checks/test-title-style.mts", "bench": "tsx scripts/bench/run.mts", "check": "npx prek run --all-files --stage pre-commit && npx prek run --all-files --stage manual", "check:diff": "npx prek run --from-ref origin/main --to-ref HEAD --stage pre-commit && npx commitlint --from origin/main --to HEAD && npx prek run --from-ref origin/main --to-ref HEAD --stage pre-push", - "checks": "tsx scripts/checks/run.ts", + "checks": "tsx scripts/checks/run.mts", "lint": "npx @biomejs/biome lint . && npm run checks", "lint:fix": "npx @biomejs/biome lint --write . && npm run checks", "lint:ts": "cd nemoclaw && npm run check", diff --git a/scripts/checks/dependency-pins.ts b/scripts/checks/dependency-pins.mts similarity index 100% rename from scripts/checks/dependency-pins.ts rename to scripts/checks/dependency-pins.mts diff --git a/scripts/checks/direct-credential-env.ts b/scripts/checks/direct-credential-env.mts similarity index 98% rename from scripts/checks/direct-credential-env.ts rename to scripts/checks/direct-credential-env.mts index 0873c5ab913..e203a521e10 100644 --- a/scripts/checks/direct-credential-env.ts +++ b/scripts/checks/direct-credential-env.mts @@ -13,8 +13,8 @@ import { readFileSync } from "node:fs"; import path from "node:path"; import { fileURLToPath } from "node:url"; import * as ts from "typescript"; -import { SUPPORTED_CREDENTIAL_ENV_NAMES } from "../../src/lib/security/credential-env"; +const { SUPPORTED_CREDENTIAL_ENV_NAMES } = await import("../../src/lib/security/credential-env"); const CREDENTIAL_ENV_KEYS = SUPPORTED_CREDENTIAL_ENV_NAMES; const MESSAGE = @@ -251,7 +251,7 @@ function scriptKindForPath(filePath: string): ts.ScriptKind { function main(): void { const filePaths = process.argv.slice(2).filter((arg) => arg !== "--"); if (filePaths.length === 0) { - console.error("Usage: tsx scripts/checks/direct-credential-env.ts FILE..."); + console.error("Usage: tsx scripts/checks/direct-credential-env.mts FILE..."); process.exitCode = 2; return; } diff --git a/scripts/checks/e2e-mock-parity.ts b/scripts/checks/e2e-mock-parity.mts similarity index 100% rename from scripts/checks/e2e-mock-parity.ts rename to scripts/checks/e2e-mock-parity.mts diff --git a/scripts/checks/hermes-light-skin-boundary.ts b/scripts/checks/hermes-light-skin-boundary.mts similarity index 91% rename from scripts/checks/hermes-light-skin-boundary.ts rename to scripts/checks/hermes-light-skin-boundary.mts index f9d784dbbc4..75c70a4ac58 100644 --- a/scripts/checks/hermes-light-skin-boundary.ts +++ b/scripts/checks/hermes-light-skin-boundary.mts @@ -5,7 +5,9 @@ import fs from "node:fs"; import path from "node:path"; import { fileURLToPath } from "node:url"; -import { NEMOCLAW_HERMES_LIGHT_SKIN_REVIEWED_HERMES_VERSIONS } from "../../src/lib/domain/sandbox/connect-env"; +const { NEMOCLAW_HERMES_LIGHT_SKIN_REVIEWED_HERMES_VERSIONS } = await import( + "../../src/lib/domain/sandbox/connect-env" +); const REPO_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../.."); const HERMES_DOCKERFILE_BASE = "agents/hermes/Dockerfile.base"; diff --git a/scripts/checks/layer-import-boundaries.ts b/scripts/checks/layer-import-boundaries.mts similarity index 100% rename from scripts/checks/layer-import-boundaries.ts rename to scripts/checks/layer-import-boundaries.mts diff --git a/scripts/checks/local-credential-helper-pin.ts b/scripts/checks/local-credential-helper-pin.mts similarity index 100% rename from scripts/checks/local-credential-helper-pin.ts rename to scripts/checks/local-credential-helper-pin.mts diff --git a/scripts/checks/no-coverage-ignore.ts b/scripts/checks/no-coverage-ignore.mts similarity index 100% rename from scripts/checks/no-coverage-ignore.ts rename to scripts/checks/no-coverage-ignore.mts diff --git a/scripts/checks/no-test-dist-imports.ts b/scripts/checks/no-test-dist-imports.mts similarity index 100% rename from scripts/checks/no-test-dist-imports.ts rename to scripts/checks/no-test-dist-imports.mts diff --git a/scripts/checks/no-unit-blocks-in-live-e2e.ts b/scripts/checks/no-unit-blocks-in-live-e2e.mts similarity index 100% rename from scripts/checks/no-unit-blocks-in-live-e2e.ts rename to scripts/checks/no-unit-blocks-in-live-e2e.mts diff --git a/scripts/checks/openshell-policy-mutation-read.ts b/scripts/checks/openshell-policy-mutation-read.mts similarity index 100% rename from scripts/checks/openshell-policy-mutation-read.ts rename to scripts/checks/openshell-policy-mutation-read.mts diff --git a/scripts/checks/run.ts b/scripts/checks/run.mts similarity index 83% rename from scripts/checks/run.ts rename to scripts/checks/run.mts index ee5138e6a4d..278e466dc8f 100644 --- a/scripts/checks/run.ts +++ b/scripts/checks/run.mts @@ -32,7 +32,7 @@ export const CHECKS: readonly CheckCommand[] = [ name: "direct-credential-env", command: TSX, args: [ - "scripts/checks/direct-credential-env.ts", + "scripts/checks/direct-credential-env.mts", "src/lib/onboard.ts", "src/lib/onboard/provider-key-bridge.ts", "src/lib/onboard/providers.ts", @@ -41,57 +41,57 @@ export const CHECKS: readonly CheckCommand[] = [ { name: "local-credential-helper-pin", command: TSX, - args: ["scripts/checks/local-credential-helper-pin.ts"], + args: ["scripts/checks/local-credential-helper-pin.mts"], }, { name: "hermes-light-skin-boundary", command: TSX, - args: ["scripts/checks/hermes-light-skin-boundary.ts"], + args: ["scripts/checks/hermes-light-skin-boundary.mts"], }, { name: "dependency-pins", command: TSX, - args: ["scripts/checks/dependency-pins.ts"], + args: ["scripts/checks/dependency-pins.mts"], }, { name: "no-coverage-ignore", command: TSX, - args: ["scripts/checks/no-coverage-ignore.ts"], + args: ["scripts/checks/no-coverage-ignore.mts"], }, { name: "openshell-policy-mutation-read", command: TSX, - args: ["scripts/checks/openshell-policy-mutation-read.ts"], + args: ["scripts/checks/openshell-policy-mutation-read.mts"], }, { name: "layer-import-boundaries", command: TSX, - args: ["scripts/checks/layer-import-boundaries.ts"], + args: ["scripts/checks/layer-import-boundaries.mts"], }, { name: "no-test-dist-imports", command: TSX, - args: ["scripts/checks/no-test-dist-imports.ts"], + args: ["scripts/checks/no-test-dist-imports.mts"], }, { name: "test-create-require-budget", command: TSX, - args: ["scripts/checks/test-create-require-budget.ts"], + args: ["scripts/checks/test-create-require-budget.mts"], }, { name: "vitest-project-overlap", command: TSX, - args: ["scripts/checks/vitest-project-overlap.ts"], + args: ["scripts/checks/vitest-project-overlap.mts"], }, { name: "test-title-style", command: TSX, - args: ["scripts/checks/test-title-style.ts"], + args: ["scripts/checks/test-title-style.mts"], }, { name: "no-unit-blocks-in-live-e2e", command: TSX, - args: ["scripts/checks/no-unit-blocks-in-live-e2e.ts"], + args: ["scripts/checks/no-unit-blocks-in-live-e2e.mts"], }, ]; diff --git a/scripts/checks/test-create-require-budget.ts b/scripts/checks/test-create-require-budget.mts similarity index 100% rename from scripts/checks/test-create-require-budget.ts rename to scripts/checks/test-create-require-budget.mts diff --git a/scripts/checks/test-title-style.ts b/scripts/checks/test-title-style.mts similarity index 100% rename from scripts/checks/test-title-style.ts rename to scripts/checks/test-title-style.mts diff --git a/scripts/checks/vitest-project-overlap.ts b/scripts/checks/vitest-project-overlap.mts similarity index 100% rename from scripts/checks/vitest-project-overlap.ts rename to scripts/checks/vitest-project-overlap.mts diff --git a/src/lib/messaging/manifest/types.test.ts b/src/lib/messaging/manifest/types.test.ts index 77b790c3f7f..400bf5ed531 100644 --- a/src/lib/messaging/manifest/types.test.ts +++ b/src/lib/messaging/manifest/types.test.ts @@ -292,6 +292,6 @@ describe("messaging manifest type contracts", () => { }); // Import-layer isolation for the production manifest modules is enforced by - // scripts/checks/layer-import-boundaries.ts. Keep this unit test focused on + // scripts/checks/layer-import-boundaries.mts. Keep this unit test focused on // manifest serialization and type contracts rather than walking source files. }); diff --git a/src/lib/security/credential-env.ts b/src/lib/security/credential-env.ts index afaf5813f32..3d9f7be7547 100644 --- a/src/lib/security/credential-env.ts +++ b/src/lib/security/credential-env.ts @@ -8,7 +8,7 @@ // separator-free provider parameters such as `clientSecret`, browser/session // material such as cookies, and connection strings cannot slip past the // validator. The standalone local credential helper and browser form embed this -// literal pattern; scripts/checks/local-credential-helper-pin.ts enforces exact +// literal pattern; scripts/checks/local-credential-helper-pin.mts enforces exact // parity because those reviewed artifacts cannot import this module at runtime. export const CREDENTIAL_SHAPED_NAME_PATTERN = /(?:^|[_-])(?:api[_-]?key|access[_-]?key|secret[_-]?key|auth[_-]?token|refresh[_-]?token|access[_-]?token|client[_-]?secret|private[_-]?key|pass[_-]?code|personal[_-]?access[_-]?token|connection[_-]?string|webhook(?:[_-]?url)?|key|secret|token|password|passwd|passcode|auth|authorization|credential|credentials|bearer|bearer[_-]?token|cookie|cookies|pat|private|privatekey|pin|webhookurl|dsn|connectionstring)(?:$|[_-])/i; diff --git a/test/checks-runner.test.ts b/test/checks-runner.test.ts index 6d28b487fee..053774321da 100644 --- a/test/checks-runner.test.ts +++ b/test/checks-runner.test.ts @@ -4,12 +4,12 @@ import type { SpawnSyncOptions } from "node:child_process"; import { describe, expect, it, vi } from "vitest"; -import { buildCheckSpawnInvocation, runChecks } from "../scripts/checks/run"; +import { buildCheckSpawnInvocation, runChecks } from "../scripts/checks/run.mts"; const sampleCheck = { name: "sample", command: "tsx.cmd", - args: ["scripts/checks/sample.ts"], + args: ["scripts/checks/sample.mts"], }; function successfulSpawn(): { status: number | null } { @@ -24,7 +24,7 @@ describe("checks runner", () => { }), ).toEqual({ command: "C:\\Windows\\System32\\cmd.exe", - args: ["/d", "/s", "/c", "tsx.cmd", "scripts/checks/sample.ts"], + args: ["/d", "/s", "/c", "tsx.cmd", "scripts/checks/sample.mts"], }); }); @@ -37,7 +37,7 @@ describe("checks runner", () => { it("keeps POSIX runner execution direct", () => { expect(buildCheckSpawnInvocation(sampleCheck, "linux")).toEqual({ command: "tsx.cmd", - args: ["scripts/checks/sample.ts"], + args: ["scripts/checks/sample.mts"], }); }); @@ -57,7 +57,7 @@ describe("checks runner", () => { expect(spawn).toHaveBeenCalledWith( "C:\\Windows\\System32\\cmd.exe", - ["/d", "/s", "/c", "tsx.cmd", "scripts/checks/sample.ts"], + ["/d", "/s", "/c", "tsx.cmd", "scripts/checks/sample.mts"], expect.objectContaining({ stdio: "inherit" }), ); expect(calls[0]?.shell).toBeUndefined(); @@ -72,7 +72,7 @@ describe("checks runner", () => { expect(spawn).toHaveBeenCalledWith( "tsx.cmd", - ["scripts/checks/sample.ts"], + ["scripts/checks/sample.mts"], expect.objectContaining({ stdio: "inherit" }), ); }); diff --git a/test/dependency-pins-check.test.ts b/test/dependency-pins-check.test.ts index 9599a54d1f9..67bbeadc575 100644 --- a/test/dependency-pins-check.test.ts +++ b/test/dependency-pins-check.test.ts @@ -7,7 +7,7 @@ import path from "node:path"; import { describe, expect, it } from "vitest"; -import { verifyDependencyPins } from "../scripts/checks/dependency-pins"; +import { verifyDependencyPins } from "../scripts/checks/dependency-pins.mts"; const OPENSHELL_MIN = "1.2.3"; const OPENSHELL_MAX = "1.2.4"; diff --git a/test/e2e-mock-parity.test.ts b/test/e2e-mock-parity.test.ts index 525b6ce651a..485ec9a9fb7 100644 --- a/test/e2e-mock-parity.test.ts +++ b/test/e2e-mock-parity.test.ts @@ -10,7 +10,7 @@ import { isMockParityRelevantSourceChange, type MockParityManifest, validateMockParity, -} from "../scripts/checks/e2e-mock-parity"; +} from "../scripts/checks/e2e-mock-parity.mts"; import { type CompositeAction, readYaml } from "./helpers/e2e-workflow-contract"; const live = "test/e2e/live/example.test.ts"; diff --git a/test/layer-import-boundaries.test.ts b/test/layer-import-boundaries.test.ts index daec1164a9e..6a8eac03d7f 100644 --- a/test/layer-import-boundaries.test.ts +++ b/test/layer-import-boundaries.test.ts @@ -6,7 +6,7 @@ import path from "node:path"; import { describe, expect, it } from "vitest"; -import { findLayerImportBoundaryViolations } from "../scripts/checks/layer-import-boundaries"; +import { findLayerImportBoundaryViolations } from "../scripts/checks/layer-import-boundaries.mts"; const REPO_ROOT = path.join(import.meta.dirname, ".."); let fixtureCounter = 0; diff --git a/test/local-credential-helper-pin.test.ts b/test/local-credential-helper-pin.test.ts index a2d66212772..07175bdc792 100644 --- a/test/local-credential-helper-pin.test.ts +++ b/test/local-credential-helper-pin.test.ts @@ -9,7 +9,7 @@ import { extractProcessControlRules, extractStringSet, verifyFieldSafetySourceParity, -} from "../scripts/checks/local-credential-helper-pin"; +} from "../scripts/checks/local-credential-helper-pin.mts"; const FUNCTION_NAME = "isBlocked"; const SET_NAME = "BLOCKED_NAMES"; diff --git a/test/no-coverage-ignore.test.ts b/test/no-coverage-ignore.test.ts index fe5d310cc5b..6e0de027672 100644 --- a/test/no-coverage-ignore.test.ts +++ b/test/no-coverage-ignore.test.ts @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import { describe, expect, it } from "vitest"; -import { findCoverageIgnoreDirectives } from "../scripts/checks/no-coverage-ignore"; +import { findCoverageIgnoreDirectives } from "../scripts/checks/no-coverage-ignore.mts"; const forbiddenDirective = ["v8", "ignore"].join(" "); diff --git a/test/no-direct-credential-env.test.ts b/test/no-direct-credential-env.test.ts index 6312146724a..d559ec2c6e0 100644 --- a/test/no-direct-credential-env.test.ts +++ b/test/no-direct-credential-env.test.ts @@ -14,7 +14,7 @@ import { spawnSync } from "node:child_process"; import path from "node:path"; import { describe, expect, it } from "vitest"; -import { findDirectCredentialEnvReads } from "../scripts/checks/direct-credential-env"; +import { findDirectCredentialEnvReads } from "../scripts/checks/direct-credential-env.mts"; describe("direct credential env guard", () => { it.each([ @@ -96,7 +96,7 @@ describe("direct credential env guard", () => { "npx", [ "tsx", - "scripts/checks/direct-credential-env.ts", + "scripts/checks/direct-credential-env.mts", "src/lib/onboard.ts", "src/lib/onboard/provider-key-bridge.ts", "src/lib/onboard/providers.ts", diff --git a/test/no-unit-blocks-in-live-e2e.test.ts b/test/no-unit-blocks-in-live-e2e.test.ts index bb37f9d1197..8bdc4ca4519 100644 --- a/test/no-unit-blocks-in-live-e2e.test.ts +++ b/test/no-unit-blocks-in-live-e2e.test.ts @@ -3,7 +3,10 @@ import { describe, expect, it } from "vitest"; -import { findLiveUnitBlocks, formatViolations } from "../scripts/checks/no-unit-blocks-in-live-e2e"; +import { + findLiveUnitBlocks, + formatViolations, +} from "../scripts/checks/no-unit-blocks-in-live-e2e.mts"; const FILE = "test/e2e/live/example.test.ts"; diff --git a/test/policy-mutation-read-discovery.test.ts b/test/policy-mutation-read-discovery.test.ts index 6c02027145f..e1224195d79 100644 --- a/test/policy-mutation-read-discovery.test.ts +++ b/test/policy-mutation-read-discovery.test.ts @@ -10,7 +10,7 @@ import { describe, expect, it } from "vitest"; import { auditOpenShellPolicyMutationReads, discoverPolicyReadSites, -} from "../scripts/checks/openshell-policy-mutation-read"; +} from "../scripts/checks/openshell-policy-mutation-read.mts"; describe("OpenShell policy mutation read discovery", () => { it("discovers builder and direct policy reads in new production files", () => { diff --git a/test/pr-workflow-contract.test.ts b/test/pr-workflow-contract.test.ts index 23b0246c907..9bdd3822130 100644 --- a/test/pr-workflow-contract.test.ts +++ b/test/pr-workflow-contract.test.ts @@ -975,7 +975,6 @@ describe("pull request and main workflow contracts", () => { expect(parityStep.run).toContain("base=HEAD^1"); expect(parityStep.run).toContain("head=HEAD^2"); expect(parityStep.run).toContain('base="$PUSH_BASE_SHA"'); - const trustedCapabilityProbe = requiredWorkflowStep( prWorkflow.jobs["cli-test-shards"], "Detect trusted E2E support sharding", diff --git a/test/test-boundary-guards.test.ts b/test/test-boundary-guards.test.ts index 3237fbf5b9d..65de61e42cc 100644 --- a/test/test-boundary-guards.test.ts +++ b/test/test-boundary-guards.test.ts @@ -12,7 +12,7 @@ import { findFastProjectTransitiveViolations, isFastProjectTestPath, isScannedTestPath, -} from "../scripts/checks/no-test-dist-imports"; +} from "../scripts/checks/no-test-dist-imports.mts"; import { discoverVitestCandidates, EXPECTED_VITEST_PROJECTS, @@ -22,7 +22,7 @@ import { parseProjectListing, parseProjectRoster, resolveVitestInvocation, -} from "../scripts/checks/vitest-project-overlap"; +} from "../scripts/checks/vitest-project-overlap.mts"; const REPO_ROOT = path.join(import.meta.dirname, ".."); const SOURCE_RUNTIME = path.join(REPO_ROOT, "test", "helpers", "onboard-script-mocks.cjs"); diff --git a/test/test-create-require-budget.test.ts b/test/test-create-require-budget.test.ts index 5b4f0d67a5b..749bd292bf0 100644 --- a/test/test-create-require-budget.test.ts +++ b/test/test-create-require-budget.test.ts @@ -12,7 +12,7 @@ import { collectTestSupportCreateRequireSources, containsCreateRequireIdentifier, createRequireBudgetFailure, -} from "../scripts/checks/test-create-require-budget"; +} from "../scripts/checks/test-create-require-budget.mts"; const tempDirs = new Set(); diff --git a/test/test-title-style.test.ts b/test/test-title-style.test.ts index 81b6f35acda..9b6b6395906 100644 --- a/test/test-title-style.test.ts +++ b/test/test-title-style.test.ts @@ -3,7 +3,7 @@ import { describe, expect, it } from "vitest"; -import { scanTestTitleStyle } from "../scripts/checks/test-title-style"; +import { scanTestTitleStyle } from "../scripts/checks/test-title-style.mts"; function rulesFor(source: string): string[] { return scanTestTitleStyle("test/virtual-title-style.test.ts", source).map( From 38d7bc000893d215baecf47209c84f433ba4e5a8 Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Thu, 16 Jul 2026 06:12:32 +0000 Subject: [PATCH 2/9] fix(ci): correct e2e-mock-parity usage diagnostic for .mts entrypoint Signed-off-by: Tinson Lai Signed-off-by: Apurv Kumaria --- scripts/checks/e2e-mock-parity.mts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/checks/e2e-mock-parity.mts b/scripts/checks/e2e-mock-parity.mts index b9dbf9e64e6..93bcbc23fc2 100644 --- a/scripts/checks/e2e-mock-parity.mts +++ b/scripts/checks/e2e-mock-parity.mts @@ -182,7 +182,7 @@ function changedFiles(base: string, head: string): string[] { function main(): void { const base = argument("--base"); const head = argument("--head") ?? "HEAD"; - if (!base) throw new Error("usage: e2e-mock-parity.ts --base [--head ]"); + if (!base) throw new Error("usage: e2e-mock-parity.mts --base [--head ]"); const manifestPath = path.join(REPO_ROOT, DEFAULT_PARITY_MANIFEST); const manifest = JSON.parse(fs.readFileSync(manifestPath, "utf8")) as MockParityManifest; From dbef6928beb6979519bf9df32e029856090c26d0 Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Thu, 16 Jul 2026 06:13:04 +0000 Subject: [PATCH 3/9] fix(ci): scan .mts sources for coverage exclusion directives Signed-off-by: Tinson Lai Signed-off-by: Apurv Kumaria --- scripts/checks/no-coverage-ignore.mts | 4 ++-- test/no-coverage-ignore.test.ts | 17 ++++++++++++++++- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/scripts/checks/no-coverage-ignore.mts b/scripts/checks/no-coverage-ignore.mts index 10011145773..3c518bbc920 100644 --- a/scripts/checks/no-coverage-ignore.mts +++ b/scripts/checks/no-coverage-ignore.mts @@ -15,7 +15,7 @@ import { fileURLToPath } from "node:url"; const REPO_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../.."); const SCAN_ROOTS = ["bin", "src", "scripts", "test", "nemoclaw/src"]; -const SOURCE_EXTENSIONS = new Set([".cjs", ".cts", ".js", ".mjs", ".ts", ".tsx"]); +const SOURCE_EXTENSIONS = new Set([".cjs", ".cts", ".js", ".mjs", ".mts", ".ts", ".tsx"]); const SKIP_DIRS = new Set([".git", "coverage", "dist", "node_modules"]); const FORBIDDEN_DIRECTIVE = ["v8", "ignore"].join(" "); const FORBIDDEN_DIRECTIVE_PATTERN = new RegExp( @@ -91,7 +91,7 @@ function* walkSourceFiles(dir: string): Generator { } } -function isScannedSourcePath(filePath: string): boolean { +export function isScannedSourcePath(filePath: string): boolean { return ( filePath.length > 0 && SCAN_ROOTS.some((root) => filePath === root || filePath.startsWith(`${root}/`)) && diff --git a/test/no-coverage-ignore.test.ts b/test/no-coverage-ignore.test.ts index 6e0de027672..d8865c3258a 100644 --- a/test/no-coverage-ignore.test.ts +++ b/test/no-coverage-ignore.test.ts @@ -2,7 +2,10 @@ // SPDX-License-Identifier: Apache-2.0 import { describe, expect, it } from "vitest"; -import { findCoverageIgnoreDirectives } from "../scripts/checks/no-coverage-ignore.mts"; +import { + findCoverageIgnoreDirectives, + isScannedSourcePath, +} from "../scripts/checks/no-coverage-ignore.mts"; const forbiddenDirective = ["v8", "ignore"].join(" "); @@ -29,3 +32,15 @@ describe("coverage ignore guard", () => { expect(findCoverageIgnoreDirectives(source, "src/example.ts")).toEqual([]); }); }); + +describe("scanned source path selection", () => { + it("scans .mts files under tracked roots", () => { + expect(isScannedSourcePath("scripts/checks/no-coverage-ignore.mts")).toBe(true); + expect(isScannedSourcePath("src/lib/example.mts")).toBe(true); + }); + + it("excludes non-source extensions and paths outside tracked roots", () => { + expect(isScannedSourcePath("scripts/checks/README.md")).toBe(false); + expect(isScannedSourcePath("docs/example.mts")).toBe(false); + }); +}); From 0a3120ed5fc299954540425ef527cf831ea0068e Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Thu, 16 Jul 2026 06:15:24 +0000 Subject: [PATCH 4/9] test(ci): cover the Hermes light-skin version boundary gate Signed-off-by: Tinson Lai Signed-off-by: Apurv Kumaria --- scripts/checks/hermes-light-skin-boundary.mts | 38 +++++++++++------- test/hermes-light-skin-boundary.test.ts | 39 +++++++++++++++++++ 2 files changed, 64 insertions(+), 13 deletions(-) create mode 100644 test/hermes-light-skin-boundary.test.ts diff --git a/scripts/checks/hermes-light-skin-boundary.mts b/scripts/checks/hermes-light-skin-boundary.mts index 75c70a4ac58..43539e3c7fd 100644 --- a/scripts/checks/hermes-light-skin-boundary.mts +++ b/scripts/checks/hermes-light-skin-boundary.mts @@ -12,22 +12,34 @@ const { NEMOCLAW_HERMES_LIGHT_SKIN_REVIEWED_HERMES_VERSIONS } = await import( const REPO_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../.."); const HERMES_DOCKERFILE_BASE = "agents/hermes/Dockerfile.base"; -function main(): void { - const dockerfile = fs.readFileSync(path.join(REPO_ROOT, HERMES_DOCKERFILE_BASE), "utf8"); - const pinnedVersion = dockerfile.match(/^ARG HERMES_VERSION=(\S+)$/m)?.[1]; +export function checkHermesLightSkinBoundary(options: { + dockerfileText: string; + reviewedVersions: readonly string[]; +}): string | null { + const { dockerfileText, reviewedVersions } = options; + const pinnedVersion = dockerfileText.match(/^ARG HERMES_VERSION=(\S+)$/m)?.[1]; if (!pinnedVersion) { - throw new Error(`${HERMES_DOCKERFILE_BASE}: could not find ARG HERMES_VERSION`); + return `${HERMES_DOCKERFILE_BASE}: could not find ARG HERMES_VERSION`; } - const reviewedVersions: readonly string[] = NEMOCLAW_HERMES_LIGHT_SKIN_REVIEWED_HERMES_VERSIONS; if (!reviewedVersions.includes(pinnedVersion)) { - throw new Error( - [ - "Hermes light terminal compatibility skin needs re-review.", - `${HERMES_DOCKERFILE_BASE} pins ${pinnedVersion}, but connect-env.ts was reviewed for ${reviewedVersions.join(", ")}.`, - "Remove the NemoClaw-managed light skin if upstream Hermes is readable in light terminals, or update the reviewed version constant after validating it still needs the shim.", - ].join(" "), - ); + return [ + "Hermes light terminal compatibility skin needs re-review.", + `${HERMES_DOCKERFILE_BASE} pins ${pinnedVersion}, but connect-env.ts was reviewed for ${reviewedVersions.join(", ")}.`, + "Remove the NemoClaw-managed light skin if upstream Hermes is readable in light terminals, or update the reviewed version constant after validating it still needs the shim.", + ].join(" "); } + return null; } -main(); +function main(): void { + const dockerfileText = fs.readFileSync(path.join(REPO_ROOT, HERMES_DOCKERFILE_BASE), "utf8"); + const error = checkHermesLightSkinBoundary({ + dockerfileText, + reviewedVersions: NEMOCLAW_HERMES_LIGHT_SKIN_REVIEWED_HERMES_VERSIONS, + }); + if (error) throw new Error(error); +} + +if (fileURLToPath(import.meta.url) === path.resolve(process.argv[1] ?? "")) { + main(); +} diff --git a/test/hermes-light-skin-boundary.test.ts b/test/hermes-light-skin-boundary.test.ts new file mode 100644 index 00000000000..801c3172189 --- /dev/null +++ b/test/hermes-light-skin-boundary.test.ts @@ -0,0 +1,39 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it } from "vitest"; +import { checkHermesLightSkinBoundary } from "../scripts/checks/hermes-light-skin-boundary.mts"; + +function dockerfileWithVersion(version: string): string { + return ["FROM debian:bookworm-slim", `ARG HERMES_VERSION=${version}`, ""].join("\n"); +} + +describe("hermes light-skin boundary check", () => { + it("passes when the pinned version is reviewed", () => { + expect( + checkHermesLightSkinBoundary({ + dockerfileText: dockerfileWithVersion("v2026.7.1"), + reviewedVersions: ["v2026.6.19", "v2026.7.1"], + }), + ).toBeNull(); + }); + + it("fails when the pinned version has not been reviewed", () => { + const error = checkHermesLightSkinBoundary({ + dockerfileText: dockerfileWithVersion("v2026.8.1"), + reviewedVersions: ["v2026.6.19", "v2026.7.1"], + }); + + expect(error).toContain("needs re-review"); + expect(error).toContain("v2026.8.1"); + }); + + it("fails when the Dockerfile has no HERMES_VERSION arg", () => { + const error = checkHermesLightSkinBoundary({ + dockerfileText: "FROM debian:bookworm-slim\n", + reviewedVersions: ["v2026.7.1"], + }); + + expect(error).toContain("could not find ARG HERMES_VERSION"); + }); +}); From 5f0fa7ceabdf3e8bbee4be1cb70e2c4c706b439c Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Thu, 16 Jul 2026 13:18:23 -0700 Subject: [PATCH 5/9] fix(checks): harden migrated repository guardrails Co-authored-by: Tinson Lai Signed-off-by: Apurv Kumaria --- .github/workflows/pr.yaml | 1 + scripts/checks/dependency-pins.mts | 3 + scripts/checks/layer-import-boundaries.mts | 100 +++++++--- scripts/checks/no-test-dist-imports.mts | 49 +++-- scripts/checks/no-unit-blocks-in-live-e2e.mts | 50 +++-- scripts/checks/test-create-require-budget.mts | 115 ++++++++++++ test/dependency-pins-check.test.ts | 5 + test/layer-import-boundaries.test.ts | 172 +++++++++++++++++- test/no-unit-blocks-in-live-e2e.test.ts | 20 ++ .../credentials-shim.test.ts | 4 +- test/pr-workflow-contract.test.ts | 3 + test/test-boundary-guards.test.ts | 59 +++++- test/test-create-require-budget.test.ts | 45 +++++ 13 files changed, 561 insertions(+), 65 deletions(-) rename test/{ => package-contract}/credentials-shim.test.ts (95%) diff --git a/.github/workflows/pr.yaml b/.github/workflows/pr.yaml index 13bf648ca08..a2f20092577 100644 --- a/.github/workflows/pr.yaml +++ b/.github/workflows/pr.yaml @@ -85,6 +85,7 @@ jobs: - name: Checkout uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: + fetch-depth: 0 persist-credentials: false - name: Checkout trusted CI actions diff --git a/scripts/checks/dependency-pins.mts b/scripts/checks/dependency-pins.mts index 4e44a700203..450d6ec4f57 100644 --- a/scripts/checks/dependency-pins.mts +++ b/scripts/checks/dependency-pins.mts @@ -214,6 +214,9 @@ function deriveDependencyPins(rootDir: string = REPO_ROOT): { }, }; + if (pins.openshell.minVersion && !NUMERIC_VERSION_RE.test(pins.openshell.minVersion)) { + failures.push("nemoclaw-blueprint/blueprint.yaml min_openshell_version must match X.Y.Z"); + } if (pins.openshell.maxVersion && !NUMERIC_VERSION_RE.test(pins.openshell.maxVersion)) { failures.push("nemoclaw-blueprint/blueprint.yaml max_openshell_version must match X.Y.Z"); } diff --git a/scripts/checks/layer-import-boundaries.mts b/scripts/checks/layer-import-boundaries.mts index 5d554fc94e6..8348c94877d 100644 --- a/scripts/checks/layer-import-boundaries.mts +++ b/scripts/checks/layer-import-boundaries.mts @@ -1,7 +1,7 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { existsSync, readdirSync, readFileSync, statSync } from "node:fs"; +import { existsSync, lstatSync, readdirSync, readFileSync, realpathSync } from "node:fs"; import path from "node:path"; import { fileURLToPath } from "node:url"; import ts from "typescript"; @@ -29,24 +29,26 @@ function toRepoPath(absPath: string): string { } function isProductionTsFile(absPath: string): boolean { - return absPath.endsWith(".ts") && !absPath.endsWith(".test.ts") && !absPath.endsWith(".spec.ts"); + return ( + /\.(?:cts|mts|ts|tsx)$/.test(absPath) && !/\.(?:test|spec)\.(?:cts|mts|ts|tsx)$/.test(absPath) + ); } function* walk(dir: string): Generator { if (!existsSync(dir)) return; - const rootStats = statSync(dir); + const rootStats = lstatSync(dir); + if (rootStats.isSymbolicLink()) return; if (rootStats.isFile()) { if (isProductionTsFile(dir)) yield dir; return; } if (!rootStats.isDirectory()) return; - for (const entry of readdirSync(dir)) { - if (SKIP_DIRS.has(entry)) continue; - const absPath = path.join(dir, entry); - const stats = statSync(absPath); - if (stats.isDirectory()) { + for (const entry of readdirSync(dir, { withFileTypes: true })) { + if (SKIP_DIRS.has(entry.name) || entry.isSymbolicLink()) continue; + const absPath = path.join(dir, entry.name); + if (entry.isDirectory()) { yield* walk(absPath); - } else if (stats.isFile() && isProductionTsFile(absPath)) { + } else if (entry.isFile() && isProductionTsFile(absPath)) { yield absPath; } } @@ -58,7 +60,7 @@ function sourceFileFor(absPath: string): ts.SourceFile { readFileSync(absPath, "utf8"), ts.ScriptTarget.Latest, true, - ts.ScriptKind.TS, + absPath.endsWith(".tsx") ? ts.ScriptKind.TSX : ts.ScriptKind.TS, ); } @@ -111,9 +113,19 @@ function collectImportRefs(absPath: string): ImportRef[] { function resolveInternalImport(fromAbsPath: string, specifier: string): string | null { if (!specifier.startsWith(".")) return null; const base = path.resolve(path.dirname(fromAbsPath), specifier); - const candidates = [base, `${base}.ts`, `${base}.tsx`, path.join(base, "index.ts")]; + const extensions = [".ts", ".tsx", ".mts", ".cts"]; + const candidates = [ + base, + ...extensions.map((extension) => `${base}${extension}`), + ...extensions.map((extension) => path.join(base, `index${extension}`)), + ]; const found = candidates.find((candidate) => existsSync(candidate)); - return found ? toRepoPath(found) : toRepoPath(`${base}.ts`); + if (!found) return toRepoPath(`${base}.ts`); + try { + return toRepoPath(realpathSync(found)); + } catch { + return toRepoPath(found); + } } function isDomainFile(repoPath: string): boolean { @@ -134,7 +146,7 @@ function isMessagingManifestFile(repoPath: string): boolean { function isActionFile(repoPath: string): boolean { if (repoPath.startsWith("src/lib/actions/")) return true; - return /(^|\/)[^/]+-actions?\.ts$/.test(repoPath); + return /(^|\/)[^/]+-actions?\.(?:cts|mts|ts|tsx)$/.test(repoPath); } function importTargetsForbiddenLayer( @@ -319,27 +331,59 @@ function checkMessagingManifestFile( function checkCommandFile(absPath: string, repoPath: string, violations: Violation[]): void { const sourceFile = sourceFileFor(absPath); - let commandClassCount = 0; - - function isCommandBase(expression: ts.ExpressionWithTypeArguments): boolean { - const text = expression.expression.getText(sourceFile); - return text === "Command" || text === "NemoClawCommand"; - } + const identifierBases = new Set(); + const namespaceBases = new Map>(); - function visit(node: ts.Node): void { + for (const statement of sourceFile.statements) { if ( - ts.isClassDeclaration(node) && - node.heritageClauses?.some( - (clause) => - clause.token === ts.SyntaxKind.ExtendsKeyword && clause.types.some(isCommandBase), - ) + !ts.isImportDeclaration(statement) || + !ts.isStringLiteral(statement.moduleSpecifier) || + !statement.importClause || + statement.importClause.isTypeOnly ) { - commandClassCount += 1; + continue; + } + + const moduleSpecifier = statement.moduleSpecifier.text; + const exportedBases = + moduleSpecifier === "@oclif/core" + ? new Set(["Command"]) + : resolveInternalImport(absPath, moduleSpecifier) === + "src/lib/cli/nemoclaw-oclif-command.ts" + ? new Set(["NemoClawCommand"]) + : null; + if (!exportedBases) continue; + + const bindings = statement.importClause.namedBindings; + if (bindings && ts.isNamedImports(bindings)) { + for (const binding of bindings.elements) { + if (binding.isTypeOnly) continue; + const importedName = binding.propertyName?.text ?? binding.name.text; + if (exportedBases.has(importedName)) identifierBases.add(binding.name.text); + } + } else if (bindings && ts.isNamespaceImport(bindings)) { + namespaceBases.set(bindings.name.text, exportedBases); } - ts.forEachChild(node, visit); } - visit(sourceFile); + function isCommandBase(expression: ts.ExpressionWithTypeArguments): boolean { + const base = expression.expression; + if (ts.isIdentifier(base)) return identifierBases.has(base.text); + return ( + ts.isPropertyAccessExpression(base) && + ts.isIdentifier(base.expression) && + namespaceBases.get(base.expression.text)?.has(base.name.text) === true + ); + } + + const commandClassCount = sourceFile.statements.filter( + (statement) => + ts.isClassDeclaration(statement) && + statement.heritageClauses?.some( + (clause) => + clause.token === ts.SyntaxKind.ExtendsKeyword && clause.types.some(isCommandBase), + ), + ).length; if (commandClassCount !== 1) { addViolation( violations, diff --git a/scripts/checks/no-test-dist-imports.mts b/scripts/checks/no-test-dist-imports.mts index 5ad594a6466..0c729847d08 100644 --- a/scripts/checks/no-test-dist-imports.mts +++ b/scripts/checks/no-test-dist-imports.mts @@ -1,11 +1,13 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { existsSync, readdirSync, readFileSync, realpathSync, statSync } from "node:fs"; +import { existsSync, lstatSync, readdirSync, readFileSync, realpathSync } from "node:fs"; import path from "node:path"; import { fileURLToPath, pathToFileURL } from "node:url"; import ts from "typescript"; +import { expectedProjectForTestPath } from "./vitest-project-overlap.mts"; + export type Violation = { chain?: string[]; detail: string; @@ -56,8 +58,15 @@ export function isScannedTestPath(relativePath: string): boolean { export function isFastProjectTestPath(relativePath: string): boolean { const normalized = relativePath.replaceAll("\\", "/"); - if (normalized.startsWith("src/") && normalized.split("/").includes(".claude")) return false; - return /^(?:src|nemoclaw\/src|test\/e2e\/support)\/.+\.test\.ts$/.test(normalized); + if (normalized.split("/").includes(".claude")) return false; + if (!/\.(?:test|spec)\.(?:[cm]?[jt]sx?)$/.test(normalized)) return false; + const project = expectedProjectForTestPath(normalized); + return ( + project === "cli" || + project === "integration" || + project === "plugin" || + project === "e2e-support" + ); } function isScannedTestFile(absolutePath: string): boolean { @@ -69,10 +78,13 @@ function* walk( acceptsFile: (absolutePath: string) => boolean = isScannedTestFile, ): Generator { if (!existsSync(directory)) return; + const rootStats = lstatSync(directory); + if (rootStats.isSymbolicLink() || !rootStats.isDirectory()) return; for (const entry of readdirSync(directory)) { if (SKIP_DIRS.has(entry)) continue; const absolutePath = path.join(directory, entry); - const stats = statSync(absolutePath); + const stats = lstatSync(absolutePath); + if (stats.isSymbolicLink()) continue; if (stats.isDirectory()) yield* walk(absolutePath, acceptsFile); else if (stats.isFile() && acceptsFile(absolutePath)) yield absolutePath; } @@ -861,18 +873,23 @@ function resolveRepoModule(importer: string, specifier: string): ResolvedRepoMod .resolvedModule?.resolvedFileName; if (!resolved) return undefined; const absolutePath = path.resolve(resolved); - const compiledTarget = resolvedCompiledInternalTarget(absolutePath); - if (compiledTarget) return { absolutePath, compiledTarget }; - return isRepoSourceModule(absolutePath) ? { absolutePath } : undefined; -} - -function collectFastProjectEntries(): string[] { - const acceptsFile = (absolutePath: string) => isFastProjectTestPath(repoPath(absolutePath)); - return [ - ...walk(path.join(REPO_ROOT, "src"), acceptsFile), - ...walk(path.join(REPO_ROOT, "nemoclaw", "src"), acceptsFile), - ...walk(path.join(REPO_ROOT, "test", "e2e", "support"), acceptsFile), - ].sort(); + let canonicalPath: string; + try { + canonicalPath = realpathSync(absolutePath); + } catch { + return undefined; + } + const compiledTarget = resolvedCompiledInternalTarget(canonicalPath); + if (compiledTarget) return { absolutePath: canonicalPath, compiledTarget }; + return isRepoSourceModule(canonicalPath) ? { absolutePath: canonicalPath } : undefined; +} + +export function collectFastProjectEntries(repoRoot = REPO_ROOT): string[] { + const acceptsFile = (absolutePath: string) => + isFastProjectTestPath(path.relative(repoRoot, absolutePath).split(path.sep).join("/")); + return ["src", "nemoclaw/src", "test"] + .flatMap((root) => [...walk(path.join(repoRoot, root), acceptsFile)]) + .sort(); } export function findFastProjectTransitiveViolations( diff --git a/scripts/checks/no-unit-blocks-in-live-e2e.mts b/scripts/checks/no-unit-blocks-in-live-e2e.mts index 5b9b86bffcd..fe9df3e9241 100644 --- a/scripts/checks/no-unit-blocks-in-live-e2e.mts +++ b/scripts/checks/no-unit-blocks-in-live-e2e.mts @@ -29,17 +29,20 @@ import { existsSync, readdirSync, readFileSync, statSync } from "node:fs"; import path from "node:path"; import { fileURLToPath } from "node:url"; +import ts from "typescript"; const REPO_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../.."); const LIVE_DIR = path.join(REPO_ROOT, "test", "e2e", "live"); const TEST_FILE_PATTERN = /\.(?:test|spec)\.(?:[cm]?[jt]s)$/; -// Match the vitest unit primitive `it(` — including `it.each(`, `it.only(`, -// `it.skip(`, etc. — as a call at a statement boundary. The leading boundary -// (line start or whitespace) prevents matching inside a custom identifier, and -// requiring a call paren after the optional member keeps non-call references -// from matching. -const IT_PRIMITIVE_PATTERN = - /(?:^|[\s;{(])it(?:\.(?:each|only|skip|todo|fails|concurrent|sequential))?\s*\(/; +const IT_PRIMITIVE_MEMBERS = new Set([ + "concurrent", + "each", + "fails", + "only", + "sequential", + "skip", + "todo", +]); export type LiveUnitBlockViolation = { readonly file: string; @@ -67,17 +70,32 @@ function* walkFiles(dir: string): Generator { export function findLiveUnitBlocks(source: string, file: string): LiveUnitBlockViolation[] { const violations: LiveUnitBlockViolation[] = []; const lines = source.split(/\r\n|\r|\n/); - for (let i = 0; i < lines.length; i += 1) { - const text = lines[i] ?? ""; - const trimmed = text.trimStart(); - // Skip import lines (`import { it, test } from "vitest"`) and comments. - if (trimmed.startsWith("import ") || trimmed.startsWith("//") || trimmed.startsWith("*")) { - continue; - } - if (IT_PRIMITIVE_PATTERN.test(text)) { - violations.push({ file, line: i + 1, text: trimmed }); + const sourceFile = ts.createSourceFile(file, source, ts.ScriptTarget.Latest, true); + const reportedLines = new Set(); + + function isItPrimitive(expression: ts.LeftHandSideExpression): boolean { + if (ts.isIdentifier(expression)) return expression.text === "it"; + return ( + ts.isPropertyAccessExpression(expression) && + ts.isIdentifier(expression.expression) && + expression.expression.text === "it" && + IT_PRIMITIVE_MEMBERS.has(expression.name.text) + ); + } + + function visit(node: ts.Node): void { + if (ts.isCallExpression(node) && isItPrimitive(node.expression)) { + const line = sourceFile.getLineAndCharacterOfPosition(node.getStart(sourceFile)).line + 1; + const text = (lines[line - 1] ?? "").trimStart(); + if (!text.startsWith("*") && !reportedLines.has(line)) { + reportedLines.add(line); + violations.push({ file, line, text }); + } } + ts.forEachChild(node, visit); } + + visit(sourceFile); return violations; } diff --git a/scripts/checks/test-create-require-budget.mts b/scripts/checks/test-create-require-budget.mts index 05eb7934966..5705d675503 100644 --- a/scripts/checks/test-create-require-budget.mts +++ b/scripts/checks/test-create-require-budget.mts @@ -1,6 +1,7 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +import { spawnSync } from "node:child_process"; import { existsSync, lstatSync, readdirSync, readFileSync } from "node:fs"; import path from "node:path"; import { fileURLToPath } from "node:url"; @@ -57,6 +58,104 @@ export const TEST_SUPPORT_CREATE_REQUIRE_FILES = [ "test/support/status-flow-test-harness.ts", ] as const; +export type CreateRequireAllowlists = Readonly<{ + cli: readonly string[]; + testSupport: readonly string[]; +}>; + +const ALLOWLIST_EXPORTS = { + CLI_CREATE_REQUIRE_FILES: "cli", + TEST_SUPPORT_CREATE_REQUIRE_FILES: "testSupport", +} as const; + +function arrayLiteral(initializer: ts.Expression | undefined): ts.ArrayLiteralExpression | null { + let expression = initializer; + while ( + expression && + (ts.isAsExpression(expression) || + ts.isSatisfiesExpression(expression) || + ts.isParenthesizedExpression(expression)) + ) { + expression = expression.expression; + } + return expression && ts.isArrayLiteralExpression(expression) ? expression : null; +} + +export function extractCreateRequireAllowlists( + sourceText: string, + fileName = "scripts/checks/test-create-require-budget.ts", +): CreateRequireAllowlists { + const sourceFile = ts.createSourceFile(fileName, sourceText, ts.ScriptTarget.Latest, true); + const values: Partial> = {}; + + for (const statement of sourceFile.statements) { + if (!ts.isVariableStatement(statement)) continue; + for (const declaration of statement.declarationList.declarations) { + if (!ts.isIdentifier(declaration.name)) continue; + const target = ALLOWLIST_EXPORTS[declaration.name.text as keyof typeof ALLOWLIST_EXPORTS]; + if (!target) continue; + const array = arrayLiteral(declaration.initializer); + if (!array || array.elements.some((element) => !ts.isStringLiteralLike(element))) { + throw new Error(`${declaration.name.text} must be a literal string array`); + } + if (values[target]) throw new Error(`${declaration.name.text} must be declared exactly once`); + values[target] = array.elements.map((element) => (element as ts.StringLiteralLike).text); + } + } + + if (!values.cli || !values.testSupport) { + throw new Error("createRequire allowlist source must declare both reviewed allowlists"); + } + return { cli: values.cli, testSupport: values.testSupport }; +} + +export function createRequireAllowlistExpansionFailure( + current: CreateRequireAllowlists, + baseline: CreateRequireAllowlists, +): string | null { + const cliAdditions = current.cli.filter((file) => !baseline.cli.includes(file)).sort(); + const supportAdditions = current.testSupport + .filter((file) => !baseline.testSupport.includes(file)) + .sort(); + if (cliAdditions.length === 0 && supportAdditions.length === 0) return null; + + return [ + "createRequire allowlists must not expand relative to the merge base.", + ...cliAdditions.map((file) => `- CLI_CREATE_REQUIRE_FILES: ${file}`), + ...supportAdditions.map((file) => `- TEST_SUPPORT_CREATE_REQUIRE_FILES: ${file}`), + ].join("\n"); +} + +function mergeBaseAllowlists(): CreateRequireAllowlists | null { + const baseBranch = process.env.GITHUB_BASE_REF?.trim(); + const baseRef = baseBranch ? `origin/${baseBranch}` : "origin/main"; + const mergeBase = spawnSync("git", ["merge-base", "HEAD", baseRef], { + cwd: REPO_ROOT, + encoding: "utf8", + timeout: 5_000, + }); + if (mergeBase.status !== 0 || !mergeBase.stdout.trim()) { + if (baseBranch) { + throw new Error(`could not resolve the pull-request merge base against ${baseRef}`); + } + return null; + } + + const revision = mergeBase.stdout.trim(); + for (const relativePath of [ + "scripts/checks/test-create-require-budget.mts", + "scripts/checks/test-create-require-budget.ts", + ]) { + const source = spawnSync("git", ["show", `${revision}:${relativePath}`], { + cwd: REPO_ROOT, + encoding: "utf8", + timeout: 5_000, + }); + if (source.status === 0) return extractCreateRequireAllowlists(source.stdout, relativePath); + } + throw new Error(`merge base ${revision} does not contain the createRequire budget check`); +} + function* walkTypeScriptFiles(directory: string): Generator { if (!existsSync(directory)) return; @@ -157,6 +256,22 @@ export function createRequireBudgetFailure( } function main(): void { + const baseline = mergeBaseAllowlists(); + const expansionFailure = baseline + ? createRequireAllowlistExpansionFailure( + { + cli: CLI_CREATE_REQUIRE_FILES, + testSupport: TEST_SUPPORT_CREATE_REQUIRE_FILES, + }, + baseline, + ) + : null; + if (expansionFailure) { + console.error(expansionFailure); + process.exitCode = 1; + return; + } + const productionFiles = collectProductionCreateRequireSources(); if (productionFiles.length > 0) { console.error( diff --git a/test/dependency-pins-check.test.ts b/test/dependency-pins-check.test.ts index 67bbeadc575..2a5109e6eb0 100644 --- a/test/dependency-pins-check.test.ts +++ b/test/dependency-pins-check.test.ts @@ -263,6 +263,11 @@ describe("dependency pin drift check", () => { }); it.each([ + { + name: "an unsafe OpenShell minimum", + overrides: { openshellMin: "../1.2.3" }, + failure: "nemoclaw-blueprint/blueprint.yaml min_openshell_version must match X.Y.Z", + }, { name: "an unsafe OpenShell maximum", overrides: { openshellMax: "../1.2.4" }, diff --git a/test/layer-import-boundaries.test.ts b/test/layer-import-boundaries.test.ts index 6a8eac03d7f..1c7b1c556df 100644 --- a/test/layer-import-boundaries.test.ts +++ b/test/layer-import-boundaries.test.ts @@ -11,9 +11,22 @@ import { findLayerImportBoundaryViolations } from "../scripts/checks/layer-impor const REPO_ROOT = path.join(import.meta.dirname, ".."); let fixtureCounter = 0; -function fixturePath(dir: string, label: string): string { +function fixturePath(dir: string, label: string, extension = ".ts"): string { fixtureCounter += 1; - return path.join(REPO_ROOT, dir, `__boundary-${label}-${process.pid}-${fixtureCounter}.ts`); + return path.join( + REPO_ROOT, + dir, + `__boundary-${label}-${process.pid}-${fixtureCounter}${extension}`, + ); +} + +function namedActionFixturePath(extension = ".mts"): string { + fixtureCounter += 1; + return path.join( + REPO_ROOT, + "src/lib", + `__boundary-${process.pid}-${fixtureCounter}-action${extension}`, + ); } function scanFixture(fixture: string, source: string) { @@ -89,4 +102,159 @@ describe("CLI layer import boundaries (#6245)", () => { ]), ); }); + + it.each([ + { + name: "a direct oclif import", + source: + 'import { Command } from "@oclif/core";\nexport default class Example extends Command {}\n', + }, + { + name: "an aliased oclif import", + source: + 'import { Command as OclifCommand } from "@oclif/core";\nexport default class Example extends OclifCommand {}\n', + }, + { + name: "a namespace-qualified oclif import", + source: + 'import * as oclif from "@oclif/core";\nexport default class Example extends oclif.Command {}\n', + }, + { + name: "the NemoClaw command base", + source: + 'import { NemoClawCommand as Base } from "../lib/cli/nemoclaw-oclif-command";\nexport default class Example extends Base {}\n', + }, + ])("recognizes $name by its import binding (#6245)", ({ source }) => { + const violations = scanFixture(fixturePath("src/commands", "command-binding"), source); + + expect(violations).not.toEqual( + expect.arrayContaining([expect.objectContaining({ rule: "one-command-per-file" })]), + ); + }); + + it.each([ + "Command", + "NemoClawCommand", + ])("rejects an unrelated local %s class as a command base (#6245)", (baseName) => { + const violations = scanFixture( + fixturePath("src/commands", "local-command-base"), + `class ${baseName} {}\nexport default class Example extends ${baseName} {}\n`, + ); + + expect(violations).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + detail: "command files must define exactly one registered oclif command class; found 0", + }), + ]), + ); + }); + + it.each([ + ".mts", + ".cts", + ".tsx", + ])("scans production %s modules for protected-layer violations (#6245)", (extension) => { + const violations = scanFixture( + fixturePath("src/lib/actions", "module-extension", extension), + 'import { Command } from "@oclif/core";\n', + ); + + expect(violations).toEqual( + expect.arrayContaining([expect.objectContaining({ rule: "actions-no-oclif" })]), + ); + }); + + it("recognizes alternate-extension action modules outside the actions directory (#6245)", () => { + const violations = scanFixture( + namedActionFixturePath(), + 'import { Command } from "@oclif/core";\n', + ); + + expect(violations).toEqual( + expect.arrayContaining([expect.objectContaining({ rule: "actions-no-oclif" })]), + ); + }); + + it("resolves extensionless imports to alternate TypeScript modules (#6245)", () => { + const target = fixturePath("src/lib/actions", "extensionless-target", ".mts"); + const importer = fixturePath("src/lib/domain", "extensionless-importer", ".mts"); + const specifier = path + .relative(path.dirname(importer), target) + .split(path.sep) + .join("/") + .replace(/\.mts$/, ""); + try { + fs.writeFileSync(target, "export const value = true;\n"); + fs.writeFileSync(importer, `import { value } from "${specifier}";\nexport { value };\n`); + + expect(findLayerImportBoundaryViolations(importer)).toEqual([ + expect.objectContaining({ + detail: `domain must not import ${path.relative(REPO_ROOT, target)}`, + }), + ]); + } finally { + fs.rmSync(importer, { force: true }); + fs.rmSync(target, { force: true }); + } + }); + + it.each([ + ".test.mts", + ".spec.cts", + ".test.tsx", + ])("excludes %s test modules from the production scan (#6245)", (extension) => { + expect( + scanFixture( + fixturePath("src/lib/actions", "test-module-extension", extension), + 'import { Command } from "@oclif/core";\n', + ), + ).toEqual([]); + }); + + it("does not recurse through a symbolic-link loop (#6245)", () => { + const fixtureRoot = fs.mkdtempSync( + path.join(REPO_ROOT, "src/lib/domain/__boundary-symlink-loop-"), + ); + try { + fs.writeFileSync( + path.join(fixtureRoot, "violation.mts"), + 'import { spawn } from "node:child_process";\nexport { spawn };\n', + ); + fs.symlinkSync(".", path.join(fixtureRoot, "loop"), "dir"); + + expect(findLayerImportBoundaryViolations(fixtureRoot)).toEqual([ + expect.objectContaining({ rule: "domain-purity" }), + ]); + } finally { + fs.rmSync(fixtureRoot, { force: true, recursive: true }); + } + }); + + it("classifies a symbolic-link import by its canonical protected-layer target (#6245)", () => { + const target = fixturePath("src/lib/actions", "symlink-target", ".mts"); + const importer = fixturePath("src/lib/domain", "symlink-importer", ".mts"); + const alias = fixturePath("src/lib/domain", "symlink-alias", ".mts"); + const relativeAlias = path + .relative(path.dirname(importer), alias) + .split(path.sep) + .join("/") + .replace(/\.mts$/, ""); + const specifier = relativeAlias.startsWith(".") ? relativeAlias : `./${relativeAlias}`; + try { + fs.writeFileSync(target, "export const value = true;\n"); + fs.symlinkSync(target, alias, "file"); + fs.writeFileSync(importer, `import { value } from "${specifier}";\nexport { value };\n`); + + expect(findLayerImportBoundaryViolations(importer)).toEqual([ + expect.objectContaining({ + detail: `domain must not import ${path.relative(REPO_ROOT, target)}`, + }), + ]); + } finally { + fs.rmSync(importer, { force: true }); + fs.rmSync(alias, { force: true }); + fs.rmSync(target, { force: true }); + } + }); }); diff --git a/test/no-unit-blocks-in-live-e2e.test.ts b/test/no-unit-blocks-in-live-e2e.test.ts index 8bdc4ca4519..d2a8d0d724c 100644 --- a/test/no-unit-blocks-in-live-e2e.test.ts +++ b/test/no-unit-blocks-in-live-e2e.test.ts @@ -65,6 +65,26 @@ describe("live E2E unit-block guard", () => { expect(linesFlagged(source)).toEqual([]); }); + it("ignores plain multiline block comments and resumes scanning after the terminator", () => { + const source = [ + "/*", + 'it("a commented unit case", () => {});', + "*/", + 'it("an executable unit case", () => {});', + ].join("\n"); + + expect(linesFlagged(source)).toEqual([4]); + }); + + it("handles same-line block comments without hiding following code", () => { + const source = [ + '/* it("commented", () => {}); */ it("executable", () => {});', + 'const marker = "/* it(\\"string data\\", () => {}); */";', + ].join("\n"); + + expect(linesFlagged(source)).toEqual([1]); + }); + it("does not match it inside a longer identifier", () => { const source = [ 'const wait = () => {}; wait("not a test");', diff --git a/test/credentials-shim.test.ts b/test/package-contract/credentials-shim.test.ts similarity index 95% rename from test/credentials-shim.test.ts rename to test/package-contract/credentials-shim.test.ts index 06f99d86f89..3e95b32dd2f 100644 --- a/test/credentials-shim.test.ts +++ b/test/package-contract/credentials-shim.test.ts @@ -9,12 +9,12 @@ import { afterEach, beforeEach, describe, expect, it } from "vitest"; const require = createRequire(import.meta.url); -type CredentialsShim = typeof import("../src/lib/credentials/store.js") & { +type CredentialsShim = typeof import("../../src/lib/credentials/store.js") & { CREDS_DIR: string; CREDS_FILE: string; }; -const credentials = require("../bin/lib/credentials.js") as CredentialsShim; +const credentials = require("../../bin/lib/credentials.js") as CredentialsShim; const TRACKED_ENV_KEYS = [...credentials.KNOWN_CREDENTIAL_ENV_KEYS, "TEST_KEY"]; function clearTrackedEnv() { diff --git a/test/pr-workflow-contract.test.ts b/test/pr-workflow-contract.test.ts index 9bdd3822130..f036154a584 100644 --- a/test/pr-workflow-contract.test.ts +++ b/test/pr-workflow-contract.test.ts @@ -773,6 +773,9 @@ describe("pull request and main workflow contracts", () => { "${{ github.workflow }}-${{ github.ref }}-${{ github.event.action != 'edited' || github.event.changes.base != null }}", "cancel-in-progress": true, }); + expect( + requiredWorkflowStep(prWorkflow.jobs["static-checks"], "Checkout").with?.["fetch-depth"], + ).toBe(0); for (const [jobName, stepName, trustedActionPath, mainActionPath] of [ [ "static-checks", diff --git a/test/test-boundary-guards.test.ts b/test/test-boundary-guards.test.ts index 65de61e42cc..af1ec6144d9 100644 --- a/test/test-boundary-guards.test.ts +++ b/test/test-boundary-guards.test.ts @@ -8,6 +8,7 @@ import path from "node:path"; import { describe, expect, it } from "vitest"; import { + collectFastProjectEntries, findCompiledInternalViolations, findFastProjectTransitiveViolations, isFastProjectTestPath, @@ -469,7 +470,63 @@ describe("fast-project transitive import boundary", () => { "test/e2e/live/example.test.ts", "test/package-contract/example.test.ts", ].map(isFastProjectTestPath), - ).toEqual([true, true, true, false, false, true, true, true, false, false, false]); + ).toEqual([true, true, true, false, false, true, false, true, true, false, false]); + }); + + it("discovers canonical root integration entries without following symlink loops (#6692)", () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-fast-project-roots-")); + try { + const testRoot = path.join(root, "test"); + fs.mkdirSync(testRoot, { recursive: true }); + fs.writeFileSync(path.join(testRoot, "entry.test.ts"), "export {};\n"); + fs.writeFileSync(path.join(testRoot, "helper.ts"), "export {};\n"); + fs.symlinkSync(".", path.join(testRoot, "loop"), "dir"); + + expect( + collectFastProjectEntries(root).map((file) => + path.relative(root, file).split(path.sep).join("/"), + ), + ).toEqual(["test/entry.test.ts"]); + } finally { + fs.rmSync(root, { force: true, recursive: true }); + } + }); + + it("skips a symbolic link used as a canonical scan root (#6692)", () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-fast-project-root-link-")); + const external = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-fast-project-external-")); + try { + fs.writeFileSync(path.join(external, "entry.test.ts"), "export {};\n"); + fs.symlinkSync(external, path.join(root, "test"), "dir"); + + expect(collectFastProjectEntries(root)).toEqual([]); + } finally { + fs.rmSync(root, { force: true, recursive: true }); + fs.rmSync(external, { force: true, recursive: true }); + } + }); + + it("canonicalizes transitive imports through a symbolic-link loop (#6692)", () => { + withImportGraphFixture( + { + "entry.test.ts": 'import "./loop/helper.js";\n', + "helper.ts": ['import "./loop/helper.js";', 'import "../../dist/lib/compiled.js";'].join( + "\n", + ), + }, + (root) => { + fs.symlinkSync(".", path.join(root, "loop"), "dir"); + + expect(findFastProjectTransitiveViolations([path.join(root, "entry.test.ts")])).toEqual([ + { + chain: [fixtureRepoPath(root, "entry.test.ts"), fixtureRepoPath(root, "helper.ts")], + detail: 'imports compiled CLI internals from "../../dist/lib/compiled.js"', + file: fixtureRepoPath(root, "helper.ts"), + line: 2, + }, + ]); + }, + ); }); it("reports a shortest chain through static import, export, dynamic import, and require edges (#6692)", () => { diff --git a/test/test-create-require-budget.test.ts b/test/test-create-require-budget.test.ts index 749bd292bf0..b80f2ae1d98 100644 --- a/test/test-create-require-budget.test.ts +++ b/test/test-create-require-budget.test.ts @@ -11,7 +11,9 @@ import { collectProductionCreateRequireSources, collectTestSupportCreateRequireSources, containsCreateRequireIdentifier, + createRequireAllowlistExpansionFailure, createRequireBudgetFailure, + extractCreateRequireAllowlists, } from "../scripts/checks/test-create-require-budget.mts"; const tempDirs = new Set(); @@ -124,4 +126,47 @@ describe("CLI createRequire budget", () => { expect(failure).toContain("src/new.test.ts"); expect(failure).toContain("src/retired.test.ts"); }); + + it("rejects allowlist additions relative to the merge base while permitting removals (#6245)", () => { + expect( + createRequireAllowlistExpansionFailure( + { cli: ["src/a.test.ts"], testSupport: [] }, + { cli: ["src/a.test.ts", "src/retired.test.ts"], testSupport: ["test/retired.ts"] }, + ), + ).toBeNull(); + + expect( + createRequireAllowlistExpansionFailure( + { cli: ["src/a.test.ts", "src/new.test.ts"], testSupport: ["test/new.ts"] }, + { cli: ["src/a.test.ts"], testSupport: [] }, + ), + ).toBe( + [ + "createRequire allowlists must not expand relative to the merge base.", + "- CLI_CREATE_REQUIRE_FILES: src/new.test.ts", + "- TEST_SUPPORT_CREATE_REQUIRE_FILES: test/new.ts", + ].join("\n"), + ); + }); + + it("extracts only literal reviewed allowlists from the merge-base source (#6245)", () => { + const source = [ + 'export const CLI_CREATE_REQUIRE_FILES = ["src/a.test.ts"] as const;', + 'export const TEST_SUPPORT_CREATE_REQUIRE_FILES = ["test/helper.ts"] as const;', + ].join("\n"); + + expect(extractCreateRequireAllowlists(source)).toEqual({ + cli: ["src/a.test.ts"], + testSupport: ["test/helper.ts"], + }); + expect(() => + extractCreateRequireAllowlists( + [ + "const dynamicPath = getPath();", + "export const CLI_CREATE_REQUIRE_FILES = [dynamicPath] as const;", + "export const TEST_SUPPORT_CREATE_REQUIRE_FILES = [] as const;", + ].join("\n"), + ), + ).toThrow("CLI_CREATE_REQUIRE_FILES must be a literal string array"); + }); }); From fef37a8eb322ad1b20bb2e2cd300e01d66cae5a1 Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Thu, 16 Jul 2026 14:42:46 -0700 Subject: [PATCH 6/9] fix(checks): address exact-head guardrail review Co-authored-by: Tinson Lai Signed-off-by: Apurv Kumaria --- .github/actions/ci-static-checks/action.yaml | 4 + .../create-require-ratchet.mts | 163 ++++++++++++++++++ scripts/checks/layer-import-boundaries.mts | 2 +- .../checks/local-credential-helper-pin.mts | 16 +- scripts/checks/no-unit-blocks-in-live-e2e.mts | 8 +- .../checks/openshell-policy-mutation-read.mts | 66 ++++++- scripts/checks/test-title-style.mts | 44 ++++- test/layer-import-boundaries.test.ts | 22 +++ test/local-credential-helper-pin.test.ts | 12 ++ test/no-coverage-ignore.test.ts | 6 +- test/no-unit-blocks-in-live-e2e.test.ts | 11 ++ test/policy-mutation-read-discovery.test.ts | 16 ++ test/pr-workflow-contract.test.ts | 6 + test/test-create-require-budget.test.ts | 28 +++ test/test-title-style.test.ts | 50 +++++- 15 files changed, 426 insertions(+), 28 deletions(-) create mode 100644 .github/actions/ci-static-checks/create-require-ratchet.mts diff --git a/.github/actions/ci-static-checks/action.yaml b/.github/actions/ci-static-checks/action.yaml index 0096f9333a2..9a69bb48f82 100644 --- a/.github/actions/ci-static-checks/action.yaml +++ b/.github/actions/ci-static-checks/action.yaml @@ -33,6 +33,10 @@ runs: shell: bash run: npm install --ignore-scripts + - name: Enforce base-trusted createRequire allowlist ratchet + shell: bash + run: npx tsx "$GITHUB_ACTION_PATH/create-require-ratchet.mts" + - name: Validate config schemas shell: bash run: npm run validate:configs diff --git a/.github/actions/ci-static-checks/create-require-ratchet.mts b/.github/actions/ci-static-checks/create-require-ratchet.mts new file mode 100644 index 00000000000..0b1aeb84fbf --- /dev/null +++ b/.github/actions/ci-static-checks/create-require-ratchet.mts @@ -0,0 +1,163 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +/** + * Enforce the createRequire allowlist ratchet from the base-trusted CI action. + * + * Pull requests execute this file from the action checked out at the immutable + * base SHA. The comparison therefore cannot be weakened by changing the PR's + * scripts/checks implementation or returning early from its local check. + */ + +import { spawnSync } from "node:child_process"; +import { existsSync, readFileSync } from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +import ts from "typescript"; + +export type TrustedCreateRequireAllowlists = Readonly<{ + cli: readonly string[]; + testSupport: readonly string[]; +}>; + +const ALLOWLIST_PATHS = [ + "scripts/checks/test-create-require-budget.mts", + "scripts/checks/test-create-require-budget.ts", +] as const; + +const ALLOWLIST_EXPORTS = { + CLI_CREATE_REQUIRE_FILES: "cli", + TEST_SUPPORT_CREATE_REQUIRE_FILES: "testSupport", +} as const; + +function arrayLiteral(initializer: ts.Expression | undefined): ts.ArrayLiteralExpression | null { + let expression = initializer; + while ( + expression && + (ts.isAsExpression(expression) || + ts.isSatisfiesExpression(expression) || + ts.isParenthesizedExpression(expression)) + ) { + expression = expression.expression; + } + return expression && ts.isArrayLiteralExpression(expression) ? expression : null; +} + +export function extractTrustedCreateRequireAllowlists( + sourceText: string, + fileName: string = ALLOWLIST_PATHS[0], +): TrustedCreateRequireAllowlists { + const sourceFile = ts.createSourceFile(fileName, sourceText, ts.ScriptTarget.Latest, true); + const values: Partial> = {}; + + for (const statement of sourceFile.statements) { + if (!ts.isVariableStatement(statement)) continue; + for (const declaration of statement.declarationList.declarations) { + if (!ts.isIdentifier(declaration.name)) continue; + const target = ALLOWLIST_EXPORTS[declaration.name.text as keyof typeof ALLOWLIST_EXPORTS]; + if (!target) continue; + const array = arrayLiteral(declaration.initializer); + if (!array || array.elements.some((element) => !ts.isStringLiteralLike(element))) { + throw new Error(`${declaration.name.text} must be a literal string array`); + } + if (values[target]) throw new Error(`${declaration.name.text} must be declared exactly once`); + values[target] = array.elements.map((element) => (element as ts.StringLiteralLike).text); + } + } + + if (!values.cli || !values.testSupport) { + throw new Error("createRequire allowlist source must declare both reviewed allowlists"); + } + return { cli: values.cli, testSupport: values.testSupport }; +} + +export function trustedCreateRequireExpansionFailure( + current: TrustedCreateRequireAllowlists, + baseline: TrustedCreateRequireAllowlists, +): string | null { + const cliAdditions = current.cli.filter((file) => !baseline.cli.includes(file)).sort(); + const supportAdditions = current.testSupport + .filter((file) => !baseline.testSupport.includes(file)) + .sort(); + if (cliAdditions.length === 0 && supportAdditions.length === 0) return null; + + return [ + "createRequire allowlists must not expand relative to the merge base.", + ...cliAdditions.map((file) => `- CLI_CREATE_REQUIRE_FILES: ${file}`), + ...supportAdditions.map((file) => `- TEST_SUPPORT_CREATE_REQUIRE_FILES: ${file}`), + ].join("\n"); +} + +function resolveMergeBase(repoRoot: string): string | null { + const baseBranch = process.env.GITHUB_BASE_REF?.trim(); + const baseRef = baseBranch ? `origin/${baseBranch}` : "origin/main"; + const mergeBase = spawnSync("git", ["merge-base", "HEAD", baseRef], { + cwd: repoRoot, + encoding: "utf8", + timeout: 5_000, + }); + if (mergeBase.status !== 0 || !mergeBase.stdout.trim()) { + if (baseBranch) { + throw new Error(`could not resolve the pull-request merge base against ${baseRef}`); + } + return null; + } + return mergeBase.stdout.trim(); +} + +function currentAllowlistSource(repoRoot: string): { path: string; source: string } { + for (const relativePath of ALLOWLIST_PATHS) { + const absolutePath = path.join(repoRoot, relativePath); + if (existsSync(absolutePath)) { + return { path: relativePath, source: readFileSync(absolutePath, "utf8") }; + } + } + throw new Error("current checkout does not contain the createRequire budget check"); +} + +function baselineAllowlistSource( + repoRoot: string, + revision: string, +): { path: string; source: string } { + for (const relativePath of ALLOWLIST_PATHS) { + const result = spawnSync("git", ["show", `${revision}:${relativePath}`], { + cwd: repoRoot, + encoding: "utf8", + timeout: 5_000, + }); + if (result.status === 0) return { path: relativePath, source: result.stdout }; + } + throw new Error(`merge base ${revision} does not contain the createRequire budget check`); +} + +export function verifyTrustedCreateRequireRatchet(repoRoot: string): string | null { + const revision = resolveMergeBase(repoRoot); + if (!revision) return null; + const current = currentAllowlistSource(repoRoot); + const baseline = baselineAllowlistSource(repoRoot, revision); + return trustedCreateRequireExpansionFailure( + extractTrustedCreateRequireAllowlists(current.source, current.path), + extractTrustedCreateRequireAllowlists(baseline.source, baseline.path), + ); +} + +function main(): void { + const repoRoot = path.resolve(process.env.GITHUB_WORKSPACE || process.cwd()); + const failure = verifyTrustedCreateRequireRatchet(repoRoot); + if (failure) { + console.error(failure); + process.exitCode = 1; + return; + } + console.log("Base-trusted createRequire allowlist ratchet passed."); +} + +if (fileURLToPath(import.meta.url) === path.resolve(process.argv[1] ?? "")) { + try { + main(); + } catch (error) { + console.error(error instanceof Error ? error.message : String(error)); + process.exitCode = 1; + } +} diff --git a/scripts/checks/layer-import-boundaries.mts b/scripts/checks/layer-import-boundaries.mts index 8348c94877d..b1d65379d65 100644 --- a/scripts/checks/layer-import-boundaries.mts +++ b/scripts/checks/layer-import-boundaries.mts @@ -115,9 +115,9 @@ function resolveInternalImport(fromAbsPath: string, specifier: string): string | const base = path.resolve(path.dirname(fromAbsPath), specifier); const extensions = [".ts", ".tsx", ".mts", ".cts"]; const candidates = [ - base, ...extensions.map((extension) => `${base}${extension}`), ...extensions.map((extension) => path.join(base, `index${extension}`)), + base, ]; const found = candidates.find((candidate) => existsSync(candidate)); if (!found) return toRepoPath(`${base}.ts`); diff --git a/scripts/checks/local-credential-helper-pin.mts b/scripts/checks/local-credential-helper-pin.mts index 5373a533ced..91e2cf97654 100644 --- a/scripts/checks/local-credential-helper-pin.mts +++ b/scripts/checks/local-credential-helper-pin.mts @@ -46,6 +46,13 @@ function escapeRegExp(value: string): string { return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); } +export function immutableRawArtifactUrlPattern(relativePath: string, flags = ""): RegExp { + return new RegExp( + `https://raw\\.githubusercontent\\.com/NVIDIA/NemoClaw/([0-9a-f]{40})/${escapeRegExp(relativePath)}(?=$|[\\s\\u0060])`, + flags, + ); +} + function findCredentialSection(promptSource: string): string { const match = promptSource.match( /## Handle Tokens Securely and Visually([\s\S]*?)\nUse this provider mapping/, @@ -58,10 +65,7 @@ function verifyArtifact(section: string, artifact: ReviewedArtifact): string[] { const failures: string[] = []; const currentBytes = fs.readFileSync(path.join(REPO_ROOT, artifact.relativePath)); const currentDigest = sha256(currentBytes); - const urlPattern = new RegExp( - `https://raw\\.githubusercontent\\.com/NVIDIA/NemoClaw/([0-9a-f]{40})/${escapeRegExp(artifact.relativePath)}`, - "g", - ); + const urlPattern = immutableRawArtifactUrlPattern(artifact.relativePath, "g"); const matches = [...section.matchAll(urlPattern)]; const match = matches[0]; if (matches.length !== 1 || !match?.[1] || match.index === undefined) { @@ -403,9 +407,7 @@ function main(): void { sha256(fs.readFileSync(path.join(REPO_ROOT, relativePath))), ); const pinnedCommits = REVIEWED_ARTIFACTS.flatMap(({ relativePath }) => { - const pattern = new RegExp( - `https://raw\\.githubusercontent\\.com/NVIDIA/NemoClaw/([0-9a-f]{40})/${escapeRegExp(relativePath)}`, - ); + const pattern = immutableRawArtifactUrlPattern(relativePath); const commit = section.match(pattern)?.[1]; return commit ? [commit] : []; }); diff --git a/scripts/checks/no-unit-blocks-in-live-e2e.mts b/scripts/checks/no-unit-blocks-in-live-e2e.mts index fe9df3e9241..fc3d1668bca 100644 --- a/scripts/checks/no-unit-blocks-in-live-e2e.mts +++ b/scripts/checks/no-unit-blocks-in-live-e2e.mts @@ -38,9 +38,12 @@ const IT_PRIMITIVE_MEMBERS = new Set([ "concurrent", "each", "fails", + "for", "only", + "runIf", "sequential", "skip", + "skipIf", "todo", ]); @@ -77,9 +80,8 @@ export function findLiveUnitBlocks(source: string, file: string): LiveUnitBlockV if (ts.isIdentifier(expression)) return expression.text === "it"; return ( ts.isPropertyAccessExpression(expression) && - ts.isIdentifier(expression.expression) && - expression.expression.text === "it" && - IT_PRIMITIVE_MEMBERS.has(expression.name.text) + IT_PRIMITIVE_MEMBERS.has(expression.name.text) && + isItPrimitive(expression.expression) ); } diff --git a/scripts/checks/openshell-policy-mutation-read.mts b/scripts/checks/openshell-policy-mutation-read.mts index 012759ac620..6f63f5a5b13 100644 --- a/scripts/checks/openshell-policy-mutation-read.mts +++ b/scripts/checks/openshell-policy-mutation-read.mts @@ -22,6 +22,8 @@ import { existsSync, readdirSync, readFileSync } from "node:fs"; import path from "node:path"; import { fileURLToPath } from "node:url"; +import ts from "typescript"; + const REPO_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../.."); interface AuditedMutationRead { @@ -79,9 +81,63 @@ export interface DiscoveredPolicyReadSite { readonly readCalls: number; } -const POLICY_GET_BUILDER_CALL = /\bbuildPolicyGet(?:Full)?Command\s*\(/gu; -const DIRECT_POLICY_GET_CALL = - /\[\s*(?:["'`]openshell["'`]\s*,\s*)?["'`]policy["'`]\s*,\s*["'`]get["'`]\s*,\s*["'`]--(?:base|full)["'`]/gu; +const POLICY_GET_BUILDERS = new Set(["buildPolicyGetCommand", "buildPolicyGetFullCommand"]); + +function calledName(expression: ts.LeftHandSideExpression): string | null { + if (ts.isIdentifier(expression)) return expression.text; + if (ts.isPropertyAccessExpression(expression)) return expression.name.text; + if ( + ts.isElementAccessExpression(expression) && + expression.argumentExpression && + ts.isStringLiteralLike(expression.argumentExpression) + ) { + return expression.argumentExpression.text; + } + return null; +} + +function literalText(expression: ts.Expression): string | null { + return ts.isStringLiteralLike(expression) ? expression.text : null; +} + +function isDirectPolicyRead(expression: ts.ArrayLiteralExpression): boolean { + const first = expression.elements[0]; + if (!first || !ts.isExpression(first)) return false; + const firstText = literalText(first); + const offset = + firstText === "policy" + ? 0 + : firstText === "openshell" || + (ts.isCallExpression(first) && calledName(first.expression) === "resolveOpenshellBinary") + ? 1 + : -1; + if (offset < 0) return false; + const values = expression.elements.map((element) => + ts.isExpression(element) ? literalText(element) : null, + ); + return ( + values[offset] === "policy" && + values[offset + 1] === "get" && + (values[offset + 2] === "--base" || values[offset + 2] === "--full") + ); +} + +export function countPolicyReadCalls(source: string, fileName: string): number { + const sourceFile = ts.createSourceFile(fileName, source, ts.ScriptTarget.Latest, true); + let readCalls = 0; + + function visit(node: ts.Node): void { + if (ts.isCallExpression(node) && POLICY_GET_BUILDERS.has(calledName(node.expression) ?? "")) { + readCalls += 1; + } else if (ts.isArrayLiteralExpression(node) && isDirectPolicyRead(node)) { + readCalls += 1; + } + ts.forEachChild(node, visit); + } + + visit(sourceFile); + return readCalls; +} function productionTypeScriptFiles(directory: string): string[] { if (!existsSync(directory)) return []; @@ -104,9 +160,7 @@ export function discoverPolicyReadSites(repoRoot: string): DiscoveredPolicyReadS .flatMap((sourceRoot) => productionTypeScriptFiles(path.join(repoRoot, sourceRoot))) .flatMap((sourcePath) => { const source = readFileSync(sourcePath, "utf8"); - const readCalls = - (source.match(POLICY_GET_BUILDER_CALL) ?? []).length + - (source.match(DIRECT_POLICY_GET_CALL) ?? []).length; + const readCalls = countPolicyReadCalls(source, sourcePath); return readCalls > 0 ? [ { diff --git a/scripts/checks/test-title-style.mts b/scripts/checks/test-title-style.mts index 6bae0dbf086..34a2986d346 100755 --- a/scripts/checks/test-title-style.mts +++ b/scripts/checks/test-title-style.mts @@ -2,7 +2,7 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { existsSync, readdirSync, readFileSync, statSync } from "node:fs"; +import { existsSync, readdirSync, readFileSync } from "node:fs"; import path from "node:path"; import { fileURLToPath, pathToFileURL } from "node:url"; @@ -35,6 +35,8 @@ const LOCAL_ISSUE_REFERENCE_PATTERN = /(? { + const aliases = new Map(); + for (const statement of sourceFile.statements) { + if ( + !ts.isImportDeclaration(statement) || + !ts.isStringLiteral(statement.moduleSpecifier) || + statement.moduleSpecifier.text !== "vitest" + ) { + continue; + } + if (statement.importClause?.isTypeOnly) continue; + const bindings = statement.importClause?.namedBindings; + if (!bindings || !ts.isNamedImports(bindings)) continue; + for (const element of bindings.elements) { + if (element.isTypeOnly) continue; + const importedName = element.propertyName?.text ?? element.name.text; + if (TEST_CALL_NAMES.has(importedName)) { + aliases.set(element.name.text, importedName as TestCallName); + } + } + } + return aliases; +} + function literalTitle(argument: ts.Expression | undefined): string | null { if (argument === undefined) return null; if (ts.isStringLiteral(argument) || ts.isNoSubstitutionTemplateLiteral(argument)) { @@ -102,10 +128,12 @@ export function scanTestTitleStyle(file: string, source: string): readonly TestT scriptKindFor(file), ); const violations: TestTitleViolation[] = []; + const aliases = vitestCallAliases(sourceFile); function visit(node: ts.Node): void { if (ts.isCallExpression(node)) { - const call = rootCallName(node.expression); + const root = rootCallName(node.expression); + const call = root === null ? null : (aliases.get(root) ?? root); const title = literalTitle(node.arguments[0]); if (call !== null && TEST_CALL_NAMES.has(call) && title !== null) { const location = sourceFile.getLineAndCharacterOfPosition(node.getStart(sourceFile)); @@ -114,7 +142,7 @@ export function scanTestTitleStyle(file: string, source: string): readonly TestT file, line: location.line + 1, column: location.character + 1, - call: call as TestTitleViolation["call"], + call: call as TestCallName, title, ...violation, }); @@ -136,13 +164,13 @@ function isSkipped(absolutePath: string): boolean { function* walkTestFiles(directory: string): Generator { if (!existsSync(directory) || isSkipped(directory)) return; - for (const entry of readdirSync(directory)) { - const absolutePath = path.join(directory, entry); + for (const entry of readdirSync(directory, { withFileTypes: true })) { + if (entry.isSymbolicLink()) continue; + const absolutePath = path.join(directory, entry.name); if (isSkipped(absolutePath)) continue; - const stats = statSync(absolutePath); - if (stats.isDirectory()) { + if (entry.isDirectory()) { yield* walkTestFiles(absolutePath); - } else if (stats.isFile() && TEST_FILE_PATTERN.test(entry)) { + } else if (entry.isFile() && TEST_FILE_PATTERN.test(entry.name)) { yield absolutePath; } } diff --git a/test/layer-import-boundaries.test.ts b/test/layer-import-boundaries.test.ts index 1c7b1c556df..6587de33e5a 100644 --- a/test/layer-import-boundaries.test.ts +++ b/test/layer-import-boundaries.test.ts @@ -199,6 +199,28 @@ describe("CLI layer import boundaries (#6245)", () => { } }); + it("resolves an extensionless directory import to its index module (#6245)", () => { + const targetDir = fs.mkdtempSync( + path.join(REPO_ROOT, "src/lib/actions/__boundary-extensionless-directory-"), + ); + const target = path.join(targetDir, "index.mts"); + const importer = fixturePath("src/lib/domain", "extensionless-directory-importer", ".mts"); + const specifier = path.relative(path.dirname(importer), targetDir).split(path.sep).join("/"); + try { + fs.writeFileSync(target, "export const value = true;\n"); + fs.writeFileSync(importer, `import { value } from "${specifier}";\nexport { value };\n`); + + expect(findLayerImportBoundaryViolations(importer)).toEqual([ + expect.objectContaining({ + detail: `domain must not import ${path.relative(REPO_ROOT, target)}`, + }), + ]); + } finally { + fs.rmSync(importer, { force: true }); + fs.rmSync(targetDir, { force: true, recursive: true }); + } + }); + it.each([ ".test.mts", ".spec.cts", diff --git a/test/local-credential-helper-pin.test.ts b/test/local-credential-helper-pin.test.ts index 07175bdc792..ded245b0cd3 100644 --- a/test/local-credential-helper-pin.test.ts +++ b/test/local-credential-helper-pin.test.ts @@ -8,6 +8,7 @@ import { extractEmbeddedFormDigest, extractProcessControlRules, extractStringSet, + immutableRawArtifactUrlPattern, verifyFieldSafetySourceParity, } from "../scripts/checks/local-credential-helper-pin.mts"; @@ -120,6 +121,17 @@ function fieldSafetySources( } describe("local credential helper pin predicate parity", () => { + it("accepts only a complete immutable artifact URL path (#5048)", () => { + const commit = "a".repeat(40); + const relativePath = "scripts/local-credential-helper.mts"; + const url = `https://raw.githubusercontent.com/NVIDIA/NemoClaw/${commit}/${relativePath}`; + + expect(`${url}\``.match(immutableRawArtifactUrlPattern(relativePath))?.[1]).toBe(commit); + expect(`${url}.bak\``.match(immutableRawArtifactUrlPattern(relativePath))).toBeNull(); + expect(`${url}?download=1\``.match(immutableRawArtifactUrlPattern(relativePath))).toBeNull(); + expect(`${url}#fragment\``.match(immutableRawArtifactUrlPattern(relativePath))).toBeNull(); + }); + it("accepts exact canonical helper and form field-safety parity (#5048)", () => { expect(verifyFieldSafetySourceParity(fieldSafetySources())).toEqual([]); }); diff --git a/test/no-coverage-ignore.test.ts b/test/no-coverage-ignore.test.ts index d8865c3258a..a7420507df8 100644 --- a/test/no-coverage-ignore.test.ts +++ b/test/no-coverage-ignore.test.ts @@ -33,14 +33,16 @@ describe("coverage ignore guard", () => { }); }); -describe("scanned source path selection", () => { - it("scans .mts files under tracked roots", () => { +describe("scanned source path selection (#6921)", () => { + it("scans .mts and .tsx files under tracked roots", () => { expect(isScannedSourcePath("scripts/checks/no-coverage-ignore.mts")).toBe(true); expect(isScannedSourcePath("src/lib/example.mts")).toBe(true); + expect(isScannedSourcePath("src/lib/example.tsx")).toBe(true); }); it("excludes non-source extensions and paths outside tracked roots", () => { expect(isScannedSourcePath("scripts/checks/README.md")).toBe(false); expect(isScannedSourcePath("docs/example.mts")).toBe(false); + expect(isScannedSourcePath("src/lib/example.tsx.bak")).toBe(false); }); }); diff --git a/test/no-unit-blocks-in-live-e2e.test.ts b/test/no-unit-blocks-in-live-e2e.test.ts index d2a8d0d724c..afdac41cd7f 100644 --- a/test/no-unit-blocks-in-live-e2e.test.ts +++ b/test/no-unit-blocks-in-live-e2e.test.ts @@ -35,6 +35,17 @@ describe("live E2E unit-block guard", () => { expect(linesFlagged(source)).toEqual([1, 2, 3]); }); + it("flags conditional and nested it modifier chains", () => { + const source = [ + 'it.skipIf(false)("conditional skip", () => {});', + 'it.runIf(true)("conditional run", () => {});', + 'it.for([1, 2])("parameterized", () => {});', + 'it.concurrent.skip("nested modifier", () => {});', + ].join("\n"); + + expect(linesFlagged(source)).toEqual([1, 2, 3, 4]); + }); + it("does not flag test(...) — the live-case primitive", () => { const source = [ 'test("live case", async ({ host }) => {});', diff --git a/test/policy-mutation-read-discovery.test.ts b/test/policy-mutation-read-discovery.test.ts index e1224195d79..f09e0b723c0 100644 --- a/test/policy-mutation-read-discovery.test.ts +++ b/test/policy-mutation-read-discovery.test.ts @@ -9,10 +9,26 @@ import { describe, expect, it } from "vitest"; import { auditOpenShellPolicyMutationReads, + countPolicyReadCalls, discoverPolicyReadSites, } from "../scripts/checks/openshell-policy-mutation-read.mts"; describe("OpenShell policy mutation read discovery", () => { + it("counts executable policy reads without comment or string decoys", () => { + const source = [ + "// buildPolicyGetCommand(commentedSandbox);", + 'const decoy = "buildPolicyGetFullCommand(stringSandbox)";', + "policyBuilders.buildPolicyGetCommand(sandboxName);", + 'policyBuilders["buildPolicyGetFullCommand"](sandboxName);', + '["openshell", "policy", "get", "--base", sandboxName];', + '["policy", "get", "--full", sandboxName];', + 'const arrayDecoy = `["policy", "get", "--base", sandboxName]`;', + '["not-openshell", "policy", "get", "--base", sandboxName];', + ].join("\n"); + + expect(countPolicyReadCalls(source, "fixture.ts")).toBe(4); + }); + it("discovers builder and direct policy reads in new production files", () => { const repoRoot = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-policy-read-discovery-")); const mutationPath = path.join(repoRoot, "src", "lib", "new-policy-mutation.ts"); diff --git a/test/pr-workflow-contract.test.ts b/test/pr-workflow-contract.test.ts index f036154a584..0377499adfd 100644 --- a/test/pr-workflow-contract.test.ts +++ b/test/pr-workflow-contract.test.ts @@ -1018,6 +1018,12 @@ describe("pull request and main workflow contracts", () => { expect(stepRuns(sharedActions.staticChecks).join("\n")).not.toContain( "skills-frontmatter.test.ts", ); + expect( + requiredStep( + sharedActions.staticChecks, + "Enforce base-trusted createRequire allowlist ratchet", + ).run, + ).toBe('npx tsx "$GITHUB_ACTION_PATH/create-require-ratchet.mts"'); }); // source-shape-contract: security -- Downloaded CI tooling must use a committed digest rather than upstream metadata diff --git a/test/test-create-require-budget.test.ts b/test/test-create-require-budget.test.ts index b80f2ae1d98..2132e2ea980 100644 --- a/test/test-create-require-budget.test.ts +++ b/test/test-create-require-budget.test.ts @@ -7,6 +7,10 @@ import path from "node:path"; import { afterEach, describe, expect, it } from "vitest"; +import { + extractTrustedCreateRequireAllowlists, + trustedCreateRequireExpansionFailure, +} from "../.github/actions/ci-static-checks/create-require-ratchet.mts"; import { collectProductionCreateRequireSources, collectTestSupportCreateRequireSources, @@ -169,4 +173,28 @@ describe("CLI createRequire budget", () => { ), ).toThrow("CLI_CREATE_REQUIRE_FILES must be a literal string array"); }); + + it("duplicates the ratchet in base-trusted CI code that rejects PR additions (#6245)", () => { + const baselineSource = [ + 'export const CLI_CREATE_REQUIRE_FILES = ["src/a.test.ts"] as const;', + "export const TEST_SUPPORT_CREATE_REQUIRE_FILES = [] as const;", + ].join("\n"); + const currentSource = [ + 'export const CLI_CREATE_REQUIRE_FILES = ["src/a.test.ts", "src/new.test.ts"] as const;', + 'export const TEST_SUPPORT_CREATE_REQUIRE_FILES = ["test/new.ts"] as const;', + ].join("\n"); + + expect( + trustedCreateRequireExpansionFailure( + extractTrustedCreateRequireAllowlists(currentSource), + extractTrustedCreateRequireAllowlists(baselineSource), + ), + ).toContain("src/new.test.ts"); + expect( + trustedCreateRequireExpansionFailure( + extractTrustedCreateRequireAllowlists(currentSource), + extractTrustedCreateRequireAllowlists(baselineSource), + ), + ).toContain("test/new.ts"); + }); }); diff --git a/test/test-title-style.test.ts b/test/test-title-style.test.ts index 9b6b6395906..c029de35735 100644 --- a/test/test-title-style.test.ts +++ b/test/test-title-style.test.ts @@ -1,9 +1,15 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +import fs from "node:fs"; +import path from "node:path"; + import { describe, expect, it } from "vitest"; -import { scanTestTitleStyle } from "../scripts/checks/test-title-style.mts"; +import { + findTestTitleStyleViolations, + scanTestTitleStyle, +} from "../scripts/checks/test-title-style.mts"; function rulesFor(source: string): string[] { return scanTestTitleStyle("test/virtual-title-style.test.ts", source).map( @@ -65,4 +71,46 @@ describe("enforces behavior-oriented Vitest titles", () => { expect(violations).toEqual([]); }); + + it("resolves aliased Vitest calls without treating non-Vitest aliases as test primitives", () => { + const violations = scanTestTitleStyle( + "test/virtual-title-style.test.ts", + ` + import { describe as suite, it as caseIt, test as caseTest } from "vitest"; + import { describe as otherSuite } from "other-test-library"; + suite("issue #1234 aliased suite", () => { + caseIt.only("#1234 aliased case", () => {}); + caseTest("input → output", () => {}); + }); + otherSuite("issue #9999 external alias", () => {}); + `, + ); + + expect(violations.map(({ call, rule }) => ({ call, rule }))).toEqual([ + { call: "describe", rule: "issue-reference-suffix" }, + { call: "describe", rule: "leading-metadata" }, + { call: "it", rule: "issue-reference-suffix" }, + { call: "it", rule: "leading-metadata" }, + { call: "test", rule: "result-arrow" }, + ]); + }); + + it("does not follow symbolic links while walking test roots", () => { + const root = fs.mkdtempSync(path.join(import.meta.dirname, "__title-style-root-")); + const outside = fs.mkdtempSync(path.join(import.meta.dirname, "__title-style-outside-")); + try { + fs.writeFileSync( + path.join(outside, "bad.test.ts"), + 'it("issue #1234 hidden behind a symlink", () => {});\n', + ); + fs.symlinkSync(outside, path.join(root, "linked"), "dir"); + + expect( + findTestTitleStyleViolations([path.relative(import.meta.dirname + "/..", root)]), + ).toEqual([]); + } finally { + fs.rmSync(root, { force: true, recursive: true }); + fs.rmSync(outside, { force: true, recursive: true }); + } + }); }); From d40e954bff39003f34a9091a7778086bc1c418e7 Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Thu, 16 Jul 2026 15:35:37 -0700 Subject: [PATCH 7/9] fix(checks): resolve policy builder bindings Co-authored-by: Tinson Lai Signed-off-by: Apurv Kumaria --- .../checks/openshell-policy-mutation-read.mts | 110 +++++++++++++++++- test/policy-mutation-read-discovery.test.ts | 37 +++++- 2 files changed, 142 insertions(+), 5 deletions(-) diff --git a/scripts/checks/openshell-policy-mutation-read.mts b/scripts/checks/openshell-policy-mutation-read.mts index 6f63f5a5b13..1e237dd92d0 100644 --- a/scripts/checks/openshell-policy-mutation-read.mts +++ b/scripts/checks/openshell-policy-mutation-read.mts @@ -83,6 +83,17 @@ export interface DiscoveredPolicyReadSite { const POLICY_GET_BUILDERS = new Set(["buildPolicyGetCommand", "buildPolicyGetFullCommand"]); +interface PolicyBuilderBindings { + readonly identifiers: ReadonlySet; + readonly namespaces: ReadonlySet; +} + +const POLICY_BUILDER_MODULE_SUFFIXES = [ + "/src/lib/policy", + "/src/lib/policy/index", + "/src/lib/policy/commands", +] as const; + function calledName(expression: ts.LeftHandSideExpression): string | null { if (ts.isIdentifier(expression)) return expression.text; if (ts.isPropertyAccessExpression(expression)) return expression.name.text; @@ -96,6 +107,102 @@ function calledName(expression: ts.LeftHandSideExpression): string | null { return null; } +function isPolicyBuilderModule(fileName: string, moduleSpecifier: string): boolean { + if (!moduleSpecifier.startsWith(".")) return false; + const resolved = path + .resolve(path.dirname(fileName), moduleSpecifier) + .split(path.sep) + .join("/") + .replace(/\.[cm]?[jt]sx?$/u, ""); + return POLICY_BUILDER_MODULE_SUFFIXES.some((suffix) => resolved.endsWith(suffix)); +} + +function requireModuleSpecifier(expression: ts.Expression | undefined): string | null { + if ( + !expression || + !ts.isCallExpression(expression) || + !ts.isIdentifier(expression.expression) || + expression.expression.text !== "require" || + expression.arguments.length !== 1 + ) { + return null; + } + const [moduleSpecifier] = expression.arguments; + return moduleSpecifier && ts.isStringLiteralLike(moduleSpecifier) ? moduleSpecifier.text : null; +} + +function collectRequiredPolicyBindings( + declaration: ts.VariableDeclaration, + fileName: string, + identifiers: Set, + namespaces: Set, +): void { + const moduleSpecifier = requireModuleSpecifier(declaration.initializer); + if (!moduleSpecifier || !isPolicyBuilderModule(fileName, moduleSpecifier)) return; + if (ts.isIdentifier(declaration.name)) { + namespaces.add(declaration.name.text); + return; + } + if (!ts.isObjectBindingPattern(declaration.name)) return; + for (const element of declaration.name.elements) { + if (element.dotDotDotToken || !ts.isIdentifier(element.name)) continue; + const importedName = element.propertyName ?? element.name; + if ( + (ts.isIdentifier(importedName) || ts.isStringLiteralLike(importedName)) && + POLICY_GET_BUILDERS.has(importedName.text) + ) { + identifiers.add(element.name.text); + } + } +} + +function collectPolicyBuilderBindings( + sourceFile: ts.SourceFile, + fileName: string, +): PolicyBuilderBindings { + const identifiers = new Set(); + const namespaces = new Set(); + for (const statement of sourceFile.statements) { + if ( + ts.isImportDeclaration(statement) && + ts.isStringLiteralLike(statement.moduleSpecifier) && + statement.importClause && + !statement.importClause.isTypeOnly && + isPolicyBuilderModule(fileName, statement.moduleSpecifier.text) + ) { + const { namedBindings } = statement.importClause; + if (namedBindings && ts.isNamespaceImport(namedBindings)) { + namespaces.add(namedBindings.name.text); + } else if (namedBindings) { + for (const element of namedBindings.elements) { + if (element.isTypeOnly) continue; + const importedName = element.propertyName?.text ?? element.name.text; + if (POLICY_GET_BUILDERS.has(importedName)) identifiers.add(element.name.text); + } + } + } else if (ts.isVariableStatement(statement)) { + for (const declaration of statement.declarationList.declarations) { + collectRequiredPolicyBindings(declaration, fileName, identifiers, namespaces); + } + } + } + return { identifiers, namespaces }; +} + +function isPolicyBuilderCall( + expression: ts.LeftHandSideExpression, + bindings: PolicyBuilderBindings, +): boolean { + if (ts.isIdentifier(expression)) return bindings.identifiers.has(expression.text); + const memberName = calledName(expression); + if (!memberName || !POLICY_GET_BUILDERS.has(memberName)) return false; + const target = + ts.isPropertyAccessExpression(expression) || ts.isElementAccessExpression(expression) + ? expression.expression + : null; + return !!target && ts.isIdentifier(target) && bindings.namespaces.has(target.text); +} + function literalText(expression: ts.Expression): string | null { return ts.isStringLiteralLike(expression) ? expression.text : null; } @@ -124,10 +231,11 @@ function isDirectPolicyRead(expression: ts.ArrayLiteralExpression): boolean { export function countPolicyReadCalls(source: string, fileName: string): number { const sourceFile = ts.createSourceFile(fileName, source, ts.ScriptTarget.Latest, true); + const builderBindings = collectPolicyBuilderBindings(sourceFile, fileName); let readCalls = 0; function visit(node: ts.Node): void { - if (ts.isCallExpression(node) && POLICY_GET_BUILDERS.has(calledName(node.expression) ?? "")) { + if (ts.isCallExpression(node) && isPolicyBuilderCall(node.expression, builderBindings)) { readCalls += 1; } else if (ts.isArrayLiteralExpression(node) && isDirectPolicyRead(node)) { readCalls += 1; diff --git a/test/policy-mutation-read-discovery.test.ts b/test/policy-mutation-read-discovery.test.ts index f09e0b723c0..73ef04e2b87 100644 --- a/test/policy-mutation-read-discovery.test.ts +++ b/test/policy-mutation-read-discovery.test.ts @@ -13,20 +13,43 @@ import { discoverPolicyReadSites, } from "../scripts/checks/openshell-policy-mutation-read.mts"; -describe("OpenShell policy mutation read discovery", () => { - it("counts executable policy reads without comment or string decoys", () => { +describe("OpenShell policy mutation read discovery (#6921)", () => { + it("counts canonical builder bindings and direct argv reads", () => { const source = [ + 'import { buildPolicyGetCommand as buildBase } from "./policy/commands";', + 'import * as policyBuilders from "./policy/index";', + 'const { buildPolicyGetFullCommand: buildFull } = require("./policy");', + 'const requiredPolicyBuilders = require("./policy");', "// buildPolicyGetCommand(commentedSandbox);", 'const decoy = "buildPolicyGetFullCommand(stringSandbox)";', + "buildBase(sandboxName);", "policyBuilders.buildPolicyGetCommand(sandboxName);", 'policyBuilders["buildPolicyGetFullCommand"](sandboxName);', + "buildFull(sandboxName);", + "requiredPolicyBuilders.buildPolicyGetCommand(sandboxName);", '["openshell", "policy", "get", "--base", sandboxName];', '["policy", "get", "--full", sandboxName];', 'const arrayDecoy = `["policy", "get", "--base", sandboxName]`;', '["not-openshell", "policy", "get", "--base", sandboxName];', ].join("\n"); - expect(countPolicyReadCalls(source, "fixture.ts")).toBe(4); + expect(countPolicyReadCalls(source, "/repo/src/lib/fixture.ts")).toBe(7); + }); + + it("ignores similarly named calls without canonical policy bindings", () => { + const source = [ + 'import { buildPolicyGetCommand as unrelated } from "./fixture-helpers";', + 'import * as fixtureBuilders from "./fixture-helpers";', + 'const requiredFixtureBuilders = require("./fixture-helpers");', + "const fixture = { buildPolicyGetCommand() {}, buildPolicyGetFullCommand() {} };", + "unrelated(sandboxName);", + "fixtureBuilders.buildPolicyGetCommand(sandboxName);", + "requiredFixtureBuilders.buildPolicyGetFullCommand(sandboxName);", + "fixture.buildPolicyGetCommand(sandboxName);", + 'fixture["buildPolicyGetFullCommand"](sandboxName);', + ].join("\n"); + + expect(countPolicyReadCalls(source, "/repo/src/lib/fixture.ts")).toBe(0); }); it("discovers builder and direct policy reads in new production files", () => { @@ -35,7 +58,13 @@ describe("OpenShell policy mutation read discovery", () => { const diagnosticPath = path.join(repoRoot, "nemoclaw", "src", "new-policy-diagnostic.ts"); fs.mkdirSync(path.dirname(mutationPath), { recursive: true }); fs.mkdirSync(path.dirname(diagnosticPath), { recursive: true }); - fs.writeFileSync(mutationPath, "runCapture(buildPolicyGetCommand(sandboxName));\n"); + fs.writeFileSync( + mutationPath, + [ + 'import { buildPolicyGetCommand } from "./policy/commands";', + "runCapture(buildPolicyGetCommand(sandboxName));", + ].join("\n"), + ); fs.writeFileSync( diagnosticPath, 'runCmd(["openshell", "policy", "get", "--full", sandboxName]);\n', From 5cc587571183880af8721cfd56987447b607fa79 Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Fri, 17 Jul 2026 05:05:47 -0700 Subject: [PATCH 8/9] fix(checks): bind policy reads to canonical imports Co-authored-by: Tinson Lai Signed-off-by: Apurv Kumaria --- .../checks/openshell-policy-mutation-read.mts | 114 +++++++++++++----- test/policy-mutation-read-discovery.test.ts | 42 ++++++- 2 files changed, 126 insertions(+), 30 deletions(-) diff --git a/scripts/checks/openshell-policy-mutation-read.mts b/scripts/checks/openshell-policy-mutation-read.mts index 1e237dd92d0..c0677c8f704 100644 --- a/scripts/checks/openshell-policy-mutation-read.mts +++ b/scripts/checks/openshell-policy-mutation-read.mts @@ -84,14 +84,14 @@ export interface DiscoveredPolicyReadSite { const POLICY_GET_BUILDERS = new Set(["buildPolicyGetCommand", "buildPolicyGetFullCommand"]); interface PolicyBuilderBindings { - readonly identifiers: ReadonlySet; - readonly namespaces: ReadonlySet; + readonly identifiers: ReadonlySet; + readonly namespaces: ReadonlySet; } -const POLICY_BUILDER_MODULE_SUFFIXES = [ - "/src/lib/policy", - "/src/lib/policy/index", - "/src/lib/policy/commands", +const POLICY_BUILDER_MODULE_PATHS = [ + "src/lib/policy", + "src/lib/policy/index", + "src/lib/policy/commands", ] as const; function calledName(expression: ts.LeftHandSideExpression): string | null { @@ -107,14 +107,18 @@ function calledName(expression: ts.LeftHandSideExpression): string | null { return null; } -function isPolicyBuilderModule(fileName: string, moduleSpecifier: string): boolean { +function isPolicyBuilderModule( + fileName: string, + moduleSpecifier: string, + repoRoot: string, +): boolean { if (!moduleSpecifier.startsWith(".")) return false; const resolved = path .resolve(path.dirname(fileName), moduleSpecifier) - .split(path.sep) - .join("/") .replace(/\.[cm]?[jt]sx?$/u, ""); - return POLICY_BUILDER_MODULE_SUFFIXES.some((suffix) => resolved.endsWith(suffix)); + return POLICY_BUILDER_MODULE_PATHS.some( + (relativePath) => resolved === path.resolve(repoRoot, relativePath), + ); } function requireModuleSpecifier(expression: ts.Expression | undefined): string | null { @@ -134,13 +138,16 @@ function requireModuleSpecifier(expression: ts.Expression | undefined): string | function collectRequiredPolicyBindings( declaration: ts.VariableDeclaration, fileName: string, - identifiers: Set, - namespaces: Set, + repoRoot: string, + checker: ts.TypeChecker, + identifiers: Set, + namespaces: Set, ): void { const moduleSpecifier = requireModuleSpecifier(declaration.initializer); - if (!moduleSpecifier || !isPolicyBuilderModule(fileName, moduleSpecifier)) return; + if (!moduleSpecifier || !isPolicyBuilderModule(fileName, moduleSpecifier, repoRoot)) return; if (ts.isIdentifier(declaration.name)) { - namespaces.add(declaration.name.text); + const symbol = checker.getSymbolAtLocation(declaration.name); + if (symbol) namespaces.add(symbol); return; } if (!ts.isObjectBindingPattern(declaration.name)) return; @@ -151,7 +158,8 @@ function collectRequiredPolicyBindings( (ts.isIdentifier(importedName) || ts.isStringLiteralLike(importedName)) && POLICY_GET_BUILDERS.has(importedName.text) ) { - identifiers.add(element.name.text); + const symbol = checker.getSymbolAtLocation(element.name); + if (symbol) identifiers.add(symbol); } } } @@ -159,30 +167,42 @@ function collectRequiredPolicyBindings( function collectPolicyBuilderBindings( sourceFile: ts.SourceFile, fileName: string, + repoRoot: string, + checker: ts.TypeChecker, ): PolicyBuilderBindings { - const identifiers = new Set(); - const namespaces = new Set(); + const identifiers = new Set(); + const namespaces = new Set(); for (const statement of sourceFile.statements) { if ( ts.isImportDeclaration(statement) && ts.isStringLiteralLike(statement.moduleSpecifier) && statement.importClause && !statement.importClause.isTypeOnly && - isPolicyBuilderModule(fileName, statement.moduleSpecifier.text) + isPolicyBuilderModule(fileName, statement.moduleSpecifier.text, repoRoot) ) { const { namedBindings } = statement.importClause; if (namedBindings && ts.isNamespaceImport(namedBindings)) { - namespaces.add(namedBindings.name.text); + const symbol = checker.getSymbolAtLocation(namedBindings.name); + if (symbol) namespaces.add(symbol); } else if (namedBindings) { for (const element of namedBindings.elements) { if (element.isTypeOnly) continue; const importedName = element.propertyName?.text ?? element.name.text; - if (POLICY_GET_BUILDERS.has(importedName)) identifiers.add(element.name.text); + if (!POLICY_GET_BUILDERS.has(importedName)) continue; + const symbol = checker.getSymbolAtLocation(element.name); + if (symbol) identifiers.add(symbol); } } } else if (ts.isVariableStatement(statement)) { for (const declaration of statement.declarationList.declarations) { - collectRequiredPolicyBindings(declaration, fileName, identifiers, namespaces); + collectRequiredPolicyBindings( + declaration, + fileName, + repoRoot, + checker, + identifiers, + namespaces, + ); } } } @@ -192,15 +212,46 @@ function collectPolicyBuilderBindings( function isPolicyBuilderCall( expression: ts.LeftHandSideExpression, bindings: PolicyBuilderBindings, + checker: ts.TypeChecker, ): boolean { - if (ts.isIdentifier(expression)) return bindings.identifiers.has(expression.text); + if (ts.isIdentifier(expression)) { + const symbol = checker.getSymbolAtLocation(expression); + return !!symbol && bindings.identifiers.has(symbol); + } const memberName = calledName(expression); if (!memberName || !POLICY_GET_BUILDERS.has(memberName)) return false; const target = ts.isPropertyAccessExpression(expression) || ts.isElementAccessExpression(expression) ? expression.expression : null; - return !!target && ts.isIdentifier(target) && bindings.namespaces.has(target.text); + if (!target || !ts.isIdentifier(target)) return false; + const symbol = checker.getSymbolAtLocation(target); + return !!symbol && bindings.namespaces.has(symbol); +} + +function createBoundSourceFile( + source: string, + fileName: string, +): { readonly sourceFile: ts.SourceFile; readonly checker: ts.TypeChecker } { + const absoluteFileName = path.resolve(fileName); + const compilerOptions: ts.CompilerOptions = { + module: ts.ModuleKind.ESNext, + noLib: true, + noResolve: true, + target: ts.ScriptTarget.Latest, + }; + const host = ts.createCompilerHost(compilerOptions, true); + host.fileExists = (candidate) => path.resolve(candidate) === absoluteFileName; + host.readFile = (candidate) => + path.resolve(candidate) === absoluteFileName ? source : undefined; + host.getSourceFile = (candidate, languageVersion) => + path.resolve(candidate) === absoluteFileName + ? ts.createSourceFile(candidate, source, languageVersion, true) + : undefined; + const program = ts.createProgram([absoluteFileName], compilerOptions, host); + const sourceFile = program.getSourceFile(absoluteFileName); + if (!sourceFile) throw new Error(`Unable to parse policy read source: ${fileName}`); + return { sourceFile, checker: program.getTypeChecker() }; } function literalText(expression: ts.Expression): string | null { @@ -229,13 +280,20 @@ function isDirectPolicyRead(expression: ts.ArrayLiteralExpression): boolean { ); } -export function countPolicyReadCalls(source: string, fileName: string): number { - const sourceFile = ts.createSourceFile(fileName, source, ts.ScriptTarget.Latest, true); - const builderBindings = collectPolicyBuilderBindings(sourceFile, fileName); +export function countPolicyReadCalls( + source: string, + fileName: string, + repoRoot = REPO_ROOT, +): number { + const { sourceFile, checker } = createBoundSourceFile(source, fileName); + const builderBindings = collectPolicyBuilderBindings(sourceFile, fileName, repoRoot, checker); let readCalls = 0; function visit(node: ts.Node): void { - if (ts.isCallExpression(node) && isPolicyBuilderCall(node.expression, builderBindings)) { + if ( + ts.isCallExpression(node) && + isPolicyBuilderCall(node.expression, builderBindings, checker) + ) { readCalls += 1; } else if (ts.isArrayLiteralExpression(node) && isDirectPolicyRead(node)) { readCalls += 1; @@ -268,7 +326,7 @@ export function discoverPolicyReadSites(repoRoot: string): DiscoveredPolicyReadS .flatMap((sourceRoot) => productionTypeScriptFiles(path.join(repoRoot, sourceRoot))) .flatMap((sourcePath) => { const source = readFileSync(sourcePath, "utf8"); - const readCalls = countPolicyReadCalls(source, sourcePath); + const readCalls = countPolicyReadCalls(source, sourcePath, repoRoot); return readCalls > 0 ? [ { diff --git a/test/policy-mutation-read-discovery.test.ts b/test/policy-mutation-read-discovery.test.ts index 73ef04e2b87..e9c00e34747 100644 --- a/test/policy-mutation-read-discovery.test.ts +++ b/test/policy-mutation-read-discovery.test.ts @@ -33,7 +33,7 @@ describe("OpenShell policy mutation read discovery (#6921)", () => { '["not-openshell", "policy", "get", "--base", sandboxName];', ].join("\n"); - expect(countPolicyReadCalls(source, "/repo/src/lib/fixture.ts")).toBe(7); + expect(countPolicyReadCalls(source, "/repo/src/lib/fixture.ts", "/repo")).toBe(7); }); it("ignores similarly named calls without canonical policy bindings", () => { @@ -49,7 +49,45 @@ describe("OpenShell policy mutation read discovery (#6921)", () => { 'fixture["buildPolicyGetFullCommand"](sandboxName);', ].join("\n"); - expect(countPolicyReadCalls(source, "/repo/src/lib/fixture.ts")).toBe(0); + expect(countPolicyReadCalls(source, "/repo/src/lib/fixture.ts", "/repo")).toBe(0); + }); + + it("ignores a named policy builder import when a nested binding shadows its alias", () => { + const source = [ + 'import { buildPolicyGetCommand as buildBase } from "./policy/commands";', + "buildBase(rootSandbox);", + "function inspect(buildBase: (sandbox: string) => string[]) {", + " buildBase(shadowedSandbox);", + "}", + ].join("\n"); + + expect(countPolicyReadCalls(source, "/repo/src/lib/fixture.ts", "/repo")).toBe(1); + }); + + it("ignores a namespace policy import when a nested binding shadows its alias", () => { + const source = [ + 'import * as policyBuilders from "./policy/index";', + "policyBuilders.buildPolicyGetCommand(rootSandbox);", + "function inspect(policyBuilders: Record string[]>) {", + " policyBuilders.buildPolicyGetCommand(shadowedSandbox);", + ' policyBuilders["buildPolicyGetFullCommand"](shadowedSandbox);', + "}", + ].join("\n"); + + expect(countPolicyReadCalls(source, "/repo/src/lib/fixture.ts", "/repo")).toBe(1); + }); + + it("ignores policy-shaped modules outside the repository-root canonical policy path", () => { + const source = [ + 'import { buildPolicyGetCommand as decoyBase } from "./vendor/src/lib/policy";', + 'import * as decoyBuilders from "./vendor/src/lib/policy/commands";', + 'const requiredDecoyBuilders = require("./vendor/src/lib/policy/index");', + "decoyBase(sandboxName);", + "decoyBuilders.buildPolicyGetFullCommand(sandboxName);", + "requiredDecoyBuilders.buildPolicyGetCommand(sandboxName);", + ].join("\n"); + + expect(countPolicyReadCalls(source, "/repo/src/lib/fixture.ts", "/repo")).toBe(0); }); it("discovers builder and direct policy reads in new production files", () => { From 209d93adf1ed0d6f0675fe8fbd7355b8a2e40915 Mon Sep 17 00:00:00 2001 From: Carlos Villela Date: Sat, 18 Jul 2026 12:36:43 -0700 Subject: [PATCH 9/9] test(checks): use trusted ratchet core --- test/test-create-require-budget.test.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/test/test-create-require-budget.test.ts b/test/test-create-require-budget.test.ts index 2132e2ea980..3fbfac134b5 100644 --- a/test/test-create-require-budget.test.ts +++ b/test/test-create-require-budget.test.ts @@ -6,11 +6,12 @@ import os from "node:os"; import path from "node:path"; import { afterEach, describe, expect, it } from "vitest"; +import ts from "typescript"; import { extractTrustedCreateRequireAllowlists, trustedCreateRequireExpansionFailure, -} from "../.github/actions/ci-static-checks/create-require-ratchet.mts"; +} from "../.github/actions/ci-static-checks/create-require-ratchet-core.mts"; import { collectProductionCreateRequireSources, collectTestSupportCreateRequireSources, @@ -186,14 +187,14 @@ describe("CLI createRequire budget", () => { expect( trustedCreateRequireExpansionFailure( - extractTrustedCreateRequireAllowlists(currentSource), - extractTrustedCreateRequireAllowlists(baselineSource), + extractTrustedCreateRequireAllowlists(ts, currentSource, "current-checker.mts"), + extractTrustedCreateRequireAllowlists(ts, baselineSource, "baseline-checker.mts"), ), ).toContain("src/new.test.ts"); expect( trustedCreateRequireExpansionFailure( - extractTrustedCreateRequireAllowlists(currentSource), - extractTrustedCreateRequireAllowlists(baselineSource), + extractTrustedCreateRequireAllowlists(ts, currentSource, "current-checker.mts"), + extractTrustedCreateRequireAllowlists(ts, baselineSource, "baseline-checker.mts"), ), ).toContain("test/new.ts"); });