chore(repo): rewrite knip config to lean on plugin defaults by kanadgupta · Pull Request #6581 · supabase/cli

kanadgupta · 2026-09-11T23:52:24Z

Why

With an empty knip.json, knip's plugins already find every test file through each workspace's vitest config, and its GitHub Actions and package.json script parsers find most scripts. Nearly everything in the existing config was either redundant with those defaults or a blanket ignore that hid real findings.

The implicit script discovery was also fragile: knip drops cross-workspace entries found in workflow run: steps as soon as any root plugin contributes a **/ pattern, which is what happens when vitest is added as a root devDependency. That produced false "unused devDependency" reports for @anthropic-ai/claude-agent-sdk, its peer SDKs, and semantic-release.

What

The config becomes knip.jsonc, and every remaining exception carries the reason it exists, so it can be deleted when the reason goes away.

Declare only entries knip cannot infer: apps/cli/src/shared/cli/bin.ts (bundled by bun build), the scripts/ directories in apps/cli and packages/config, the semantic-release plugin file (knip's plugin is root-only), the platform smoke tests (string dispatched), and apps/docs/mdx-components.tsx.
Replace the apps/cli-go/** file ignore with ignoreWorkspaces.
One top-level ignoreBinaries for machine-provided tools, each annotated with the file that shells out to it.
Keep verdaccio and prettier ignored because both are spawned through a node_modules/.bin path knip cannot trace; ws stays ignored with a TODO while the cause is investigated.
Remove exclude: ["catalogReferences"], the .repos/** ignore, every src/**/*.test.ts entry, the .github/scripts and tools/release root entries, and the undici ignore.

Findings the old ignores were hiding are fixed rather than re-ignored, one commit each:

packages/api drops its unused undici dependency.
Test helpers in apps/cli/tests/helpers lose exports that were only used in-file, and five mocks with no callers are deleted.
ProjectLinkRemote service, its LinkedServiceVersions type, and the PropFlags telemetry constant had no consumer outside those mocks and are removed; ProfileResponse is no longer exported.
ReleaseDuePlan and NoReleasePlan in packages/config are no longer exported.
The npm-registry smoke helper resolves sync-versions.ts relative to itself so knip can follow the reference.
The .github/scripts bun:test suites run through a new root test:github-scripts script, which the workflow now calls; knip's Bun plugin derives the test entries from it.

🤖 Generated with Claude Code

github-actions

🤖 AI Review

One major tooling regression is confirmed: Knip no longer excludes initialized .repos submodules. A stale documentation reference and two configuration-polish issues are also confirmed. Three findings are refuted because they describe hypothetical path movement, misunderstand JSON Schema versus JSONC parsing, or overstate the effect of private constituent types.

Findings

Severity	Location	Category	Sources	Claim
🟠 MAJOR	`knip.jsonc:3`	`tooling`	claude	Removing the `.repos/**` ignore causes Knip to inspect initialized reference submodules, making the repository-wide Knip check impractical for standard development checkouts.
🟡 MINOR	`packages/config/docs/cli-config-loading.md:49`	`documentation`	claude	The configuration vocabulary documentation references the deleted `ProjectLinkRemote` service.
⚪ NIT	`knip.jsonc:5`	`tooling`	claude	Placing app-specific binary suppressions in the root `ignoreBinaries` list unnecessarily disables those checks for every workspace.
⚪ NIT	`knip.jsonc:22`	`documentation`	claude	The Verdaccio suppression comment incorrectly says both callers spawn it through a `node_modules/.bin` path.

Findings outside the diff

🟡 MINOR packages/config/docs/cli-config-loading.md:49 — The configuration vocabulary documentation references the deleted ProjectLinkRemote service.

Refuted findings (kept for transparency, not posted as review comments)

apps/cli/tests/helpers/npm-registry.ts:269 (maintainability): The new file-relative sync-versions.ts invocation is fragile because moving the helper would break its script path.
Refuted: The working directory makes the current path deterministic and it resolves to the existing apps/cli/scripts/sync-versions.ts. The existing root constant at line 18 is itself file-location-relative, so a hypothetical file move would already require updating path calculations.
knip.jsonc:2 (tooling): A JSONC Knip configuration must reference a separate schema-jsonc.json instead of schema.json.
Refuted: JSONC parsing and JSON Schema validation are separate: the editor/parser removes JSONC syntax before applying the semantic schema. Comments and trailing commas therefore do not make schema.json inappropriate, and the proposed alternate path is unsupported by repository evidence.
apps/cli/tests/helpers/command-mocks.ts:471 (maintainability): Making constituent helper types private prevents consumers from naming types exposed through exported helper APIs.
Refuted: The exported APIs remain structurally usable and existing callbacks receive contextual typing through ApiHandler. Consumers can also derive names through exported shapes, such as MockCommandPlatformApiResult["requests"][number], NonNullable<SetupStorageOptions["routes"]>[number], and ComputeHttpRoutes[string]; direct exports are not required.

Stats

Claude findings: 7 · Codex findings: 0 · Confirmed: 4 · Refuted: 3 · Uncertain: 0

Models: claude-opus-5 + gpt-5.6-sol · Trigger: auto · Workflow run

This review runs once per PR. A maintainer can request another with a /ai-review comment.

github-actions · 2026-09-12T00:21:55Z

+  // Binaries the machine provides rather than npm. `go` comes from mise.
+  "ignoreBinaries": [
+    "go",
+    // apps/cli/tests/smoke-test-macos.ts
+    "brew",
+    // apps/cli/tests/smoke-test-windows.ts
+    "cygpath",
+    // apps/cli/src/commands/seed/buckets/buckets.integration.test.ts
+    "mkfifo",
+    // apps/cli/scripts/build.ts
+    "nfpm",
+    // apps/cli/scripts/build.ts
+    "rcodesign",
+    // apps/cli/tests/smoke-test-windows.ts
+    "scoop",
+    // The CLI under test in apps/cli/tests/smoke-test-{macos,windows}.ts.
+    "supabase",


⚪ NIT · tooling · source: claude

Placing app-specific binary suppressions in the root ignoreBinaries list unnecessarily disables those checks for every workspace.

Evidence: knip.jsonc:5-21 globally ignores brew, cygpath, mkfifo, nfpm, rcodesign, scoop, and supabase; their actual invocations are all under apps/cli, whereas go is also invoked from tools/release/local-release.ts.

Suggested fix: Keep go global and move the app-specific binary ignores into workspaces["apps/cli"].ignoreBinaries.

github-actions · 2026-09-12T00:21:55Z

+  ],
+  // Spawned through a node_modules/.bin path in tools/release/local-registry.ts and
+  // apps/cli/tests/helpers/npm-registry.ts, which knip cannot trace.


⚪ NIT · documentation · source: claude

The Verdaccio suppression comment incorrectly says both callers spawn it through a node_modules/.bin path.

Evidence: tools/release/local-registry.ts:99 constructs a node_modules/.bin/verdaccio path, but apps/cli/tests/helpers/npm-registry.ts:49 invokes bunx verdaccio.

Suggested fix: State that one caller uses node_modules/.bin and the other uses bunx.

Nine symbols in apps/cli/tests/helpers were exported but only referenced inside their own module: DEFAULT_USER_AGENT and RecordedRequest in command-mocks, RecordedRequest, StubResponse, StubTransportFailure and RouteHandler in compute, LiveWorkspace in live, and StorageRoute and RecordedStorageRequest in storage. knip did not report them because the whole tests/ directory was excluded via `ignore` in knip.json. That exclusion is being removed so the helpers are analysed like any other source; dropping the unused exports first keeps that change from surfacing findings it should not have to carry. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

ReleaseDuePlan and NoReleasePlan exist only to form the exported ReleasePlan union in the same file; nothing imports them directly. knip hid this because scripts/release-plan.ts was an entry file, and exports of entry files are not checked by default. The upcoming knip config keeps the file as an entry, so this would otherwise stay invisible. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

PropFlags was declared in the telemetry event catalog but never referenced by any event definition, emitter, or test. knip did not report it because event-catalog.ts was excluded via `ignore` in knip.json; that exclusion is being removed so the catalog is analysed like the rest of the source. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

@supabase/api declared undici as a runtime dependency since the package was created, but nothing in the package or the repo imports it. The only consumer is @effect/platform-node, which declares undici as its own dependency and therefore keeps receiving it transitively. knip reported this correctly; the report was silenced with ignoreDependencies in knip.json. Removing the dependency retires the workaround instead of keeping it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

The bun:test suites under .github/scripts were only ever invoked inline from the github-scripts-ci workflow. Moving the invocation into a root `test:github-scripts` script gives developers a local command and lets knip's Bun plugin derive the test-file entries from the script, which is what allows the hard-coded `.github/scripts/**/*.ts` entry to leave the knip config. The workflow now calls the script and keeps its existing guard against silent zero-test discovery. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Analysing apps/cli/tests with knip for the first time surfaced helpers that nothing imports. mockCrypto, mockCredentials, mockApi, mockProjectLinkRemote and withEnv in mocks.ts had no callers at all; the two tests that mention withEnv define their own local version. They are deleted together with the imports only they used. mockProjectLinkState, mockCliProjectLocalServiceVersions, mockComputeHttp and spawnSupabase are still used inside their own modules, so they lose the export keyword instead. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Deleting the unused test mocks left three production symbols with no consumer: - ProjectLinkRemote in project-link-remote.service.ts was only ever referenced by mockProjectLinkRemote; no command or layer imports the service, so the file is removed. - LinkedServiceVersions in project-link-state.service.ts was only used by that service. The schema it derived from stays, since the state schema still embeds it. - ProfileResponse in api.service.ts is still used by the Api service signature but was only imported by mockApi, so it is no longer exported. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

The local-registry smoke helper invoked sync-versions.ts through `pnpm exec bun apps/cli/scripts/...` with the repo root as cwd. knip resolves Bun shell commands from the directory of the file that contains them, so it reported the script path as unresolved. Running `bun` with a path relative to the helper and cwd set to the helper's own directory keeps the behaviour (sync-versions.ts locates the repo root from import.meta.dir, not from cwd) and lets knip follow the reference. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

With an empty config on develop, knip's plugins already find every test file through each workspace's vitest config, and the GitHub Actions and package.json script parsers find most scripts. Nearly everything in knip.json was either redundant with those defaults or a blanket ignore hiding real findings, and the implicit script discovery broke as soon as a root plugin (vitest at the root) contributed a `**/` pattern, because knip then drops cross-workspace workflow entries. The config becomes knip.jsonc so every remaining exception carries the reason it exists: - `ignoreWorkspaces` replaces the `apps/cli-go/**` file ignore; the Go workspace only has a package.json for turbo. - One top-level `ignoreBinaries` lists tools the machine provides, each with the file that shells out to it. - `verdaccio` and `prettier` stay ignored because they are spawned through a node_modules/.bin path knip cannot trace. - `apps/cli` declares only the entries knip cannot infer: bin.ts (bundled by `bun build`), the scripts directory, the semantic-release plugin file (the knip plugin is root-only), and the platform smoke tests (string dispatched). Only the generated feedback types and the macro-read compute templates remain ignored. - `packages/config` declares its scripts directory for the same reason. - `packages/stack` keeps the `ws` ignore with a TODO, since knip does not credit the import in the e2e test and the cause is not yet known. - `exclude: ["catalogReferences"]`, the `.repos/**` ignore (submodules knip already skips), the `tools/release` and `.github/scripts` root entries, every `src/**/*.test.ts` entry, and the `undici` ignore are removed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

kanadgupta marked this pull request as ready for review September 12, 2026 00:05

kanadgupta requested a review from a team as a code owner September 12, 2026 00:05

kanadgupta added this pull request to stack #6584 September 12, 2026 00:12

github-actions Bot reviewed Sep 12, 2026

View reviewed changes

kanadgupta force-pushed the kanad-claude/cli-2422-knip-cleanup branch 2 times, most recently from 1699b1f to 11dc2e2 Compare September 12, 2026 00:28

kanadgupta and others added 12 commits September 12, 2026 10:16

chore: wtf

1204c69

chore: tweaks based on feedback

527a0e3

chore: more copy edits based on feedback

c9aa3b7

kanadgupta force-pushed the kanad-claude/cli-2422-knip-cleanup branch from b4aaecc to c9aa3b7 Compare September 12, 2026 17:16

Navigation Menu

Uh oh!

chore(repo): rewrite knip config to lean on plugin defaults - #6581

chore(repo): rewrite knip config to lean on plugin defaults#6581
kanadgupta wants to merge 12 commits into
developfrom
kanad-claude/cli-2422-knip-cleanup

kanadgupta commented Sep 11, 2026

Uh oh!

github-actions Bot left a comment

Uh oh!

Uh oh!

github-actions Bot Sep 12, 2026

Uh oh!

github-actions Bot Sep 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

kanadgupta commented Sep 11, 2026

Why

What

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

🤖 AI Review

Findings

Findings outside the diff

Stats

Uh oh!

Uh oh!

github-actions Bot Sep 12, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 12, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant