feat(cli): in-place npm docs — reference node_modules directly#59
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
0 issues found across 4 files (changes from recent commits).
Requires human review: This PR introduces a significant architectural change in how documentation is stored and referenced (in-place node_modules), modifying core installation and discovery logic.
There was a problem hiding this comment.
1 issue found across 18 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/schema/src/resolved.ts">
<violation number="1" location="packages/schema/src/resolved.ts:32">
P2: Enforce the schema invariant that `inPlacePath` is required when `materialization` is `'in-place'` to avoid accepting inconsistent resolved entries.</violation>
</file>
Architecture diagram
sequenceDiagram
participant User as CLI User
participant CLI as CLI Command (index.ts)
participant Install as Install Service (install.ts)
participant Discovery as Local Discovery
participant NPM as node_modules
participant AskStorage as .ask/docs/ (Vendored)
participant ResolvedStore as resolved.json (Cache)
participant AgentsMD as AGENTS.md
User->>CLI: ask install [--no-in-place]
CLI->>Install: runInstall(options)
Note over CLI,Install: NEW: Resolve inPlace priority:<br/>CLI flag > ask.json > default(true)
loop Each Library
Install->>Discovery: runLocalDiscovery()
Discovery->>NPM: Check conventions (dist/docs, etc.)
alt NEW: In-place allowed AND docs found in node_modules
NPM-->>Discovery: Found docs path
Discovery-->>Install: DocsDiscoveryResult (inPlace: true)
Install->>Install: NEW: materializeInPlace()
Install->>AskStorage: NEW: removeDocs() (Clean up any stale copies)
opt Force or Version Mismatch
Install->>ResolvedStore: NEW: Update entry (materialization: 'in-place', inPlacePath)
end
else Standard Copy Path (Opt-out or Not Found)
Discovery-->>Install: Not found or disabled
Install->>Install: materialize (Copy pipeline)
Install->>AskStorage: saveDocs() (Download/Copy to .ask/docs/)
Install->>ResolvedStore: Update entry (materialization: 'copy')
end
end
Install->>AgentsMD: generateAgentsMd()
AgentsMD->>ResolvedStore: Read materialization metadata
alt CHANGED: Entry is 'in-place'
AgentsMD->>AgentsMD: Emit "shipped by package" block<br/>pointing to node_modules
else Entry is 'copy'
AgentsMD->>AgentsMD: Emit standard block<br/>pointing to .ask/docs/
end
Install-->>User: Install Summary
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
0 issues found across 5 files (changes from recent commits).
Requires human review: Significant feature change modifying core installation logic and default storage behavior. Changes the materialization strategy for npm docs and touches critical paths in the CLI.
When convention-based discovery finds docs shipped inside an npm package (e.g. node_modules/next/dist/docs/), ASK now references them in place instead of copying into .ask/docs/. This eliminates disk duplication and keeps docs in sync with `bun install` automatically. - Add `inPlace?: boolean` to ask.json schema (default true) - Add `--no-in-place` CLI flag for install/add commands - Add `materialization` and `inPlacePath` to resolved entry schema - Discovery adapters set `inPlace: true` on local results - Install orchestrator branches on in-place for skip-copy path - AGENTS.md differentiates in-place blocks with "shipped by the package" - Clean up stale .ask/docs/ on transition from copy to in-place - Integration tests for SC-1 through SC-8 Closes #56
…place Add superRefine to ResolvedEntrySchema ensuring inPlacePath is present when materialization is 'in-place'. Addresses cubic review suggestion.
SC-4 used the real `lodash` package, so the fall-through path triggered a real npm tarball download via `curl -sL`. On slower CI this exceeded the 5s bun:test default and the test timed out. The assertion only needs to prove in-place discovery returned null and the entry did not materialize as in-place — it does not need a real tarball. Switch to a fake package name so `npm view` 404s quickly, matching the pattern already used by SC-5/SC-6. Local file runtime: 4.20s → 1.99s.
d6c3343 to
da010cd
Compare
Summary
node_modules/next/dist/docs/), ASK now references them in place instead of copying into.ask/docs/--no-in-placeCLI flag andask.jsoninPlacefield for opt-out (precedence: CLI > ask.json > defaulttrue)bun installkeeps them in sync" wordingKey changes
ask-json.ts,resolved.tsinPlace?: boolean,materialization,inPlacePathfieldstypes.ts,local-conventions.ts,local-ask.tsinPlace?: truemarker on discovery resultsinstall.tsmaterializeInPlace()— skip saveDocs, clean stale dirs, stamp resolved cacheagents.ts,storage.tsindex.ts--no-in-placeflag oninstall/addcommandsCloses #56
Test plan
bun run build— all packages compilebun run --cwd packages/schema test— 57 passbun run --cwd packages/cli test— 240 pass (10 new)bun run --cwd packages/cli lint— cleanask installon a project withnpm:next(canary withdist/docs/) produces in-place AGENTS.mdask install --no-in-placeon the same project copies into.ask/docs/ask remove npm:nexton an in-place entry cleans resolved entry without touchingnode_modules/Summary by cubic
ASK now references docs shipped inside npm packages directly from
node_modulesinstead of copying to.ask/docs/, cutting disk use and staying in sync with your lockfile. Adds an opt‑out via--no-in-placeand anask.jsoninPlacefield, and enforces schema validation for in‑place entries. Closes #56.New Features
node_modules/<pkg>/<subdir>/, skip copy and recordmaterialization: 'in-place'withinPlacePath.--no-in-placeonask installandask addto force copy.ask.jsonsupportsinPlace?: boolean(defaulttrue).bun installkeeps them in sync”..ask/docs/<pkg>@*/when switching from copy to in‑place.inPlacePathwhenmaterializationis'in-place'.Migration
ask install --no-in-placeor set"inPlace": falseinask.json(precedence: CLI >ask.json> default).Written for commit da010cd. Summary will update on new commits.