← Back to PRs

#21979: chore: add knip dependency check and remove unused dependencies

by chansuke open 2026-02-20 16:01 View on GitHub →
size: S
## Summary Describe the problem and fix in 2–5 bullets: - Problem: Unused dependencies remained in the repo, and there was no CI guard to catch future dependency drift. - Why it matters: Dependency drift increases maintenance/security surface and can silently regress over time without automated checks. - What changed: Removed knip-reported unused deps/files from core + `extensions/msteams`, added `knip` + `knip:ci`, added `knip.json` (root workspace, dependency-only, production mode, `signal-utils` ignore), and wired `pnpm knip:ci` into CI `check` job. - What did NOT change (scope boundary): No gateway/runtime behavior changes, no protocol/API contract changes, no auth/token flow changes, no channel routing logic changes. ## Change Type (select all) - [ ] Bug fix - [ ] Feature - [x] Refactor - [ ] Docs - [ ] Security hardening - [x] Chore/infra ## Scope (select all touched areas) - [ ] Gateway / orchestration - [ ] Skills / tool execution - [ ] Auth / tokens - [ ] Memory / storage - [x] Integrations - [ ] API / contracts - [ ] UI / DX - [x] CI/CD / infra ## Linked Issue/PR - Closes #N/A - Related #N/A ## User-visible / Behavior Changes None. ## Security Impact (required) - New permissions/capabilities? (`Yes/No`): No - Secrets/tokens handling changed? (`Yes/No`): No - New/changed network calls? (`Yes/No`): No - Command/tool execution surface changed? (`Yes/No`): No - Data access scope changed? (`Yes/No`): No - If any `Yes`, explain risk + mitigation: N/A ## Repro + Verification ### Environment - OS: Linux - Runtime/container: Node 22 + pnpm 10.23.0 - Model/provider: N/A - Integration/channel (if any): CI pipeline + repo deps (`extensions/msteams`) - Relevant config (redacted): `knip.json` (root workspace dependency check, `ignoreDependencies: ["signal-utils"]`) ### Steps 1. Run dependency hygiene scan: `pnpm dlx knip --dependencies --no-progress`. 2. Remove verified unused deps and update lockfile; add CI knip command/config. 3. Validate with `corepack pnpm@10.23.0 knip:ci`, `corepack pnpm@10.23.0 build`, and `corepack pnpm@10.23.0 test`. ### Expected - Unused dependencies are removed. - CI `check` job runs knip dependency check and fails on new drift. - Build/test remain green. ### Actual - Implemented as expected; commands passed after updates. ## Evidence Attach at least one: - [x] Failing test/log before + passing after - [x] Trace/log snippets - [ ] Screenshot/recording - [ ] Perf numbers (if relevant) Log snippets: ```text before (build failure when removing required dep): TS2307: Cannot find module 'signal-utils/array' after: > openclaw@2026.2.20 knip:ci > knip --include dependencies --workspace . --production --config knip.json --no-progress (build) exited 0 (test) Test Files 773 passed, Tests 6261 passed ``` ## Human Verification (required) What you personally verified (not just CI), and how: - Verified scenarios: Ran knip dependency scans, then ran `knip:ci`, full `build`, and full `test` locally after cleanup and CI wiring. - Edge cases checked: Confirmed `signal-utils` is still required by A2UI/vendor build path and therefore excluded from CI knip false-positive failure. - What you did **not** verify: GitHub-hosted CI runtime execution on this branch (local-only verification performed). ## Compatibility / Migration - Backward compatible? (`Yes/No`): Yes - Config/env changes? (`Yes/No`): No - Migration needed? (`Yes/No`): No - If yes, exact upgrade steps: N/A ## Failure Recovery (if this breaks) - How to disable/revert this change quickly: Revert CI hook commit (`git revert 10a52a68d`) to stop gating, or revert both knip commits (`git revert 10a52a68d f8e9e06aa`) to fully remove knip integration. - Files/config to restore: `.github/workflows/ci.yml`, `package.json`, `pnpm-lock.yaml`, `knip.json`. - Known bad symptoms reviewers should watch for: CI `check` failing on `pnpm knip:ci` due to new false positives or dependency graph changes. ## Risks and Mitigations List only real risks for this PR. Add/remove entries as needed. If none, write `None`. - Risk: `knip` false positives can block CI. - Mitigation: Scope is intentionally narrow (`dependencies` + root workspace + production mode) and known false positive (`signal-utils`) is explicitly ignored in `knip.json`. - Risk: Added dev tooling dependency increases lockfile churn. - Mitigation: Limited to dev/CI path; no runtime dependency or behavior impact. <!-- greptile_comment --> <h3>Greptile Summary</h3> Added knip dependency checker to CI with focused scope (production dependencies only) and made media understanding tests Windows-compatible. The PR correctly excludes `signal-utils` (used by A2UI bundler) and `long` (transitive dependency of `protobufjs`) from knip's false-positive detection. Test changes use `path.delimiter` instead of hardcoded colons and create `.cmd` files on Windows. <h3>Confidence Score: 5/5</h3> - This PR is safe to merge with minimal risk - Score reflects well-scoped changes with clear purpose: dependency hygiene tooling (knip) with appropriate exclusions, platform-agnostic test improvements, and no runtime behavior changes. The narrow focus (dependencies-only check, production mode) minimizes false-positive risk. - No files require special attention <sub>Last reviewed commit: 80baad5</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs