chore: slim CLI install by making heavy deps optional#115
Conversation
Move @skillkit/api, @skillkit/tui, @skillkit/mesh, @skillkit/messaging to optionalDependencies. Drop @skillkit/memory entirely — the CLI never imported it (only @skillkit/mcp-memory consumes it). Killing @skillkit/memory removes cozo-node (native Rust) and @xenova/transformers (~300MB ML stack) from the critical install path. Making @skillkit/tui optional removes @opentui/core's prebuild-install toolchain, which was the source of the inflight/gauge/npmlog/tar/glob deprecation warnings. Add skipNodeModulesBundle + platform=node + target=node18 to apps/skillkit tsup config so optional deps stay external. All TUI/mesh/messaging/api commands already lazy-import via dynamic import() with surrounding try/catch, so missing optional deps surface as a graceful error rather than a crash. Measured impact (npm i skillkit in a clean cache): Scenario Packages Time Deprecations Vulns (crit/high) main 544 37s 10 33 (5/5) slim (default) 475 18s 3 25 (0/0) slim --omit=optional 118 9s 0 0 Core commands (list, find, translate, recommend, install, --help, --version) smoke-tested in all three scenarios. Optional commands (ui, mesh, message, serve) confirmed to degrade with a clean "Cannot find package" error under --omit=optional.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (5)
📝 WalkthroughWalkthroughOptionalizing several feature packages, updating build target to Node 18 and skipping node_modules bundling, making TUI start dynamic, and enhancing CLI install onboarding to pre-detect agents and cancel when none are selected. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant InstallCmd as InstallCommand
participant Detector as detectAgent()
participant Prompt as quickAgentSelect
participant Installer as Installer
User->>InstallCmd: run `skillkit install`
InstallCmd->>Detector: detectAgent()
Detector-->>InstallCmd: agentId or undefined
InstallCmd->>Prompt: quickAgentSelect({ agents, detected: agentId })
Prompt-->>User: show options (detected / last / select / all)
alt user picks "detected"
Prompt-->>InstallCmd: { agents: [detected], method: "detected" }
else user picks "all"
Prompt->>User: confirm "Install for all?"
alt confirmed
Prompt-->>InstallCmd: { agents: allAgents, method: "all" }
else canceled
Prompt-->>InstallCmd: { agents: [], method: "all" }
end
else user picks "select" or "last"
Prompt-->>InstallCmd: { agents: selectedAgents, method: "select" }
end
InstallCmd->>InstallCmd: if agents empty -> cancel("Installation cancelled") & exit
InstallCmd->>Installer: proceed with chosen agents
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Before: first-time users (no history) saw "All detected agents"
as the first option in the agent picker. Mashing Enter would
install to all 32 agents, writing files across every adapter's
config dir. Selecting "Select specific agents" also pre-ticked
all 32 agents, requiring the user to un-tick by hand.
After:
- Default option is "Just <detected> (detected)" — one agent,
low blast radius.
- "Select specific agents" moves above "All detected agents"
and pre-ticks only the detected agent.
- "All detected agents" gets a second confirm gate
("Install to all N agents?") with initialValue: false.
- Hint text on "All" warns "use only if you know".
Returning users still see "Same as last time (Recommended)" as
the first option; their flow is unchanged.
Non-TTY path is unchanged (still auto-detects a single agent).
Wire detectAgent() through quickAgentSelect so the picker can
label and pre-select the detected agent. Empty selection from
the confirm gate is treated as cancellation.
| "clipanion": "^4.0.0-rc.4" | ||
| }, | ||
| "optionalDependencies": { | ||
| "@skillkit/api": "workspace:*", |
There was a problem hiding this comment.
🔴 Static re-export of optional dependency @skillkit/tui will crash the package entry point when the dep is not installed
apps/skillkit/src/index.ts:6 has a static re-export: export { startTUI } from '@skillkit/tui'. However, this PR moved @skillkit/tui from dependencies to optionalDependencies in apps/skillkit/package.json:62. Optional dependencies may not be installed, and a static import/re-export will throw a module-not-found error at load time, crashing the entire skillkit package's main entry point (./dist/index.js). This defeats the purpose of making the dependency optional. All other usages of the now-optional packages (in packages/cli/src) correctly use dynamic await import() inside try/catch blocks, but this static export was missed.
Prompt for agents
The file apps/skillkit/src/index.ts line 6 has a static re-export of @skillkit/tui:
export { startTUI } from '@skillkit/tui';
But @skillkit/tui was moved to optionalDependencies in apps/skillkit/package.json. When the optional dep is not installed, this static export will crash the entire module (the package's main entry point).
To fix this, either:
1. Remove the static re-export from apps/skillkit/src/index.ts entirely, or
2. Keep @skillkit/tui in regular dependencies (not optionalDependencies) in apps/skillkit/package.json, or
3. Replace the static re-export with a dynamic export pattern, e.g. export an async function that dynamically imports @skillkit/tui and returns startTUI.
Was this helpful? React with 👍 or 👎 to provide feedback.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Devin review caught a hard bug: apps/skillkit/src/index.ts had a
static re-export of startTUI from @skillkit/tui. Now that tui is an
optional dependency, importing the skillkit package would crash with
ERR_MODULE_NOT_FOUND whenever the optional dep was skipped
(--omit=optional). Convert the re-export to an async wrapper that
dynamically imports @skillkit/tui on call. The error is surfaced
naturally with a clear Cannot-find-package message.
Docs updates so users understand the new split:
- README.md: add "Install sizes" section with full vs slim table,
cold-install measurements (475 pkgs / 18 s vs 118 / 9 s),
per-feature package mapping, and how to add a single optional
package later.
- apps/skillkit/README.md (npm landing): add "What's optional"
table + install mode guidance.
- docs/fumadocs/content/docs/installation.mdx: add "Full vs slim
install" section covering all four optional features.
- docs/skillkit/components/Documentation.tsx +
docs/fumadocs/src/components/Documentation.tsx: add "Optional
Features" block to the Quick Start step on the website.
Verified: after `npm install --omit=optional`, `import('skillkit')`
succeeds with 622 exports loaded, and `await startTUI()` fails with
a clean "Cannot find package '@skillkit/tui'" message instead of
crashing at module-load time.
Two nits from CodeRabbit on the first-user picker: 1. detectAgent() returns 'universal' as a fallback when no concrete adapter is detected. Passing that through to quickAgentSelect made the picker show "Just Universal (detected)" even when no real agent was found — misleading. Treat 'universal' as "not detected" and fall through to the plain select/all options instead. 2. The "All detected agents" label on the bottom option was wrong: agents at the call site is getAllAdapters() (all supported adapters), not only detected ones. Rename to "All supported agents" and change the hint to "writes to every adapter" so users aren't tricked into thinking this is a safe detected-only install.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/skillkit/src/index.ts`:
- Around line 4-7: The wrapper widened the public API by accepting unknown args
and returning Promise<unknown>; restore the original signature by changing
export async function startTUI to accept no parameters and return the original
Promise<never> (or the exact return type of `@skillkit/tui.startTUI`), then
dynamically import '@skillkit/tui' and call mod.startTUI() with no arguments
(you can narrow the return with a type assertion to the original Promise<never>
if needed), ensuring the exported startTUI exactly matches the original public
signature and runtime behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e8ea350f-9900-47d5-96a8-bf4cdd2ac3fe
📒 Files selected for processing (6)
README.mdapps/skillkit/README.mdapps/skillkit/src/index.tsdocs/fumadocs/content/docs/installation.mdxdocs/fumadocs/src/components/Documentation.tsxdocs/skillkit/components/Documentation.tsx
✅ Files skipped from review due to trivial changes (5)
- docs/fumadocs/content/docs/installation.mdx
- docs/skillkit/components/Documentation.tsx
- README.md
- docs/fumadocs/src/components/Documentation.tsx
- apps/skillkit/README.md
| export async function startTUI(...args: unknown[]): Promise<unknown> { | ||
| const mod = await import('@skillkit/tui'); | ||
| return mod.startTUI(...(args as Parameters<typeof mod.startTUI>)); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Confirm the wrapper signature diverges from the actual `@skillkit/tui` startTUI signature.
printf '\napps/skillkit wrapper:\n'
rg -n -C2 'export async function startTUI' apps/skillkit/src/index.ts
printf '\n@skillkit/tui implementation:\n'
rg -n -C2 'export async function startTUI\(\): Promise<never>' packages/tui/src/index.tsxRepository: rohitg00/skillkit
Length of output: 499
Preserve the original startTUI public signature.
The dynamic import approach is correct, but this wrapper widens the API from startTUI(): Promise<never> to startTUI(...args: unknown[]): Promise<unknown>. The type cast on line 6 is compile-time only and does not affect runtime behavior. Since @skillkit/tui.startTUI takes no parameters, restore the wrapper to the original signature to avoid confusion and maintain API compatibility.
Suggested fix
-export async function startTUI(...args: unknown[]): Promise<unknown> {
+export async function startTUI(): Promise<never> {
const mod = await import('@skillkit/tui');
- return mod.startTUI(...(args as Parameters<typeof mod.startTUI>));
+ return mod.startTUI();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export async function startTUI(...args: unknown[]): Promise<unknown> { | |
| const mod = await import('@skillkit/tui'); | |
| return mod.startTUI(...(args as Parameters<typeof mod.startTUI>)); | |
| } | |
| export async function startTUI(): Promise<never> { | |
| const mod = await import('@skillkit/tui'); | |
| return mod.startTUI(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/skillkit/src/index.ts` around lines 4 - 7, The wrapper widened the
public API by accepting unknown args and returning Promise<unknown>; restore the
original signature by changing export async function startTUI to accept no
parameters and return the original Promise<never> (or the exact return type of
`@skillkit/tui.startTUI`), then dynamically import '@skillkit/tui' and call
mod.startTUI() with no arguments (you can narrow the return with a type
assertion to the original Promise<never> if needed), ensuring the exported
startTUI exactly matches the original public signature and runtime behavior.
| export async function startTUI(...args: unknown[]): Promise<unknown> { | ||
| const mod = await import('@skillkit/tui'); | ||
| return mod.startTUI(...(args as Parameters<typeof mod.startTUI>)); | ||
| } |
There was a problem hiding this comment.
🟡 Public API type regression: startTUI signature loosened from () => Promise<never> to (...args: unknown[]) => Promise<unknown>
The startTUI wrapper in apps/skillkit/src/index.ts uses ...args: unknown[] and returns Promise<unknown>, whereas the original re-export (export { startTUI } from '@skillkit/tui') exposed the actual signature (): Promise<never> (see packages/tui/src/index.tsx:25). This is a breaking change to the public TypeScript API of the skillkit package—any downstream consumer that relied on the return type being Promise<never> or the zero-argument signature will now get type errors. The wrapper could be correctly typed as (): Promise<never> since startTUI takes no arguments.
| export async function startTUI(...args: unknown[]): Promise<unknown> { | |
| const mod = await import('@skillkit/tui'); | |
| return mod.startTUI(...(args as Parameters<typeof mod.startTUI>)); | |
| } | |
| export async function startTUI(): Promise<never> { | |
| const mod = await import('@skillkit/tui'); | |
| return mod.startTUI(); | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/src/onboarding/prompts.ts`:
- Around line 192-199: The current logic builds initialValues by checking
lastSelected.length which treats a non-empty but fully stale saved selection as
valid and prevents falling back to the detected agent; update the computation
used before calling agentMultiselect so you first compute a filtered list (e.g.,
const filtered = lastSelected.filter(a => agents.includes(a))) and then set
initialValues = filtered.length > 0 ? filtered : hasDetected ? [detected!] : [];
change the reference to initialValues used by agentMultiselect accordingly so
when saved selections are stale it falls back to the detected agent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8d0428ae-71b1-4270-a4a8-0595f1aab399
📒 Files selected for processing (2)
packages/cli/src/commands/install.tspackages/cli/src/onboarding/prompts.ts
✅ Files skipped from review due to trivial changes (1)
- packages/cli/src/commands/install.ts
| const initialValues = lastSelected.length > 0 | ||
| ? lastSelected.filter(a => agents.includes(a)) | ||
| : hasDetected ? [detected!] : []; | ||
|
|
||
| const selected = await agentMultiselect({ | ||
| message: 'Select agents', | ||
| agents, | ||
| initialValues: lastSelected.length > 0 ? lastSelected : agents, | ||
| initialValues, |
There was a problem hiding this comment.
Fall back to the detected agent when saved selections are stale.
Line 192 checks raw lastSelected.length, so a non-empty but fully invalid saved selection produces initialValues: [] and skips the detected-agent default.
Suggested fix
- const initialValues = lastSelected.length > 0
- ? lastSelected.filter(a => agents.includes(a))
- : hasDetected ? [detected!] : [];
+ const validLastSelected = lastSelected.filter(a => agents.includes(a));
+ const initialValues = validLastSelected.length > 0
+ ? validLastSelected
+ : hasDetected ? [detected!] : [];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const initialValues = lastSelected.length > 0 | |
| ? lastSelected.filter(a => agents.includes(a)) | |
| : hasDetected ? [detected!] : []; | |
| const selected = await agentMultiselect({ | |
| message: 'Select agents', | |
| agents, | |
| initialValues: lastSelected.length > 0 ? lastSelected : agents, | |
| initialValues, | |
| const validLastSelected = lastSelected.filter(a => agents.includes(a)); | |
| const initialValues = validLastSelected.length > 0 | |
| ? validLastSelected | |
| : hasDetected ? [detected!] : []; | |
| const selected = await agentMultiselect({ | |
| message: 'Select agents', | |
| agents, | |
| initialValues, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/onboarding/prompts.ts` around lines 192 - 199, The current
logic builds initialValues by checking lastSelected.length which treats a
non-empty but fully stale saved selection as valid and prevents falling back to
the detected agent; update the computation used before calling agentMultiselect
so you first compute a filtered list (e.g., const filtered =
lastSelected.filter(a => agents.includes(a))) and then set initialValues =
filtered.length > 0 ? filtered : hasDetected ? [detected!] : []; change the
reference to initialValues used by agentMultiselect accordingly so when saved
selections are stale it falls back to the detected agent.
npx skillkit add <owner/repo> is the most common first-touch path, so call it out explicitly. Cover: - Full vs slim invocation (npx skillkit add vs npx --omit=optional skillkit add), with the same 118/9s slim numbers. - How the npx cache at ~/.npm/_npx/ makes subsequent runs instant until a new version ships. - When to graduate to a global install: skips the prompt-to-proceed and kills the per-release refetch cost. Updated in all four install-docs surfaces: root README, apps/skillkit README (npm landing), docs/fumadocs/content/docs/installation.mdx, and the Documentation.tsx components used on the website and the fumadocs mirror.
1 participant
Problem
`npx skillkit add …` was slow (~37s on cold cache) and printed 10
`npm warn deprecated` lines every install. Root cause: the CLI package
pulled a pile of workspace deps it rarely uses at runtime, including
`@skillkit/memory` (which the CLI never imported at all).
`@skillkit/memory` drags in `cozo-node` (native Rust binding) and
`@xenova/transformers` (~300 MB ML stack). `@skillkit/tui` drags in
`@opentui/core`, whose `prebuild-install` toolchain is the source of
the `inflight`/`gauge`/`npmlog`/`tar`/`glob` deprecation noise.
Change
entirely (only `@skillkit/mcp-memory` consumes it), move
`@skillkit/api`, `@skillkit/tui`, `@skillkit/mesh`, and
`@skillkit/messaging` to `optionalDependencies`.
`platform: 'node'`, `target: 'node18'` so optional deps remain
external in the built bundle.
All optional-feature commands (`ui`, `mesh`, `message`, `serve`)
already `await import()` their optional package inside a `try/catch`,
so missing deps surface as a clean error, not a crash.
Measured impact
Cold `npm install skillkit@1.22.1` in a fresh cache:
`pnpm-lock.yaml` shrinks by ~3,500 lines.
Test plan
`list`, `find`, `translate`, `recommend`, `install` all run.
Optional commands emit a friendly error:
`Cannot find package '@skillkit/tui' imported from …`
`Failed to initialize mesh: Cannot find package '@skillkit/mesh' …`
`Failed to start server: Cannot find package '@skillkit/api' …`
`Failed to send message: Cannot find package '@skillkit/messaging' …`
`skillkit mesh status`, `skillkit message inbox` all work.
Summary by CodeRabbit
Chores
New Features
Documentation