diff --git a/.github/workflows/codebase-growth-guardrails.yaml b/.github/workflows/codebase-growth-guardrails.yaml index c1ebb94edf3..10fc44dfacb 100644 --- a/.github/workflows/codebase-growth-guardrails.yaml +++ b/.github/workflows/codebase-growth-guardrails.yaml @@ -12,6 +12,7 @@ on: types: [opened, reopened, synchronize, ready_for_review] permissions: + contents: read pull-requests: read jobs: @@ -105,3 +106,220 @@ jobs: in the top-level onboard entrypoint. EOF exit 1 + + - name: Require changed test files to stay within size budget + env: + GH_TOKEN: ${{ github.token }} + PR_NUMBER: ${{ github.event.pull_request.number }} + REPO: ${{ github.repository }} + BASE_SHA: ${{ github.event.pull_request.base.sha }} + HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + set -euo pipefail + + changed_files_file="$(mktemp)" + rows_file="$(mktemp)" + base_budget_file="$(mktemp)" + head_budget_file="$(mktemp)" + base_budget_mode_file="$(mktemp)" + budget_changed=false + + gh api --paginate "/repos/${REPO}/pulls/${PR_NUMBER}/files" \ + --jq '.[] | [.status, .filename, (.previous_filename // "")] | @tsv' \ + > "$changed_files_file" + + gh api --paginate "/repos/${REPO}/pulls/${PR_NUMBER}/files" \ + --jq '.[] | select((.status != "removed") and (.filename | test("^(test|src|nemoclaw/src)/.*\\.(test|spec)\\.(ts|js|mts|mjs|cts|cjs)$"))) | [.filename] | @tsv' \ + > "$rows_file" + + if gh api "/repos/${REPO}/contents/ci/test-file-size-budget.json?ref=${BASE_SHA}" --jq .content 2>/dev/null \ + | base64 --decode > "$base_budget_file" && [ -s "$base_budget_file" ]; then + echo "base" > "$base_budget_mode_file" + else + echo "WARN: budget file not found at base SHA; using conservative default fallback." + printf '%s\n' '{"defaultMaxLines":1500,"legacyMaxLines":{}}' > "$base_budget_file" + echo "fallback" > "$base_budget_mode_file" + fi + + while IFS=$'\t' read -r _file_status file_path previous_path; do + if [ "$file_path" = "ci/test-file-size-budget.json" ] || [ "$previous_path" = "ci/test-file-size-budget.json" ]; then + budget_changed=true + break + fi + done < "$changed_files_file" + + if [ "$budget_changed" = true ]; then + if ! gh api "/repos/${HEAD_REPO}/contents/ci/test-file-size-budget.json?ref=${HEAD_SHA}" --jq .content \ + | base64 --decode > "$head_budget_file" || [ ! -s "$head_budget_file" ]; then + echo "FAIL: ci/test-file-size-budget.json must remain present and parseable at the PR head." + exit 1 + fi + else + cp "$base_budget_file" "$head_budget_file" + fi + + node - "$base_budget_file" "$head_budget_file" "$base_budget_mode_file" "$rows_file" <<'NODE' + const fs = require("node:fs"); + + function countLines(text) { + if (text.length === 0) return 0; + const newlineCount = text.match(/\r\n|\r|\n/g)?.length ?? 0; + return newlineCount + (/(?:\r\n|\r|\n)$/.test(text) ? 0 : 1); + } + + function parseBudget(sourceText, label) { + const parsed = JSON.parse(sourceText); + if (!Number.isInteger(parsed.defaultMaxLines) || parsed.defaultMaxLines <= 0) { + throw new Error(`${label} must define positive integer defaultMaxLines`); + } + + const legacyMaxLines = parsed.legacyMaxLines ?? {}; + if ( + typeof legacyMaxLines !== "object" || + legacyMaxLines === null || + Array.isArray(legacyMaxLines) + ) { + throw new Error(`${label} legacyMaxLines must be an object`); + } + + for (const [file, maxLines] of Object.entries(legacyMaxLines)) { + if (!Number.isInteger(maxLines) || maxLines <= 0) { + throw new Error(`${label} has invalid legacy budget for ${file}: ${maxLines}`); + } + } + + return { defaultMaxLines: parsed.defaultMaxLines, legacyMaxLines }; + } + + function changedTestFiles(rowsPath) { + const rowsText = fs.readFileSync(rowsPath, "utf8").trim(); + return rowsText.length === 0 ? [] : rowsText.split(/\n/).filter(Boolean); + } + + function githubContentsUrl(file) { + const repo = process.env.HEAD_REPO; + const headSha = process.env.HEAD_SHA; + if (!repo || !headSha) { + throw new Error("HEAD_REPO and HEAD_SHA must be set"); + } + const encodedPath = file.split("/").map(encodeURIComponent).join("/"); + return `https://api.github.com/repos/${repo}/contents/${encodedPath}?ref=${encodeURIComponent(headSha)}`; + } + + async function fetchHeadTextFile(file) { + const response = await fetch(githubContentsUrl(file), { + headers: { + Authorization: `Bearer ${process.env.GH_TOKEN}`, + "X-GitHub-Api-Version": "2022-11-28", + }, + }); + if (response.status === 404) return null; + if (!response.ok) { + throw new Error(`Could not fetch ${file}: HTTP ${response.status}`); + } + const body = await response.json(); + if (body.type !== "file" || body.encoding !== "base64" || typeof body.content !== "string") { + throw new Error(`Could not decode file contents for ${file}`); + } + return Buffer.from(body.content.replace(/\s/g, ""), "base64").toString("utf8"); + } + + async function validateBudgetChange(baseBudget, headBudget, baseBudgetMode) { + if (baseBudgetMode === "fallback") return []; + + const violations = []; + if (headBudget.defaultMaxLines > baseBudget.defaultMaxLines) { + violations.push( + `defaultMaxLines increased from ${baseBudget.defaultMaxLines} to ${headBudget.defaultMaxLines}`, + ); + } + + for (const [file, baseMax] of Object.entries(baseBudget.legacyMaxLines)) { + const headMax = headBudget.legacyMaxLines[file]; + if (headMax === undefined) { + const text = await fetchHeadTextFile(file); + if (text !== null && countLines(text) > headBudget.defaultMaxLines) { + violations.push( + `${file} removed its legacy budget while still exceeding defaultMaxLines`, + ); + } + } else if (headMax > baseMax) { + violations.push(`${file} legacy budget increased from ${baseMax} to ${headMax}`); + } + } + + for (const [file, headMax] of Object.entries(headBudget.legacyMaxLines)) { + if (baseBudget.legacyMaxLines[file] === undefined && headMax > headBudget.defaultMaxLines) { + violations.push( + `${file} adds a new legacy budget (${headMax}) above defaultMaxLines (${headBudget.defaultMaxLines})`, + ); + } + const text = await fetchHeadTextFile(file); + if (text === null) { + violations.push(`${file} has a legacy budget but no matching test file at the PR head`); + } else if (countLines(text) > headMax) { + violations.push(`${file} has ${countLines(text)} line(s), above its legacy budget ${headMax}`); + } + } + + return violations; + } + + async function main() { + const [baseBudgetPath, headBudgetPath, baseBudgetModePath, rowsPath] = process.argv.slice(2); + const baseBudget = parseBudget(fs.readFileSync(baseBudgetPath, "utf8"), "base budget"); + const headBudget = parseBudget(fs.readFileSync(headBudgetPath, "utf8"), "head budget"); + const baseBudgetMode = fs.readFileSync(baseBudgetModePath, "utf8").trim(); + const rows = changedTestFiles(rowsPath); + + function maxLinesFor(file) { + return headBudget.legacyMaxLines[file] ?? headBudget.defaultMaxLines; + } + + const budgetViolations = await validateBudgetChange(baseBudget, headBudget, baseBudgetMode); + const violations = []; + + for (const file of rows) { + const text = await fetchHeadTextFile(file); + if (text === null) { + throw new Error(`Changed test file ${file} was not found at the PR head`); + } + const lines = countLines(text); + const maxLines = maxLinesFor(file); + if (lines > maxLines) { + violations.push(`${file}: ${lines} line(s) > ${maxLines}`); + } + const legacyMax = headBudget.legacyMaxLines[file]; + if (legacyMax !== undefined && lines < legacyMax) { + violations.push(`${file}: ${lines} line(s) < ${legacyMax} legacy budget; lower the budget entry`); + } + } + + if (budgetViolations.length > 0 || violations.length > 0) { + if (budgetViolations.length > 0) { + console.error("FAIL: ci/test-file-size-budget.json weakens the base budget."); + for (const violation of budgetViolations) { + console.error(`- ${violation}`); + } + } + if (violations.length > 0) { + console.error("FAIL: one or more changed test files exceed or underrun the size budget."); + console.error("Split large tests into focused files, or shrink a legacy oversized test before adding more coverage there."); + for (const violation of violations) { + console.error(`- ${violation}`); + } + } + process.exit(1); + } + + console.log( + `PASS: test size budget policy is monotonic and ${rows.length} changed test file(s) are within budget.`, + ); + } + + main().catch((error) => { + console.error(error); + process.exit(1); + }); + NODE diff --git a/.github/workflows/pr.yaml b/.github/workflows/pr.yaml index 1d45a2c1595..b5c3853a295 100644 --- a/.github/workflows/pr.yaml +++ b/.github/workflows/pr.yaml @@ -112,11 +112,15 @@ jobs: --skip test-cli \ --skip test-plugin \ --skip source-shape-test-budget \ + --skip test-file-size-budget \ --skip test-skills-yaml - name: Run source-shape budget run: npm run source-shape:check + - name: Run test file size budget + run: npm run test-size:check + - name: Run skills YAML tests run: npx vitest run test/skills-frontmatter.test.ts diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 2d09b6cdd1a..b09bc8e2dc6 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,3 +1,6 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + # NemoClaw — prek hook configuration # prek: https://github.com/j178/prek — single binary, no Python required for the runner # Installed as an npm devDependency (@j178/prek) — available after `npm install`. @@ -304,6 +307,14 @@ repos: files: ^(test/|scripts/find-source-shape-tests\.ts$|ci/source-shape-test-budget\.json$) priority: 20 + - id: test-file-size-budget + name: Test file size budget + entry: npm run test-size:check + language: system + pass_filenames: false + files: ^(test/|src/.*\.(test|spec)\.(ts|js|mts|mjs|cts|cjs)$|nemoclaw/src/.*\.(test|spec)\.(ts|js|mts|mjs|cts|cjs)$|scripts/check-test-file-size-budget\.ts$|ci/test-file-size-budget\.json$) + priority: 20 + - id: test-skills-yaml name: Test (skills YAML) entry: npx vitest run test/skills-frontmatter.test.ts diff --git a/ci/test-file-size-budget.json b/ci/test-file-size-budget.json new file mode 100644 index 00000000000..236a84beffe --- /dev/null +++ b/ci/test-file-size-budget.json @@ -0,0 +1,18 @@ +{ + "$comment": "SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.\nSPDX-License-Identifier: Apache-2.0", + "defaultMaxLines": 1500, + "legacyMaxLines": { + "nemoclaw/src/commands/migration-state.test.ts": 1566, + "src/lib/inference/nim.test.ts": 2079, + "src/lib/onboard/preflight.test.ts": 1905, + "test/channels-add-preset.test.ts": 1915, + "test/generate-openclaw-config.test.ts": 2106, + "test/install-preflight.test.ts": 4397, + "test/nemoclaw-start.test.ts": 5319, + "test/onboard-messaging.test.ts": 2122, + "test/onboard-selection.test.ts": 7757, + "test/onboard.test.ts": 4887, + "test/policies.test.ts": 3147, + "test/sandbox-connect-inference.test.ts": 1577 + } +} diff --git a/docs/reference/troubleshooting.mdx b/docs/reference/troubleshooting.mdx index 326f13b5f43..bd7f35dcbba 100644 --- a/docs/reference/troubleshooting.mdx +++ b/docs/reference/troubleshooting.mdx @@ -1028,8 +1028,9 @@ $$nemoclaw onboard These are build-time settings baked into the sandbox image. Changing them after onboarding requires re-running `$$nemoclaw onboard` to rebuild the image. -When `HTTP_PROXY` or `HTTPS_PROXY` is set on the host, NemoClaw adds `localhost` and `127.0.0.1` to `NO_PROXY` for managed subprocesses. -This keeps local Ollama health checks and model pulls from being routed through a corporate or desktop proxy while preserving the proxy for external hosts. +When `HTTP_PROXY` or `HTTPS_PROXY` is set on the host, NemoClaw adds `localhost`, `127.0.0.1`, `::1`, `0.0.0.0`, the container-host aliases `host.docker.internal` and `host.containers.internal`, and the managed inference hostname `inference.local` to `NO_PROXY` for host-side subprocesses and for the env forwarded into `openshell sandbox create`. +This keeps local Ollama health checks, model pulls, and managed inference traffic from being chained through a corporate or desktop proxy at the sandbox-create boundary, while preserving the proxy for external hosts. +Inside the running sandbox, processes continue to use the OpenShell L7 proxy for `inference.local` so OpenShell's internal routing, DNS, and audit boundaries stay intact. ### Agent cannot reach a host-side HTTP service diff --git a/nemoclaw/src/lib/subprocess-env.ts b/nemoclaw/src/lib/subprocess-env.ts index a1819ce4292..de497710b5c 100644 --- a/nemoclaw/src/lib/subprocess-env.ts +++ b/nemoclaw/src/lib/subprocess-env.ts @@ -49,11 +49,26 @@ const ALLOWED_ENV_PREFIXES = ["LC_", "XDG_", "OPENSHELL_", "GRPC_"]; // ── Public API ───────────────────────────────────────────────── /** - * When any HTTP proxy is forwarded, ensure local host-bound traffic is not - * routed through it. Without this, tools that respect HTTP_PROXY (curl, Node.js - * http, Python requests) will tunnel loopback or WSL Windows-host requests to - * the user's proxy (e.g. Privoxy), which fails with HTTP 500. - * See: #2616 + * When any HTTP proxy is forwarded, augment NO_PROXY so the host proxy is + * never asked to forward traffic destined for the host loopback, the + * container-host aliases, or the OpenShell-managed inference hostname. + * + * Boundary: the helper covers host-side subprocesses (curl, Node.js http, + * Python requests) and the env forwarded into `openshell sandbox create + * -- env ...`. The latter is what determines whether OpenShell's L7 proxy + * chains a hostname through the host HTTP_PROXY when the host has one set + * (for example Privoxy at 127.0.0.1:8118 on macOS + Colima). Adding + * `inference.local` here is the seed that keeps OpenShell-internal + * inference traffic off the host proxy chain. + * + * The sandbox runtime's own NO_PROXY is set later by + * `scripts/nemoclaw-start.sh` against the OpenShell L7 proxy address and + * intentionally does not include `inference.local`, which is orthogonal + * to this seed and unaffected by the augmentation. + * + * Removal condition: when OpenShell's host-side proxy chaining no longer + * consults the caller's NO_PROXY for sandbox-create env decisions, this + * augmentation can be dropped. */ export function withLocalNoProxy(env: Record): void { const hasProxy = env.HTTP_PROXY || env.HTTPS_PROXY || env.http_proxy || env.https_proxy; @@ -65,7 +80,15 @@ export function withLocalNoProxy(env: Record): void { .map((s) => s.trim()) .filter(Boolean); let changed = false; - for (const host of ["localhost", "127.0.0.1", "host.docker.internal", "::1", "0.0.0.0"]) { + for (const host of [ + "localhost", + "127.0.0.1", + "host.docker.internal", + "host.containers.internal", + "::1", + "0.0.0.0", + "inference.local", + ]) { if (!parts.includes(host)) { parts.push(host); changed = true; diff --git a/package.json b/package.json index d5686f173ec..9179ec36a36 100644 --- a/package.json +++ b/package.json @@ -1,4 +1,5 @@ { + "$comment": "SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.\nSPDX-License-Identifier: Apache-2.0", "name": "nemoclaw", "version": "0.1.0", "description": "NemoClaw — run OpenClaw inside OpenShell with NVIDIA inference", @@ -35,6 +36,7 @@ "type-safety:hotspots": "tsx scripts/type-safety-hotspots.ts", "source-shape:scan": "tsx scripts/find-source-shape-tests.ts --metrics", "source-shape:check": "tsx scripts/find-source-shape-tests.ts --check", + "test-size:check": "tsx scripts/check-test-file-size-budget.ts", "bump:version": "tsx scripts/bump-version.ts", "release:plan": "tsx scripts/release-plan.ts", "release:cut": "bash scripts/release-cut-tag.sh", diff --git a/scripts/check-test-file-size-budget.ts b/scripts/check-test-file-size-budget.ts new file mode 100755 index 00000000000..932d197d8a4 --- /dev/null +++ b/scripts/check-test-file-size-budget.ts @@ -0,0 +1,225 @@ +#!/usr/bin/env -S npx tsx +// 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 path from "node:path"; +import { fileURLToPath } from "node:url"; + +export type TestFileSizeBudget = { + readonly defaultMaxLines: number; + readonly legacyMaxLines?: Readonly>; +}; + +export type TestFileSizeEntry = { + readonly file: string; + readonly lines: number; +}; + +export type TestFileSizeViolation = + | { + readonly kind: "oversized"; + readonly file: string; + readonly lines: number; + readonly maxLines: number; + readonly budgetKind: "default" | "legacy"; + } + | { + readonly kind: "legacy-ratchet"; + readonly file: string; + readonly lines: number; + readonly maxLines: number; + } + | { + readonly kind: "stale-legacy-budget"; + readonly file: string; + readonly maxLines: number; + }; + +const REPO_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); +const BUDGET_PATH = path.join(REPO_ROOT, "ci", "test-file-size-budget.json"); +const TEST_FILE_PATTERN = /\.(?:test|spec)\.(?:[cm]?[jt]s)$/; +const SCAN_ROOTS = ["test", "src", "nemoclaw/src"]; +const SKIP_DIRS = new Set([ + ".git", + ".venv", + "coverage", + "dist", + "docs/_build", + "nemoclaw/dist", + "nemoclaw/node_modules", + "node_modules", +]); + +function toRepoPath(absPath: string): string { + return path.relative(REPO_ROOT, absPath).split(path.sep).join("/"); +} + +function isSkipped(absPath: string): boolean { + const rel = toRepoPath(absPath); + return [...SKIP_DIRS].some((skipDir) => rel === skipDir || rel.startsWith(`${skipDir}/`)); +} + +function* walkFiles(dir: string): Generator { + if (!existsSync(dir) || isSkipped(dir)) return; + + for (const entry of readdirSync(dir)) { + const absPath = path.join(dir, entry); + if (isSkipped(absPath)) continue; + + const stats = statSync(absPath); + if (stats.isDirectory()) { + yield* walkFiles(absPath); + } else if (stats.isFile() && TEST_FILE_PATTERN.test(entry)) { + yield absPath; + } + } +} + +export function countLines(text: string): number { + if (text.length === 0) return 0; + const newlineCount = text.match(/\r\n|\r|\n/g)?.length ?? 0; + return newlineCount + (/(?:\r\n|\r|\n)$/.test(text) ? 0 : 1); +} + +export function collectTestFileSizes(roots = SCAN_ROOTS): TestFileSizeEntry[] { + return roots + .flatMap((root) => [...walkFiles(path.join(REPO_ROOT, root))]) + .map((absPath) => ({ + file: toRepoPath(absPath), + lines: countLines(readFileSync(absPath, "utf-8")), + })) + .sort((a, b) => a.file.localeCompare(b.file)); +} + +function assertPositiveInteger(value: unknown, label: string): number { + if (!Number.isInteger(value) || Number(value) <= 0) { + throw new Error(`${label} must be a positive integer`); + } + return Number(value); +} + +export function parseBudget(sourceText: string, filePath = BUDGET_PATH): TestFileSizeBudget { + const parsed = JSON.parse(sourceText) as { + readonly defaultMaxLines?: unknown; + readonly legacyMaxLines?: unknown; + }; + const defaultMaxLines = assertPositiveInteger( + parsed.defaultMaxLines, + `${filePath}: defaultMaxLines`, + ); + + if (parsed.legacyMaxLines !== undefined && !isRecord(parsed.legacyMaxLines)) { + throw new Error(`${filePath}: legacyMaxLines must be an object when present`); + } + + const legacyMaxLines: Record = {}; + for (const [legacyPath, value] of Object.entries(parsed.legacyMaxLines ?? {})) { + legacyMaxLines[legacyPath] = assertPositiveInteger( + value, + `${filePath}: legacyMaxLines.${legacyPath}`, + ); + } + + return { defaultMaxLines, legacyMaxLines }; +} + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +export function evaluateTestFileSizeBudget( + entries: readonly TestFileSizeEntry[], + budget: TestFileSizeBudget, +): TestFileSizeViolation[] { + const legacyMaxLines = budget.legacyMaxLines ?? {}; + const seenFiles = new Set(entries.map((entry) => entry.file)); + const violations: TestFileSizeViolation[] = []; + + for (const entry of entries) { + const legacyMax = legacyMaxLines[entry.file]; + const maxLines = legacyMax ?? budget.defaultMaxLines; + const budgetKind = legacyMax === undefined ? "default" : "legacy"; + + if (entry.lines > maxLines) { + violations.push({ + kind: "oversized", + file: entry.file, + lines: entry.lines, + maxLines, + budgetKind, + }); + } else if (legacyMax !== undefined && entry.lines < legacyMax) { + violations.push({ + kind: "legacy-ratchet", + file: entry.file, + lines: entry.lines, + maxLines: legacyMax, + }); + } + } + + for (const [legacyPath, maxLines] of Object.entries(legacyMaxLines)) { + if (!seenFiles.has(legacyPath)) { + violations.push({ kind: "stale-legacy-budget", file: legacyPath, maxLines }); + } + } + + return violations.sort((a, b) => a.file.localeCompare(b.file)); +} + +export function formatViolations( + violations: readonly TestFileSizeViolation[], + budgetPath = "ci/test-file-size-budget.json", +): string { + const lines = [ + "Test file size budget failed.", + "", + `Default test-file ceiling is configured in ${budgetPath}.`, + "Split oversized tests into focused files, or ratchet the legacy budget down after shrinking them.", + "", + ]; + + for (const violation of violations) { + if (violation.kind === "oversized") { + lines.push( + `- ${violation.file}: ${violation.lines} line(s) > ${violation.maxLines} ${violation.budgetKind} budget`, + ); + } else if (violation.kind === "legacy-ratchet") { + lines.push( + `- ${violation.file}: ${violation.lines} line(s) < ${violation.maxLines} legacy budget; lower the budget entry`, + ); + } else { + lines.push( + `- ${violation.file}: legacy budget entry (${violation.maxLines}) has no matching test file; remove it`, + ); + } + } + + return lines.join("\n"); +} + +function main(): void { + const budget = parseBudget(readFileSync(BUDGET_PATH, "utf-8"), BUDGET_PATH); + const entries = collectTestFileSizes(); + const violations = evaluateTestFileSizeBudget(entries, budget); + + if (violations.length > 0) { + console.error(formatViolations(violations)); + process.exitCode = 1; + return; + } + + const maxEntry = entries.reduce( + (max, entry) => (max === null || entry.lines > max.lines ? entry : max), + null, + ); + const maxText = maxEntry ? `${maxEntry.file} (${maxEntry.lines} lines)` : "no test files"; + console.log( + `Test file size budget passed: ${entries.length} files scanned; largest is ${maxText}.`, + ); +} + +if (fileURLToPath(import.meta.url) === path.resolve(process.argv[1] ?? "")) { + main(); +} diff --git a/src/lib/onboard/http-proxy-preflight.test.ts b/src/lib/onboard/http-proxy-preflight.test.ts index 80cee3c6538..06cd8d7a289 100644 --- a/src/lib/onboard/http-proxy-preflight.test.ts +++ b/src/lib/onboard/http-proxy-preflight.test.ts @@ -5,7 +5,7 @@ import { describe, expect, it } from "vitest"; import { redactProxyCredentials, warnIfHostProxyMissesLoopback } from "./http-proxy-preflight"; -describe("redactProxyCredentials (#2616)", () => { +describe("redactProxyCredentials", () => { it("returns plain proxy URLs unchanged", () => { expect(redactProxyCredentials("http://127.0.0.1:8118")).toBe("http://127.0.0.1:8118"); expect(redactProxyCredentials("http://corp-proxy.example.com:3128")).toBe( @@ -35,7 +35,7 @@ describe("redactProxyCredentials (#2616)", () => { }); }); -describe("warnIfHostProxyMissesLoopback (#2616)", () => { +describe("warnIfHostProxyMissesLoopback", () => { it("does not warn when no HTTP_PROXY is set", () => { const lines: string[] = []; const fired = warnIfHostProxyMissesLoopback({}, (line) => lines.push(line)); @@ -43,37 +43,50 @@ describe("warnIfHostProxyMissesLoopback (#2616)", () => { expect(lines).toEqual([]); }); - it("does not warn when NO_PROXY already includes localhost", () => { + it("does not warn when NO_PROXY includes loopback and the managed inference hostname", () => { const lines: string[] = []; const fired = warnIfHostProxyMissesLoopback( - { http_proxy: "http://127.0.0.1:8118", NO_PROXY: "localhost,127.0.0.1" }, + { + http_proxy: "http://127.0.0.1:8118", + NO_PROXY: "localhost,127.0.0.1,inference.local", + }, (line) => lines.push(line), ); expect(fired).toBe(false); expect(lines).toEqual([]); }); - it("warns when NO_PROXY only has localhost (127.0.0.1 still proxied) (CodeRabbit #3801)", () => { + it("warns when NO_PROXY has loopback but is missing the managed inference hostname", () => { + const lines: string[] = []; + const fired = warnIfHostProxyMissesLoopback( + { http_proxy: "http://127.0.0.1:8118", NO_PROXY: "localhost,127.0.0.1" }, + (line) => lines.push(line), + ); + expect(fired).toBe(true); + expect(lines.join("\n")).toContain("inference.local"); + }); + + it("warns when NO_PROXY only has localhost (127.0.0.1 still proxied)", () => { const lines: string[] = []; const fired = warnIfHostProxyMissesLoopback( { http_proxy: "http://127.0.0.1:8118", NO_PROXY: "localhost" }, (line) => lines.push(line), ); expect(fired).toBe(true); - expect(lines.join("\n")).toContain("export NO_PROXY=localhost,127.0.0.1"); + expect(lines.join("\n")).toContain("export NO_PROXY=localhost,127.0.0.1,inference.local"); }); - it("warns when NO_PROXY only has 127.0.0.1 (localhost still proxied) (CodeRabbit #3801)", () => { + it("warns when NO_PROXY only has 127.0.0.1 (localhost still proxied)", () => { const lines: string[] = []; const fired = warnIfHostProxyMissesLoopback( { http_proxy: "http://127.0.0.1:8118", NO_PROXY: "127.0.0.1" }, (line) => lines.push(line), ); expect(fired).toBe(true); - expect(lines.join("\n")).toContain("export NO_PROXY=localhost,127.0.0.1"); + expect(lines.join("\n")).toContain("export NO_PROXY=localhost,127.0.0.1,inference.local"); }); - it("warns when HTTP_PROXY is set without NO_PROXY=localhost", () => { + it("warns when HTTP_PROXY is set without NO_PROXY", () => { const lines: string[] = []; const fired = warnIfHostProxyMissesLoopback( { http_proxy: "http://127.0.0.1:8118" }, @@ -82,10 +95,10 @@ describe("warnIfHostProxyMissesLoopback (#2616)", () => { expect(fired).toBe(true); expect(lines.join("\n")).toContain("HTTP_PROXY/http_proxy is set"); expect(lines.join("\n")).toContain("Detected proxy: http://127.0.0.1:8118"); - expect(lines.join("\n")).toContain("export NO_PROXY=localhost,127.0.0.1"); + expect(lines.join("\n")).toContain("export NO_PROXY=localhost,127.0.0.1,inference.local"); }); - it("redacts credentials in the proxy URL it logs (CodeRabbit #3801)", () => { + it("redacts credentials in the proxy URL it logs", () => { const lines: string[] = []; warnIfHostProxyMissesLoopback( { http_proxy: "http://alice:s3cret@proxy.example.com:3128" }, @@ -107,4 +120,15 @@ describe("warnIfHostProxyMissesLoopback (#2616)", () => { expect(fired).toBe(true); expect(lines.join("\n")).toContain("corp-proxy:3128"); }); + + it("surfaces the managed inference hostname in the suggested NO_PROXY export", () => { + const lines: string[] = []; + warnIfHostProxyMissesLoopback({ http_proxy: "http://127.0.0.1:8118" }, (line) => + lines.push(line), + ); + const joined = lines.join("\n"); + expect(joined).toContain("inference.local"); + expect(joined).toContain("export NO_PROXY=localhost,127.0.0.1,inference.local"); + expect(joined).toContain("export no_proxy=localhost,127.0.0.1,inference.local"); + }); }); diff --git a/src/lib/onboard/http-proxy-preflight.ts b/src/lib/onboard/http-proxy-preflight.ts index 1a068de178b..11538a582b2 100644 --- a/src/lib/onboard/http-proxy-preflight.ts +++ b/src/lib/onboard/http-proxy-preflight.ts @@ -3,7 +3,7 @@ /** * Preflight warning when the user's shell has HTTP_PROXY set without a - * NO_PROXY=localhost,127.0.0.1 bypass. See #2616. + * NO_PROXY bypass for loopback and the managed inference hostname. * * NemoClaw's own subprocess spawn helpers (`buildSubprocessEnv`) inject * NO_PROXY for loopback hosts, so NemoClaw-managed processes are safe. But @@ -19,19 +19,25 @@ export function warnIfHostProxyMissesLoopback( const proxyEnv = env.HTTP_PROXY || env.http_proxy; if (!proxyEnv) return false; const noProxyEnv = env.NO_PROXY || env.no_proxy || ""; - // Require BOTH entries — HTTP libraries match the literal hostname against - // NO_PROXY, so `NO_PROXY=localhost` alone still proxies `127.0.0.1` requests - // (and vice versa). Only suppress the warning when both are present. + // Require all three entries — HTTP libraries match the literal hostname + // against NO_PROXY, so partial coverage still proxies the missing entries. + // Suppress the warning only when localhost, 127.0.0.1, and the managed + // inference hostname are all present. const hasLocalhost = /(^|,)\s*localhost\s*(,|$)/.test(noProxyEnv); const hasLoopback = /(^|,)\s*127\.0\.0\.1\s*(,|$)/.test(noProxyEnv); - if (hasLocalhost && hasLoopback) return false; - warn(" ⚠ HTTP_PROXY/http_proxy is set without NO_PROXY=localhost,127.0.0.1."); + const hasInference = /(^|,)\s*inference\.local\s*(,|$)/.test(noProxyEnv); + if (hasLocalhost && hasLoopback && hasInference) return false; + warn( + " ⚠ HTTP_PROXY/http_proxy is set without NO_PROXY=localhost,127.0.0.1,inference.local.", + ); warn(` Detected proxy: ${redactProxyCredentials(proxyEnv)}`); - warn(" NemoClaw injects NO_PROXY for its own subprocess spawns, but any tool you run"); - warn(" that respects HTTP_PROXY (curl, Node fetch, Python requests) will still tunnel"); - warn(" localhost traffic through your host proxy. To bypass loopback (see #2616):"); - warn(" export NO_PROXY=localhost,127.0.0.1"); - warn(" export no_proxy=localhost,127.0.0.1"); + warn(" NemoClaw injects NO_PROXY for its own subprocess spawns (loopback hosts,"); + warn(" container-host aliases, and the managed inference hostname inference.local),"); + warn(" but any tool you run that respects HTTP_PROXY (curl, Node fetch, Python"); + warn(" requests) will still tunnel localhost traffic through your host proxy."); + warn(" To bypass loopback and the managed inference hostname:"); + warn(" export NO_PROXY=localhost,127.0.0.1,inference.local"); + warn(" export no_proxy=localhost,127.0.0.1,inference.local"); return true; } diff --git a/src/lib/subprocess-env.test.ts b/src/lib/subprocess-env.test.ts index 02abf5c65f7..0b5dc239a65 100644 --- a/src/lib/subprocess-env.test.ts +++ b/src/lib/subprocess-env.test.ts @@ -4,7 +4,8 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { withLocalNoProxy } from "../../dist/lib/subprocess-env"; -const LOCAL_NO_PROXY = "localhost,127.0.0.1,host.docker.internal,::1,0.0.0.0"; +const LOCAL_NO_PROXY = + "localhost,127.0.0.1,host.docker.internal,host.containers.internal,::1,0.0.0.0,inference.local"; describe("withLocalNoProxy", () => { it("does nothing when no proxy vars are present", () => { @@ -62,8 +63,12 @@ describe("withLocalNoProxy", () => { no_proxy: "example.com,localhost", }; withLocalNoProxy(env); - expect(env.NO_PROXY).toBe("example.com,localhost,127.0.0.1,host.docker.internal,::1,0.0.0.0"); - expect(env.no_proxy).toBe("example.com,localhost,127.0.0.1,host.docker.internal,::1,0.0.0.0"); + expect(env.NO_PROXY).toBe( + "example.com,localhost,127.0.0.1,host.docker.internal,host.containers.internal,::1,0.0.0.0,inference.local", + ); + expect(env.no_proxy).toBe( + "example.com,localhost,127.0.0.1,host.docker.internal,host.containers.internal,::1,0.0.0.0,inference.local", + ); }); it("does not duplicate entries when all local hosts are already present", () => { @@ -87,6 +92,49 @@ describe("withLocalNoProxy", () => { expect(env.NO_PROXY).toBe(`corp.internal,.nvidia.com,${LOCAL_NO_PROXY}`); expect(env.no_proxy).toBe(`corp.internal,.nvidia.com,${LOCAL_NO_PROXY}`); }); + + it("bypasses the host proxy for the managed inference hostname when HTTP_PROXY is set", () => { + const env: Record = { HTTP_PROXY: "http://127.0.0.1:8118" }; + withLocalNoProxy(env); + expect(env.NO_PROXY?.split(",")).toContain("inference.local"); + expect(env.no_proxy?.split(",")).toContain("inference.local"); + }); + + it("bypasses the host proxy for the rootless container host alias when HTTPS_PROXY is set", () => { + const env: Record = { HTTPS_PROXY: "http://127.0.0.1:8118" }; + withLocalNoProxy(env); + expect(env.NO_PROXY?.split(",")).toContain("host.containers.internal"); + expect(env.no_proxy?.split(",")).toContain("host.containers.internal"); + }); + + it("does not inject a broad .local suffix or arbitrary *.local hostnames", () => { + const env: Record = { HTTP_PROXY: "http://127.0.0.1:8118" }; + withLocalNoProxy(env); + for (const key of ["NO_PROXY", "no_proxy"] as const) { + const parts = (env[key] ?? "").split(","); + expect(parts).not.toContain(".local"); + expect(parts).not.toContain("*.local"); + expect(parts).not.toContain("evil.local"); + expect(parts).not.toContain("attacker.local"); + expect(parts.filter((p) => p.endsWith(".local"))).toEqual(["inference.local"]); + } + }); + + it("preserves a caller-provided .local entry without expanding the bypass", () => { + const env: Record = { + HTTP_PROXY: "http://127.0.0.1:8118", + NO_PROXY: "trusted.local", + no_proxy: "trusted.local", + }; + withLocalNoProxy(env); + for (const key of ["NO_PROXY", "no_proxy"] as const) { + const parts = (env[key] ?? "").split(","); + expect(parts).toContain("trusted.local"); + expect(parts).toContain("inference.local"); + expect(parts).not.toContain(".local"); + expect(parts).not.toContain("*.local"); + } + }); }); describe("buildSubprocessEnv NO_PROXY injection", () => { diff --git a/src/lib/subprocess-env.ts b/src/lib/subprocess-env.ts index a829a0a66c0..0ff8b59bdc5 100644 --- a/src/lib/subprocess-env.ts +++ b/src/lib/subprocess-env.ts @@ -49,20 +49,46 @@ const ALLOWED_ENV_PREFIXES = ["LC_", "XDG_", "OPENSHELL_", "GRPC_"]; // ── Public API ───────────────────────────────────────────────── /** - * When any HTTP proxy is forwarded, ensure local host-bound traffic is not - * routed through it. Without this, tools that respect HTTP_PROXY (curl, Node.js - * http, Python requests) will tunnel loopback or WSL Windows-host requests to - * the user's proxy (e.g. Privoxy), which fails with HTTP 500. - * See: #2616 + * When any HTTP proxy is forwarded, augment NO_PROXY so the host proxy is + * never asked to forward traffic destined for the host loopback, the + * container-host aliases, or the OpenShell-managed inference hostname. + * + * Boundary: the helper covers host-side subprocesses (curl, Node.js http, + * Python requests) and the env forwarded into `openshell sandbox create + * -- env ...`. The latter is what determines whether OpenShell's L7 proxy + * chains a hostname through the host HTTP_PROXY when the host has one set + * (for example Privoxy at 127.0.0.1:8118 on macOS + Colima). Adding + * `inference.local` here is the seed that keeps OpenShell-internal + * inference traffic off the host proxy chain. + * + * The sandbox runtime's own NO_PROXY is set later by + * `scripts/nemoclaw-start.sh` against the OpenShell L7 proxy address and + * intentionally does not include `inference.local`, which is orthogonal + * to this seed and unaffected by the augmentation. + * + * Removal condition: when OpenShell's host-side proxy chaining no longer + * consults the caller's NO_PROXY for sandbox-create env decisions, this + * augmentation can be dropped. */ export function withLocalNoProxy(env: Record): void { const hasProxy = env.HTTP_PROXY || env.HTTPS_PROXY || env.http_proxy || env.https_proxy; if (!hasProxy) return; for (const key of ["NO_PROXY", "no_proxy"] as const) { const current = env[key] ?? ""; - const parts = current.split(",").map((s) => s.trim()).filter(Boolean); + const parts = current + .split(",") + .map((s) => s.trim()) + .filter(Boolean); let changed = false; - for (const host of ["localhost", "127.0.0.1", "host.docker.internal", "::1", "0.0.0.0"]) { + for (const host of [ + "localhost", + "127.0.0.1", + "host.docker.internal", + "host.containers.internal", + "::1", + "0.0.0.0", + "inference.local", + ]) { if (!parts.includes(host)) { parts.push(host); changed = true; diff --git a/test/credential-exposure.test.ts b/test/credential-exposure.test.ts index a77d163476a..f1189341f81 100644 --- a/test/credential-exposure.test.ts +++ b/test/credential-exposure.test.ts @@ -112,8 +112,12 @@ describe("credential exposure in process arguments", () => { withLocalNoProxy(env); - expect(env.NO_PROXY).toBe("corp.internal,localhost,127.0.0.1,host.docker.internal,::1,0.0.0.0"); - expect(env.no_proxy).toBe("corp.internal,localhost,127.0.0.1,host.docker.internal,::1,0.0.0.0"); + expect(env.NO_PROXY).toBe( + "corp.internal,localhost,127.0.0.1,host.docker.internal,host.containers.internal,::1,0.0.0.0,inference.local", + ); + expect(env.no_proxy).toBe( + "corp.internal,localhost,127.0.0.1,host.docker.internal,host.containers.internal,::1,0.0.0.0,inference.local", + ); } }); diff --git a/test/host-proxy-inference-local-e2e.test.ts b/test/host-proxy-inference-local-e2e.test.ts new file mode 100644 index 00000000000..51399992a5e --- /dev/null +++ b/test/host-proxy-inference-local-e2e.test.ts @@ -0,0 +1,166 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { execFileSync, spawn } from "node:child_process"; +import http from "node:http"; +import type { AddressInfo } from "node:net"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; + +import { buildSubprocessEnv } from "../dist/lib/subprocess-env"; + +function runCurl( + args: string[], + env: NodeJS.ProcessEnv, +): Promise<{ status: number | null; stdout: string; stderr: string }> { + return new Promise((resolve, reject) => { + const child = spawn("curl", args, { env }); + const stdoutChunks: Buffer[] = []; + const stderrChunks: Buffer[] = []; + child.stdout.on("data", (c) => stdoutChunks.push(c)); + child.stderr.on("data", (c) => stderrChunks.push(c)); + child.on("error", reject); + child.on("close", (status) => { + resolve({ + status, + stdout: Buffer.concat(stdoutChunks).toString("utf-8"), + stderr: Buffer.concat(stderrChunks).toString("utf-8"), + }); + }); + }); +} + +function curlAvailable(): boolean { + try { + execFileSync("curl", ["--version"], { stdio: "pipe" }); + return true; + } catch { + return false; + } +} + +const curlOk = curlAvailable(); +if (!curlOk && process.env.CI === "true") { + throw new Error( + "[host-proxy-inference-local-e2e] CI=true but curl unavailable. " + + "This test must not silently skip in CI — install curl on the runner.", + ); +} + +// Boundary: this E2E proves that the env produced by `buildSubprocessEnv()` +// causes a host-side `curl` to reach `inference.local` directly when a host +// HTTP proxy is set, exercising the seed list `withLocalNoProxy()` injects. +// The test is run against a deliberately unreachable proxy (127.0.0.1:1) +// so that the negative control case fails fast when the bypass is absent, +// and the positive case proves that `no_proxy` (lowercase, the form curl +// honours for plain http:// URLs) is responsible for routing the request +// directly to the local listener. +// +// The full sandbox path on macOS + Colima (where OpenShell's L7 proxy +// chains through the host HTTP_PROXY and must bypass for `inference.local`) +// requires a macOS + Colima runner and is not covered here. +describe("inference.local bypass via host NO_PROXY seed", () => { + const saved: Record = {}; + let server: http.Server; + let port: number; + let received: { url: string | undefined; host: string | undefined }[]; + + const curlArgs = () => [ + "-sS", + "--max-time", + "5", + "--resolve", + `inference.local:${port}:127.0.0.1`, + `http://inference.local:${port}/v1/chat/completions`, + ]; + + const stripInferenceLocal = (env: Record) => { + for (const key of ["NO_PROXY", "no_proxy"] as const) { + const cur = env[key] ?? ""; + env[key] = cur + .split(",") + .map((p) => p.trim()) + .filter((p) => p && p !== "inference.local") + .join(","); + } + }; + + beforeEach(async () => { + received = []; + server = http.createServer((req, res) => { + received.push({ url: req.url, host: req.headers.host }); + res.writeHead(200, { "content-type": "text/plain" }); + res.end("inference-local-direct"); + }); + await new Promise((resolve, reject) => { + server.once("error", reject); + server.listen(0, "127.0.0.1", () => resolve()); + }); + const addr = server.address() as AddressInfo | null; + if (!addr) throw new Error("listener address unavailable"); + port = addr.port; + + for (const key of [ + "HTTP_PROXY", + "HTTPS_PROXY", + "NO_PROXY", + "http_proxy", + "https_proxy", + "no_proxy", + ]) { + saved[key] = process.env[key]; + } + // Set both cases — curl honours lowercase `http_proxy` for http:// URLs + // and uppercase HTTPS_PROXY for https:// URLs. Pointing both at a + // deliberately unreachable address (127.0.0.1:1, refused) ensures a + // proxied request fails fast. + process.env.HTTP_PROXY = "http://127.0.0.1:1"; + process.env.HTTPS_PROXY = "http://127.0.0.1:1"; + process.env.http_proxy = "http://127.0.0.1:1"; + process.env.https_proxy = "http://127.0.0.1:1"; + delete process.env.NO_PROXY; + delete process.env.no_proxy; + }); + + afterEach(async () => { + for (const [key, value] of Object.entries(saved)) { + if (value === undefined) delete process.env[key]; + else process.env[key] = value; + } + await new Promise((resolve) => server.close(() => resolve())); + }); + + it.skipIf(!curlOk)( + "negative control: without inference.local in no_proxy, curl is routed through the broken proxy and the listener never sees the request", + async () => { + const env = buildSubprocessEnv(); + stripInferenceLocal(env); + expect(env.NO_PROXY?.split(",")).not.toContain("inference.local"); + expect(env.no_proxy?.split(",")).not.toContain("inference.local"); + + const result = await runCurl(curlArgs(), env); + + expect( + result.status, + `curl should fail when routed through the broken proxy; stderr: ${result.stderr}`, + ).not.toBe(0); + expect(received).toHaveLength(0); + }, + ); + + it.skipIf(!curlOk)( + "positive: subprocess env carries inference.local in no_proxy so curl bypasses the broken proxy and reaches the listener", + async () => { + const env = buildSubprocessEnv(); + expect(env.NO_PROXY?.split(",")).toContain("inference.local"); + expect(env.no_proxy?.split(",")).toContain("inference.local"); + + const result = await runCurl(curlArgs(), env); + + expect(result.status, `curl exit ${result.status}, stderr: ${result.stderr}`).toBe(0); + expect(result.stdout).toBe("inference-local-direct"); + expect(received).toHaveLength(1); + expect(received[0]?.url).toBe("/v1/chat/completions"); + expect(received[0]?.host).toBe(`inference.local:${port}`); + }, + ); +}); diff --git a/test/onboard.test.ts b/test/onboard.test.ts index 761a9ad198e..bc9af37d668 100644 --- a/test/onboard.test.ts +++ b/test/onboard.test.ts @@ -173,6 +173,34 @@ describe("onboard helpers", () => { } }); + it("seeds inference.local and host.containers.internal into the sandbox-create NO_PROXY/no_proxy", () => { + // Boundary pin: appendHostProxyEnvArgs() forwards env into `openshell + // sandbox create -- env ...`, and OpenShell consults the seeded + // NO_PROXY at sandbox-create time when deciding whether to chain its + // L7 proxy through the host HTTP_PROXY for a given hostname. Both + // `inference.local` (OpenShell-managed inference) and + // `host.containers.internal` (rootless container host alias) must be + // emitted here so the L7 proxy never tunnels them through the host + // proxy. The complementary runtime exclusion (nemoclaw-start.sh sets a + // narrower NO_PROXY without inference.local once sandbox boots) is + // asserted in test/service-env.test.ts. + const envArgs: string[] = []; + + appendHostProxyEnvArgs(envArgs, { + HTTP_PROXY: "http://127.0.0.1:8118", + }); + + const upper = envArgs.find((e) => e.startsWith("NO_PROXY=")); + const lower = envArgs.find((e) => e.startsWith("no_proxy=")); + expect(upper, "NO_PROXY should be synthesized").toBeDefined(); + expect(lower, "no_proxy should be synthesized").toBeDefined(); + for (const v of [upper, lower]) { + const parts = (v ?? "").split("=")[1]?.split(",") ?? []; + expect(parts).toContain("inference.local"); + expect(parts).toContain("host.containers.internal"); + } + }); + it("propagates NEMOCLAW_MINIMAL_BOOTSTRAP=1 from host into sandbox env (#2598)", () => { const envArgs: string[] = []; appendHostProxyEnvArgs(envArgs, { NEMOCLAW_MINIMAL_BOOTSTRAP: "1" }); diff --git a/test/pr-workflow-contract.test.ts b/test/pr-workflow-contract.test.ts index 63cbcc2749c..96a801f2603 100644 --- a/test/pr-workflow-contract.test.ts +++ b/test/pr-workflow-contract.test.ts @@ -13,6 +13,10 @@ type PullRequestWorkflow = { jobs: Record; }; +type CodebaseGrowthGuardrailsWorkflow = { + jobs: Record; +}; + function stepRuns(job: WorkflowJob): string[] { return (job.steps ?? []).flatMap((step) => (step.run ? [step.run] : [])); } @@ -116,6 +120,7 @@ describe("pull request workflow contract", () => { "test-cli", "test-plugin", "source-shape-test-budget", + "test-file-size-budget", "test-skills-yaml", ]) { expect(staticPrekRun).toContain(`--skip ${skippedHook}`); @@ -134,9 +139,28 @@ describe("pull request workflow contract", () => { expect(pluginTestRun).toContain("npx vitest run --project plugin"); expect(pluginTestRun).toContain("npx tsx scripts/check-coverage-ratchet.ts"); expect(staticRuns).toContain("npm run source-shape:check"); + expect(staticRuns).toContain("npm run test-size:check"); expect(staticRuns).toContain("npx vitest run test/skills-frontmatter.test.ts"); }); + it("keeps the trusted test-size guard closed around budget policy changes", () => { + const growthGuardrails = readYaml( + ".github/workflows/codebase-growth-guardrails.yaml", + ); + const guardRun = stepRuns(growthGuardrails.jobs["codebase-growth-guardrails"]).join( + "\n", + ); + + expect(guardRun).toContain("HEAD_REPO"); + expect(guardRun).toContain("HEAD_SHA"); + expect(guardRun).not.toContain(".raw_url"); + expect(guardRun).toContain('(.previous_filename // "")'); + expect(guardRun).toContain("[ \"$budget_changed\" = true ]"); + expect(guardRun).toContain( + "has a legacy budget but no matching test file at the PR head", + ); + }); + it("uploads CLI Vitest JSON results for timing analysis", () => { const uploadStep = workflow.jobs["cli-tests"].steps?.find( (step) => step.name === "Upload CLI Vitest timing report", diff --git a/test/test-file-size-budget.test.ts b/test/test-file-size-budget.test.ts new file mode 100644 index 00000000000..9bce33c207a --- /dev/null +++ b/test/test-file-size-budget.test.ts @@ -0,0 +1,98 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it } from "vitest"; + +import { + countLines, + evaluateTestFileSizeBudget, + formatViolations, + parseBudget, +} from "../scripts/check-test-file-size-budget"; + +describe("test file size budget", () => { + it("counts trailing-newline and non-trailing-newline files consistently", () => { + expect(countLines("")).toBe(0); + expect(countLines("one")).toBe(1); + expect(countLines("one\n")).toBe(1); + expect(countLines("one\r\ntwo")).toBe(2); + }); + + it("flags non-grandfathered test files above the default ceiling", () => { + const violations = evaluateTestFileSizeBudget( + [{ file: "test/new-large.test.ts", lines: 151 }], + { defaultMaxLines: 150 }, + ); + + expect(violations).toEqual([ + { + kind: "oversized", + file: "test/new-large.test.ts", + lines: 151, + maxLines: 150, + budgetKind: "default", + }, + ]); + }); + + it("grandfathers existing large tests without allowing growth", () => { + const budget = { defaultMaxLines: 150, legacyMaxLines: { "test/legacy.test.ts": 250 } }; + + expect( + evaluateTestFileSizeBudget([{ file: "test/legacy.test.ts", lines: 250 }], budget), + ).toEqual([]); + expect( + evaluateTestFileSizeBudget([{ file: "test/legacy.test.ts", lines: 251 }], budget), + ).toEqual([ + { + kind: "oversized", + file: "test/legacy.test.ts", + lines: 251, + maxLines: 250, + budgetKind: "legacy", + }, + ]); + }); + + it("requires legacy budgets to ratchet down when oversized tests shrink", () => { + const violations = evaluateTestFileSizeBudget( + [{ file: "test/legacy.test.ts", lines: 200 }], + { defaultMaxLines: 150, legacyMaxLines: { "test/legacy.test.ts": 250 } }, + ); + + expect(violations).toEqual([ + { + kind: "legacy-ratchet", + file: "test/legacy.test.ts", + lines: 200, + maxLines: 250, + }, + ]); + expect(formatViolations(violations)).toContain("lower the budget entry"); + }); + + it("rejects stale legacy budget entries", () => { + const violations = evaluateTestFileSizeBudget([], { + defaultMaxLines: 150, + legacyMaxLines: { "test/deleted.test.ts": 200 }, + }); + + expect(violations).toEqual([ + { kind: "stale-legacy-budget", file: "test/deleted.test.ts", maxLines: 200 }, + ]); + }); + + it("parses the JSON budget format", () => { + expect( + parseBudget( + JSON.stringify({ + defaultMaxLines: 1500, + legacyMaxLines: { "test/legacy.test.ts": 2000 }, + }), + ), + ).toEqual({ + defaultMaxLines: 1500, + legacyMaxLines: { "test/legacy.test.ts": 2000 }, + }); + }); +});