feat: maintainer skill publishing with skilld author #45
Conversation
Replaces the untracked `publish.ts` with a proper `author` command that: - Auto-detects monorepo workspaces (pnpm-workspace.yaml, npm workspaces) - Prompts to select which packages should ship skills - Resolves docs from monorepo-level docs/ or docs/content/ when per-package docs are empty - Creates or patches package.json `files` array to include `skills` - Prints consumer guidance (skilld prepare) - `publish` kept as hidden alias for backwards compat
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdded a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as CLI Router
participant AuthorCmd as Author Command
participant WorkspaceDetect as Workspace Detector
participant DocResolver as Doc Resolver
participant Cache
participant GitHub as GitHub API
participant LLM as LLM Engine
participant FileSystem as File System
User->>CLI: skilld author
CLI->>AuthorCmd: invoke author command
AuthorCmd->>WorkspaceDetect: detect monorepo?
alt Monorepo detected
WorkspaceDetect-->>AuthorCmd: workspace packages list
AuthorCmd->>User: prompt package selection
User-->>AuthorCmd: selected packages
else Single package
WorkspaceDetect-->>AuthorCmd: use local package.json
end
AuthorCmd->>FileSystem: create skills/<name> directory
AuthorCmd->>DocResolver: resolve documentation
DocResolver->>FileSystem: check `package/docs/`
DocResolver->>FileSystem: check monorepo `docs/` and `docs/content/`
DocResolver->>FileSystem: check `llms.txt` and `README.md`
DocResolver-->>AuthorCmd: resolved docs content
AuthorCmd->>Cache: store docs & changelog
AuthorCmd->>GitHub: fetch issues & discussions?
GitHub-->>AuthorCmd: remote content
AuthorCmd->>Cache: store remote content
AuthorCmd->>LLM: enhance SKILL.md (if configured)
LLM-->>AuthorCmd: enhanced content
AuthorCmd->>FileSystem: write SKILL.md
AuthorCmd->>FileSystem: eject reference files
AuthorCmd->>FileSystem: patch package.json (`files` includes "skills")
AuthorCmd-->>User: success message
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
CLAUDE.md (1)
43-45: Consider documenting additional flags.The
authorcommand supports additional flags (-y/--yes,-f/--force,--debug) that aren't documented here. Consider adding examples if these are commonly used:skilld author -f # Force regenerate (clear cache) skilld author -y # Skip prompts, use defaults🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLAUDE.md` around lines 43 - 45, The CLAUDE.md section listing the "skilld author" examples is missing documentation for the additional supported flags; update the "author" command examples to include and briefly describe the -f/--force (force regenerate/clear cache), -y/--yes (skip prompts/use defaults) and --debug flags and add example usages such as "skilld author -f" and "skilld author -y" alongside the existing examples so users see both flag names and short intent; reference the "author" command and each flag (-f/--force, -y/--yes, --debug) when editing the examples.src/commands/author.ts (2)
303-308: Reconsider hardcodingdistin files array.When creating a new
filesarray, assumingdistmay not match all packages (some uselib,build,out, etc.). Consider only addingskillsand letting the user specify their build output:♻️ Safer approach
if (!Array.isArray(pkg.files)) { - // Create files array with common defaults - pkg.files = ['dist', 'skills'] + pkg.files = ['skills'] writeFileSync(pkgPath, `${JSON.stringify(pkg, null, 2)}\n`) - p.log.success('Created `files` array in package.json with `["dist", "skills"]`. Verify this matches your package.') + p.log.warn('Created `files` array with `["skills"]`. Add your build output directory (e.g., "dist", "lib") to publish correctly.') return }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/author.ts` around lines 303 - 308, The code currently creates a new files array with ['dist','skills'] when pkg.files is missing; change this to only add 'skills' (i.e., set pkg.files = ['skills']) so we don't hardcode a build output like 'dist', update the writeFileSync usage that persists pkgPath and adjust the success message in p.log.success to inform the user that only 'skills' was added and they should verify/add their build output (e.g., lib/build/out) as needed; ensure you modify the block handling pkg.files, the writeFileSync call, and the p.log.success text accordingly.
566-594: Consider adding-bbackground mode flag.Per coding guidelines, commands should "Support CLI background mode with -b flag for pnpm prepare hooks (non-interactive, auto-uses configured model)". While
authoris primarily interactive, a-bflag could be useful for CI-driven skill generation in monorepos.If intentionally omitted for this command, this can be ignored.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/author.ts` around lines 566 - 594, Add a background mode flag to the author command by extending the args object in src/commands/author.ts: add a new boolean arg named background (alias 'b') with description like "Run in background/non-interactive mode (auto-use configured model for CI/pnpm prepare hooks)" and default false, and ensure any codepaths that check interactivity (e.g., where args.yes or args.model are used in functions handling SKILL.md generation) respect args.background by skipping prompts and auto-selecting the configured model; update places that branch on interactive flow to treat background the same as yes/non-interactive mode.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/author.ts`:
- Around line 77-80: Replace the vulnerable regex-based extraction (the
content.match(...) and subsequent mapping that sets patterns) with a
deterministic line-by-line parser: split content by '\n', trim each line, filter
for lines starting with '-', remove the leading '-' and surrounding quotes, trim
again, and collect non-empty results into patterns; remove the use of matches
and the problematic /^\s*-\s+['"]?([^'"#\n]+)['"]?\s*$/gm to eliminate the ReDoS
risk while preserving the same output shape.
- Line 404: Current logic (!opts.yes || opts.model) causes -y alone to skip LLM;
change it so -y uses the configured default model: replace the condition with
one that runs LLM when LLMs are enabled and EITHER -y is set or a model is
explicitly provided, e.g. use if (!globalConfig.skipLlm && (opts.yes ||
opts.model)), and update the author command help text to state that -y uses the
configured default model for LLM enhancement unless --model is provided;
reference opts.yes, opts.model and globalConfig.skipLlm in your change.
---
Nitpick comments:
In `@CLAUDE.md`:
- Around line 43-45: The CLAUDE.md section listing the "skilld author" examples
is missing documentation for the additional supported flags; update the "author"
command examples to include and briefly describe the -f/--force (force
regenerate/clear cache), -y/--yes (skip prompts/use defaults) and --debug flags
and add example usages such as "skilld author -f" and "skilld author -y"
alongside the existing examples so users see both flag names and short intent;
reference the "author" command and each flag (-f/--force, -y/--yes, --debug)
when editing the examples.
In `@src/commands/author.ts`:
- Around line 303-308: The code currently creates a new files array with
['dist','skills'] when pkg.files is missing; change this to only add 'skills'
(i.e., set pkg.files = ['skills']) so we don't hardcode a build output like
'dist', update the writeFileSync usage that persists pkgPath and adjust the
success message in p.log.success to inform the user that only 'skills' was added
and they should verify/add their build output (e.g., lib/build/out) as needed;
ensure you modify the block handling pkg.files, the writeFileSync call, and the
p.log.success text accordingly.
- Around line 566-594: Add a background mode flag to the author command by
extending the args object in src/commands/author.ts: add a new boolean arg named
background (alias 'b') with description like "Run in background/non-interactive
mode (auto-use configured model for CI/pnpm prepare hooks)" and default false,
and ensure any codepaths that check interactivity (e.g., where args.yes or
args.model are used in functions handling SKILL.md generation) respect
args.background by skipping prompts and auto-selecting the configured model;
update places that branch on interactive flow to treat background the same as
yes/non-interactive mode.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 60d8f6a2-4803-4889-89b3-85147f8f640a
📒 Files selected for processing (3)
CLAUDE.mdsrc/cli.tssrc/commands/author.ts
src/commands/author.ts
Outdated
|
|
||
| // LLM enhancement | ||
| const globalConfig = readConfig() | ||
| if (!globalConfig.skipLlm && (!opts.yes || opts.model)) { |
There was a problem hiding this comment.
Clarify -y flag behavior with LLM enhancement.
The condition !opts.yes || opts.model means -y alone skips LLM enhancement entirely, while -y -m haiku would run it. This differs from typical -y semantics ("accept defaults").
If the intent is that -y should use the configured default model (similar to background mode -b in other commands), consider:
- if (!globalConfig.skipLlm && (!opts.yes || opts.model)) {
+ if (!globalConfig.skipLlm) {
+ const model = opts.model ?? (opts.yes ? globalConfig.model as OptimizeModel : undefined)
+ const llmConfig = model ? { model, sections: [], promptOnly: false } : await selectLlmConfig(undefined)Otherwise, document this behavior clearly in the help text.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/author.ts` at line 404, Current logic (!opts.yes || opts.model)
causes -y alone to skip LLM; change it so -y uses the configured default model:
replace the condition with one that runs LLM when LLMs are enabled and EITHER -y
is set or a model is explicitly provided, e.g. use if (!globalConfig.skipLlm &&
(opts.yes || opts.model)), and update the author command help text to state that
-y uses the configured default model for LLM enhancement unless --model is
provided; reference opts.yes, opts.model and globalConfig.skipLlm in your
change.
- Prompt LLM config once for all packages in monorepo mode (not per-package) - Error on --out in monorepo mode (each package needs its own skills/ dir) - Fall back to monorepo root for CHANGELOG, llms.txt, and README resolution - Inherit repoUrl from monorepo root when child packages lack their own - Preserve package.json formatting when patching files array (targeted insertion)
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/commands/author.ts (1)
492-505:⚠️ Potential issue | 🟠 Major
authorstill lacks the required background mode.There is no
-bflag here, andresolveLlmConfig()returnsundefinedfor-ywithout--model, so this command still has no non-interactive path that auto-uses the configured model. That blocks the pnpm prepare-hook workflow this repo expects for commands.As per coding guidelines, "src/commands/**/*.ts: Support CLI background mode with -b flag for pnpm prepare hooks (non-interactive, auto-uses configured model)".
Also applies to: 614-643
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/author.ts` around lines 492 - 505, The author command needs a background mode flag so it can run non-interactively and auto-use the configured model: add a background?: boolean field to the authorCommand opts and expose a -b flag in the CLI; update resolveLlmConfig signature to accept a background boolean (e.g. resolveLlmConfig(model?: OptimizeModel, yes?: boolean, background?: boolean)) and change its logic so when background is true and model is not provided it returns the configured model (via readConfig()/selectLlmConfig()) instead of undefined; update all calls in authorCommand to pass the new background flag so the pnpm prepare-hook path uses the configured model automatically.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/commands/author.ts`:
- Around line 434-482: The .skilld/ removal must run even if enhanceSkillWithLLM
rejects: wrap the logic that calls writePromptFiles or await enhanceSkillWithLLM
and the subsequent ejectReferences(...) success path in a try block, keep
ejectReferences(...) in the try (success) path, and move the skilldDir rmSync
cleanup into a finally block so the join(outDir, '.skilld') ->
existsSync(skilldDir) -> rmSync(skilldDir, { recursive: true, force: true })
sequence always executes; do not swallow the original error—rethrow after
finally so failures still propagate. Reference functions/vars: writePromptFiles,
enhanceSkillWithLLM, ejectReferences, outDir, skilldDir.
- Around line 367-372: The current use of opts.out to compute outDir (via
computeSkillDirName and join(packageDir, 'skills', sanitizedName)) can allow
dangerous paths like '.' or '..' to point at package root or parents before
deletion; update the logic that computes outDir to resolve and normalize the
provided opts.out, reject any path that resolves to packageDir or an ancestor of
packageDir, and only perform rmSync/existSync/mkdirSync when the resolved outDir
is strictly within join(packageDir, 'skills') (i.e.,
path.startsWith(path.resolve(join(packageDir, 'skills')))); likewise only modify
package.json "files" entries when outDir lives under ./skills; if validation
fails, throw or return a clear error instead of deleting.
- Around line 174-178: The cached markdown is written raw to writeToCache, so
run each doc content through the sanitizer exported from src/core/sanitize.ts
before pushing into cachedDocs (e.g., replace f.content with sanitize(f.content)
or sanitizeMarkdown(f.content) depending on the exported name), and then call
writeToCache with the sanitized cachedDocs; repeat the same change for the other
writeToCache sites in this file (the blocks that build cachedDocs from
walkMarkdownFiles / README / CHANGELOG / llms.txt) so all persisted docs are
sanitized before saving.
---
Duplicate comments:
In `@src/commands/author.ts`:
- Around line 492-505: The author command needs a background mode flag so it can
run non-interactively and auto-use the configured model: add a background?:
boolean field to the authorCommand opts and expose a -b flag in the CLI; update
resolveLlmConfig signature to accept a background boolean (e.g.
resolveLlmConfig(model?: OptimizeModel, yes?: boolean, background?: boolean))
and change its logic so when background is true and model is not provided it
returns the configured model (via readConfig()/selectLlmConfig()) instead of
undefined; update all calls in authorCommand to pass the new background flag so
the pnpm prepare-hook path uses the configured model automatically.
- 12 unit tests covering detectMonorepoPackages and patchPackageJsonFiles - Tests: workspace detection, pnpm-workspace.yaml, private pkg filtering, quoted entries, repo URL parsing, files array patching - README: new "For Maintainers" section documenting skilld author workflow, consumer setup with skilld prepare, and command options - Export detectMonorepoPackages and patchPackageJsonFiles for testability
skilld author command with monorepo supportskilld author
- Sanitize all cached markdown via sanitizeMarkdown() before writing (docs, llms.txt, README, CHANGELOG) to prevent prompt injection - Validate --out path: reject package root or parent directories - Only patch package.json files array when output is under skills/ - Wrap LLM enhancement in try/finally to always clean up .skilld/ symlinks
🔗 Linked issue
Related to #33
❓ Type of change
📚 Description
Package authors had no guided way to generate and ship skills with their npm packages, especially in monorepos. This adds
skilld author, a command that:docs/ordocs/content/when per-package docs are empty (common in Nuxt/UnJS monorepos)filesarray in package.json to includeskillsskilld preparepicks up shipped skillspublishis kept as a hidden alias for backwards compatSummary by CodeRabbit
New Features
skilld authorcommand to generate portable skill packages for npm publishing, with single-package and monorepo-aware modes.Documentation
skilld authorsubcommands and usage examples.