Repository navigation
chore(ci): make knip a blocking dead-code gate + burn down 587 -> 5 baselined issues - #2135
Merged
Merged
Conversation
Wires knip into the lint job as a blocking check via a baseline/ratchet script (scripts/knip-ratchet.mjs), modeled on scripts/coverage-ratchet.mjs. knip:check fails only on issues not already in knip-baseline.json; knip:ratchet regenerates the baseline after cleanup, refusing to let it grow. Also fixes real knip.json config drift found while generating the baseline: - packages/lib and packages/db entry globs pointed at deleted barrel files (src/index.ts, src/server.ts, src/client.ts) instead of the packages' actual ~320/70 subpath exports, badly inflating unused-exports/types counts - apps/android and apps/ios Capacitor plugins were false-positive "unused deps" (auto-linked natively, never imported in TS) - packages/cli's bin.ts/bin-pagespace-mcp.ts (package.json "bin" field) had no matching knip entry, so they showed as unused files - 14 stale ignoreDependencies entries and 4 redundant entry patterns already covered by knip's built-in plugins Baseline lands at 408 real issues (down from a raw 587 before the config fixes). Follow-up PRs against this branch burn that down to zero.
…are route (#2116) The VersionHistoryPanel/VersionHistoryItem components and their barrel were knip-baselined as unused; the /api/pages/[pageId]/versions/compare route's only caller was that UI (knip can't flag route.ts files — manual audit finding). RollbackConfirmDialog.tsx stays: it's live via ActivityItem and SidebarActivityTab. Removes the route's now-stale AUDIT_EXEMPT_ROUTES entry and ratchets the knip baseline down by the 3 UI files. Claude-Session: https://claude.ai/code/session_01ABo3G5XFj1jDWgx4jEhF8D Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Removes ToolBuilder.tsx, JsonSchemaBuilder.tsx, and OpenAPIImportDialog.tsx — an unreferenced closed cluster (they import only each other and shared UI primitives) — and drops their three entries from the knip baseline. Claude-Session: https://claude.ai/code/session_01X9MVgoceVrAtKxpzM6BdBc Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
The vendored "AI Elements" kit under apps/web/src/components/ai/ui/ was mostly dead: 26 of its 30 components had zero imports anywhere in the repo (verified by knip and a full-repo grep including dynamic imports). The four live files — code-block.tsx, conversation.tsx, tool.tsx, web-preview.tsx — are kept, along with their test. Baseline shrinks by 26 entries via knip:ratchet. Claude-Session: https://claude.ai/code/session_01XdBnRUGsKhJnkq7DjheNmg Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…marketing) (#2119) Removes 15 unused dependencies, adds 4 missing (unlisted) ones, removes 3 orphaned package.json exports subpaths, and encodes 7 knip false positives as ignoreDependencies. Baseline shrinks 405 -> 361. Removed deps: - apps/web: @ai-sdk/anthropic, @ai-sdk/google, @ai-sdk/openai, @ai-sdk/xai (abandoned AI SDK 7 migration remnants), pg, recharts, @xyflow/react, @radix-ui/react-accordion, embla-carousel-react - apps/control-plane: dotenv - apps/processor: @aws-sdk/s3-request-presigner, ts-node - apps/realtime: cookie - packages/lib: mammoth, pdf-parse-debugging-disabled Added (were unlisted): - root: drizzle-orm, pg (scripts/ imports) - apps/web: server-only - apps/admin: postcss Orphaned exports removed (source deleted in earlier refactors): - packages/lib ./services/sandbox/lifecycle (14f50df) - packages/lib ./services/rate-limit-cache (e5659ae) - packages/db ./schema/sandbox-sessions (88b10b4) plus lib's matching dangling typesVersions entries. knip.json ignoreDependencies (false positives, mirroring apps/web's existing pattern): - apps/admin eslint stack (used by `next lint` + eslint.config.mjs) - apps/android + apps/ios `web` (turbo build-order dep for cap sync) - packages/lib react-dom (peer-dep host for @react-email/render) Left alone: apps/web tokenlens + @radix-ui/react-hover-card — their only importers are knip-dead but still-typechecked files, and nothing else declares them, so removal breaks typecheck until a dead-file deletion PR lands. Claude-Session: https://claude.ai/code/session_014q2Rfkf5PFkjiacLKLGXoD Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…eps (#2118) Pure deletion PR in the knip-baseline burn-down series. Every removed file was flagged unused by knip and verified to have no references anywhere in apps/marketing (including dynamic imports and cross-imports — the only importers of these files were other files in the same deletion set). Each removed dependency was imported only by the deleted files. Marketing build passes; knip baseline shrinks 405 → 338. Claude-Session: https://claude.ai/code/session_011RdhxswpnTxfygoeSm46gz Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
apps/web/src/components/ui/{accordion,carousel}.tsx had zero real
consumers (carousel.tsx's only consumer, ai/ui/inline-citation.tsx, was
deleted in the AI Elements cleanup). They were invisible to knip
(src/components/ui/** is knip-ignored) and only kept typechecking because
apps/marketing still declared @radix-ui/react-accordion/embla-carousel-react
and bun's hoisting gave web a free resolution path — a path removed by the
marketing + deps-sweep cleanup PRs. Deleting them removes the fragile
cross-workspace hoisting dependency entirely.
- apps/marketing: add ignoreDependencies for its eslint stack (same FlatCompat-resolved-but-not-imported pattern already ignored for apps/web/apps/admin); add postcss/postcss-load-config as explicit devDeps instead of relying on hoisting from another workspace - apps/admin: add src/components/ui/** to knip ignore, matching apps/web/apps/marketing convention for vendored shadcn kits — was producing 13 false-positive "unused export" entries - apps/web: remove tokenlens + @radix-ui/react-hover-card (only consumers were the already-deleted ai/ui/* files); delete components/ui/hover-card.tsx (zero real consumers, same knip-ignored-but-dead pattern as the accordion/carousel cleanup) Baseline: 262 -> 241.
- vitest.workspace.ts: 'setupFiles: ./src/test/setup.ts' resolves fine at runtime (vitest remaps it relative to the web project's root: './apps/web'), but knip resolves it relative to vitest.workspace.ts's own location instead — false positive, added ignoreUnresolved. - scripts/__tests__/changelog-shell-safety.test.ts: tested a module tree (scripts/changelog/*) deleted wholesale in 5c47614 ("remove stale docs/, prune cross-references") — the test was never cleaned up alongside it. Deleted. Note: root package.json's "changelog:generate" script still points at the deleted scripts/changelog/index.ts and is therefore broken — left as-is, this is a pre-existing, already-known issue (not a dead-code/knip finding) and out of scope here. Baseline: 241 -> 237.
…lags (#2122) Deleted 13 genuinely dead files (zero references anywhere in the repo, confirmed via repo-wide grep for imports, dynamic require()/import(), and string-keyed lookups): - apps/control-plane/src/repositories/index.ts (dead barrel; createTenantRepository is imported directly from ./tenant-repository everywhere) - apps/web/src/components/layout/left-sidebar/DriveList.tsx - apps/web/src/components/layout/left-sidebar/workspace-selector.tsx - apps/web/src/components/layout/right-sidebar/ai-assistant/SidebarSettingsTab.tsx - apps/web/src/components/shared/AuthButtons.tsx (superseded by OAuthButtons) - apps/web/src/components/shared/ContactForm.tsx (superseded by marketing's own ContactForm) - apps/web/src/components/tasks/TaskMobileCard.tsx - apps/web/src/components/tasks/task-hooks.ts - apps/web/src/hooks/useDirectUpload.ts (superseded by useAttachmentUpload + lib/upload/*) - apps/web/src/lib/ai/types/global-prompt.ts (superseded by admin's local copy of the type) - apps/web/src/lib/repositories/index.ts (dead barrel; individual repository files are imported directly everywhere) - apps/web/src/lib/tabs/index.ts (dead barrel; individual tab modules are imported directly) - packages/lib/src/email-templates/TenantProvisioningCompleteEmail.tsx Kept 3 files that are genuinely reachable only through mechanisms knip's static analysis can't trace, and reclassified them in knip.json instead of deleting: - apps/web/src/test/next-server-stub.ts — resolved via vitest.config.ts's resolve.alias substitute for 'next/server'; added to apps/web's ignore list. - apps/e2e/support/mock-server-main.ts — invoked as a shell command (`bun run support/mock-server-main.ts`) by Playwright's webServer config; added a new apps/e2e workspace block with it in ignore. - packages/db/src/migrate-pending-invites.ts — standalone CLI script invoked via the package.json `migrate-pending-invites` script; added to packages/db's entry list alongside its sibling scripts (migrate.ts, migrate-admin.ts, provision-admin-users.ts). Ran `bun run knip:ratchet` to lock in the shrink (237 -> 221 baseline issues). Verified with a full `bunx turbo run typecheck --force` and `bunx turbo run lint --force` across the whole repo (not just affected workspaces). Claude-Session: https://claude.ai/code/session_01HX9rxrXVhWCJVdU3haUciU Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…exports (#2132) * chore(knip): PR8b — resolve remaining unused exports/types/duplicate-exports Removed unused export keywords, dead symbols, and duplicate export forms (named + default) flagged by knip across web, admin, marketing, desktop, control-plane, realtime, e2e, and the lib/sdk packages. Deleted six fully dead component files and VoiceModeOverlay.tsx (zero importers repo-wide; prior pass had stripped its exports but left the file in place). Every removed symbol was cross-checked with a repo-wide ripgrep sweep for remaining consumers before deletion. * chore(knip): fix cascade breakage from PR8b + ratchet baseline Merging pu/knip-ci's tip exposed 4 new knip flags plus a handful of typecheck/lint errors caused by symbols becoming dead only after PR8b's exports were stripped (a removed component/type left its sibling constant, prop type, or union member with zero remaining consumers). - apps/desktop/fetch-proxy-types.ts: FetchProxyResponse union was itself dead (verified: no repo-wide consumer), which cascaded to its four member interfaces once removed — deleted all five. - apps/web/lib/mentions/mentionConfig.ts: MentionConfigManager's removal in PR8b left `globalConfig`/DEFAULT_GLOBAL_CONFIG/GlobalMentionConfig with no reader — deleted. - apps/web/lib/utils/formatters.ts: LANGUAGE_EXTENSION_MAP was only used by a function already removed in PR8b — deleted. - apps/web/lib/monitoring/monitoring-types.ts: last remaining interface (MonitoringWidgetProps) had zero consumers after DetailedWidgetProps was removed; the admin app has its own separate copy that's very much alive, this web-only copy was dead — deleted the whole file. - apps/web/components/ai/ui/tool.tsx: ToolInputProps/ToolOutputProps types were only used by the ToolInput/ToolOutput components PR8b removed — deleted. - apps/web/lib/editor/pagination: getPageSize had zero importers outside its own dead barrel re-export — deleted. - apps/admin/lib/auth/auth-fetch.ts: fetchJSON had zero callers anywhere in the admin app — deleted (unrelated to the merge, pre-existing). Ratcheted knip-baseline.json: 221 -> 8 issues. The remaining 8 are deliberately left alone (not oversights): - capacitor-bridge.ts (isAndroid, getInjectedPlatform, callNative): exercised by a dedicated test suite via dynamic import; removing would break the tests, and this reads as a stable platform-detection surface not yet called from every code path. - page-agent-repository.ts PageAgentConfig: explicitly documented in its own comment as a forward-looking contract for sibling Terminal PRs to build against. - replay-dedupe.ts MAX_ANCHOR_BYTES/MAX_DISCARDABLE_GIVEUP_BYTES: semantically distinct constants that are intentionally equal by design (documented in a comment), not an accidental alias. - cli keys/list.ts and keys/revoke.ts handler/Handler pairs: explicitly documented as split so tests can call the plain function without going through the router. - device-auth-helpers.ts createDeviceToken/createWebDeviceToken: @deprecated alias with an active migration still in progress across real call sites (device/register, google/callback, apple/callback) — a behavior migration, not an export-hygiene fix. Verified with `rm -rf apps/*/.next && bunx turbo run typecheck --force` and `bunx turbo run lint --force` across the whole repo (both clean), plus the affected unit test suites.
apps/web/src/lib/capacitor-bridge.ts: isAndroid, getInjectedPlatform, callNative had zero real callers in application code — only exercised by their own dedicated unit tests (test-only usage doesn't count per this repo's knip config). Removed the functions and their test blocks; 23/23 tests in capacitor-bridge.test.ts still pass. Baseline: 8 -> 5. The remaining 5 are deliberately kept, not oversights: - 4 "duplicate exports" (tokensList/tokensListHandler, tokensRevoke/tokensRevokeHandler in packages/cli, and createDeviceToken/createWebDeviceToken in apps/web) follow an intentional two-tier naming convention — bare name is the reusable business-logic function (part of @pagespace/cli's published library API), Handler-suffixed alias is what the CLI router imports. A generic duplicate-export detector can't distinguish that from an accidental leftover alias; consolidating risks breaking external consumers of a published package. - 1 "unused type" (PageAgentConfig in page-agent-repository.ts) is explicitly documented as "the contract sibling Terminal PRs (tool-group, settings UI, session routing) build against" — active in-flight design work, not dead code. knip:check / typecheck / lint all clean, full repo, uncached.
# Conflicts: # apps/web/src/components/version-history/VersionHistoryItem.tsx # apps/web/src/components/version-history/VersionHistoryPanel.tsx
Merging master's broadcast admin UI feature (#2107) into pu/knip-ci surfaced real breakage: PR8b had removed post/fetchJSON from auth-fetch.ts and AudienceDefinitionInput/BroadcastActionInput from broadcasts/schema.ts because they had zero consumers *in PR8b's branch* — the broadcast admin UI didn't exist there yet. Two PRs landing in parallel, neither could see the other's need for these. Restored exactly what master's broadcast-composer.tsx/composer-form.ts actually import; left `del` and `TemplateCreateInput` removed since nothing currently uses either (verified via repo-wide grep — the one `del(` hit was `stripe.customers.del`, unrelated). Full repo typecheck (16/16) + lint (14/14) clean; 28/28 broadcast component tests pass.
Contributor
|
Important Review skippedToo many files! This PR contains 134 files, which is 34 over the limit of 100. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (238)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b72a3f3e9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
# Conflicts: # apps/control-plane/package.json # apps/realtime/package.json # bun.lock
Merged 13 more master commits (task-board crash-prevention epic phases 1-8, Sentry crash reporting, payment receipt emails). Two real conflicts: - apps/control-plane/package.json, apps/realtime/package.json: the Sentry commit (#2131) re-added dotenv/cookie next to its new @sentry/node dependency in both files — same class of parallel-PR collision as the earlier admin/broadcast one. Verified via grep: still zero real consumers of dotenv (control-plane) or cookie (realtime) post-merge. Kept @sentry/node, dropped both. - knip flagged 2 new "unused files": apps/web/src/__fixtures__/ eslint-unbounded-findmany/*.ts. These are intentional TDD fixtures for a new no-restricted-syntax ESLint rule (#2129, relocated by #2136) — files meant to be *linted*, not imported; their pass/fail lint state is the test itself. Added a knip.json ignore rather than treating them as dead code. Baseline unchanged at 5. Full uncached typecheck (16/16) + lint (14/14) clean.
Two real bugs flagged by automated PR review on #2135: 1. knip's `duplicates` entries are arrays of group members (each group is 2+ symbols that are the same export under different names), not flat {name} objects like every other category. The old code did `item?.name` on the group array itself, which is always undefined, so every duplicate group in a file collapsed onto the identical `duplicates::file::` fingerprint. A file with two distinct duplicate groups would only ever register one — the second would silently match the already-baselined (empty-name) fingerprint. Fixed by fingerprinting the whole group as its sorted, comma-joined member names. 2. Check mode printed a "you could shrink the baseline" suggestion but still exited 0. A stale fingerprint for now-fixed dead code remains permitted indefinitely — reintroducing that exact dead code later would silently pass CI. Check mode now fails when the baseline can shrink, same severity as finding a new issue; verified with a scenario test (injected a stale fingerprint, confirmed check mode fails with a clear message, confirmed it passes clean once removed). Regenerated knip-baseline.json with the corrected duplicate fingerprints (same 5 issues, now uniquely identified). knip:check / full typecheck / full lint / scripts lint all clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
From the 2026-07-17 solo-tells audit (cheap-fix #10: "Turn knip on in CI and burn down the 100-file dead list"). knip was installed and configured but never enforced — dead code just accumulated (100 unused files, 164 unused exports, 55 unused deps per the audit's raw count).
This PR:
lintCI job as a blocking gate with a monotonic baseline/ratchet (scripts/knip-ratchet.mjs,knip-baseline.json) —bun run knip:checkfails only on issues not already in the baseline;bun run knip:ratchetregenerates it after cleanup and refuses to let it grow.knip.jsonconfig drift found while generating the baseline (stale entry globs pointing at long-deleted barrel files, missing Capacitor/eslint-stack ignores, a missingpackages/clibin entry, a missingsrc/components/ui/**vendored-kit ignore forapps/admin) — this alone dropped the raw baseline from 587 to 411 before any actual code was deleted.require()/import()before any deletion, full uncachedtypecheck+lintafter).What's deleted
~19,000 lines of dead code: a 26-file unused "AI Elements" UI kit (zero imports since 2025-12-15), an orphaned version-history UI + its backing
/versions/compareAPI route, a dead ToolBuilder/JsonSchemaBuilder/OpenAPIImportDialog cluster, 45 unused marketing shadcn components + 22 unused deps, abandoned AI SDK 7 migration deps, ~150 unused exports/types, and assorted smaller dead files.Near-misses caught (things that looked dead but weren't)
apps/web/src/lib/auth/platform-storage/{desktop,ios,web}-storage.ts— reached only via runtimerequire()inside platform-detection branches; knip's static analysis can't trace that. Reclassified as a knip ignore, not deleted — deleting would have broken desktop/iOS auth storage.packages/cli/src/{bin.ts,bin-pagespace-mcp.ts}— the CLI's actual binaries (package.jsonbinfield), invisible to knip's default entry detection. Added an explicit workspace entry instead of letting a sweep delete them.apps/android/apps/iospackage.json— auto-linked natively, never imported in TS.ignoreDependencies, not removal.apps/web/src/lib/repositories/global-machine-config-repository.tsetc. "using"PageAgentConfig— turned out to be JSDoc prose references, not real imports; the type genuinely was unused, but its docstring flagged it as forward-looking design work for in-flight Terminal epics, so it's baselined rather than deleted.packages/cli'stokensList/tokensListHandler(and the revoke/device-token equivalents) — look like accidental duplicate exports, are actually an intentional two-tier naming convention (bare name = published library API,Handlersuffix = what the CLI router imports). Baselined rather than "fixed."post/AudienceDefinitionInput/BroadcastActionInputbecause they were genuinely unused in this branch — the broadcast admin UI didn't exist yet when that commit ran. Restored exactly what's needed, left everything else that's still actually dead removed.Final state (5 deliberately-baselined issues, not oversights)
capacitor-bridge.ts: none remaining (isAndroid/getInjectedPlatform/callNative were genuinely dead — test-only usage — and removed)PageAgentConfig) that's documented in-repo as forward-looking contract work for active Terminal epicsTest plan
bun run knip:checkpasses (5 issues, all within baseline)bunx turbo run typecheck --force— 16/16 cleanbunx turbo run lint --force— 14/14 cleanSecurity Test Suite,CodeQL,Static Security Analysis,Secret Scanning,Dependency Audit— greenci / Unit Tests(DB-backed) — runningci / Lint & TypeScript Check(includesknip:check) — runningPost-open updates
masteris a fast-moving target (this repo merges continuously) — reconciled two waves of drift after opening this PR, each with real, non-mechanical conflicts:apps/web/src/components/version-history/{VersionHistoryItem,VersionHistoryPanel}.tsxconflicted modify/delete — a concurrent, unrelated toast-migration commit touched them right before this PR's deletion landed. Confirmed viagit log/grep it was a mechanical bulk edit, not a revival; kept the deletion. Also restored 3 admin exports (post/AudienceDefinitionInput/BroadcastActionInput) that an earlier commit in this series had removed as unused — correct at the time, but master's newly-merged broadcast admin UI (which didn't exist yet in that commit's branch) needs them. Two PRs in flight, neither could see the other's need.apps/control-plane/package.json+apps/realtime/package.jsonconflicted — master's Sentry crash-reporting commit re-addeddotenv/cookienext to its new@sentry/nodedep in both files (same class of parallel-PR collision). Verified both are still genuinely unused post-merge; kept@sentry/node, dropped both. Also added a knip ignore forapps/web/src/__fixtures__/eslint-unbounded-findmany/**— intentional TDD fixtures for a new lint rule, meant to be linted not imported.Automated review (2 threads, both addressed): a Codex-based reviewer caught two real bugs in
scripts/knip-ratchet.mjs— (1)duplicatesentries are nested arrays of group members, not flat{name}objects, so the fingerprint logic collapsed every duplicate-export group in a file onto the same identical fingerprint (fixed: fingerprint the whole group by its sorted member names); (2) check mode only suggested runningknip:ratchetwhen the baseline could shrink but still exited 0, so a stale fingerprint for now-fixed dead code stayed permitted indefinitely (fixed: check mode now fails when the baseline can shrink, verified with a scenario test). Both replied to with the exact fix commit; left unresolved for the bot to re-verify against the new commits rather than self-resolving.Baseline unaffected (still 5, same deliberately-kept issues) — all reconciliation was fingerprint-format/drift bookkeeping, not new dead code.
https://claude.ai/code/session_019fX3BVrtbTHdcjHT8Cevxi