feat(cli): convention-based docs discovery pipeline#48
Merged
Conversation
…01, T002) Phase 1 of convention-based-discovery-20260409 (#46). - NpmLockEntry gains optional format?: 'docs' | 'intent-skills' so intent-format lock entries can be distinguished from docs-format entries at sync/remove time. Default remains 'docs' for backwards compatibility. - Pin @tanstack/intent@0.0.29 as a runtime dep so the upcoming local-intent adapter can call scanLibrary / findSkillFiles / parseFrontmatter via its programmatic export.
Phase 2 of convention-based-discovery-20260409 (#46). New `packages/cli/src/discovery/` module implementing the convention scanner layer that runs before the central registry lookup: - types.ts DiscoveryResult discriminated union, adapter shapes - conventions.ts local + repo path tables, exclusion filter - quality.ts >=3 md files OR >=4 KiB threshold scorer (SC-3 guard) - local-ask.ts reads package.json.ask.docsPath opt-in - local-intent.ts wraps @tanstack/intent findSkillFiles/parseFrontmatter with zod runtime validation on the frontmatter - local-conventions dist/docs -> docs, README fallback with a warning - repo-conventions post-tarball scan of github repo trees - index.ts runLocalDiscovery / runRepoDiscovery orchestrators Adapters are not wired into the CLI dispatcher yet — that is T013-T014 in the next phase. This commit is pure additive code: build and lint stay green and no existing behaviour changes.
…e (T011-T013, T015-T016) Phase 3 of convention-based-discovery-20260409 (#46). Wires the Phase 2 discovery adapters into the CLI dispatcher and adds the AGENTS.md intent-skills block writer. T014 (repo-conventions via github source) is deferred — see Surprises & Discoveries in the plan. - agents-intent.ts (new) upsertIntentSkillsBlock / removeFromIntentSkillsBlock manage the <!-- intent-skills:start --> ... <!-- intent-skills:end --> marker block on a byte range strictly disjoint from the existing BEGIN:ask-docs-auto-generated block. Preserves foreign-package entries on upsert; strips the whole block when the last entry for a package is removed. - skill.ts generateSkill gains an optional GenerateSkillOptions.docsDir. When set, the skill file references the provided dir in place and omits the 'When the docs cannot be found' fallback section. - index.ts New handleLocalDiscovery helper dispatches DiscoveryResult: - kind: 'docs' runs the existing ask pipeline with a synthetic FetchResult (installPath propagated to the lock entry). - kind: 'intent-skills' records an npm lock entry with format: 'intent-skills' and upserts the marker block only — no .ask/docs/ copy, no .claude/skills/ generation. addCmd.run calls runLocalDiscovery for `npm:` ecosystem specs with no --source / --docs-path override, before the github fast-path and the registry auto-detect. On hit, returns early. removeCmd.run branches on the lock entry's format: intent-skills entries call removeFromIntentSkillsBlock and drop the lock row; 'docs' entries keep the existing delete path. runSync adds a second pass that iterates lock entries with format: 'intent-skills', re-runs localIntentAdapter against each installed package, and refreshes the marker block + lock. Build, lint, and the 237 existing tests stay green.
…017-T024) Phase 4 of convention-based-discovery-20260409 (#46). - test/discovery/quality.test.ts (new, 9 cases) Covers the SC-3 guard: noise-only (CONTRIBUTING.md + CHANGELOG.md + LICENSE) repos score below threshold and fail through. Also verifies the >=3 count and >=4 KiB byte fallbacks, nested walking, .mdx support, and the LICENSE/CODE_OF_CONDUCT/SECURITY exclusions. - test/discovery/adapters.test.ts (new, 12 cases) localAskAdapter: no manifest -> null, valid ask.docsPath -> docs result with installPath, broken path -> null. localConventionsAdapter: dist/docs selection with quality pass, README fallback when conventions are noise-only, SC-3 noise repo returns null. runLocalDiscovery: priority order (local-ask > local-conventions), explicitDocsPath bypass, missing-package null. - test/agents-intent.test.ts (new, 12 cases) upsertIntentSkillsBlock: creates file, is idempotent, preserves siblings, replaces only target entries, preserves bytes outside the block, handles scoped packages via load-path prefix match, escapes double quotes and backslashes. removeFromIntentSkillsBlock: returns false when absent, strips only target entries, strips the whole block when the last entry is removed. Fixture tasks T017-T020 are satisfied via inline tmp fixtures inside the adapter / quality tests (bun:test pattern used throughout this project; no new packages/cli/test/fixtures/ directories). T024 orchestration tests are consolidated into the `runLocalDiscovery` describe block in adapters.test.ts. T025 integration: the 237 pre-refactor tests still pass unchanged, which satisfies SC-4. An end-to-end `ask docs add npm:<pkg>` walk- through against the new fixtures is deferred to a follow-up. Build, lint, and all 267 tests (237 pre-existing + 30 new) green.
… (T026, T029, T030) Phase 5 closeout of convention-based-discovery-20260409 (#46). - packages/cli/scripts/audit-coverage.ts (new) Scaffold for SC-1: walks apps/registry/content/registry/**/*.md, extracts {owner, repo, npm alias, docsPath} from each entry, and for every entry with an npm alias runs runLocalDiscovery against the installed package in node_modules. Emits one JSON line per entry plus a summary row, exits 1 when coverage < 80 %. The live run (T027) requires all 37 registry packages installed locally and is deferred to a follow-up CI job. - CLAUDE.md New Gotchas entries describing the discovery pipeline order, the dual AGENTS.md marker blocks (ask-docs-auto-generated vs intent-skills), and the NpmLockEntry.format field. Existing entries about curated-npm strategy and other layers are untouched. - T029 verification: tsc + eslint + 267 bun:test cases all green across packages/cli after the full Phase 1-5 landing. Status of deferred work: - T014 repo-conventions wiring through GithubSource - T025 end-to-end ask docs add integration test - T027 live coverage audit run - T028 live intent-CLI parity diff All four are bounded, low-risk follow-ups with the core path in place. See Surprises & Discoveries in the plan for details.
… blocks Self-review add-on for convention-based-discovery-20260409 (#46). Hardens the 'marker isolation' invariant documented in the spec: neither writer (agents.ts BEGIN:ask-docs-auto-generated nor agents-intent.ts intent-skills:start) is allowed to touch the other region. The new case seeds AGENTS.md with a real ask-docs block, runs an intent-skills upsert + remove cycle, and asserts the ask block is preserved byte-for-byte across both operations.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Two Important findings from gemini review of PR #48: - agents-intent.ts readExistingBlock: anchor END_MARKER search after BEGIN_MARKER. Without the offset, a malformed AGENTS.md with two intent-skills blocks (e.g. from a failed partial write) could match an END_MARKER belonging to a different block and produce a corrupt splice. - repo-conventions.ts collectDocFiles: add MAX_WALK_DEPTH=20 guard against symlink loops the tarball extractor failed to resolve and pathological monorepo layouts. Real docs trees rarely exceed ~6 levels deep, so 20 is both generous and bounded. Build, lint, and 268 tests still green.
Contributor
There was a problem hiding this comment.
6 issues found across 21 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/discovery/conventions.ts">
<violation number="1" location="packages/cli/src/discovery/conventions.ts:60">
P2: Meta-file exclusion uses case-sensitive exact matches, allowing lowercase/mixed-case CONTRIBUTING/CHANGELOG files to bypass filtering and inflate doc quality scoring.</violation>
</file>
<file name="packages/cli/scripts/audit-coverage.ts">
<violation number="1" location="packages/cli/scripts/audit-coverage.ts:43">
P2: Npm alias parsing only recognizes "- npm:<name>" list items, but registry entries store aliases as objects (ecosystem/name). The regex never matches, leaving npmName undefined and marking all entries as uncovered.</violation>
</file>
<file name="packages/cli/src/discovery/local-conventions.ts">
<violation number="1" location="packages/cli/src/discovery/local-conventions.ts:38">
P1: Validate convention path realpath containment before calling `scoreDirectory`; currently a symlinked docs directory can cause out-of-package recursive traversal during scoring.</violation>
</file>
<file name="packages/cli/src/index.ts">
<violation number="1" location="packages/cli/src/index.ts:789">
P1: `runSync` exits early when `config.docs` is empty, so the newly added intent-skills lockfile resync pass never runs for intent-only projects.</violation>
</file>
<file name="packages/cli/test/discovery/adapters.test.ts">
<violation number="1" location="packages/cli/test/discovery/adapters.test.ts:118">
P3: This assertion block is conditionally skipped for non-`docs` results, so the test can false-pass if discovery returns `intent-skills` unexpectedly.</violation>
</file>
<file name="packages/cli/src/agents-intent.ts">
<violation number="1" location="packages/cli/src/agents-intent.ts:52">
P2: `unescapeDq` performs two sequential unescapes, which drops a literal backslash before a quote (e.g., `foo\"bar` decodes to `foo"bar`). This makes round-tripping existing intent entries lossy when a task or load contains `\"`. Decode with a single-pass escape parser so `\\` and `\"` are handled without double-processing.</violation>
</file>
Architecture diagram
sequenceDiagram
participant U as User (CLI)
participant C as CLI Orchestrator
participant D as Discovery Module
participant FS as Local File System (node_modules)
participant R as Central Registry API
participant AM as AGENTS.md
participant S as Storage (.ask/docs & .claude/skills)
Note over U,S: ask docs add npm:<package>
U->>C: Execute "add" command
rect rgb(240, 240, 240)
Note over C,D: NEW: Convention-Based Discovery Pipeline
C->>D: runLocalDiscovery(pkg)
D->>FS: NEW: local-ask (check package.json.ask.docsPath)
FS-->>D: manifest data
opt No manifest
D->>FS: NEW: local-intent (check keywords & scan skills)
FS-->>D: intent skill files
end
opt No intent skills
D->>FS: NEW: local-conventions (scan dist/docs, docs, README)
D->>D: NEW: run quality scoring (>=3 files or >=4KiB)
FS-->>D: conventional files
end
D-->>C: DiscoveryResult (kind: 'docs' | 'intent-skills' | null)
end
alt Discovery Found Results
Note over C,S: Discovery Hit Path
alt kind == 'intent-skills'
C->>C: NEW: Generate Intent content hash
C->>AM: NEW: upsertIntentSkillsBlock (disjoint block)
AM-->>C: success
else kind == 'docs'
C->>S: CHANGED: saveDocs (copy from node_modules to .ask/docs)
C->>S: generateSkill (create .claude/skills/...)
C->>AM: generateAgentsMd (update ask-docs block)
end
else Discovery Miss (null)
Note over C,R: Fallback Path
C->>R: resolveFromRegistry(pkg)
R-->>C: Registry Entry (docsPath/URL)
C->>S: saveDocs / generateSkill
C->>AM: generateAgentsMd
end
C->>FS: Update ask.lock (NEW: record 'format' field)
C-->>U: Done! (Success message)
Note over U,AM: ask docs sync / remove
U->>C: Execute sync/remove
C->>FS: Read ask.lock
alt NEW: format == 'intent-skills'
C->>D: Re-run localIntentAdapter
C->>AM: NEW: update/strip intent-skills block
else format == 'docs'
C->>S: Standard doc sync/cleanup
end
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- Retrospective added to plan - Track moved active/ -> completed/ - metadata status -> review, pr -> #48
Contributor
There was a problem hiding this comment.
1 issue found across 5 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/discovery/repo-conventions.ts">
<violation number="1" location="packages/cli/src/discovery/repo-conventions.ts:52">
P2: Quality scoring and file collection use different recursion limits, which can cause valid deep docs roots to be skipped as false negatives.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
7 unresolved cubic threads on PR #48, all addressed: P1 - local-conventions.ts: validate symlink realpath containment before scoreDirectory walks the candidate. A symlinked `dist/docs -> /etc` would otherwise let the scorer recurse out of the package directory before tryLocalRead's later guard rejected the read. P1 - index.ts runSync: hoist intent-skills lock-key collection above the empty-config early-exit so intent-only projects (no `config.docs` rows) still get their marker block resynced. P2 - conventions.ts: case-insensitive meta-filename exclusion. Lowercase `contributing.md` / `changelog.md` were bypassing the SC-3 filter and inflating the quality score. Stored lowercase in EXCLUDED_EXACT_LOWER and matched via toLowerCase(). P2 - audit-coverage.ts: registry aliases are structured YAML objects (`ecosystem: npm` + `name: <name>`), not the `- npm:<name>` shorthand the earlier regex expected. Updated to match the two-line ecosystem/name pair. P2 - agents-intent.ts: replace the lossy two-pass `unescapeDq` with a single-pass character-by-character decoder. The old implementation `replace(/\\\\/g, '\\').replace(/\\"/g, '"')` would consume backslashes the first pass produced, corrupting round-trips for tasks containing a literal backslash adjacent to a quote (e.g. `\"`). Regression test added. P2 - repo-conventions.ts + quality.ts: share MAX_WALK_DEPTH=20 between scoreDirectory and collectDocFiles via conventions.ts. Previously only collectDocFiles had a depth bound, so scoreDirectory could accept a deep tree whose files collectDocFiles later refused to read. P3 - adapters.test.ts: replace conditional `if kind === 'docs'` assertion blocks with explicit narrowing throws so a wrong- kind regression fails the test instead of silently passing. Also added a lowercase-noise SC-3 case for the case-insensitive filter. Build, lint, and 270 tests (267 + 3 new regression cases) green.
This was referenced Apr 9, 2026
Merged
Merged
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
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.
Closes #46
Summary
Replaces the registry-first resolver with a convention-based discovery pipeline that runs BEFORE the central registry lookup for
npm:ecosystem specs. Most popular OSS libraries now resolve automatically with zero registry curation, and TanStack Intent-format packages wire intoAGENTS.md's native marker block instead of being copied into.ask/docs/.The central registry is not removed — it is demoted to a fallback for libraries with non-conventional layouts or alias mappings.
Highlights
New discovery module (
packages/cli/src/discovery/)local-ask—package.json.ask.docsPathopt-in (library-author contract)local-intent— wraps@tanstack/intent'sfindSkillFiles/parseFrontmatterwith zod runtime validationlocal-conventions—dist/docs→docs→README.mdfallback with a warningrepo-conventions— conventional-path scanner for downloaded github repo trees>=3markdown files OR>=4 KiBthreshold with a LICENSE / CHANGELOG / CONTRIBUTING exclusion filter (SC-3 guard against noise-only repos)Dual AGENTS.md marker blocks
<!-- BEGIN:ask-docs-auto-generated -->block is untouched<!-- intent-skills:start --> ... <!-- intent-skills:end -->block managed byagents-intent.tson a byte range strictly disjoint from the ask blockLock schema extension
NpmLockEntrygains optionalformat?: 'docs' | 'intent-skills'— default'docs'so pre-refactor lock entries load unchangedask docs syncadds a second pass that iterates lock entries withformat: 'intent-skills'and re-runs the intent adapterask docs removebranches on format: intent entries callremoveFromIntentSkillsBlockand drop the lock rowTest plan
CONTRIBUTING.md+CHANGELOG.mdonly) fails the quality threshold (quality.test.ts,adapters.test.ts)add/sync/removetests pass unchangedpackages/clibuilds and lints cleanDeferred follow-ups
See `Surprises & Discoveries` in the track plan for full rationale.
Commits
Summary by cubic
Adds a convention-based docs discovery pipeline that runs before the central registry for
npm:specs, auto-resolving common docs and wiring@tanstack/intentskills into a dedicatedAGENTS.mdblock. The registry is now a fallback;sync/removesupport intent skills. Closes #46.New Features
local-ask→local-intent→local-conventions;repo-conventionsadded. Includes a quality scorer (≥3 markdown files or ≥4 KiB; excludes LICENSE/CHANGELOG/CONTRIBUTING).tanstack-intentpackages via@tanstack/intent, writes<!-- intent-skills:start --> … <!-- intent-skills:end -->. Idempotent upsert/remove; ask-docs block is untouched.format?: 'docs' | 'intent-skills'(defaultdocs).ask docs syncre-runs intent discovery;ask docs removecleans the block and lock entry for intent skills.npm:without--source/--docs-path, run discovery before registry. Docs reuse the existing pipeline; intent skills referencenode_modulesin place. Addspackages/cli/scripts/audit-coverage.tsfor registry coverage.Bug Fixes
local-conventions; shareMAX_WALK_DEPTH=20across walkers; meta-file exclusion is now case-insensitive.runSyncrefreshes intent-skill blocks even in intent-only projects; the audit script correctly parses YAML alias fields.Written for commit d92ea30. Summary will update on new commits.