Skip to content

chore(ci): run the knip dead-code gate for the repository root and every workspace - #4942

Draft
orangeCatDeveloper wants to merge 2 commits into
apache:mainfrom
orangeCatDeveloper:chore/knip-dead-code-gate
Draft

chore(ci): run the knip dead-code gate for the repository root and every workspace#4942
orangeCatDeveloper wants to merge 2 commits into
apache:mainfrom
orangeCatDeveloper:chore/knip-dead-code-gate

Conversation

@orangeCatDeveloper

@orangeCatDeveloper orangeCatDeveloper commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Stacked on #4938 (the dead-code deletions); this draft carries the gate configuration and CI wiring. Until #4938 merges, the diff below shows both PRs combined; after it merges, this PR's delta is exactly knip.json + ci.yml.

  • knip.json gains per-workspace configs for runtime, runtime-host, storage, core, mcp, computer-use, eval, cli and website, plus the repository root (workspaces["."]). Dynamic reachability is registered explicitly — spawn-loaded test fixtures, the filesystem worker bundle entry, the PowerShell-driven Windows IPC trust fixture, eval's Python-spawned harness entry, the dynamically dispatched computer-use command modules, and the two modules that actions/github-script loads from with: blocks (knip's github-actions plugin only reads run: steps) — so the scan doesn't guess.
    so the scan doesn't guess. The runtime-host workspace also carries a file-scoped ignoreIssues entry accepting the knip duplicates finding for the three intentional protocol alias files kept by chore(cleanup): drop verified dead exports and unused types #4938 (the encode/decode result-codec pairs and the PAGE/RANGE bound pair): probe-verified that with the exception an injected unused export in artifact.ts is still reported, so only duplicates is silenced and only in those three files.
  • ci.yml replaces the two existing per-workspace Knip steps with a single plain npx knip: a strict superset that covers the root plus all 11 workspaces in ~2s and respects every workspace's project patterns. The step fails when a package.json workspace is missing from knip.json, so a newly added workspace cannot silently enter the repo ungated.
  • website keeps its exact cookie@2.0.1 pin declared as an ignored dependency: astro's prerenderer imports { parseCookie } from 'cookie' and vite resolves the bare specifier through the website root, so the workspace pin decides which copy loads.
  • Hoisted build/test tooling that root scripts import (electron, electron-builder, electron-updater, app-builder-lib, builder-util, @playwright/test, @babel/parser, ws) is declared in the root package.json with the versions the tree resolves — instead of blanket-silenced, so a hoisting change that would break check:tui-copy or the packaging scripts surfaces as a knip finding. patch-package stays the one silenced exception: apply-dependency-patches resolves it through createRequire by design, so the gate cannot see that use.
  • The CI step additionally guards the coupling this declaration created: the root pins for electron-builder's internal packages must track electron-builder's own requirement, or the deep-import packaging tests would silently verify an old archive algorithm while the real build uses a new one.
  • packages/ui/scripts joins the project set (a benchmark script sat in the gate's blind zone), and the two intentional exceptions are documented in the CI step itself so the reasons persist in-repo.
  • Known coverage boundary: files surfaced through a package.json exports map are entry files, so their unused exports sit outside knip's default report (core exposes 147 of 266 source files this way, runtime 110 of 506, storage 55 of 249); --include-entry-exports surfaces ~250 public-API findings that need their own sweep with explicit keep decisions.

Verification

npx knip (plain)                       → 0 findings (root + all 11 workspaces, ~2s)
gate sensitivity probe                 → an unimported src/copy file and an unused packages/mcp file are both reported
package.json ↔ knip.json coverage assert → implemented in the CI step
duplicates probe                       → injected unused export in artifact.ts still reported with the alias-file exception

AI use

  • Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code — knip.json workspace configuration, the scan/verification loop (probe-tested coverage), CI wiring, and this description.

Checklist

  • Lint, format, typecheck and the affected suites pass locally
  • Commit carries Generated-by: Claude Code

Does this PR entail a change in behavior?

  • Yes
  • No

@github-actions github-actions Bot added the effort/L Under 1000 readable lines label Sep 7, 2026
@orangeCatDeveloper
orangeCatDeveloper force-pushed the chore/knip-dead-code-gate branch 2 times, most recently from a2a3e77 to d7dc56a Compare September 7, 2026 02:13
Remove the dead code that the knip sweep across every workspace and the
repository root surfaced, with each symbol verified consumer-free by a
repo-wide grep spanning .ts, .tsx, .mjs, .ps1, .py and workflow files:

- unused exported functions, types and helpers in runtime
  (context-budget-helpers, model-history's text-only projection subtree,
  agent-run-inspect, stream-graph-supervisor-tools,
  runtime-event-backfill, model-adapter, workspace-executor,
  compaction-boundary, memory-extraction-proposal, filesystem-authority,
  plugin-runtime's alias), runtime-host (agent-graph-coordinator,
  host-kernel, client-capability-coordinator's type re-export,
  peer-mesh), storage (workspace-version-authority-internal, a fixture's
  unused helper), cli (workspace-root's type re-exports, an unused
  wrapper, an unused union) and core (the computer-use action-type
  alias);
- the storage readHead registration plumbing that only fed the deleted
  readWorkspaceHeadInternal (the live store method and its tests stay);
- two dead root-script exports and two ambient .d.mts declarations that
  no module resolution reaches;
- two project-catalog path-limit constants unexported, declared as a
  compatible extension at the current epoch (123, rebased onto main).

Kept deliberately: the decode/encode and PAGE/RANGE codec alias pairs
whose both names have live consumers stay as plain aliases so the page
and range bounds and the wire contract remain true by construction.
Their knip duplicates finding is handled by a scoped ignoreIssues
exception for the three alias files in the follow-up gate PR instead of
restructuring the declarations. The website keeps its exact cookie@2.0.1
pin: astro's prerenderer resolves it through the website root.

The scan configuration that surfaced these (knip.json workspaces) and
the CI wiring live in the follow-up gate PR; this PR is deletions only.

No behaviour change.

Generated-by: Claude Code
…pository root

Extend knip.json to all nine previously-ungated workspaces (runtime,
runtime-host, storage, core, mcp, computer-use, eval, cli, website) plus
the repository root, and replace the eleven per-workspace Knip CI steps
with a single plain 'npx knip' run - a strict superset that also covers
the repository root and respects every workspace's project patterns.
Workspace globs are deliberately avoided: -W 'packages/*' silently skips
per-workspace project patterns (probe-verified). The step fails when a
package.json workspace is missing from knip.json, and when a knip.json
key no longer resolves to a declared workspace.

Dynamic reachability is registered explicitly - spawn-loaded test
fixtures, the filesystem worker bundle entry, the PowerShell-driven
Windows IPC trust fixture, eval's Python-spawned harness entry, the
dynamically dispatched computer-use command modules, and the two
modules that actions/github-script loads from its with: script block
(knip's github-actions plugin only reads run: steps) - so the scan
doesn't guess. The website's entry surface is scoped to the Astro
routes, the config and the executed scripts; components, layouts and
copy modules stay in the project set and are judged by the real import
graph (probe-verified). The root project glob excludes ambient
declarations, which TS resolution consumes without an import statement.

Remove what the scan surfaced and repo-wide greps confirmed unreachable:
unused exports, types and helpers across runtime, runtime-host, storage,
cli, core and the root scripts; the storage readHead registration
plumbing that only fed the deleted readWorkspaceHeadInternal; and export
aliases whose second name had no consumers.

Kept deliberately: the decode/encode and PAGE/RANGE codec alias pairs
whose both names have live consumers. Their aliases become distinct
declarations (thin wrapper functions, an independently declared range
bound asserted against the page bound in the protocol test) so the
duplicates check resolves without exemption tags. The website keeps its
exact cookie@2.0.1 pin as a declared exception: astro's prerenderer
resolves it through the website root. Hoisted build/test tooling that
root scripts import (electron, electron-builder, electron-updater,
app-builder-lib, builder-util, @playwright/test, @babel/parser, ws) is
declared in the root package.json instead of blanket-silenced, and the
CI step asserts the two electron-builder internal-package pins track
electron-builder's own requirement.

The coverage and pin-parity guards live in
scripts/check-knip-workspace-coverage.mjs with node --test coverage
(single-level glob semantics on both sides, the stale-key regression
case, pin parity including the absent-pin case) rather than as untested
inline scripts.

Known coverage boundary, surfaced by review: files listed in a
package.json exports map are entry files, so their unused exports sit
outside the default report (core exposes 147 of 266 source files this
way, runtime 110 of 506, storage 55 of 249); --include-entry-exports
surfaces ~250 public-API findings that need their own sweep with
explicit keep decisions.

No behaviour change.

The knip duplicates finding on the three intentional protocol alias
files (the encode/decode result-codec pairs and the PAGE/RANGE bound
pair, kept as plain aliases by the cleanup PR) is accepted with a
file-scoped ignoreIssues entry in the runtime-host workspace. The
duplicates check stays meaningful elsewhere: probe-verified that an
injected unused export in artifact.ts is still reported, and the
full npx knip run is clean.

Generated-by: Claude Code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/L Under 1000 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant