Skip to content

Fix CLI typecheck failures and missing devDependencies - #1200

Open
nordicnode wants to merge 1 commit into
CodebuffAI:mainfrom
nordicnode:fix-cli-dev-dependencies
Open

Fix CLI typecheck failures and missing devDependencies#1200
nordicnode wants to merge 1 commit into
CodebuffAI:mainfrom
nordicnode:fix-cli-dev-dependencies

Conversation

@nordicnode

Copy link
Copy Markdown

Problem & Context

Running TypeScript typechecking on the CLI package (bun x tsc --noEmit -p cli or workspace tsc --build) failed with 10 compilation errors across 10 files:

  1. TS2307: Cannot find module 'tar' or its corresponding type declarations in cli/src/__tests__/release/wrapper-safety.test.ts:265. Furthermore, cli/release-core/launcher.js:12 requires tar, and running bun test cli/src/__tests__/release/wrapper-safety.test.ts failed with Cannot find module 'tar' because tar was not declared in cli/package.json devDependencies.
  2. 9 files across cli/src/components/... (e.g. grid-layout.test.tsx, message-block.completion.test.tsx, render-ui.test.tsx) failed with TS7016: Could not find a declaration file for module 'react-dom/server' because @types/react-dom was omitted from cli/package.json devDependencies despite react-dom being declared.

Changes Made

  • Added "@types/react-dom": "19.2.3" to cli/package.json devDependencies (aligning with root package.json React 19 overrides).
  • Added "tar": "^7.0.0" to cli/package.json devDependencies (matching cli/release/package.json).
  • Added "@types/tar": "^6.1.13" to cli/package.json devDependencies to provide type declarations for tar.
  • Updated bun.lock accordingly.

Architecture & Conventions Conformance

  • Adheres to Dependency Injection (contracts defined in common/src/types/contracts/, no module monkey patching)
  • Terminal commands use terminalCommandBroker (no direct spawn or TUI-process bypass)
  • Environment hygiene respected (getCliEnv() for CLI, getSdkEnv() for SDK, no forbidden getProcessEnv() imports)
  • Freebuff mode compatibility (IS_FREEBUFF preserved, no paid features introduced)
  • Imports ordered and explicit (import type used for types)

Scope Verification

  • All modified files are within allowed public directories: cli/package.json, bun.lock
  • NO modifications to web/, freebuff/web/, packages/internal/, packages/billing/, packages/bigquery/, or packages/build-tools/

Testing & Verification

  • bun x tsc --noEmit -p cli passed with 0 errors (was 10 errors).
  • bun test cli/src/__tests__/release/wrapper-safety.test.ts passed 17/17 tests (was 3 failures).
  • bun run build:sdk passed cleanly.
  • bun run build:freebuff passed cleanly.
  • bun cli/scripts/smoke-binary.ts cli/bin/freebuff passed cleanly.
  • Anti-flake principles observed.

@codebuff-team

Copy link
Copy Markdown
Contributor

Good, minimal, well-scoped fix. Adding @types/react-dom and tar/@types/tar to cli/package.json devDependencies addresses real TS2307/TS7016 errors that would otherwise block tsc --noEmit and the wrapper-safety test — this is a legitimate gap since launcher.js already requires tar at runtime but it was never declared as a dependency anywhere reachable by the CLI package's own typecheck/test run.

A few things worth double-checking before porting:

  • tar is added under devDependencies even though cli/release-core/launcher.js appears to use it at runtime (not just in tests). If that file ships in the built CLI binary, tar probably needs to be a real dependency, not a devDependency, or it needs to already be satisfied transitively via cli/release/package.json at runtime. Worth confirming this doesn't break the packaged binary if tar isn't otherwise bundled.
  • The bun.lock diff bumps chownr 1.1.4→3.0.0 and yallist 4.0.0→5.0.0 as transitive deps of tar@7.5.22. That's expected given tar@^7, but worth a note in the description since it's a wider lockfile churn than the two-line package.json change suggests.
  • No test changes accompany this fix (fixing the failing existing test suite, not adding new coverage) — reasonable here since this is a dependency/type-declaration fix, not new behavior.

Overall: correct diagnosis, small blast radius, in-scope files only. Worth a maintainer's few minutes to confirm the runtime-vs-dev dependency placement for tar before merging upstream.

@codebuff-team codebuff-team added bot:triaged Classified by the community triage bot pr:port-candidate Worth porting into the private source tree labels Sep 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:triaged Classified by the community triage bot pr:port-candidate Worth porting into the private source tree

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants