feat(cli): npm tarball dist/docs support with monorepo disambiguation#35
Merged
Conversation
…stra-ai/mastra These libraries ship author-curated agent docs in dist/docs of their npm tarballs. Registering them with an explicit npm strategy lets ASK pull docs from node_modules (or the tarball) instead of GitHub, matching the installed version exactly. Refs #33
An npm strategy carrying an explicit docsPath is treated as author-curated (e.g. vercel/ai's dist/docs) and outranks every other strategy in the same entry. Without docsPath, the static SOURCE_PRIORITY table still wins, so non-curated npm entries do not regress github-first behavior. This unblocks the existing vercel/next.js entry that was already shipping an npm strategy with dist/docs but was being shadowed by the github strategy in selectBestStrategy. Refs #33
When the requested package is already installed and its package.json version satisfies the request, NpmSource.fetch now reads docs directly from node_modules/<pkg>/<docsPath> and skips the tarball download entirely. This makes 'ask docs add npm:ai' work fully offline whenever the package is in the project's dependency tree. The npm lock entry schema now accepts either tarball (network fetch) or installPath (local read). Callers must supply at least one — the invariant is enforced in buildLockEntry rather than via Zod refine because discriminatedUnion does not accept ZodEffects branches. NpmSource is no longer marked @deprecated. The deprecation rationale (prefer resolver + github source) does not apply to author-curated dist/docs cases. Refs #33
When the .ask/docs/<pkg> directory is missing or stale, the agent now
has explicit guidance to walk node_modules/<pkg>/{dist/docs,docs,*.md}
as a discovery path, and to suggest 'ask docs add npm:<pkg>' for formal
registration once usable docs are found.
This is the Tier 2 of the npm-tarball-docs strategy: Tier 1 is the
curated registry entry, Tier 2 is the LLM walking node_modules at
runtime when no entry exists.
Refs #33
- design/telemetry.md: full opt-in telemetry design for the future Registry auto-promotion loop, with prior-art analysis of vercel-labs/skills src/telemetry.ts. No code in this track. - CLAUDE.md gotchas: document the curated-npm > github selection rule and the local-first NpmSource behavior with the new installPath lock entry shape. - plan.md: mark all 15 tasks complete. Refs #33
Make Step 3 of add-docs explicit: when the package appears in the project's manifest/lockfile, prefer the ecosystem prefix (npm:<name>) so the CLI's NpmSource can short-circuit to the local node_modules read with no network call. Only fall back to GitHub shorthand when there is no manifest evidence. Refs #33
The CLI is deterministic — it does not fall back between sources. The LLM driving add-docs must read the CLI error, classify it, and retry with a different spec when appropriate. The most common case: an npm strategy was selected but the tarball did not actually ship the curated docs directory; the skill must then resolve the package's GitHub repo and retry as <owner>/<repo>. Resource ladder for finding owner/repo uses only always-available resources (training knowledge, node_modules/<pkg>/package.json, WebFetch of registry.npmjs.org, WebSearch, AskUserQuestion). MCP servers like deepwiki or context_grep are intentionally excluded — we cannot assume they are installed in the user's environment. Includes a recovery decision tree per error class, a cost-ordered resource ladder, and explicit limits (max 1 retry, preserve user version intent, always report which path was taken). Refs #33
The Recovery section's error classification table, resource ladder, and worked examples are only needed when the CLI actually fails — which should be rare given proper upfront planning in Step 3. Keep SKILL.md focused on the happy path and link out for the exception path. Refs #33
.claude/agent-memory/ is version controlled per CLAUDE.md so agent discoveries persist across sessions. These files were left untracked from a prior session.
The mastra-ai/mastra registry entry holds two npm strategies, one for @mastra/core and one for @mastra/memory. selectBestStrategy was picking the first curated npm regardless of which package the user asked for, so 'ask docs add npm:@mastra/memory' returned @mastra/core docs. Fix: - selectBestStrategy now accepts an optional context with requestedPackage. When set and a curated npm strategy has a matching package field, that strategy wins (Rule 1). The previous 'first curated npm' behavior is retained as Rule 2 fallback. - resolveFromRegistry threads the requested name into selectBestStrategy. - For monorepo entries (>1 npm strategy), the resolved name is the requested package name rather than the entry's display name, so docs for @mastra/core and @mastra/memory are saved under distinct slugs instead of overwriting each other. Caught by /review:code-review-loop iteration 1. Refs #33
When the monorepo disambiguation path returns the requested package name
(e.g. '@mastra/memory'), that name flows into both the storage path
('.ask/docs/<name>@<ver>/') and the Claude Code skill name. Both
surfaces reject scoped-name characters: '@' and '/' would create stray
nested directories and the Claude Code skill name regex is [a-z0-9-]+
only.
slugifyPackageName converts '@scope/pkg' → 'scope-pkg' for both
surfaces. Unscoped names pass through unchanged so existing entries
keep their slugs.
Caught by /review:code-review-loop iteration 2.
Refs #33
The registry server is the right place to handle scoped-package disambiguation for monorepo entries. It already has full context (which alias matched, the entry, all strategies) and serving every client (CLI today, future SDKs) from one source of truth is cleaner than asking each client to reimplement the rule. Server changes: - New disambiguateStrategies(all, requestedPackage): for alias-based lookups, reorder strategies so the curated npm strategy whose package field matches the requested alias comes first. - New slugifyPackageName: converts @scope/pkg -> scope-pkg. - New resolvedName response field: server-computed slug for monorepo entries, falls back to entry display name otherwise. Client changes: - selectBestStrategy reverts to single-arg signature; server has already put the right strategy first. - slugifyPackageName removed from client (lives on server now). - resolveFromRegistry trusts entry.resolvedName and falls back to entry.name for backward compat with older server responses. - New RegistryApiResponse type augments RegistryEntry with resolvedName. Tests: - Drop ctx-based and slugify tests (moved server-side). - Add test documenting that selectBestStrategy honors server's declared order for monorepo entries. Refs #33
17 tasks
17 tasks
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Deploying ask-registry with
|
| Latest commit: |
90087cc
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b81c156a.ask-registry.pages.dev |
| Branch Preview URL: | https://33-track-npm-tarball-docs-20.ask-registry.pages.dev |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 22 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/cli/src/sources/npm.ts">
<violation number="1" location="packages/cli/src/sources/npm.ts:93">
P1: Validate `docsPath` stays inside the installed package directory before reading from disk to prevent path traversal in local-first docs loading.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Agent as Agent / User
participant CLI as CLI (Registry & Dispatcher)
participant RegSvr as Registry API (Nuxt)
participant NpmSrc as NpmSource (Local-First)
participant LocalFS as Local node_modules
participant NpmReg as npm Registry (Tarball)
Note over Agent,NpmReg: Documentation Addition Flow (npm Strategy)
Agent->>CLI: ask docs add npm:<package>
CLI->>RegSvr: NEW: GET /api/registry/ecosystem/name
Note over RegSvr: Disambiguate strategies<br/>(Move matching npm to head)
RegSvr-->>CLI: Entry + NEW: resolvedName + prioritized strategies
CLI->>CLI: CHANGED: selectBestStrategy()<br/>(Prioritize curated npm + docsPath)
CLI->>NpmSrc: fetch(strategy)
NpmSrc->>LocalFS: NEW: tryLocalRead(node_modules/pkg/docsPath)
alt NEW: Local docs found (Satisfies version)
LocalFS-->>NpmSrc: return markdown files
Note right of NpmSrc: Meta: source = 'node_modules'
else Local miss or version mismatch
NpmSrc->>NpmReg: download tarball
NpmReg-->>NpmSrc: tarball stream
NpmSrc->>NpmSrc: extract docsPath
end
NpmSrc-->>CLI: FetchResult (files, resolvedVersion)
CLI->>CLI: CHANGED: buildLockEntry()
Note right of CLI: NEW: Lock accepts 'installPath' (local)<br/>OR 'tarball' (network)
CLI->>LocalFS: Write .ask/docs/<resolvedName>@<ver>/
CLI-->>Agent: Success
Note over Agent,LocalFS: Tier 2 Fallback (No Registry Entry)
opt Registry 404
CLI->>CLI: NEW: generate SKILL.md with fallback instructions
CLI-->>Agent: Generated skill guides Agent to walk node_modules
Agent->>LocalFS: NEW: Agent manually scans node_modules/<pkg>/{dist/docs,docs,*.md}
end
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
2 issues found across 6 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".please/docs/product-specs/cli/sources.md">
<violation number="1" location=".please/docs/product-specs/cli/sources.md:24">
P2: Use `docsPath` consistently in the normative requirement text; `docspath` conflicts with the stated field name and can produce incorrect implementations.</violation>
<violation number="2" location=".please/docs/product-specs/cli/sources.md:106">
P2: The requirement body conflicts with its MUST-level heading (`SHOULD` vs `MUST`) and changes contract naming; align the normative sentence to avoid ambiguous implementation expectations.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
…Path cubic-dev-ai review on PR #35 flagged that path.join(pkgDir, docsPath) could escape the installed package directory if a registry strategy declared a malicious docsPath like '../../../etc/passwd'. Although docsPath comes from a trusted registry, defense-in-depth is the right move — the local-first read now path.resolve()s the join and rejects anything that lands outside pkgDir or is absolute. Two tests added covering both rejection cases. Also remove the malformed product-specs/cli/sources.md generated by sync-product-specs during finalize. The script (in the please plugin cache, not this repo) corrupts requirement bodies (lowercases code identifiers, downgrades MUST → SHOULD, double-period typos). Will be regenerated correctly once the upstream generator is fixed. Refs #33
Contributor
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/cli/src/sources/npm.ts">
<violation number="1" location="packages/cli/src/sources/npm.ts:98">
P1: The new docsPath boundary check is bypassable via symlinks because it compares unresolved paths. Validate containment using real paths (`fs.realpathSync`) after existence checks.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
cubic-dev-ai noted that the previous string-level check is bypassable via symlinks: a symlink at node_modules/<pkg>/dist/docs pointing to /etc would lexically pass the path.relative check but still escape the package directory at read time. Two-stage check now: 1. String-level pre-check (cheap, blocks obvious traversal before touching the filesystem). 2. fs.realpathSync on both pkgDir and docsDir after existence check, re-running the containment test against the resolved real paths. Wrapped in try/catch so a broken symlink (realpath throws) is treated as a miss rather than crashing the read. New test plants a symlink at node_modules/ai/dist/docs pointing to a temp dir outside the project and asserts tryLocalRead returns null. Refs #33
Contributor
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Requires human review: This PR introduces significant logic changes to core CLI source resolution, Registry API response shapes, and the lockfile schema, which requires a human review for potential side effects.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #33.
Make ASK exploit
dist/docs/directories that npm libraries (ai,@mastra/core,@mastra/memory,nextcanary, ...) ship inside their published tarballs. Today the CLI delegates npm specs to the GitHub source even when an npm strategy is registered, and theNpmSourceclass is@deprecatedand lacks any local-node_modules shortcut. The result is unnecessary network IO and missed first-party docs.What changed
Two-tier strategy:
docsPathin the ASK Registry, the CLI executes it deterministically. Seed entries added forvercel/aiandmastra-ai/mastra(with@mastra/coreand@mastra/memoryaliases).*-docsSKILL.md guides the agent to walknode_modules/<pkg>/{dist/docs,docs,*.md}at runtime and suggest formal registration viaask docs add npm:<pkg>.CLI changes:
SOURCE_PRIORITYwith rule-layeredselectBestStrategy— a curated npm strategy (source: npmwithdocsPath) outranks github regardless of declaration order. WithoutdocsPath, the static priority table still wins.NpmSource.fetchis now local-first: it readsnode_modules/<pkg>/<docsPath>directly when the installedpackage.jsonversion satisfies the request, and only falls through to a tarball download on miss.NpmSourceis no longer marked@deprecated.NpmLockEntryschema accepts eithertarball(network fetch) orinstallPath(local read); the invariant is enforced inbuildLockEntry.Server-side changes (
apps/registry/server/api/registry/[...slug].get.ts):ecosystem/name), the server now disambiguates strategies — the curated npm strategy whosepackagefield matches the requested alias is moved to the head of the response.resolvedNamefield returned in the response: for monorepo entries (multiple npm strategies under one repo), the server returns a slugified package name (@mastra/memory→mastra-memory); for non-monorepo entries it equals the entry's display name. This keeps the disambiguation rule centralized in one place that all clients (CLI today, future SDKs tomorrow) benefit from.User-facing skill (
skills/add-docs/):references/recovery.mdso SKILL.md stays focused on the happy path.Telemetry design:
design/telemetry.mdin the track directory captures the opt-in model, collected fields, aggregation flow, and prior-art analysis ofvercel-labs/skills/src/telemetry.ts. Implementation is a follow-up track.Key files
packages/cli/src/registry.ts—selectBestStrategyrule layering,RegistryApiResponsetype,resolvedNameconsumptionpackages/cli/src/sources/npm.ts—tryLocalRead+ tarball fallbackpackages/cli/src/index.ts—buildLockEntryacceptstarballorinstallPathpackages/schema/src/lock.ts—NpmLockEntryshape relaxedpackages/cli/src/skill.ts— fallback section in generated SKILL.mdapps/registry/server/api/registry/[...slug].get.ts— server-side disambiguation + slugifyapps/registry/content/registry/{vercel/ai,mastra-ai/mastra}.md— seed registry entriesskills/add-docs/SKILL.md+references/recovery.md— user-facing skill update.please/docs/tracks/active/npm-tarball-docs-20260408/design/telemetry.md— telemetry designNotable findings during implementation
source: 'npm'+docsPath— the real blocker wasSOURCE_PRIORITY = { github: 0, npm: 1 }killing the existingvercel/next.jsentry that was already shipping the npm strategy. Fixing the priority lit up dead code.getSource()already dispatched toNpmSource, so no separate dispatcher was needed once the priority bug was fixed./review:code-review-loopcaught and fixed: (1) the mastra entry conflated@mastra/coreand@mastra/memory, (2) the resulting scoped names broke filesystem and skill name slugs. Both fixes were then refactored to live on the registry server instead of in the CLI client.Test plan
tsc)eslint)nuxt buildwithexperimental.sqliteConnector: native)bun add ai, runnode packages/cli/dist/index.js docs add npm:aiand verify the log showsUsing local node_modules for ai@<ver>and.ask/docs/...mirrorsnode_modules/ai/dist/docsnpm:@mastra/coreandnpm:@mastra/memory— confirm they land in distinct slugs (e.g.mastra-core@<ver>andmastra-memory@<ver>)ask docs syncon a project withlodash,axios,jqueryproduces the same file count as before this branchFollow-up tracks (out of scope here)
feat(registry): edge caching for /api/registry/** route/api/raw/<ecosystem>/<name>)Summary by cubic
Adds npm tarball
dist/docssupport with a localnode_modulesfast path and monorepo-aware disambiguation, preferring curated npm docs over GitHub. Also hardensNpmSourcewith a strongerdocsPathtraversal guard. Closes #33.New Features
source: 'npm'withdocsPath) over GitHub; otherwise the existing priority table applies.NpmSourceis local-first: readsnode_modules/<pkg>/<docsPath>when the installed version satisfies the request, falling back to tarball; lock entries may recordinstallPath(local) ortarball(network).resolvedName(slug, e.g.mastra-memory) for consistent client naming; seeded entries forvercel/aiandmastra-ai/mastraenable the npm fast path.node_modules/<pkg>/{dist/docs,docs,*.md}and suggestsask docs add npm:<pkg>;skills/add-docsdocuments the npm-vs-GitHub decision tree.Bug Fixes
NpmSource.tryLocalRead: ensuredocsPathresolves insidenode_modules/<pkg>and reject absolute/escaping paths, including symlink-based escapes via a realpath containment check; tests cover both cases.Written for commit 90087cc. Summary will update on new commits.