Skip to content

fix(cli): accept source prefixes in the wizard package prompt - #86

Closed
mrrobertkent wants to merge 1 commit into
skilld-dev:mainfrom
mrrobertkent:fix/wizard-source-prefixes
Closed

fix(cli): accept source prefixes in the wizard package prompt#86
mrrobertkent wants to merge 1 commit into
skilld-dev:mainfrom
mrrobertkent:fix/wizard-source-prefixes

Conversation

@mrrobertkent

Copy link
Copy Markdown
Contributor

Problem

The interactive wizard rejects the source prefixes documented for skilld add, with a stack trace rather than a message:

◇  Enter package names (space or comma-separated)
│  npm:unlighthouse
│
◑  npm:unlighthouse: npm registryError: Invalid package name: npm:unlighthouse
    at getCacheDir (dist/_chunks/prepare.mjs:15:40)
    at resolvePkgDir (dist/_chunks/prepare.mjs:30:29)
    at getShippedSkills (dist/_chunks/prepare.mjs:53:18)
    at handleShippedSkills (dist/_chunks/skill-installer2.mjs:14:24)
    at npmResolver (dist/_chunks/sync.mjs:34:20)

■  Canceled

add normalizes its inputs:

// src/commands/sync/add.ts
const items = rawInputs.map(parseSkillInput)

The wizard does not:

// src/cli.ts
selected = input.split(COMMA_OR_WHITESPACE_RE).map(s => s.trim()).filter(Boolean)

So npm:unlighthouse travels as a literal package name until getCacheDir rejects it, and the throw takes down the whole run.

Change

Normalize the same way add does, at both manual-entry prompts. npm: prefixed and bare names resolve; other sources report where they belong instead of failing later:

gh:owner/repo is not an npm package. Install it with `skilld add gh:owner/repo`.

Second, resolvePkgDir no longer lets the validation error escape. It probes for an optional cache hit, so a malformed name is a miss. Traversal is still rejected, it just no longer aborts a sync from a read path. getShippedSkills is the only caller that could reach it with unvalidated input, and it already treats "not found" as empty.

Testing

test/unit/pkg-dir-probe.test.ts covers the probe returning null for npm:vue, gh:owner/repo, and ../escape rather than throwing, and getShippedSkills reporting none.

Normalization verified against the wizard's own splitting:

npm:unlighthouse           -> ["unlighthouse"]
unlighthouse               -> ["unlighthouse"]
vue npm:nuxt pinia         -> ["vue","nuxt","pinia"]
gh:harlan-zw/unlighthouse  -> rejected with guidance
crate:serde                -> rejected with guidance
Test Files  57 passed (57)
     Tests  908 passed (908)

pnpm typecheck clean.

Docs

README notes that the same prefixes work in the wizard prompt, and the placeholder now shows one (vue npm:nuxt pinia).

Out of scope

resolvePkgDir('', cwd, v) returns <cwd>/node_modules, because join(cwd, 'node_modules', '') exists. No caller passes an empty name, so I left it alone rather than widen this PR.

The wizard passed manually-entered packages through untouched, so the `npm:`
prefix documented for `skilld add` crashed instead of resolving:

  Enter package names > npm:unlighthouse
  Error: Invalid package name: npm:unlighthouse
      at getCacheDir (core/prepare.ts)
      at getShippedSkills

`add` normalizes with `parseSkillInput`; the wizard did not. Do the same there,
and reject non-npm sources with a message pointing at `skilld add`.

Also stop `resolvePkgDir` propagating the validation error. It probes for an
optional cache hit, so a malformed name is a miss. The guard still rejects
traversal, it just no longer aborts the whole sync from a read path.
@mrrobertkent

Copy link
Copy Markdown
Contributor Author

Closing in favour of a clean branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant