| author | |
| committer | |
| log | 5fe5c6fe9bb72c6e8ce9c1724a3887381d3f5cf7 |
| tree | adeb24e8f78f180c5d96f02733d20dd2ec8b5710 |
| parent | 2ae3b712b4b7dc7a3f44e9ceff404dfe1572ab93 |
| signature | Signed by PGP key B5690EEEBB952194 |
7 files changed, 264 insertions(+), 5 deletions(-)
docs/operations/development.md+4-2| ... | @@ -72,8 +72,10 @@ Windows investigation while that suite is not a required gate. | ... | @@ -72,8 +72,10 @@ Windows investigation while that suite is not a required gate. |
| 72 | ### Unused code | 72 | ### Unused code |
| 73 | 73 | ||
| 74 | `vp run knip:check` checks unused files and dependencies across the repo, then | 74 | `vp run knip:check` checks unused files and dependencies across the repo, then |
| 75 | unused exports and types in `packages/tailscale` and `packages/effect-codex-app-server`. | 75 | unused runtime exports in every internal package under `packages/`. CI enforces both checks. |
| 76 | CI enforces both checks. | 76 | Exported types and Effect schemas are allowed without consumers. The schema preprocessor |
| 77 | recognizes schema types, including aliases and schema classes; functions that create or decode | ||
| 78 | schemas remain checked. Completely unused files remain checked too. | ||
| 77 | Use `vp run knip --workspace apps/web` to audit one workspace, including exports, | 79 | Use `vp run knip --workspace apps/web` to audit one workspace, including exports, |
| 78 | or `vp run knip:production --workspace apps/web` to find code kept alive only by tests. | 80 | or `vp run knip:production --workspace apps/web` to find code kept alive only by tests. |
| 79 | The full export audit still has findings and is not a repo-wide CI gate. Extend the | 81 | The full export audit still has findings and is not a repo-wide CI gate. Extend the |
knip.jsonc+6| ... | @@ -1,5 +1,7 @@ | ... | @@ -1,5 +1,7 @@ |
| 1 | { | 1 | { |
| 2 | "$schema": "https://unpkg.com/knip@6/schema.json", | 2 | "$schema": "https://unpkg.com/knip@6/schema.json", |
| 3 | // Exported types are part of our contracts even before they have consumers. | ||
| 4 | "rules": { "types": "off", "nsTypes": "off" }, | ||
| 3 | // These executables are supplied by the OS or installed separately from npm. | 5 | // These executables are supplied by the OS or installed separately from npm. |
| 4 | "ignoreBinaries": ["eas", "file", "mkfifo", "pkill", "pkg-config", "plutil", "sips"], | 6 | "ignoreBinaries": ["eas", "file", "mkfifo", "pkill", "pkg-config", "plutil", "sips"], |
| 5 | "workspaces": { | 7 | "workspaces": { |
| ... | @@ -10,6 +12,10 @@ | ... | @@ -10,6 +12,10 @@ |
| 10 | "vite": { "entry": [".github/**/*.test.cjs"] }, | 12 | "vite": { "entry": [".github/**/*.test.cjs"] }, |
| 11 | "vitest": { "entry": [".github/**/*.test.cjs"] }, | 13 | "vitest": { "entry": [".github/**/*.test.cjs"] }, |
| 12 | }, | 14 | }, |
| 15 | "scripts": { | ||
| 16 | // Knip loads its preprocessor through a CLI option, not a source import. | ||
| 17 | "entry": ["knip-schemas.ts"], | ||
| 18 | }, | ||
| 13 | "apps/server": { | 19 | "apps/server": { |
| 14 | // Vite+ pack entries and the launcher used by installed background services. | 20 | // Vite+ pack entries and the launcher used by installed background services. |
| 15 | "entry": [ | 21 | "entry": [ |
package.json+3-3| ... | @@ -25,9 +25,9 @@ | ... | @@ -25,9 +25,9 @@ |
| 25 | "typecheck": "vp run -r --concurrency-limit 2 typecheck", | 25 | "typecheck": "vp run -r --concurrency-limit 2 typecheck", |
| 26 | "tc": "vp run -r --concurrency-limit 2 typecheck", | 26 | "tc": "vp run -r --concurrency-limit 2 typecheck", |
| 27 | "lint": "vp lint --report-unused-disable-directives", | 27 | "lint": "vp lint --report-unused-disable-directives", |
| 28 | "knip": "knip", | 28 | "knip": "knip --preprocessor ./scripts/knip-schemas.ts", |
| 29 | "knip:check": "knip --include files,dependencies --no-config-hints && knip --workspace packages/tailscale --workspace packages/effect-codex-app-server --workspace packages/ssh --exports --no-config-hints", | 29 | "knip:check": "knip --include files,dependencies --no-config-hints && knip --workspace packages/client-runtime --workspace packages/contracts --workspace packages/effect-acp --workspace packages/effect-codex-app-server --workspace packages/shared --workspace packages/ssh --workspace packages/tailscale --exports --preprocessor ./scripts/knip-schemas.ts --no-config-hints", |
| 30 | "knip:production": "knip --production", | 30 | "knip:production": "knip --production --preprocessor ./scripts/knip-schemas.ts", |
| 31 | "lint:mobile": "node scripts/mobile-native-static-check.ts", | 31 | "lint:mobile": "node scripts/mobile-native-static-check.ts", |
| 32 | "test": "vp run -r test", | 32 | "test": "vp run -r test", |
| 33 | "test:resource-monitor": "cargo test --locked --manifest-path native/resource-monitor/Cargo.toml", | 33 | "test:resource-monitor": "cargo test --locked --manifest-path native/resource-monitor/Cargo.toml", |
pnpm-lock.yaml+3| ... | @@ -991,6 +991,9 @@ importers: | ... | @@ -991,6 +991,9 @@ importers: |
| 991 | '@types/pngjs': | 991 | '@types/pngjs': |
| 992 | specifier: 6.0.5 | 992 | specifier: 6.0.5 |
| 993 | version: 6.0.5 | 993 | version: 6.0.5 |
| 994 | typescript: | ||
| 995 | specifier: 'catalog:' | ||
| 996 | version: 6.0.3 | ||
| 994 | vite-plus: | 997 | vite-plus: |
| 995 | specifier: 'catalog:' | 998 | specifier: 'catalog:' |
| 996 | version: 0.3.0(@types/node@24.12.4)(bufferutil@4.1.0)(esbuild@0.28.1)(jiti@2.7.0)(msw@2.12.11(@types/node@24.12.4)(typescript@6.0.3))(terser@5.48.0)(typescript@6.0.3)(unrun@0.2.39)(utf-8-validate@6.0.6)(yaml@2.9.0) | 999 | version: 0.3.0(@types/node@24.12.4)(bufferutil@4.1.0)(esbuild@0.28.1)(jiti@2.7.0)(msw@2.12.11(@types/node@24.12.4)(typescript@6.0.3))(terser@5.48.0)(typescript@6.0.3)(unrun@0.2.39)(utf-8-validate@6.0.6)(yaml@2.9.0) |
scripts/knip-schemas.test.ts created+151| ... | @@ -0,0 +1,151 @@ | ||
| 1 | // @effect-diagnostics nodeBuiltinImport:off - Runs the real Knip CLI against a disposable on-disk project. | ||
| 2 | import * as NodeChildProcess from "node:child_process"; | ||
| 3 | import * as NodeFS from "node:fs"; | ||
| 4 | import * as NodeModule from "node:module"; | ||
| 5 | import * as NodePath from "node:path"; | ||
| 6 | import * as NodeProcess from "node:process"; | ||
| 7 | import { expect, it } from "vite-plus/test"; | ||
| 8 | |||
| 9 | const require = NodeModule.createRequire(import.meta.url); | ||
| 10 | const cli = NodePath.join(NodePath.dirname(require.resolve("knip")), "cli.js"); | ||
| 11 | const preprocessor = NodePath.join(import.meta.dirname, "knip-schemas.ts"); | ||
| 12 | |||
| 13 | it("allows types and schemas through the real Knip CLI without hiding runtime or file findings", () => { | ||
| 14 | // Keeping the disposable project here gives it the same Effect installation as the scripts. | ||
| 15 | const cwd = NodeFS.mkdtempSync(NodePath.join(import.meta.dirname, ".knip-test-")); | ||
| 16 | const write = (file: string, content: string) => | ||
| 17 | NodeFS.writeFileSync(NodePath.join(cwd, file), content); | ||
| 18 | const run = (filtered: boolean) => | ||
| 19 | NodeChildProcess.spawnSync( | ||
| 20 | NodeProcess.execPath, | ||
| 21 | [ | ||
| 22 | cli, | ||
| 23 | "--directory", | ||
| 24 | cwd, | ||
| 25 | "--config", | ||
| 26 | NodePath.join(cwd, "knip.json"), | ||
| 27 | "--no-config-hints", | ||
| 28 | "--include", | ||
| 29 | "files,dependencies,exports,nsExports,types,nsTypes,duplicates", | ||
| 30 | "--reporter", | ||
| 31 | "json", | ||
| 32 | ...(filtered ? ["--preprocessor", preprocessor] : []), | ||
| 33 | ], | ||
| 34 | { encoding: "utf8" }, | ||
| 35 | ); | ||
| 36 | try { | ||
| 37 | write( | ||
| 38 | "package.json", | ||
| 39 | JSON.stringify({ private: true, type: "module", dependencies: { effect: "*" } }), | ||
| 40 | ); | ||
| 41 | write( | ||
| 42 | "tsconfig.json", | ||
| 43 | JSON.stringify({ compilerOptions: { module: "NodeNext", strict: true } }), | ||
| 44 | ); | ||
| 45 | write( | ||
| 46 | "knip.json", | ||
| 47 | JSON.stringify({ | ||
| 48 | entry: ["entry.ts"], | ||
| 49 | project: ["*.ts"], | ||
| 50 | includeEntryExports: true, | ||
| 51 | include: [ | ||
| 52 | "exports", | ||
| 53 | "nsExports", | ||
| 54 | "types", | ||
| 55 | "nsTypes", | ||
| 56 | "duplicates", | ||
| 57 | "files", | ||
| 58 | "dependencies", | ||
| 59 | ], | ||
| 60 | rules: { types: "off", nsTypes: "off" }, | ||
| 61 | }), | ||
| 62 | ); | ||
| 63 | write( | ||
| 64 | "entry.ts", | ||
| 65 | ` | ||
| 66 | import * as S from "effect/Schema"; | ||
| 67 | import * as ns from "./namespace.ts"; | ||
| 68 | console.log(ns); | ||
| 69 | export { Remote as Reexported } from "./remote.ts"; | ||
| 70 | export const Text = S.String; | ||
| 71 | export const Alias = Text; | ||
| 72 | const Internal = S.Boolean; | ||
| 73 | export type Internal = typeof Internal.Type; | ||
| 74 | export const PublicAlias = Internal; | ||
| 75 | export default Text; | ||
| 76 | export const Record = S.Struct({ name: Text }).annotate({ title: "record" }); | ||
| 77 | export const Branded = S.String.pipe(S.brand("Name")); | ||
| 78 | export class Failure extends S.TaggedErrorClass<Failure>()("Failure", { reason: Text }) {} | ||
| 79 | export class Person extends S.Class<Person>("Person")({ name: Text }) {} | ||
| 80 | export type UnusedType = { name: string }; | ||
| 81 | export interface UnusedInterface { name: string } | ||
| 82 | `, | ||
| 83 | ); | ||
| 84 | write( | ||
| 85 | "remote.ts", | ||
| 86 | ` | ||
| 87 | import { Schema as S } from "effect"; | ||
| 88 | export const Remote = S.Struct({ id: S.Number }); | ||
| 89 | throw new Error("The preprocessor must never evaluate application modules"); | ||
| 90 | `, | ||
| 91 | ); | ||
| 92 | write( | ||
| 93 | "namespace.ts", | ||
| 94 | ` | ||
| 95 | import * as S from "effect/Schema"; | ||
| 96 | export const Unused = S.Number; | ||
| 97 | export type UnusedType = string; | ||
| 98 | `, | ||
| 99 | ); | ||
| 100 | const baseline = run(false); | ||
| 101 | expect(baseline.status, baseline.stderr).toBe(1); | ||
| 102 | expect(baseline.stdout).toContain('"Text"'); | ||
| 103 | const allowed = run(true); | ||
| 104 | expect(allowed.status, allowed.stderr + allowed.stdout).toBe(0); | ||
| 105 | expect(JSON.parse(allowed.stdout).issues).toEqual([]); | ||
| 106 | |||
| 107 | NodeFS.appendFileSync( | ||
| 108 | NodePath.join(cwd, "entry.ts"), | ||
| 109 | ` | ||
| 110 | export const decode = S.decodeUnknownSync(Text); | ||
| 111 | export const makeSchema = () => S.String; | ||
| 112 | export const LooksLikeSchema = { ast: "not a schema" }; | ||
| 113 | export const ordinary = 123; | ||
| 114 | export const duplicate = ordinary; | ||
| 115 | `, | ||
| 116 | ); | ||
| 117 | NodeFS.appendFileSync(NodePath.join(cwd, "namespace.ts"), `export const helper = () => 1;`); | ||
| 118 | write("unused.ts", `export const orphan = 1;`); | ||
| 119 | write( | ||
| 120 | "package.json", | ||
| 121 | JSON.stringify({ | ||
| 122 | private: true, | ||
| 123 | type: "module", | ||
| 124 | dependencies: { effect: "*", "unused-knip-fixture-dependency": "*" }, | ||
| 125 | }), | ||
| 126 | ); | ||
| 127 | const rejected = run(true); | ||
| 128 | expect(rejected.status, rejected.stderr).toBe(1); | ||
| 129 | const issues = JSON.parse(rejected.stdout).issues; | ||
| 130 | const entry = issues.find((issue: { file: string }) => issue.file === "entry.ts"); | ||
| 131 | expect(entry.exports.map((issue: { name: string }) => issue.name).sort()).toEqual([ | ||
| 132 | "LooksLikeSchema", | ||
| 133 | "decode", | ||
| 134 | "duplicate", | ||
| 135 | "makeSchema", | ||
| 136 | "ordinary", | ||
| 137 | ]); | ||
| 138 | expect(entry.duplicates).toHaveLength(1); | ||
| 139 | expect( | ||
| 140 | issues.find((issue: { file: string }) => issue.file === "namespace.ts").nsExports, | ||
| 141 | ).toEqual([expect.objectContaining({ name: "helper" })]); | ||
| 142 | expect(issues.find((issue: { file: string }) => issue.file === "unused.ts").files).toHaveLength( | ||
| 143 | 1, | ||
| 144 | ); | ||
| 145 | expect( | ||
| 146 | issues.find((issue: { file: string }) => issue.file === "package.json").dependencies, | ||
| 147 | ).toEqual([expect.objectContaining({ name: "unused-knip-fixture-dependency" })]); | ||
| 148 | } finally { | ||
| 149 | NodeFS.rmSync(cwd, { recursive: true, force: true }); | ||
| 150 | } | ||
| 151 | }); | ||
scripts/knip-schemas.ts created+96| ... | @@ -0,0 +1,96 @@ | ||
| 1 | // @effect-diagnostics nodeBuiltinImport:off - Knip and the TypeScript compiler host use synchronous Node paths. | ||
| 2 | import * as NodePath from "node:path"; | ||
| 3 | import type { Preprocessor } from "knip"; | ||
| 4 | import ts from "typescript"; | ||
| 5 | |||
| 6 | // Effect 4 schemas carry this marker, including aliases and Schema.Class constructors. | ||
| 7 | // Checking the type avoids evaluating application modules or exempting schema factories/decoders. | ||
| 8 | const schemaTypeId = "~effect/Schema/Schema"; | ||
| 9 | |||
| 10 | const preprocess: Preprocessor = (options) => { | ||
| 11 | const categories = ["exports", "nsExports", "duplicates"] as const; | ||
| 12 | const projects = new Map<string | undefined, Set<string>>(); | ||
| 13 | for (const category of categories) { | ||
| 14 | for (const [filePath, issues] of Object.entries(options.issues[category])) { | ||
| 15 | if (Object.keys(issues).length === 0) continue; | ||
| 16 | const configPath = ts.findConfigFile( | ||
| 17 | NodePath.dirname(NodePath.resolve(options.cwd, filePath)), | ||
| 18 | ts.sys.fileExists, | ||
| 19 | ); | ||
| 20 | const files = projects.get(configPath) ?? new Set<string>(); | ||
| 21 | files.add(filePath); | ||
| 22 | projects.set(configPath, files); | ||
| 23 | } | ||
| 24 | } | ||
| 25 | |||
| 26 | for (const [configPath, files] of projects) { | ||
| 27 | const config = configPath | ||
| 28 | ? ts.getParsedCommandLineOfConfigFile( | ||
| 29 | configPath, | ||
| 30 | {}, | ||
| 31 | { | ||
| 32 | ...ts.sys, | ||
| 33 | onUnRecoverableConfigFileDiagnostic: (diagnostic) => { | ||
| 34 | throw new Error(ts.flattenDiagnosticMessageText(diagnostic.messageText, "\n")); | ||
| 35 | }, | ||
| 36 | }, | ||
| 37 | ) | ||
| 38 | : undefined; | ||
| 39 | if (config?.errors.length) { | ||
| 40 | throw new Error( | ||
| 41 | config.errors | ||
| 42 | .map((error) => ts.flattenDiagnosticMessageText(error.messageText, "\n")) | ||
| 43 | .join("\n"), | ||
| 44 | ); | ||
| 45 | } | ||
| 46 | const program = ts.createProgram( | ||
| 47 | [...files].map((file) => NodePath.resolve(options.cwd, file)), | ||
| 48 | { | ||
| 49 | module: ts.ModuleKind.NodeNext, | ||
| 50 | allowJs: true, | ||
| 51 | ...config?.options, | ||
| 52 | noEmit: true, | ||
| 53 | }, | ||
| 54 | ); | ||
| 55 | const checker = program.getTypeChecker(); | ||
| 56 | for (const filePath of files) { | ||
| 57 | const source = program.getSourceFile(NodePath.resolve(options.cwd, filePath)); | ||
| 58 | const module = source && checker.getSymbolAtLocation(source); | ||
| 59 | if (!source || !module) continue; | ||
| 60 | const schemas = new Set( | ||
| 61 | checker.getExportsOfModule(module).flatMap((symbol) => { | ||
| 62 | const exported = | ||
| 63 | symbol.flags & ts.SymbolFlags.Alias ? checker.getAliasedSymbol(symbol) : symbol; | ||
| 64 | // Duplicate exports can name a private value with a public type of the same name. | ||
| 65 | const target = | ||
| 66 | exported.flags & ts.SymbolFlags.Value | ||
| 67 | ? exported | ||
| 68 | : checker.resolveName(symbol.name, source, ts.SymbolFlags.Value, false); | ||
| 69 | if (!target) return []; | ||
| 70 | const type = checker.getTypeOfSymbolAtLocation(target, target.valueDeclaration ?? source); | ||
| 71 | const marker = type.getProperty(schemaTypeId); | ||
| 72 | if (!marker) return []; | ||
| 73 | const markerType = checker.getTypeOfSymbolAtLocation(marker, source); | ||
| 74 | return markerType.isStringLiteral() && markerType.value === schemaTypeId | ||
| 75 | ? [symbol.name] | ||
| 76 | : []; | ||
| 77 | }), | ||
| 78 | ); | ||
| 79 | for (const category of categories) { | ||
| 80 | const issues = options.issues[category][filePath]; | ||
| 81 | if (!issues) continue; | ||
| 82 | for (const [key, issue] of Object.entries(issues)) { | ||
| 83 | const symbols = issue.symbols ?? [{ symbol: issue.symbol }]; | ||
| 84 | if (symbols.length > 0 && symbols.every(({ symbol }) => schemas.has(symbol))) { | ||
| 85 | delete issues[key]; | ||
| 86 | options.counters[category]--; | ||
| 87 | } | ||
| 88 | } | ||
| 89 | if (Object.keys(issues).length === 0) delete options.issues[category][filePath]; | ||
| 90 | } | ||
| 91 | } | ||
| 92 | } | ||
| 93 | return options; | ||
| 94 | }; | ||
| 95 | |||
| 96 | export default preprocess; | ||
scripts/package.json+1| ... | @@ -18,6 +18,7 @@ | ... | @@ -18,6 +18,7 @@ |
| 18 | "devDependencies": { | 18 | "devDependencies": { |
| 19 | "@effect/vitest": "catalog:", | 19 | "@effect/vitest": "catalog:", |
| 20 | "@types/pngjs": "6.0.5", | 20 | "@types/pngjs": "6.0.5", |
| 21 | "typescript": "catalog:", | ||
| 21 | "vite-plus": "catalog:" | 22 | "vite-plus": "catalog:" |
| 22 | } | 23 | } |
| 23 | } | 24 | } |