diff --git a/Makefile b/Makefile index 63377a1f5dc..47d7a5b2784 100644 --- a/Makefile +++ b/Makefile @@ -100,7 +100,7 @@ include fmt.mk .PHONY: storybook storybook-run storybook-build storybook-flake-check test-storybook storybook-budget .PHONY: benchmark-terminal bench bench-compare test-loop-baseline first-load-js page-timing first-run-view-parity bug-bash mcp-apps-e2e test-stories generate-phone-chrome check-phone-chrome .PHONY: ensure-deps mux -.PHONY: check-startup-imports check-startup-imports-runtime check-react-compiler check-test-routing check-test-seam-comments test-bench-scripts +.PHONY: check-startup-imports check-startup-imports-runtime check-react-compiler check-test-routing check-test-seam-comments check-test-only-exports test-bench-scripts # Use the package binary instead of its internal path so native-preview can change wrappers safely. TSGO := bun run tsgo @@ -539,7 +539,7 @@ build/icon.png: docs/img/logo-white.svg scripts/generate-icons.ts # (.oxlintrc.json, kept in step by check-oxlint-config); static-check-full also runs type-aware # ESLint on all files, so CI still catches what oxlint cannot check, including type-aware # findings that a change surfaces in files it did not touch. -STATIC_CHECKS := typecheck fmt-check check-startup-imports check-react-compiler check-code-docs-links check-test-routing check-test-seam-comments check-oxlint-config lint-shellcheck lint-hadolint +STATIC_CHECKS := typecheck fmt-check check-startup-imports check-react-compiler check-code-docs-links check-test-routing check-test-seam-comments check-test-only-exports check-oxlint-config lint-shellcheck lint-hadolint static-check: lint $(STATIC_CHECKS) ## Run fast local static checks @@ -622,6 +622,11 @@ check-test-seam-comments: node_modules/.installed ## Fail when production code g @bun test ./scripts/check-test-seam-comments.test.ts @bun scripts/check-test-seam-comments.ts +# ~3 s: fails when a production export under src/ has no non-test reference (see the script). +check-test-only-exports: node_modules/.installed ## Fail when a production export under src/ is unlisted and only tests (or nothing) reference it + @bun test ./scripts/check-test-only-exports.test.ts + @bun scripts/check-test-only-exports.ts + check-bench-agent: node_modules/.installed src/version.ts $(BUILTIN_SKILLS_GENERATED) $(BUILTIN_WORKFLOWS_GENERATED) $(WORKFLOW_RUNTIME_SOURCES_GENERATED) ## Verify terminal-bench agent configuration and imports @./scripts/check-bench-agent.sh diff --git a/scripts/check-test-only-exports.allowlist.json b/scripts/check-test-only-exports.allowlist.json new file mode 100644 index 00000000000..92635146ad0 --- /dev/null +++ b/scripts/check-test-only-exports.allowlist.json @@ -0,0 +1,52 @@ +{ + "allowed": [ + { "file": "src/browser/contexts/ChatHostContext.tsx", "symbol": "ChatHostContextProvider", "reason": "consumer outside src/: packages/chat-components/src/index.ts re-exports it and vscode/src/webview/App.tsx renders it" }, + { "file": "src/browser/stores/DraftStore.ts", "symbol": "resetDraftStoreForTests", "reason": "module-singleton reset hook named ...ForTests: chatCommands.fork.test.ts; no production caller" }, + { "file": "src/browser/utils/aiSelectionIntent.ts", "symbol": "resetAiSelectionIntentForTests", "reason": "module-singleton reset hook named ...ForTests: aiSelectionIntent.test.ts, chatCommands.test.ts, tests/ui/agents/creationSlashCommands.test.ts, vscode/src/webview/App.test.tsx and the Storybook meta decorator; no production caller" }, + { "file": "src/browser/utils/highlighting/highlightWorkerClient.ts", "symbol": "__pendingWorkerCallCountForTests", "reason": "module-singleton reset hook named ...ForTests: highlightWorkerClient.test.ts (read-only retention witness, #6303); no production caller" }, + { "file": "src/browser/utils/highlighting/highlightWorkerClient.ts", "symbol": "__resetForTests", "reason": "module-singleton reset hook named ...ForTests: highlightWorkerClient.test.ts and HighlightedCode.test.tsx; no production caller" }, + { "file": "src/browser/utils/workspaceStorageGc.ts", "symbol": "resetWorkspaceStorageGcForTests", "reason": "module-singleton reset hook named ...ForTests: workspaceStorageGc.test.ts and WorkspaceContext.test.tsx; no production caller" }, + { "file": "src/common/compat/macPackagedApp.ts", "symbol": "resolveMacPackagedAppNames", "reason": "consumers outside src/: scripts/checkMacAttachFileRuntime.ts and the Makefile's macOS packaging recipe" }, + { "file": "src/common/orpc/schemas/drafts.ts", "symbol": "CreationDraftScope", "reason": "story helper type: the Storybook drafts mock (src/browser/stories/mocks/drafts.ts) types its creation-draft scope with it" }, + { "file": "src/common/orpc/schemas/reviewState.ts", "symbol": "ReviewSchemasMatchTypes", "reason": "compile-time guard: the type fails typecheck when the persisted Review schemas and the shared Review types diverge; it is exported only so no-unused-vars accepts it" }, + { "file": "src/common/utils/sessionTapes/sessionTapeLoader.ts", "symbol": "summarizeSessionTape", "reason": "consumer outside src/: scripts/perf/tapeInfo.ts" }, + { "file": "src/common/utils/tokens/updateModelsData.ts", "symbol": "pruneModelData", "reason": "consumer outside src/: scripts/update_models.ts (the logic lives in src so unit tests cover it)" }, + { "file": "src/common/utils/tokens/updateModelsData.ts", "symbol": "sanitizePricing", "reason": "consumer outside src/: scripts/update_models.ts (the logic lives in src so unit tests cover it)" }, + { "file": "src/common/utils/tokens/updateModelsData.ts", "symbol": "validateModelData", "reason": "consumer outside src/: scripts/update_models.ts (the logic lives in src so unit tests cover it)" }, + { "file": "src/common/utils/tokens/updateModelsData.ts", "symbol": "serializeModelData", "reason": "consumer outside src/: scripts/update_models.ts (the logic lives in src so unit tests cover it)" }, + { "file": "src/constants/layout.ts", "symbol": "MOBILE_TOUCH_TARGET_PX", "reason": "consumer outside src/: .storybook/test-runner.ts checks touch targets with it (globals.css keeps the same value)" }, + { "file": "src/constants/layout.ts", "symbol": "TABLET_TOUCH_TARGET_PX", "reason": "consumer outside src/: .storybook/test-runner.ts checks tablet touch targets with it" }, + { "file": "src/node/runtime/containerCli.ts", "symbol": "resetContainerCliCacheForTests", "reason": "module-singleton reset hook named ...ForTests: containerCli.test.ts; no production caller" }, + { "file": "src/node/services/log.ts", "symbol": "__resetFileSinkForTests", "reason": "module-singleton reset hook named ...ForTests: log.test.ts; no production caller" }, + { "file": "src/node/services/mock/mockAiRouter.ts", "symbol": "buildMockStreamStartGateMessage", "reason": "test-support builder kept next to the mock router's parser of the same gate prompt, so tests/ipc/streaming/queuedMessages.starting.test.ts and tests/ipc/tasks/persistentSubagentCompaction.test.ts cannot drift from the format" }, + { "file": "src/node/services/ptc/staticAnalysis.ts", "symbol": "disposeAnalysisContextForTests", "reason": "module-singleton reset hook named ...ForTests: staticAnalysis.test.ts; no production caller" }, + { "file": "src/node/services/ptc/typeGenerator.ts", "symbol": "clearTypeCacheForTests", "reason": "module-singleton reset hook named ...ForTests: typeGenerator.test.ts; no production caller" }, + { "file": "src/node/utils/concurrency/processLiveness.ts", "symbol": "setSelfIdentityForTests", "reason": "module-singleton reset hook named ...ForTests: crossProcessLock.test.ts; no production caller" }, + { "file": "src/node/utils/extensionMetadata.ts", "symbol": "readExtensionMetadata", "reason": "consumer outside src/: vscode/src/xumConfig.ts" }, + { "file": "src/node/utils/main/bashPath.ts", "symbol": "resetBashPathCacheForTests", "reason": "module-singleton reset hook named ...ForTests: bashPath.test.ts; no production caller" }, + { "file": "src/node/utils/main/tokenizer.ts", "symbol": "__resetTokenizerForTests", "reason": "module-singleton reset hook named ...ForTests: tokenizer.test.ts and tokenizerWarmModels.test.ts; no production caller" }, + { "file": "src/vite/devServerEnv.ts", "symbol": "resolveViteDevServerEnv", "reason": "consumer outside src/: vite.config.ts" }, + { "file": "src/vite/novncCompatPlugin.ts", "symbol": "novncCompatPlugin", "reason": "consumer outside src/: vite.config.ts" }, + { "file": "src/vite/reactCompilerConfig.ts", "symbol": "reactCompilerConfig", "reason": "consumers outside src/: vite.config.ts and scripts/check_react_compiler_coverage.ts" }, + { "file": "src/vite/streamdownRehypeKatexPlugin.ts", "symbol": "streamdownRehypeKatexPlugin", "reason": "consumer outside src/: vite.config.ts" }, + { "file": "src/vite/streamdownShikiLangsPlugin.ts", "symbol": "streamdownShikiLangsPlugin", "reason": "consumer outside src/: vite.config.ts" }, + { "file": "src/common/orpc/onChatCursorFingerprint.ts", "symbol": "computePriorHistoryFingerprint", "reason": "reference implementation: the tests of computePriorHistoryFingerprintAsync (production) require the same result for every input" }, + { "file": "src/node/services/replay/replayFixture.ts", "symbol": "appendReplayFixtureCompactionBoundary", "reason": "test-support: the replay golden-fixture harness" }, + { "file": "src/node/services/replay/replayFixture.ts", "symbol": "generateReplayFixtureSession", "reason": "test-support: the replay golden-fixture harness" }, + { "file": "src/common/staticWorkspaceView/phoneChrome.ts", "symbol": "PhoneChrome", "reason": "test-support type: it types the generated PHONE_CHROME_JSON skeleton, which only tests parse" }, + { "file": "src/node/worktree/codeWorkspaceSync.ts", "symbol": "updateCodeWorkspaceFile", "reason": "unit boundary of the .code-workspace edit algorithm (23 tests); syncProjectCodeWorkspace runs the same updateCodeWorkspaceFileLocked under its own lock" } + ], + "pendingDeletion": [ + { "file": "src/browser/features/RightSidebar/ArtifactsTab/artifactDocument.ts", "symbol": "buildSandboxedHtmlDocument", "owner": "P3-4 (test-audit phase 3, frontend)", "reason": "no non-test reference at 7d2498b1d0 (2026-10-26 audit); the owner deletes it, or moves it to \"allowed\" with a reason" }, + { "file": "src/browser/features/Settings/Sections/GoalsSection.tsx", "symbol": "GoalsSection", "owner": "P3-4 (test-audit phase 3, frontend)", "reason": "no non-test reference at 7d2498b1d0 (2026-10-26 audit); the owner deletes it, or moves it to \"allowed\" with a reason" }, + { "file": "src/browser/hooks/useModelsFromSettings.ts", "symbol": "getSuggestedModels", "owner": "P3-4 (test-audit phase 3, frontend)", "reason": "no non-test reference at 7d2498b1d0 (2026-10-26 audit); the owner deletes it, or moves it to \"allowed\" with a reason" }, + { "file": "src/browser/stores/DraftStore.ts", "symbol": "DraftAttachmentMatchesChatAttachment", "owner": "P3-4 (test-audit phase 3, frontend)", "reason": "no non-test reference at 7d2498b1d0 (2026-10-26 audit); the owner deletes it, or moves it to \"allowed\" with a reason" }, + { "file": "src/browser/utils/highlighting/highlightDiffChunk.ts", "symbol": "highlightDiffChunk", "owner": "P3-4 (test-audit phase 3, frontend)", "reason": "no non-test reference at 7d2498b1d0 (2026-10-26 audit); the owner deletes it, or moves it to \"allowed\" with a reason" }, + { "file": "src/browser/utils/messages/StreamingTPSCalculator.ts", "symbol": "calculateTPS", "owner": "P3-4 (test-audit phase 3, frontend)", "reason": "no non-test reference at 7d2498b1d0 (2026-10-26 audit); the owner deletes it, or moves it to \"allowed\" with a reason" }, + { "file": "src/browser/utils/messages/StreamingTPSCalculator.ts", "symbol": "calculateTokenCount", "owner": "P3-4 (test-audit phase 3, frontend)", "reason": "no non-test reference at 7d2498b1d0 (2026-10-26 audit); the owner deletes it, or moves it to \"allowed\" with a reason" }, + { "file": "src/browser/utils/messages/StreamingTPSCalculator.ts", "symbol": "DeltaRecord", "owner": "P3-4 (test-audit phase 3, frontend)", "reason": "no non-test reference at 7d2498b1d0 (2026-10-26 audit); the owner deletes it, or moves it to \"allowed\" with a reason" }, + { "file": "src/browser/utils/rightSidebarLayout.ts", "symbol": "addToolToFocusedTabset", "owner": "P3-4 (test-audit phase 3, frontend)", "reason": "no non-test reference at 7d2498b1d0 (2026-10-26 audit); the owner deletes it, or moves it to \"allowed\" with a reason" }, + { "file": "src/browser/utils/workflowRunMessages.ts", "symbol": "filterWorkflowDisplayOnlyMessages", "owner": "P3-4 (test-audit phase 3, frontend)", "reason": "no non-test reference at 7d2498b1d0 (2026-10-26 audit); the owner deletes it, or moves it to \"allowed\" with a reason" }, + { "file": "src/browser/utils/workspaceStorage.ts", "symbol": "migrateWorkspaceStorage", "owner": "P3-4 (test-audit phase 3, frontend)", "reason": "no non-test reference at 7d2498b1d0 (2026-10-26 audit); the owner deletes it, or moves it to \"allowed\" with a reason" } + ] +} diff --git a/scripts/check-test-only-exports.test.ts b/scripts/check-test-only-exports.test.ts new file mode 100644 index 00000000000..e02958c1725 --- /dev/null +++ b/scripts/check-test-only-exports.test.ts @@ -0,0 +1,244 @@ +// Tests for scripts/check-test-only-exports.ts. `make check-test-only-exports` runs this file +// before the real check (scripts/ tests are not part of the `bun test src` lane). +import { expect, test } from "bun:test"; +import { + checkExports, + collectModuleFacts, + findUnreferencedExports, + isRuntimeContractExport, + resolveSpecifier, + type ModuleFacts, +} from "./check-test-only-exports"; + +/** Unreferenced exports (`file#name`) of a set of production files. */ +function unreferenced(files: Record): string[] { + const facts = new Map(); + for (const [file, text] of Object.entries(files)) facts.set(file, collectModuleFacts(file, text)); + const names = Object.keys(files); + return findUnreferencedExports(facts, names, names).map((hit) => `${hit.file}#${hit.symbol}`); +} + +test("flags an export that no production file imports or uses", () => { + // Tests are never parsed, so an export only a test imports looks exactly like this. + expect( + unreferenced({ + "src/a.ts": "export function used() {}\nexport function testOnly() {}", + "src/b.ts": 'import { used } from "./a";\nused();', + }) + ).toEqual(["src/a.ts#testOnly"]); +}); + +test("counts a use in the export's own file, but not a property with the same name", () => { + expect( + unreferenced({ + "src/a.ts": [ + "export const LIMIT = 3;", + "export function inner() {}", + "export function outer() { return inner() + LIMIT; }", + "export const lone = 1;", + "const o = { lone: 2 };", + "o.lone;", + "const { lone: renamed } = o;", + ].join("\n"), + "src/b.ts": 'import { outer } from "@/a";\nouter();', + }) + ).toEqual(["src/a.ts#lone"]); +}); + +test("follows named and star re-exports through barrels", () => { + expect( + unreferenced({ + "src/lib/a.ts": "export const viaStar = 1;\nexport const viaName = 2;\nexport default 3;", + "src/lib/index.ts": + 'export * from "./a";\nexport { viaName as renamed, default } from "./a";', + "src/b.ts": 'import { viaStar, renamed } from "./lib";\nviaStar + renamed;', + }) + ).toEqual(["src/lib/a.ts#default"]); + // `export *` does not forward `default`. + expect( + unreferenced({ + "src/a.ts": "export default 1;", + "src/index.ts": 'export * from "./a";', + "src/b.ts": 'import x from "./index";\nx;', + }) + ).toEqual(["src/a.ts#default"]); +}); + +test("treats namespace, dynamic, require and import-type uses as references", () => { + const target = "export const a = 1;\nexport const b = 2;"; + expect(unreferenced({ "src/t.ts": target, "src/u.ts": 'import * as t from "./t";\nt;' })).toEqual( + [] + ); + expect( + unreferenced({ "src/t.ts": target, "src/u.ts": 'const lazy = () => import("./t");' }) + ).toEqual([]); + expect(unreferenced({ "src/t.ts": target, "src/u.ts": 'requireDesktopModule("./t");' })).toEqual( + [] + ); + expect( + unreferenced({ "src/t.ts": target, "src/u.ts": 'type A = typeof import("./t").a;' }) + ).toEqual(["src/t.ts#b"]); + // An import-type qualifier names the other module's export, not a same-named local one. + expect( + unreferenced({ + "src/t.ts": target, + "src/u.ts": + 'export interface a {}\nexport interface c {}\nexport type R = import("./t").a;', + }) + ).toEqual(["src/t.ts#b", "src/u.ts#a", "src/u.ts#R"]); + expect( + unreferenced({ "src/t.ts": "export default 1;", "src/u.ts": 'import d from "./t";\nd;' }) + ).toEqual([]); + // A side-effect import uses no export. + expect(unreferenced({ "src/t.ts": target, "src/u.ts": 'import "./t";' })).toEqual([ + "src/t.ts#a", + "src/t.ts#b", + ]); +}); + +test("reports every exported declaration form once", () => { + expect( + unreferenced({ + "src/a.ts": [ + "export function over(x: string): void;", + "export function over(x: unknown) {}", + "export const { d1, d2: d3 } = { d1: 1, d2: 2 };", + "export interface I {}", + "export type T = 1;", + "export enum E { A }", + "export class C {}", + "const local = 1;", + "export { local as aliased };", + 'export * as ns from "./b";', + ].join("\n"), + "src/b.ts": "export const used = 1;", + }) + ).toEqual([ + "src/a.ts#over", + "src/a.ts#d1", + "src/a.ts#d3", + "src/a.ts#I", + "src/a.ts#T", + "src/a.ts#E", + "src/a.ts#C", + "src/a.ts#aliased", + "src/a.ts#ns", + ]); +}); + +test("resolves the @/ alias, relative paths, ESM .js suffixes and index files", () => { + const known = new Set(["src/a/b.ts", "src/a/index.tsx", "src/c.js"]); + expect(resolveSpecifier("@/a/b", "src/x.ts", known)).toBe("src/a/b.ts"); + expect(resolveSpecifier("./b.js", "src/a/x.ts", known)).toBe("src/a/b.ts"); + expect(resolveSpecifier("../a", "src/z/x.ts", known)).toBe("src/a/index.tsx"); + expect(resolveSpecifier("./c.js", "src/x.ts", known)).toBe("src/c.js"); + expect(resolveSpecifier("./j", "src/x.ts", new Set(["src/j/index.jsx"]))).toBe("src/j/index.jsx"); + expect(resolveSpecifier("zod", "src/x.ts", known)).toBeUndefined(); + // NodeNext sources are imported by their emitted extensions. + const nodeNext = new Set(["src/m.mts", "src/c.cts"]); + expect(resolveSpecifier("./m.mjs", "src/x.ts", nodeNext)).toBe("src/m.mts"); + expect(resolveSpecifier("./c.cjs", "src/x.ts", nodeNext)).toBe("src/c.cts"); +}); + +test("exempts only the meta and default exports of workflow scripts", () => { + const workflow = "src/node/builtinSkills/deep-research/workflow.js"; + expect(isRuntimeContractExport(workflow, "meta")).toBe(true); + expect(isRuntimeContractExport(workflow, "default")).toBe(true); + expect(isRuntimeContractExport(workflow, "helper")).toBe(false); + expect(isRuntimeContractExport("src/node/services/x.js", "default")).toBe(false); +}); + +test("does not count shadowing bindings or a declaration's own body as uses", () => { + expect( + unreferenced({ + "src/a.ts": [ + "export const param = 1;", + "export const caught = 2;", + "export const blockLocal = 3;", + "export type Generic = 4;", + "export function recursive(n: number): number { return n && recursive(n - 1); }", + "export class Self { static make() { return new Self(); } }", + "function f(param: number) { return param; }", + "try {} catch (caught) { void caught; }", + "function g() { const blockLocal = 0; return blockLocal; }", + "function h(x: Generic) { return x; }", + "export const used = 5;", + "function k(other: number) { return other + used; }", + "export const hoisted = 6;", + "function m() { if (true) { var hoisted = 0; } return hoisted; }", + ].join("\n"), + }) + ).toEqual([ + "src/a.ts#param", + "src/a.ts#caught", + "src/a.ts#blockLocal", + "src/a.ts#Generic", + "src/a.ts#recursive", + "src/a.ts#Self", + "src/a.ts#hoisted", + ]); +}); + +test("reads CommonJS exports", () => { + expect( + unreferenced({ + "src/a.cjs": 'exports.one = 1;\nmodule.exports.two = 2;\nexports["five"] = 5;', + "src/b.cjs": "module.exports = { three: 3, four: 4 };", + "src/c.cjs": "module.exports = function () {};", + "src/d.ts": 'import { one, three } from "./a.cjs";\nimport { three as t } from "./b.cjs";', + }) + ).toEqual(["src/a.cjs#two", "src/a.cjs#five", "src/b.cjs#four", "src/c.cjs#default"]); + // The default import of `module.exports = { ... }` is the whole object. + expect( + unreferenced({ + "src/b.cjs": "module.exports = { three: 3, four: 4 };", + "src/d.ts": 'import b from "./b.cjs";\nb.three;', + }) + ).toEqual([]); +}); + +test("an explicit barrel export shadows the same name from export *", () => { + expect( + unreferenced({ + "src/a.ts": "export const value = 1;", + "src/index.ts": 'export * from "./a";\nexport const value = 2;', + "src/b.ts": 'import { value } from "./index";\nvalue;', + }) + ).toEqual(["src/a.ts#value"]); + // A namespace use of the barrel holds the barrel's own `value`, not a's. + expect( + unreferenced({ + "src/a.ts": "export const value = 1;\nexport const other = 3;", + "src/index.ts": 'export * from "./a";\nexport const value = 2;', + "src/b.ts": 'import * as lib from "./index";\nlib;', + }) + ).toEqual(["src/a.ts#value"]); + // A namespace use reaches names a star target forwards by name. + expect( + unreferenced({ + "src/source.ts": "export const value = 1;", + "src/middle.ts": 'export { value } from "./source";', + "src/outer.ts": 'export * from "./middle";', + "src/b.ts": 'import * as lib from "./outer";\nlib;', + }) + ).toEqual([]); +}); + +test("reports unlisted findings, stale and duplicate entries, and pendingDeletion growth", () => { + const finding = (symbol: string) => ({ file: "src/a.ts", symbol, line: 1 }); + const entry = (symbol: string) => ({ file: "src/a.ts", symbol, reason: "r" }); + const pending = (symbol: string) => ({ ...entry(symbol), owner: "o" }); + const result = checkExports( + [finding("allowed"), finding("fresh"), finding("pending"), finding("newPending")], + { + allowed: [entry("allowed"), entry("gone"), entry("allowed")], + pendingDeletion: [pending("pending"), pending("newPending")], + }, + ["src/a.ts#pending", "src/a.ts#deleted"] + ); + expect(result.unlisted.map((hit) => hit.symbol)).toEqual(["fresh"]); + expect(result.stale.map((hit) => hit.symbol)).toEqual(["gone"]); + expect(result.duplicates.map((hit) => hit.symbol)).toEqual(["allowed"]); + expect(result.unfrozen.map((hit) => hit.symbol)).toEqual(["newPending"]); + expect(result.thawed).toEqual(["src/a.ts#deleted"]); +}); diff --git a/scripts/check-test-only-exports.ts b/scripts/check-test-only-exports.ts new file mode 100644 index 00000000000..152739535d1 --- /dev/null +++ b/scripts/check-test-only-exports.ts @@ -0,0 +1,750 @@ +#!/usr/bin/env bun +/** + * Fails when an export of a production file under src/ has no non-test reference: no + * production file imports it (directly, through a barrel, or as a namespace) and its own file + * does not use it either. Such an export is either dead or kept alive only by tests, and both + * ship dead surface. `scripts/check-test-seam-comments.ts` only reads comments, so a feature + * removal that leaves an uncommented helper behind (the 2026-10 audit found ~8 of them, plus 21 + * exports nothing referenced) passes it; this check sees the references instead. + * + * Fix a finding by deleting the export (and the tests that only exercise it), by moving the + * test to the function production calls, or by dropping `export` when only its own file uses it. + * If the export must stay, list it under "allowed" in + * scripts/check-test-only-exports.allowlist.json with a reason: a ...ForTests reset hook, a + * test-support or story helper, or a consumer outside src/ (scripts/, vscode/, a build config). + * "pendingDeletion" lists findings that another workstream owns and is deleting. It only + * shrinks: its keys must equal FROZEN_PENDING_DELETION below. + * + * Only production files are parsed (tests never count as references), one at a time, so the + * check stays fast (about 3 s) and light (under 300 MB). + * + * Usage: bun scripts/check-test-only-exports.ts (or `make check-test-only-exports`) + */ +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import path from "node:path"; +import ts from "typescript"; +import { isProductionSource, SOURCE_GLOB, typescriptProblem } from "./check-test-seam-comments"; + +export const ALLOWLIST_PATH = "scripts/check-test-only-exports.allowlist.json"; + +/** + * The `pendingDeletion` keys (file#symbol). Another workstream owns each one; remove a key here + * and in the JSON file when its export is gone. Never add a key here. + */ +export const FROZEN_PENDING_DELETION: readonly string[] = [ + "src/browser/features/RightSidebar/ArtifactsTab/artifactDocument.ts#buildSandboxedHtmlDocument", + "src/browser/features/Settings/Sections/GoalsSection.tsx#GoalsSection", + "src/browser/hooks/useModelsFromSettings.ts#getSuggestedModels", + "src/browser/stores/DraftStore.ts#DraftAttachmentMatchesChatAttachment", + "src/browser/utils/highlighting/highlightDiffChunk.ts#highlightDiffChunk", + "src/browser/utils/messages/StreamingTPSCalculator.ts#calculateTPS", + "src/browser/utils/messages/StreamingTPSCalculator.ts#calculateTokenCount", + "src/browser/utils/messages/StreamingTPSCalculator.ts#DeltaRecord", + "src/browser/utils/rightSidebarLayout.ts#addToolToFocusedTabset", + "src/browser/utils/workflowRunMessages.ts#filterWorkflowDisplayOnlyMessages", + "src/browser/utils/workspaceStorage.ts#migrateWorkspaceStorage", +]; + +/** What one file imports from one module specifier. `"*"` means every export (namespace use). */ +export interface ImportFact { + specifier: string; + names: string[]; +} + +export interface ReExportFact { + specifier: string; + /** `export * from` forwards every name; otherwise `exported` forwards `imported`. */ + star: boolean; + names: Array<{ imported: string; exported: string }>; +} + +export interface ExportFact { + name: string; + line: number; +} + +export interface ModuleFacts { + /** Exports declared in this file (not re-exports). */ + exports: ExportFact[]; + /** Declared exports that the file itself uses outside the declaration. */ + usedLocally: Set; + imports: ImportFact[]; + reExports: ReExportFact[]; + /** `module.exports = { a, b }`: a default import is the whole object, so it uses a and b. */ + defaultIsWholeModule: boolean; +} + +function scriptKind(file: string): ts.ScriptKind { + if (file.endsWith(".tsx")) return ts.ScriptKind.TSX; + if (file.endsWith(".jsx")) return ts.ScriptKind.JSX; + return /\.[cm]?js$/.test(file) ? ts.ScriptKind.JS : ts.ScriptKind.TS; +} + +function hasExportModifier(node: ts.Node): boolean { + return ( + ts.canHaveModifiers(node) && + (ts.getModifiers(node) ?? []).some((m) => m.kind === ts.SyntaxKind.ExportKeyword) + ); +} + +function hasDefaultModifier(node: ts.Node): boolean { + return ( + ts.canHaveModifiers(node) && + (ts.getModifiers(node) ?? []).some((m) => m.kind === ts.SyntaxKind.DefaultKeyword) + ); +} + +function bindingNames(name: ts.BindingName): ts.Identifier[] { + if (ts.isIdentifier(name)) return [name]; + return name.elements.flatMap((element) => + ts.isOmittedExpression(element) ? [] : bindingNames(element.name) + ); +} + +/** Names a block-like statement list declares for its own scope. */ +function statementBindings(statements: readonly ts.Statement[]): string[] { + return statements.flatMap((statement) => { + if (ts.isVariableStatement(statement)) { + return statement.declarationList.declarations.flatMap((declaration) => + bindingNames(declaration.name).map((id) => id.text) + ); + } + const name = (statement as { name?: ts.Node }).name; + return name !== undefined && ts.isIdentifier(name) ? [name.text] : []; + }); +} + +/** + * `var` names declared anywhere in a function body outside nested functions. JavaScript hoists + * them to the whole function, so they shadow a module export of the same name there. + */ +function hoistedVarNames(body: ts.Node): string[] { + const names: string[] = []; + const walk = (node: ts.Node) => { + if (ts.isFunctionLike(node)) return; + if ( + ts.isVariableDeclarationList(node) && + (node.flags & ts.NodeFlags.BlockScoped) === ts.NodeFlags.None + ) { + for (const declaration of node.declarations) { + names.push(...bindingNames(declaration.name).map((id) => id.text)); + } + } + ts.forEachChild(node, walk); + }; + ts.forEachChild(body, walk); + return names; +} + +/** + * Names `node` binds for its own subtree (parameters, type parameters, catch variables, block + * declarations, a named function or class expression's own name). Inside that subtree they + * shadow a module-level export of the same name, so they are not uses of it. + */ +function scopeBindings(node: ts.Node): string[] { + if (ts.isSourceFile(node)) return []; + const names: string[] = []; + if (ts.isFunctionLike(node)) { + for (const parameter of node.parameters) { + names.push(...bindingNames(parameter.name).map((id) => id.text)); + } + if (ts.isFunctionExpression(node) && node.name) names.push(node.name.text); + const body = (node as { body?: ts.Node }).body; + if (body !== undefined) names.push(...hoistedVarNames(body)); + } + if (ts.isClassExpression(node) && node.name) names.push(node.name.text); + const typeParameters = (node as { typeParameters?: ts.NodeArray }) + .typeParameters; + for (const typeParameter of typeParameters ?? []) names.push(typeParameter.name.text); + if (ts.isCatchClause(node) && node.variableDeclaration) { + names.push(...bindingNames(node.variableDeclaration.name).map((id) => id.text)); + } + if (ts.isBlock(node) || ts.isModuleBlock(node)) names.push(...statementBindings(node.statements)); + if (ts.isCaseBlock(node)) { + names.push(...statementBindings(node.clauses.flatMap((clause) => [...clause.statements]))); + } + if ( + (ts.isForStatement(node) || ts.isForInStatement(node) || ts.isForOfStatement(node)) && + node.initializer && + ts.isVariableDeclarationList(node.initializer) + ) { + for (const declaration of node.initializer.declarations) { + names.push(...bindingNames(declaration.name).map((id) => id.text)); + } + } + return names; +} + +/** + * The module-level names a top-level declaration declares. Uses inside the declaration itself + * (a recursive call, a class naming itself) are not uses from elsewhere in the file. + */ +function ownDeclarationNames(node: ts.Node, sourceFile: ts.SourceFile): string[] { + if ( + ts.isVariableDeclaration(node) && + ts.isVariableDeclarationList(node.parent) && + node.parent.parent.parent === sourceFile + ) { + return bindingNames(node.name).map((id) => id.text); + } + if (node.parent !== sourceFile || ts.isVariableStatement(node)) return []; + const name = (node as { name?: ts.Node }).name; + return name !== undefined && ts.isIdentifier(name) ? [name.text] : []; +} + +/** An identifier that names a property or member, not a binding in scope. */ +function isPropertyName(node: ts.Identifier): boolean { + const parent = node.parent; + if (ts.isPropertyAccessExpression(parent) || ts.isJsxAttribute(parent)) { + return parent.name === node; + } + if (ts.isQualifiedName(parent)) return parent.right === node; + // `{ a: x } = y`: `a` is the object's property. + if (ts.isBindingElement(parent)) return parent.propertyName === node; + if (ts.isImportSpecifier(parent) || ts.isExportSpecifier(parent)) return true; + if ( + ts.isPropertyAssignment(parent) || + ts.isPropertyDeclaration(parent) || + ts.isPropertySignature(parent) || + ts.isMethodDeclaration(parent) || + ts.isMethodSignature(parent) || + ts.isGetAccessorDeclaration(parent) || + ts.isSetAccessorDeclaration(parent) || + ts.isEnumMember(parent) + ) { + return parent.name === node; + } + return false; +} + +/** `exports.a` / `module.exports.a` / `exports["a"]`: the exported name, else undefined. */ +function commonJsMemberName(target: ts.Expression): string | undefined { + const isExportsObject = (object: ts.Expression) => + (ts.isIdentifier(object) && object.text === "exports") || isModuleExports(object); + if (ts.isPropertyAccessExpression(target) && isExportsObject(target.expression)) { + return target.name.text; + } + if ( + ts.isElementAccessExpression(target) && + isExportsObject(target.expression) && + ts.isStringLiteralLike(target.argumentExpression) + ) { + return target.argumentExpression.text; + } + return undefined; +} + +const isModuleExports = (node: ts.Expression) => + ts.isPropertyAccessExpression(node) && + ts.isIdentifier(node.expression) && + node.expression.text === "module" && + node.name.text === "exports"; + +/** + * CommonJS exports a statement declares: `exports.a = ...` / `module.exports.a = ...` / + * `exports["a"] = ...` declare `a`; `module.exports = { a, b }` declares `a` and `b` (and its + * default import is the whole object, see `ModuleFacts.defaultIsWholeModule`); any other `module.exports = x` declares + * `default`. Undefined for statements that are not such an assignment. + */ +function commonJsExportNames(statement: ts.Statement): string[] | undefined { + if (!ts.isExpressionStatement(statement)) return undefined; + const expression = statement.expression; + if (!ts.isBinaryExpression(expression)) return undefined; + if (expression.operatorToken.kind !== ts.SyntaxKind.EqualsToken) return undefined; + const target = expression.left; + if (isModuleExports(target)) { + const value = expression.right; + if (!ts.isObjectLiteralExpression(value)) return ["default"]; + return value.properties.flatMap((property) => { + const name = property.name; + return name !== undefined && (ts.isIdentifier(name) || ts.isStringLiteral(name)) + ? [name.text] + : []; + }); + } + const member = commonJsMemberName(target); + return member === undefined ? undefined : [member]; +} + +/** `module.exports = { ... }`. */ +function assignsModuleExportsObject(statement: ts.Statement): boolean { + return ( + ts.isExpressionStatement(statement) && + ts.isBinaryExpression(statement.expression) && + statement.expression.operatorToken.kind === ts.SyntaxKind.EqualsToken && + isModuleExports(statement.expression.left) && + ts.isObjectLiteralExpression(statement.expression.right) + ); +} + +function stringArgument(node: ts.CallExpression): string | undefined { + const [first] = node.arguments; + return first !== undefined && ts.isStringLiteralLike(first) ? first.text : undefined; +} + +/** Parses one file into the facts the reachability check needs. */ +export function collectModuleFacts(file: string, text: string): ModuleFacts { + const sourceFile = ts.createSourceFile( + file, + text, + ts.ScriptTarget.Latest, + true, + scriptKind(file) + ); + const exports: ExportFact[] = []; + const imports: ImportFact[] = []; + const reExports: ReExportFact[] = []; + let defaultIsWholeModule = false; + /** Local name -> exported names, and the identifier nodes that declare it. */ + const exportedLocals = new Map(); + const lineOf = (node: ts.Node) => + sourceFile.getLineAndCharacterOfPosition(node.getStart(sourceFile)).line + 1; + const addExport = (exported: string, local: string | undefined, at: ts.Node) => { + // Overloads and declaration merging repeat a name; the first declaration reports it. + if (!exports.some((fact) => fact.name === exported)) { + exports.push({ name: exported, line: lineOf(at) }); + } + if (local !== undefined) + exportedLocals.set(local, [...(exportedLocals.get(local) ?? []), exported]); + }; + + for (const statement of sourceFile.statements) { + if (ts.isImportDeclaration(statement) && ts.isStringLiteral(statement.moduleSpecifier)) { + const clause = statement.importClause; + const names: string[] = []; + if (clause?.name) names.push("default"); + const bindings = clause?.namedBindings; + if (bindings && ts.isNamespaceImport(bindings)) names.push("*"); + if (bindings && ts.isNamedImports(bindings)) { + for (const element of bindings.elements) { + names.push((element.propertyName ?? element.name).text); + } + } + // A side-effect import (`import "./x"`) uses no export. + imports.push({ specifier: statement.moduleSpecifier.text, names }); + continue; + } + if (ts.isExportDeclaration(statement)) { + const specifier = + statement.moduleSpecifier && ts.isStringLiteral(statement.moduleSpecifier) + ? statement.moduleSpecifier.text + : undefined; + const clause = statement.exportClause; + if (specifier !== undefined) { + if (clause === undefined) { + reExports.push({ specifier, star: true, names: [] }); + } else if (ts.isNamespaceExport(clause)) { + // `export * as ns from "./a"` declares `ns` here and uses all of a. + addExport(clause.name.text, undefined, clause.name); + imports.push({ specifier, names: ["*"] }); + } else { + reExports.push({ + specifier, + star: false, + names: clause.elements.map((element) => ({ + imported: (element.propertyName ?? element.name).text, + exported: element.name.text, + })), + }); + } + } else if (clause && ts.isNamedExports(clause)) { + for (const element of clause.elements) { + addExport(element.name.text, (element.propertyName ?? element.name).text, element.name); + } + } + continue; + } + if (ts.isExportAssignment(statement)) { + // `export default expr` / `export = expr`: the expression is used by the export itself. + addExport("default", undefined, statement); + continue; + } + const commonJs = commonJsExportNames(statement); + if (assignsModuleExportsObject(statement)) defaultIsWholeModule = true; + if (commonJs !== undefined) { + for (const name of commonJs) addExport(name, undefined, statement); + continue; + } + if (!hasExportModifier(statement)) continue; + if (ts.isVariableStatement(statement)) { + for (const declaration of statement.declarationList.declarations) { + for (const id of bindingNames(declaration.name)) { + addExport(id.text, id.text, id); + } + } + continue; + } + const named = statement as ts.Node & { name?: ts.Node }; + if (hasDefaultModifier(statement)) { + addExport( + "default", + named.name && ts.isIdentifier(named.name) ? named.name.text : undefined, + statement + ); + continue; + } + if (named.name && (ts.isIdentifier(named.name) || ts.isStringLiteral(named.name))) { + addExport(named.name.text, named.name.text, named.name); + } + } + + const usedLocally = new Set(); + const visit = (node: ts.Node, hidden: ReadonlySet) => { + const hiding = [...scopeBindings(node), ...ownDeclarationNames(node, sourceFile)].filter( + (name) => exportedLocals.has(name) && !hidden.has(name) + ); + const inner = hiding.length > 0 ? new Set([...hidden, ...hiding]) : hidden; + if (ts.isIdentifier(node)) { + const exported = inner.has(node.text) ? undefined : exportedLocals.get(node.text); + if (exported && !isPropertyName(node)) { + for (const name of exported) usedLocally.add(name); + } + } else if (ts.isCallExpression(node)) { + // `import("./x")`, `require("./x")` and require wrappers such as main.ts's + // `requireDesktopModule("./x")` use the whole module. + const isDynamicImport = node.expression.kind === ts.SyntaxKind.ImportKeyword; + const isRequire = + ts.isIdentifier(node.expression) && /^require(?:[A-Z]\w*)?$/.test(node.expression.text); + const specifier = isDynamicImport || isRequire ? stringArgument(node) : undefined; + if (specifier !== undefined) imports.push({ specifier, names: ["*"] }); + } else if (ts.isImportTypeNode(node)) { + // `typeof import("./x")` / `import("./x").Foo`. + const argument = node.argument; + if (ts.isLiteralTypeNode(argument) && ts.isStringLiteral(argument.literal)) { + const qualifier = node.qualifier; + const first = + qualifier === undefined + ? undefined + : ts.isIdentifier(qualifier) + ? qualifier + : leftmost(qualifier); + imports.push({ specifier: argument.literal.text, names: [first?.text ?? "*"] }); + } + // The qualifier names the other module's exports, not this file's. + for (const typeArgument of node.typeArguments ?? []) visit(typeArgument, inner); + return; + } + ts.forEachChild(node, (child) => visit(child, inner)); + }; + visit(sourceFile, new Set()); + return { exports, usedLocally, imports, reExports, defaultIsWholeModule }; +} + +function leftmost(name: ts.QualifiedName): ts.Identifier { + return ts.isIdentifier(name.left) ? name.left : leftmost(name.left); +} + +/** Suffixes an extensionless specifier can stand for. */ +const RESOLVE_SUFFIXES = [ + "", + ".ts", + ".tsx", + ".d.ts", + ".mts", + ".cts", + ".js", + ".jsx", + ".mjs", + ".cjs", + "/index.ts", + "/index.tsx", + "/index.mts", + "/index.cts", + "/index.d.ts", + "/index.js", + "/index.jsx", + "/index.mjs", + "/index.cjs", +]; + +/** Sources an emitted-extension specifier (`./a.js`, NodeNext `./a.mjs`) can stand for. */ +const EMITTED_SOURCES: Readonly> = { + ".js": [".ts", ".tsx", ".d.ts"], + ".jsx": [".tsx"], + ".mjs": [".mts", ".d.mts"], + ".cjs": [".cts", ".d.cts"], +}; + +/** Resolves a specifier to a known repo-relative file, or undefined for packages and assets. */ +export function resolveSpecifier( + specifier: string, + fromFile: string, + knownFiles: ReadonlySet +): string | undefined { + let base: string; + const alias = /^(?:@|@shared)\//.exec(specifier); + if (alias) base = `src/${specifier.slice(alias[0].length)}`; + else if (specifier.startsWith(".")) { + base = path.posix.join(path.posix.dirname(fromFile), specifier); + } else return undefined; + for (const suffix of RESOLVE_SUFFIXES) { + if (knownFiles.has(base + suffix)) return base + suffix; + } + const extension = /\.[cm]?jsx?$/.exec(base)?.[0]; + if (extension === undefined) return undefined; + for (const source of EMITTED_SOURCES[extension]) { + const candidate = base.slice(0, -extension.length) + source; + if (knownFiles.has(candidate)) return candidate; + } + return undefined; +} + +/** + * Built-in workflow scripts (the .js files under src/node/builtinSkills/) are run by the workflow + * runtime, which reads their `meta` and `default` exports by contract; no module imports them. + * Their other exports are checked like any other. + */ +export function isRuntimeContractExport(file: string, symbol: string): boolean { + return ( + file.startsWith("src/node/builtinSkills/") && + file.endsWith(".js") && + (symbol === "meta" || symbol === "default") + ); +} + +function explicitlyExports(module: ModuleFacts, name: string): boolean { + return ( + module.exports.some((fact) => fact.name === name) || + module.reExports.some((reExport) => reExport.names.some((n) => n.exported === name)) + ); +} + +export interface UnreferencedExport { + file: string; + symbol: string; + line: number; +} + +/** + * Marks every export some referrer reaches and returns the declared exports of `checkedFiles` + * that nothing reaches. `facts` must hold every referrer and every barrel a referrer imports. + */ +export function findUnreferencedExports( + facts: ReadonlyMap, + referrers: readonly string[], + checkedFiles: readonly string[] +): UnreferencedExport[] { + const knownFiles = new Set(facts.keys()); + const used = new Set(); + const visited = new Set(); + /** + * The names `file`'s namespace holds: its own exports (named re-exports included) plus, through + * each `export *`, the target's names except `default` and the names `file` shadows. + */ + const namespaceNames = (file: string, seen: Set = new Set()): string[] => { + const module = facts.get(file); + if (module === undefined || seen.has(file)) return []; + seen.add(file); + const names = module.exports.map((fact) => fact.name); + for (const reExport of module.reExports) { + // `export { a } from "./b"` puts `a` in this namespace too. + names.push(...reExport.names.map((forwarded) => forwarded.exported)); + const target = reExport.star + ? resolveSpecifier(reExport.specifier, file, knownFiles) + : undefined; + if (target === undefined) continue; + for (const name of namespaceNames(target, seen)) { + if (name !== "default" && !explicitlyExports(module, name)) names.push(name); + } + } + return names; + }; + const markUse = (file: string, name: string) => { + const key = `${file}#${name}`; + if (visited.has(key)) return; + visited.add(key); + const module = facts.get(file); + if (module === undefined) return; + // The default import of `module.exports = { a, b }` is the whole object. + if (name === "default" && module.defaultIsWholeModule) { + markUse(file, "*"); + return; + } + if (name === "*") { + for (const fact of module.exports) used.add(`${file}#${fact.name}`); + } else { + used.add(key); + } + for (const reExport of module.reExports) { + const target = resolveSpecifier(reExport.specifier, file, knownFiles); + if (target === undefined) continue; + if (reExport.star) { + // `export * from` never forwards `default`, nor a name the barrel exports explicitly, so + // a namespace use of the barrel reaches only the target's other names. + const names = name === "*" ? namespaceNames(target) : [name]; + for (const forwarded of names) { + if (forwarded !== "default" && !explicitlyExports(module, forwarded)) { + markUse(target, forwarded); + } + } + continue; + } + for (const forwarded of reExport.names) { + if (name === "*" || forwarded.exported === name) markUse(target, forwarded.imported); + } + } + }; + for (const file of referrers) { + const module = facts.get(file); + assert(module !== undefined, `no facts for referrer ${file}`); + for (const name of module.usedLocally) used.add(`${file}#${name}`); + for (const fact of module.imports) { + const target = resolveSpecifier(fact.specifier, file, knownFiles); + if (target === undefined) continue; + for (const name of fact.names) markUse(target, name); + } + } + return checkedFiles.flatMap((file) => + (facts.get(file)?.exports ?? []) + .filter((fact) => !used.has(`${file}#${fact.name}`)) + .map((fact) => ({ file, symbol: fact.name, line: fact.line })) + ); +} + +export interface AllowlistEntry { + file: string; + symbol: string; + reason: string; +} + +export interface PendingEntry extends AllowlistEntry { + /** The workstream or person deleting it. */ + owner: string; +} + +export interface Allowlist { + allowed: AllowlistEntry[]; + pendingDeletion: PendingEntry[]; +} + +function parseEntries(json: unknown, list: string, extra: string[]): T[] { + assert(Array.isArray(json), `${ALLOWLIST_PATH}: "${list}" must be an array`); + return json.map((entry: unknown, index) => { + const record = (entry ?? {}) as Record; + const where = `${ALLOWLIST_PATH}: ${list}[${index}]`; + for (const field of ["file", "symbol", "reason", ...extra]) { + const value = record[field]; + assert(typeof value === "string" && value.trim().length > 0, `${where}.${field} is required`); + } + return record as unknown as T; + }); +} + +export function parseAllowlist(json: unknown): Allowlist { + assert(typeof json === "object" && json !== null, `${ALLOWLIST_PATH} must be a JSON object`); + const { allowed, pendingDeletion } = json as Record; + return { + allowed: parseEntries(allowed, "allowed", []), + pendingDeletion: parseEntries(pendingDeletion, "pendingDeletion", ["owner"]), + }; +} + +const entryKey = (entry: { file: string; symbol: string }) => `${entry.file}#${entry.symbol}`; + +export interface CheckResult { + unlisted: UnreferencedExport[]; + /** Entries whose export now has a production reference or no longer exists. */ + stale: AllowlistEntry[]; + duplicates: AllowlistEntry[]; + /** `pendingDeletion` keys missing from the frozen list (the list only shrinks). */ + unfrozen: PendingEntry[]; + /** Frozen keys no longer pending; remove them from FROZEN_PENDING_DELETION too. */ + thawed: string[]; +} + +export function checkExports( + findings: readonly UnreferencedExport[], + allowlist: Allowlist, + frozenPending: readonly string[] = FROZEN_PENDING_DELETION +): CheckResult { + const listed = new Map(); + const duplicates: AllowlistEntry[] = []; + for (const entry of [...allowlist.allowed, ...allowlist.pendingDeletion]) { + if (listed.has(entryKey(entry))) duplicates.push(entry); + listed.set(entryKey(entry), entry); + } + const found = new Set(findings.map(entryKey)); + const frozen = new Set(frozenPending); + const pendingKeys = new Set(allowlist.pendingDeletion.map(entryKey)); + return { + unlisted: findings.filter((finding) => !listed.has(entryKey(finding))), + stale: [...listed.values()].filter((entry) => !found.has(entryKey(entry))), + duplicates, + unfrozen: allowlist.pendingDeletion.filter((entry) => !frozen.has(entryKey(entry))), + thawed: [...frozen].filter((key) => !pendingKeys.has(key)), + }; +} + +function main(): number { + const tag = "check-test-only-exports:"; + const problem = typescriptProblem(ts); + if (problem !== undefined) { + console.error(`${tag} ${problem}`); + return 2; + } + const root = path.resolve(import.meta.dir, ".."); + const files = [...new Bun.Glob(SOURCE_GLOB).scanSync({ cwd: root })] + .map((file) => file.split(path.sep).join("/")) + .filter(isProductionSource) + .sort(); + assert(files.length > 100, `expected production sources under src/, found ${files.length}`); + const facts = new Map(); + for (const file of files) { + facts.set(file, collectModuleFacts(file, readFileSync(path.join(root, file), "utf8"))); + } + const findings = findUnreferencedExports(facts, files, files).filter( + (finding) => !isRuntimeContractExport(finding.file, finding.symbol) + ); + const allowlist = parseAllowlist( + JSON.parse(readFileSync(path.join(root, ALLOWLIST_PATH), "utf8")) as unknown + ); + const { unlisted, stale, duplicates, unfrozen, thawed } = checkExports(findings, allowlist); + + for (const finding of unlisted) { + console.error( + `${tag} ${finding.file}:${finding.line} export \`${finding.symbol}\` has no non-test reference` + ); + } + if (unlisted.length > 0) { + console.error( + [ + "", + "Nothing in production imports these exports, and their own files do not use them.", + "Delete the export (and tests that only exercise it), move the test to the function", + "production calls, or drop `export` if only its own file uses it. If it must stay (a", + "...ForTests reset hook, a test-support or story helper, a consumer outside src/), add it", + `to "allowed" in ${ALLOWLIST_PATH} with the reason:`, + ...unlisted.map( + (finding) => + ` { "file": "${finding.file}", "symbol": "${finding.symbol}", "reason": "" }` + ), + ].join("\n") + ); + } + for (const entry of stale) { + console.error( + `${tag} stale allowlist entry ${entryKey(entry)}: the export is gone or has a production reference now; remove it from ${ALLOWLIST_PATH}` + ); + } + for (const entry of duplicates) { + console.error(`${tag} duplicate allowlist entry ${entryKey(entry)} in ${ALLOWLIST_PATH}`); + } + for (const entry of unfrozen) { + console.error( + `${tag} new pendingDeletion entry ${entryKey(entry)}: pendingDeletion only shrinks; delete the export or list it under "allowed" with a reason` + ); + } + for (const key of thawed) { + console.error( + `${tag} ${key} left pendingDeletion; remove it from FROZEN_PENDING_DELETION in scripts/check-test-only-exports.ts too` + ); + } + const failures = + unlisted.length + stale.length + duplicates.length + unfrozen.length + thawed.length; + return failures > 0 ? 1 : 0; +} + +if (import.meta.main) { + process.exit(main()); +} diff --git a/scripts/check-test-routing.sh b/scripts/check-test-routing.sh index 3e901c4e612..67d8da4d717 100755 --- a/scripts/check-test-routing.sh +++ b/scripts/check-test-routing.sh @@ -24,6 +24,7 @@ cd "$(dirname "${BASH_SOURCE[0]}")/.." other_routes=( $'scripts/check-startup-imports.test.ts\tmake check-startup-imports (static-check), right before its analyzer' $'scripts/check-test-seam-comments.test.ts\tmake check-test-seam-comments (static-check), right before its checker' + $'scripts/check-test-only-exports.test.ts\tmake check-test-only-exports (static-check), right before its checker' $'scripts/crap/check.test.ts\tmake test-crap-scripts (static-check-full): needs uv, which Test / Unit does not install' $'scripts/crap/agentGate.test.ts\tmake test-crap-scripts (static-check-full): needs uv, which Test / Unit does not install' $'scripts/dry/check.test.ts\tmake test-dry-scripts (static-check-full): needs uv, which Test / Unit does not install' diff --git a/scripts/check-test-seam-comments.test.ts b/scripts/check-test-seam-comments.test.ts index 4e66424913e..89eaf608af6 100644 --- a/scripts/check-test-seam-comments.test.ts +++ b/scripts/check-test-seam-comments.test.ts @@ -239,6 +239,7 @@ test("separates production sources from tests, support code, stories and generat "src/node/runtime/testRemoteRuntime.ts", "src/node/utils/concurrency/fileLockTestHelpers.ts", "src/browser/features/Tools/GoogleSearchToolCall.fixtures.ts", + "src/browser/features/Settings/Sections/settingsStoryUtils.tsx", "src/node/services/__tests__/foo.ts", "src/common/utils/testing/fuzzHelpers.ts", "src/browser/stories/mocks/orpc.ts", diff --git a/scripts/check-test-seam-comments.ts b/scripts/check-test-seam-comments.ts index 6b1a5f52996..9dc5f52a318 100644 --- a/scripts/check-test-seam-comments.ts +++ b/scripts/check-test-seam-comments.ts @@ -118,6 +118,8 @@ export function isProductionSource(relPath: string): boolean { // refinementTestHelpers.ts, fileLockTestHelpers.ts, workspaceStoreTestOverlay.ts, // GoogleSearchToolCall.fixtures.ts, DesktopBridgeServer.nodeFixture.ts. if (/Test(?:Helpers?|Harness|Utils|Fixtures?|Overlay|Child)\b/.test(base)) return false; + // settingsStoryUtils.tsx: helpers only stories import. + if (/Story(?:Utils|Helpers?)\b/.test(base)) return false; if (/\.(?:\w*[fF]ixtures?)\.[cm]?[jt]sx?$/.test(base)) return false; // Generated at build time (see Makefile's version target). if (relPath === "src/version.ts") return false; diff --git a/scripts/test-unit-ci.sh b/scripts/test-unit-ci.sh index 04d506ab480..ca69d46d45b 100755 --- a/scripts/test-unit-ci.sh +++ b/scripts/test-unit-ci.sh @@ -258,9 +258,9 @@ done # Bun-run test trees outside src/ (Jest ignores them, see jest.config.js): Storybook # policy tests, the VS Code extension's pure helpers (incl. the webview oRPC allowlist -# guard) and tooling tests under scripts/. check-startup-imports.test.ts and -# check-test-seam-comments.test.ts are excluded because their static-check make -# targets run them right before the guard itself; crap/check.test.ts and +# guard) and tooling tests under scripts/. check-startup-imports.test.ts, +# check-test-seam-comments.test.ts and check-test-only-exports.test.ts are excluded +# because their static-check make targets run them right before the guard itself; crap/check.test.ts and # crap/agentGate.test.ts need uv (make test-crap-scripts runs them), as do # dry/check.test.ts and mutate/check.test.ts (make test-dry-scripts and # test-mutate-scripts); orpcConnection.integration.test.ts @@ -281,6 +281,7 @@ done < <( find "${tooling_roots[@]}" -type f \( -name '*.test.ts' -o -name '*.test.tsx' \) \ ! -path ./scripts/check-startup-imports.test.ts \ ! -path ./scripts/check-test-seam-comments.test.ts \ + ! -path ./scripts/check-test-only-exports.test.ts \ ! -path ./scripts/crap/check.test.ts \ ! -path ./scripts/crap/agentGate.test.ts \ ! -path ./scripts/dry/check.test.ts \