Conversation
…alidation Phase 1-4 of positioning SkillKit as THE runtime implementing all three AI agent skill standards (Agent Skills, AGENTS.md, well-known discovery). New modules: - save/ — ContentExtractor, SkillGenerator, AutoTagger for URL-to-Skill - agents-md/ — AGENTS.md generator with managed section markers - validation/ — SpecValidator with strict mode for agentskills.io compliance New CLI commands: - `skillkit save <url>` — save any URL/text/file as a reusable SKILL.md - `skillkit agents init|sync|show` — generate and manage AGENTS.md Updated: - Well-known provider: Mintlify convention paths, discoverFromUrl() - Install command: smart URL fallback (well-known → offer save) - Remove command: auto-updates AGENTS.md on skill removal - Validate command: --strict flag for spec compliance checks - Quality module: name-directory match + token budget warnings - Schema: agents enum updated from 7 → 44 agents
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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. 📝 WalkthroughWalkthroughThis PR introduces multiple new CLI commands (Save, AGENTS.md management), core utility modules for content extraction and skill generation, enhances existing commands with AGENTS.md integration, implements spec validation, updates the skill schema, and expands provider discovery capabilities with well-known path support. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant SaveCommand
participant ContentExtractor
participant AutoTagger
participant SkillGenerator
participant FileSystem
Client->>SaveCommand: execute(url/text/file)
SaveCommand->>SaveCommand: validate single source
SaveCommand->>ContentExtractor: extract from source
ContentExtractor->>FileSystem: read file or fetch URL
ContentExtractor->>ContentExtractor: parse HTML to Markdown if needed
ContentExtractor-->>SaveCommand: ExtractedContent
SaveCommand->>AutoTagger: detectTags(content)
AutoTagger->>AutoTagger: analyze headings, code, keywords
AutoTagger-->>SaveCommand: tags[]
SaveCommand->>SkillGenerator: generate(content, options)
SkillGenerator->>FileSystem: create skill directory
SkillGenerator->>FileSystem: write SKILL.md with frontmatter
SkillGenerator-->>SaveCommand: SaveGeneratedSkill
SaveCommand->>FileSystem: optionally copy to agent directories
SaveCommand-->>Client: success with skill path & location
sequenceDiagram
participant Client
participant InstallCommand
participant WellKnownProvider
participant AgentsMdParser
participant AgentsMdGenerator
participant FileSystem
Client->>InstallCommand: execute(http(s) URL)
InstallCommand->>WellKnownProvider: discoverFromUrl(url)
WellKnownProvider->>WellKnownProvider: try well-known paths
WellKnownProvider->>FileSystem: fetch SKILL.md from paths
WellKnownProvider-->>InstallCommand: CloneResult with skills
InstallCommand->>InstallCommand: npm install if package.json exists
InstallCommand->>FileSystem: read existing AGENTS.md
InstallCommand->>AgentsMdParser: parse(content)
AgentsMdParser-->>InstallCommand: AgentsMdSection[]
InstallCommand->>AgentsMdGenerator: generate(config)
AgentsMdGenerator-->>InstallCommand: AgentsMdResult
InstallCommand->>FileSystem: updateManagedSections(sections)
InstallCommand->>FileSystem: write updated AGENTS.md
InstallCommand-->>Client: success with install details
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
schemas/skill.schema.json (2)
5-5:⚠️ Potential issue | 🟡 MinorTypo in schema description URL: "agenstskills" → "agentskills".
- "description": "Schema for SKILL.md frontmatter validation (follows Agent Skills spec: https://agenstskills.com)", + "description": "Schema for SKILL.md frontmatter validation (follows Agent Skills spec: https://agentskills.com)",
63-70:⚠️ Potential issue | 🔴 CriticalCritical mismatch: 35 agents in schema lack configurations in agent-config.ts
The schema accepts 44 agents, but
packages/core/src/agent-config.tsonly defines configurations for 9 (claude-code, cursor, codex, gemini-cli, opencode, antigravity, github-copilot, amp, clawdbot, and 35 others missing). Functions likegetSkillsDir()andgetAgentDirectoryConfig()will fail at runtime for agents likecodex,cursor,opencode,antigravity,cline,devin,bolt,lovable,tabnine, and 25 others whenAGENT_CONFIG[agent]is accessed without null-safety checks. Either remove unsupported agents from the schema or add their configurations to agent-config.ts.packages/cli/src/commands/validate.ts (1)
375-401:⚠️ Potential issue | 🟡 Minor
--strictis silently ignored in JSON output mode.When
--jsonis used, the code returns JSON results without runningSpecValidator, so--strict --jsonproduces no spec validation data. If this is intentional, document it in the usage; otherwise, include spec results in the JSON output.Sketch — add spec validation to JSON path
if (this.benchmark && r.quality) { result.benchmark = benchmarkSkill(r.name, r.quality); } + if (this.strict && r.path) { + const specValidator = new SpecValidator(); + result.specValidation = specValidator.validate(r.path, { strict: true }); + } return result; });packages/cli/src/commands/install.ts (2)
391-404:⚠️ Potential issue | 🟡 MinorNo timeout on
execFileAsync('npm', ...)— could hang indefinitely.If npm hangs (e.g., network issues, registry down), this blocks the install command forever with no recourse. Consider adding a
timeoutoption toexecFileAsync.Suggested fix
- await execFileAsync('npm', ['install', '--omit=dev'], { cwd: targetPath }); + await execFileAsync('npm', ['install', '--omit=dev'], { cwd: targetPath, timeout: 120_000 });
391-404:⚠️ Potential issue | 🟡 MinorReplace deprecated
npm install --productionwith--omit=dev.The
--productionflag is deprecated in npm 7+. Use--omit=devinstead to install without devDependencies.Suggested fix
- await execFileAsync('npm', ['install', '--production'], { cwd: targetPath }); + await execFileAsync('npm', ['install', '--omit=dev'], { cwd: targetPath });
🤖 Fix all issues with AI agents
In `@apps/skillkit/src/cli.ts`:
- Line 107: The AgentsMd* commands (AgentsMdCommand, AgentsMdInitCommand,
AgentsMdSyncCommand, AgentsMdShowCommand) are exported but not imported or
registered; import these four symbols into the import block alongside
SaveCommand and then call cli.register(AgentsMdCommand),
cli.register(AgentsMdInitCommand), cli.register(AgentsMdSyncCommand), and
cli.register(AgentsMdShowCommand) after the existing cli.register(SaveCommand)
call so the "agents init|sync|show" subcommands are available.
In `@packages/cli/src/commands/agents-md.ts`:
- Around line 38-58: Wrap the body of AgentsMdInitCommand.execute() in a
try-catch that catches errors from AgentsMdGenerator.generate() and
writeFileSync; on error, log a clear user-friendly message (e.g., using
console.error or chalk.red) including the error.message and return a non-zero
exit code. Specifically, guard calls to generator.generate() and
writeFileSync(agentsPath, ...) with the try block and in the catch log the
failure to generate or write AGENTS.md (include the thrown error), then return 1
to avoid an unhandled exception. Ensure existing checks (existsSync) remain
before the try so existing-file handling is unchanged.
In `@packages/cli/src/commands/install.ts`:
- Around line 443-455: Wrap the AGENTS.md update logic (the block that creates
AgentsMdParser, calls parser.hasManagedSections, instantiates AgentsMdGenerator
and calls gen.generate(), then calls parser.updateManagedSections and
writeFileSync) in its own try-catch so exceptions from
AgentsMdGenerator.generate() or writeFileSync do not propagate and cause the
overall install to be reported as failed; on error catch, log a non-fatal
warning (include the error message) and continue, ensuring the outer flow that
reports success when totalInstalled > 0 remains unchanged.
In `@packages/cli/src/commands/publish.ts`:
- Around line 132-140: The mintlify branch currently creates mintlifyDir before
checking SKILL.md and lacks the resolve+startsWith path-traversal guard; update
the validSkills loop so you first check existsSync(skillMdPath) and only then
create the directory, and before writing compute the resolved target path (e.g.,
resolve(mintlifyDir, 'skill.md')) and verify it startsWith resolve(outputDir)
(defense-in-depth in addition to sanitizeSkillName) — if the check fails, skip
writing; reference mintlifyDir, skillMdPath, and the validSkills loop when
making these changes.
In `@packages/cli/src/commands/validate.ts`:
- Around line 147-174: The printSpecReport function is printing duplicate
messages because SpecValidationResult items are present in both
specResult.checks and specResult.errors/specResult.warnings; to fix, keep only
the checks output and remove the redundant errors and warnings blocks: delete
the if (specResult.errors.length > 0) { ... } and if (specResult.warnings.length
> 0) { ... } sections so you only iterate specResult.checks (which already has
severity icons); ensure spacing/console.log('') calls remain appropriate around
the checks loop.
In `@packages/core/package.json`:
- Around line 56-58: Update the dependency entry for "@types/turndown" in
packages/core/package.json to a valid published version: change its version
specifier from ^5.0.6 to ^5.0.5 (the latest available on npm). Locate the
"@types/turndown" dependency in the dependencies/devDependencies block and
replace the version string so package resolution and installs succeed.
In `@packages/core/src/agents-md/generator.ts`:
- Around line 100-112: The generated Markdown table declares a "Tags" column but
every row outputs an empty cell because Skill has no tags; update the generator
to remove the Tags column to keep the table accurate: change the header line and
separator from '| Skill | Description | Tags |' and
'|-------|-------------|------|' to two-column variants, and update the row
construction (the lines.push call that uses escapeTableCell(skill.name) and
escapeTableCell(skill.description)) to emit only the Name and Description
columns; locate usages of skills and escapeTableCell in this file to make the
three-line header/row changes consistently.
In `@packages/core/src/providers/wellknown.ts`:
- Around line 88-89: The frontmatter detection regex and skill-name derivation
are incorrect: update the regex used (currently /^---\s*\n[\s\S]*?name:\s*.+/m)
to only match YAML frontmatter at the very start of the document (remove the 'm'
flag or anchor to document start) so it won't match mid-document '---' lines,
and replace the basename(baseUrl) usage (where baseUrl is a URL string) with
proper URL parsing to extract the last non-empty path segment from new
URL(baseUrl).pathname (falling back to a sensible default like 'default' if
there is no path segment); apply the same changes for the identical block in
clone() as well so both occurrences use the tightened regex and URL path-segment
extraction rather than basename(baseUrl).
In `@packages/core/src/save/extractor.ts`:
- Around line 59-99: extractFromUrl (and likewise fetchGitHubContent) currently
calls fetch without an AbortSignal and can hang; add an AbortController (or use
AbortSignal.timeout) with a sensible default timeout (or from options.timeout)
and pass controller.signal to fetch, ensure you clear/cleanup the timer if using
AbortController, and catch the abort/timeout error to throw a clear,
user-friendly error (e.g., "Request to <url> timed out") while preserving other
fetch errors; update function signatures to accept an optional timeout in
ExtractionOptions and use that value when creating the signal.
In `@packages/core/src/save/skill-generator.ts`:
- Around line 93-106: The frontmatter builder in
packages/core/src/save/skill-generator.ts currently uses .filter(Boolean) which
removes the intentional trailing '' blank line; update the array handling so
only truly optional lines are filtered (e.g., replace .filter(Boolean) with
.filter(v => v !== undefined && v !== null) to preserve empty string) or remove
the trailing '' from the array and append a '\n' after the .join('\n'); ensure
references to yamlEscape and the frontmatter array (the lines including '---',
name/description/tags/metadata, savedAt and the closing '---') are updated
accordingly.
🧹 Nitpick comments (17)
packages/core/src/quality/index.ts (2)
572-581: Misleading function name: checks line count, not token budget.
checkTokenBudgetcounts body lines (body.split('\n').length) and compares against a 500-line threshold, but the name implies token-based measurement. The codebase already has acountTokenshelper (line 156). Consider renaming tocheckLineBudgetorcheckBodyLengthto match what it actually does.Proposed rename
-function checkTokenBudget(content: string): string | null { +function checkBodyLineBudget(content: string): string | null {
586-594: Double file read of SKILL.md.
evaluateSkillFilealready reads the file content internally (line 555), and thenevaluateSkillDirectoryreads it again at line 588 for the new checks. For a CLI tool this is unlikely to matter, but you could avoid it by refactoring to read once and pass the content to bothevaluateSkillContentand the new check helpers.Sketch: read once, evaluate once
if (existsSync(skillMdPath)) { - const result = evaluateSkillFile(skillMdPath); - if (result) { - const content = readFileSync(skillMdPath, 'utf-8'); + try { + const content = readFileSync(skillMdPath, 'utf-8'); + const result = evaluateSkillContent(content); const nameWarning = checkNameDirectoryMatch(content, dirPath); if (nameWarning) result.warnings.push(nameWarning); - const budgetWarning = checkTokenBudget(content); + const budgetWarning = checkBodyLineBudget(content); if (budgetWarning) result.warnings.push(budgetWarning); + return result; + } catch { + return null; } - return result; }packages/cli/src/commands/publish.ts (1)
58-61: Format option has no validation — invalid values silently fall through to "standard".If a user passes
--format foo, it silently uses the standard flow instead of erroring. Consider validating the value.Proposed validation
Add at the beginning of
execute():async execute(): Promise<number> { + if (this.format && this.format !== 'standard' && this.format !== 'mintlify') { + console.error(chalk.red(`Unknown format: "${this.format}". Use "standard" or "mintlify".`)); + return 1; + } const basePath = this.skillPath || process.cwd();packages/core/src/providers/wellknown.ts (2)
82-107: Significant code duplication: Mintlify path discovery is copy-pasted betweendiscoverFromUrlandclone.The Mintlify-path iteration + fetch + frontmatter check + write logic is nearly identical in both methods. Extract a shared helper to reduce maintenance burden and divergence risk.
Sketch of extracted helper
+ private async tryMintlifyPaths(baseUrl: string, tempDir: string): Promise<CloneResult | null> { + for (const mintlifyPath of MINTLIFY_PATHS) { + const fullUrl = `${baseUrl}${mintlifyPath}`; + try { + const response = await fetch(fullUrl); + if (response.ok) { + const content = await response.text(); + if (/^---\s*\n[\s\S]*?name:\s*.+/m.test(content)) { + const skillName = this.extractSkillName(baseUrl); + const safeName = sanitizeSkillName(skillName) ?? 'default'; + const skillDir = join(tempDir, safeName); + mkdirSync(skillDir, { recursive: true }); + writeFileSync(join(skillDir, 'SKILL.md'), content); + return { + success: true, + path: tempDir, + tempRoot: tempDir, + skills: [safeName], + discoveredSkills: [{ name: safeName, dirName: safeName, path: skillDir }], + }; + } + } + } catch { + continue; + } + } + return null; + }Also applies to: 155-180
75-124: No timeout onfetchcalls — CLI can hang indefinitely on unresponsive servers.All
fetchcalls indiscoverFromUrl(andclone) lack anAbortSignaltimeout. If a server accepts the connection but never responds, the CLI blocks forever.Example using AbortSignal.timeout
- const response = await fetch(fullUrl); + const response = await fetch(fullUrl, { signal: AbortSignal.timeout(10_000) });packages/core/src/agents-md/parser.ts (1)
7-69: Content before the first##heading is silently discarded byparse().Any lines that appear before the first
##heading (e.g., a# AGENTS.mdtitle, preamble text, or blank lines) are not captured in any section becausecurrentSectionisnulluntil a##heading is encountered. If the AGENTS.md file has a level-1 heading or introductory text, those lines are lost when round-tripping through parse → update.This may be intentional (only
##sections are managed), but worth confirming it aligns with expected AGENTS.md formats. TheupdateManagedSectionsmethod preserves content outside the managed markers, so this only matters if someone callsparse()on the full file and expects all content back.packages/cli/src/commands/remove.ts (1)
56-66: Consider extracting the AGENTS.md sync logic into a shared helper.The read → parse → generate → update → write pattern for AGENTS.md is duplicated in both
install.tsandremove.ts. A shared utility (e.g.,syncAgentsMd(projectPath: string)) would eliminate duplication and ensure consistent behavior across commands.packages/cli/src/commands/save.ts (2)
58-84: Tags are computed twice — once here and once insideSkillGenerator.generate().
AutoTaggeris instantiated and called here (lines 60, 76), and thenSkillGenerator.generate()internally creates its ownAutoTaggerand callsdetectTagsagain (seeskill-generator.tslines 25, 32). The tags written toSKILL.mdcome from the generator's internal tagger, not from the ones assigned at line 77.Consider either:
- Passing the pre-computed tags into the generator via
SaveGenerateOptions, or- Removing the external tagger call and reading the tags back from the generated result for display.
Option A — pass tags to generator
In
skill-generator.ts, add atagsfield toSaveGenerateOptionsand use it when provided:export interface SaveGenerateOptions { name?: string; global?: boolean; outputDir?: string; + tags?: string[]; }- const tags = this.tagger.detectTags(content); + const tags = options.tags ?? this.tagger.detectTags(content);Then in
save.ts:const result = generator.generate(content, { name: this.name, global: this.global, + tags, });
3-3: Inconsistent styling:chalkused directly instead of sharedcolorstheme.Other CLI commands (e.g.,
validate.ts) use thecolorsabstraction from../onboarding/index.js, which wrapspicocolors. This file importschalkdirectly, introducing a second color library dependency and diverging from the project's styling convention.Consider replacing
chalkwith the sharedcolors/success/error/warnhelpers for consistency.packages/core/src/validation/spec-validator.ts (1)
95-107: Usedirnameinstead ofjoin(skillPath, '..').Line 96 uses
join(skillPath, '..')to get the parent directory.dirname(skillPath)is more idiomatic and explicit. Just adddirnameto the import on line 2.Proposed fix
-import { join, basename } from 'node:path'; +import { join, basename, dirname } from 'node:path';- const skillDir = skillPath.endsWith('.md') ? join(skillPath, '..') : skillPath; + const skillDir = skillPath.endsWith('.md') ? dirname(skillPath) : skillPath;packages/core/src/save/tagger.ts (1)
84-92: Static analysis ReDoS flag is a false positive here, but the'i'flag is redundant.The
new RegExp(...)uses hardcoded keywords fromTECH_KEYWORDS, not user input, so ReDoS is not a risk. However,contentis already lowercased at line 85, making the'i'flag on line 87 redundant.- const re = new RegExp(`\\b${keyword}\\b`, 'i'); - if (re.test(lower)) { + const re = new RegExp(`\\b${keyword}\\b`); + if (re.test(lower)) {packages/core/src/save/skill-generator.ts (1)
108-114:yamlEscapehandles common special characters but doesn't guard against leading YAML scalars.Values starting with
true,false,yes,no,null, or numeric strings are parsed as non-string types by some YAML parsers. If a description like"123 ways to code"or"true story"is encountered, it could be misinterpreted. Consider also quoting values that match known YAML boolean/null literals.packages/cli/src/commands/install.ts (1)
112-130: Well-known discovery failure exits the entire install — consider offering a fallback.When the URL is not recognized by a git provider and well-known discovery fails, the command hard-exits with code 1 (line 128). This means a user who pastes a URL like
https://example.com/skills-repo(a valid git repo not on a known host) can never install it without--provider. The early return pre-empts the!providerAdaptererror block at line 136 which already provides good guidance.Consider letting the flow fall through to line 136 instead of returning here, so the user gets the "Use
--providerflag" guidance alongside theskillkit savesuggestion.Suggested change
if (discovery.success) { s.stop(`Found ${discovery.skills?.length || 0} skill(s) via well-known discovery`); providerAdapter = wellKnown; result = discovery; } else { s.stop('No well-known skills found'); - warn(`No well-known skills found at ${this.source}`); - console.log(colors.muted('You can save this URL as a skill instead:')); - console.log(colors.muted(` skillkit save ${this.source}`)); - return 1; + // Fall through — if no provider is detected, the block at line 136 will handle it + // with additional guidance including the save suggestion }packages/core/src/save/extractor.ts (3)
116-138: No file size guard —readFileSyncon a large file could exhaust memory.
readFileSyncat line 121 reads the entire file into memory before applyingmaxLength. For multi-GB files accidentally targeted, this could OOM the process. Consider checkingstatSync(filePath).sizefirst and bailing out or streaming if above a threshold.
46-47: GitHub URL regex won't match URLs with query parameters or fragments.
GITHUB_URL_PATTERNanchors at$after the path, so URLs likehttps://github.com/owner/repo/blob/main/file.ts?raw=truewon't match. This isn't a bug (falls through to the general fetch path), but you may want to strip query/fragment before matching for more robust GitHub detection.
126-130:maxLengthis applied before code fencing — the fence markers add to the final size.On line 126,
maxLengthtrims the raw content, but then line 130 wraps it with```language\n...\n```markers, so the actualcontentlength exceedsmaxLength. If this is used for token budgets, the overshoot is minor but worth noting for consistency with the other extraction methods.packages/cli/src/commands/agents-md.ts (1)
50-54: Init writes the file without user confirmation after preview.The command prints a preview and then immediately writes the file. For consistency with the interactive UX in
install.ts, consider adding an optional--yesflag or a confirmation prompt before writing (especially since the generated content could be large). This is a minor UX nit —initcommands often just write.
| for (const skill of validSkills) { | ||
| const mintlifyDir = join(outputDir, '.well-known', 'skills', skill.safeName); | ||
| mkdirSync(mintlifyDir, { recursive: true }); | ||
| const skillMdPath = join(skill.path, 'SKILL.md'); | ||
| if (existsSync(skillMdPath)) { | ||
| const content = readFileSync(skillMdPath, 'utf-8'); | ||
| writeFileSync(join(mintlifyDir, 'skill.md'), content); | ||
| } | ||
| } |
There was a problem hiding this comment.
Mintlify branch lacks the path-traversal guard present in the standard branch.
The standard publishing path (lines 174–180) includes an explicit resolve + startsWith check as defense-in-depth against path traversal. The mintlify branch relies solely on sanitizeSkillName, which does cover .. and separators, but for consistency and defense-in-depth the same guard should be applied here.
Also, if SKILL.md doesn't exist at line 136, an empty directory is created at line 134 and silently left behind.
Proposed fix
for (const skill of validSkills) {
const mintlifyDir = join(outputDir, '.well-known', 'skills', skill.safeName);
+ const resolvedMintlifyDir = resolve(mintlifyDir);
+ const resolvedOutputDir = resolve(join(outputDir, '.well-known', 'skills'));
+ if (!resolvedMintlifyDir.startsWith(resolvedOutputDir)) {
+ console.log(chalk.yellow(` Skipping "${skill.name}" (path traversal detected)`));
+ continue;
+ }
mkdirSync(mintlifyDir, { recursive: true });
const skillMdPath = join(skill.path, 'SKILL.md');
if (existsSync(skillMdPath)) {
const content = readFileSync(skillMdPath, 'utf-8');
writeFileSync(join(mintlifyDir, 'skill.md'), content);
+ } else {
+ console.log(chalk.yellow(` Skipping "${skill.name}" (SKILL.md not found)`));
+ continue;
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for (const skill of validSkills) { | |
| const mintlifyDir = join(outputDir, '.well-known', 'skills', skill.safeName); | |
| mkdirSync(mintlifyDir, { recursive: true }); | |
| const skillMdPath = join(skill.path, 'SKILL.md'); | |
| if (existsSync(skillMdPath)) { | |
| const content = readFileSync(skillMdPath, 'utf-8'); | |
| writeFileSync(join(mintlifyDir, 'skill.md'), content); | |
| } | |
| } | |
| for (const skill of validSkills) { | |
| const mintlifyDir = join(outputDir, '.well-known', 'skills', skill.safeName); | |
| const resolvedMintlifyDir = resolve(mintlifyDir); | |
| const resolvedOutputDir = resolve(join(outputDir, '.well-known', 'skills')); | |
| if (!resolvedMintlifyDir.startsWith(resolvedOutputDir)) { | |
| console.log(chalk.yellow(` Skipping "${skill.name}" (path traversal detected)`)); | |
| continue; | |
| } | |
| mkdirSync(mintlifyDir, { recursive: true }); | |
| const skillMdPath = join(skill.path, 'SKILL.md'); | |
| if (existsSync(skillMdPath)) { | |
| const content = readFileSync(skillMdPath, 'utf-8'); | |
| writeFileSync(join(mintlifyDir, 'skill.md'), content); | |
| } else { | |
| console.log(chalk.yellow(` Skipping "${skill.name}" (SKILL.md not found)`)); | |
| continue; | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@packages/cli/src/commands/publish.ts` around lines 132 - 140, The mintlify
branch currently creates mintlifyDir before checking SKILL.md and lacks the
resolve+startsWith path-traversal guard; update the validSkills loop so you
first check existsSync(skillMdPath) and only then create the directory, and
before writing compute the resolved target path (e.g., resolve(mintlifyDir,
'skill.md')) and verify it startsWith resolve(outputDir) (defense-in-depth in
addition to sanitizeSkillName) — if the check fails, skip writing; reference
mintlifyDir, skillMdPath, and the validSkills loop when making these changes.
| "@mozilla/readability": "^0.6.0", | ||
| "minimatch": "^10.1.1", | ||
| "turndown": "^7.2.2", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
@mozilla/readability npm latest version 2025 2026
💡 Result:
As of February 10, 2026, the latest published npm version of @mozilla/readability is 0.6.0. [1][2]
(Security note: versions < 0.6.0 are affected by CVE-2025-2792; upgrading to 0.6.0+ is recommended.) [3]
🌐 Web query:
turndown npm latest version 2025 2026
💡 Result:
As of February 10, 2026, the latest published version of turndown on npm is 7.2.2, released/published on October 24, 2025. [1][2]
Sources: GitHub releases list v7.2.2 as latest [1]; npm version publish date is also reflected in Snyk’s npm version history. [2]
🌐 Web query:
@types/turndown npm latest version 2025 2026
💡 Result:
As of February 10, 2026, the latest npm version of @types/turndown is 5.0.5 (last updated July 23, 2024). [1][2]
Pin @types/turndown to an existing version.
@mozilla/readability (^0.6.0) and turndown (^7.2.2) are well-established libraries for HTML content extraction and HTML-to-Markdown conversion, and both versions are current as of February 2026. However, @types/turndown ^5.0.6 does not exist on npm; the latest available version is 5.0.5 (last updated July 23, 2024). Update line 74 to use @types/turndown ^5.0.5 instead.
🤖 Prompt for AI Agents
In `@packages/core/package.json` around lines 56 - 58, Update the dependency entry
for "@types/turndown" in packages/core/package.json to a valid published
version: change its version specifier from ^5.0.6 to ^5.0.5 (the latest
available on npm). Locate the "@types/turndown" dependency in the
dependencies/devDependencies block and replace the version string so package
resolution and installs succeed.
| const lines: string[] = [ | ||
| '| Skill | Description | Tags |', | ||
| '|-------|-------------|------|', | ||
| ]; | ||
|
|
||
| for (const skill of skills) { | ||
| const name = escapeTableCell(skill.name); | ||
| const desc = escapeTableCell(skill.description); | ||
| lines.push(`| ${name} | ${desc} | |`); | ||
| } | ||
|
|
||
| return lines.join('\n'); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n 'interface Skill' --type=ts -A 15Repository: rohitg00/skillkit
Length of output: 37870
🏁 Script executed:
cat -n packages/core/src/agents-md/generator.ts | head -120Repository: rohitg00/skillkit
Length of output: 4191
🏁 Script executed:
rg -n 'export.*interface Skill\b|export.*type Skill\b' packages/core/src/types.ts -A 20Repository: rohitg00/skillkit
Length of output: 798
🏁 Script executed:
rg -n 'const Skill\s*=' packages/core/src/types.ts -B 5 -A 15Repository: rohitg00/skillkit
Length of output: 799
🏁 Script executed:
rg -n 'const SkillMetadata\s*=' packages/core/src/types.ts -B 2 -A 15Repository: rohitg00/skillkit
Length of output: 725
🏁 Script executed:
rg -n 'tags' packages/core/src/agents-md/generator.tsRepository: rohitg00/skillkit
Length of output: 43
🏁 Script executed:
rg -n 'tags' packages/core/src/types.tsRepository: rohitg00/skillkit
Length of output: 126
🏁 Script executed:
rg -n 'const SkillFrontmatter\s*=' packages/core/src/types.ts -B 2 -A 20Repository: rohitg00/skillkit
Length of output: 952
🏁 Script executed:
# Check if skills are parsed from frontmatter and if tags are included
rg -n 'SkillFrontmatter\|tags' packages/core/src/skills.ts -B 3 -A 3Repository: rohitg00/skillkit
Length of output: 43
🏁 Script executed:
rg -n 'findAllSkills' packages/core/src/skills.ts -A 30Repository: rohitg00/skillkit
Length of output: 1202
🏁 Script executed:
# Let me check if there's any other code path that might include tags with skills
rg -n 'tags' packages/core/src/skills.tsRepository: rohitg00/skillkit
Length of output: 43
Remove the Tags column or populate it with skill tags.
The table header declares a "Tags" column but line 108 always outputs an empty cell. The Skill type doesn't include a tags property, so the column cannot be populated. Either remove the column header and separator to avoid confusion, or extend the Skill type to include tags from the skill's frontmatter.
🤖 Prompt for AI Agents
In `@packages/core/src/agents-md/generator.ts` around lines 100 - 112, The
generated Markdown table declares a "Tags" column but every row outputs an empty
cell because Skill has no tags; update the generator to remove the Tags column
to keep the table accurate: change the header line and separator from '| Skill |
Description | Tags |' and '|-------|-------------|------|' to two-column
variants, and update the row construction (the lines.push call that uses
escapeTableCell(skill.name) and escapeTableCell(skill.description)) to emit only
the Name and Description columns; locate usages of skills and escapeTableCell in
this file to make the three-line header/row changes consistently.
| return [ | ||
| '---', | ||
| `name: ${name}`, | ||
| `description: ${this.yamlEscape(description)}`, | ||
| tags.length > 0 ? `tags:\n${yamlTags}` : '', | ||
| 'metadata:', | ||
| source ? ` source: ${source}` : '', | ||
| ` savedAt: ${savedAt}`, | ||
| '---', | ||
| '', | ||
| ] | ||
| .filter(Boolean) | ||
| .join('\n'); | ||
| } |
There was a problem hiding this comment.
.filter(Boolean) removes the intentional blank line after closing ---.
Line 102 adds an empty string '' to produce a trailing blank line after the frontmatter fence, but .filter(Boolean) on line 104 removes all falsy values including ''. As a result, the generated SKILL.md has no blank line between the closing --- and the body content. This can cause issues with some Markdown/frontmatter parsers.
Proposed fix — filter only truly optional lines
return [
'---',
`name: ${name}`,
`description: ${this.yamlEscape(description)}`,
tags.length > 0 ? `tags:\n${yamlTags}` : '',
'metadata:',
source ? ` source: ${source}` : '',
` savedAt: ${savedAt}`,
'---',
- '',
]
- .filter(Boolean)
+ .filter((line, i, arr) => line !== '' || i === arr.length - 1)
- .join('\n');
+ .join('\n') + '\n';Alternatively, a simpler approach — build the array without the trailing blank and append \n at the end:
return [
'---',
`name: ${name}`,
`description: ${this.yamlEscape(description)}`,
tags.length > 0 ? `tags:\n${yamlTags}` : null,
'metadata:',
source ? ` source: ${source}` : null,
` savedAt: ${savedAt}`,
'---',
]
- .filter(Boolean)
+ .filter((line): line is string => line !== null)
.join('\n');🤖 Prompt for AI Agents
In `@packages/core/src/save/skill-generator.ts` around lines 93 - 106, The
frontmatter builder in packages/core/src/save/skill-generator.ts currently uses
.filter(Boolean) which removes the intentional trailing '' blank line; update
the array handling so only truly optional lines are filtered (e.g., replace
.filter(Boolean) with .filter(v => v !== undefined && v !== null) to preserve
empty string) or remove the trailing '' from the array and append a '\n' after
the .join('\n'); ensure references to yamlEscape and the frontmatter array (the
lines including '---', name/description/tags/metadata, savedAt and the closing
'---') are updated accordingly.
…leak, wrap AGENTS.md update - Register AgentsMdCommand/Init/Sync/Show in CLI entry point (were exported but never registered) - Clean up tempDir in wellknown clone() when no skills found via index or mintlify - Wrap AGENTS.md auto-update in install.ts with try-catch to prevent masking successful installs
| const isUrl = this.source.startsWith('http://') || this.source.startsWith('https://'); | ||
| if (isUrl && !this.provider && !providerAdapter) { | ||
| s.start('Checking for well-known skills...'); | ||
| const wellKnown = new WellKnownProvider(); | ||
| const discovery = await wellKnown.discoverFromUrl(this.source); | ||
| if (discovery.success) { | ||
| s.stop(`Found ${discovery.skills?.length || 0} skill(s) via well-known discovery`); | ||
| providerAdapter = wellKnown; | ||
| result = discovery; | ||
| } else { | ||
| s.stop('No well-known skills found'); | ||
| warn(`No well-known skills found at ${this.source}`); | ||
| console.log(colors.muted('You can save this URL as a skill instead:')); | ||
| console.log(colors.muted(` skillkit save ${this.source}`)); | ||
| return 1; | ||
| } | ||
| } |
There was a problem hiding this comment.
🔴 Well-known discovery with save fallback is unreachable dead code in install command
The "smart URL fallback" feature described in the PR (discoverFromUrl → suggest skillkit save) never triggers because detectProvider already matches HTTP URLs to WellKnownProvider.
Root Cause
At packages/cli/src/commands/install.ts:111, detectProvider(this.source) iterates over all providers and calls matches(). The WellKnownProvider.matches() at packages/core/src/providers/wellknown.ts:52-64 returns true for any HTTP URL that isn't github.com/gitlab.com/bitbucket.org. So for a URL like https://react.dev/learn, providerAdapter is already set to WellKnownProvider.
Then at line 115, the condition isUrl && !this.provider && !providerAdapter evaluates to true && true && false → false. The entire block (lines 115-130) including the discoverFromUrl() call and the helpful save fallback message is never reached.
Instead, execution falls through to line 146 where providerAdapter.clone() is called directly. If clone() fails, the user gets a generic error like "No skills found at …/index.json" instead of the intended UX:
You can save this URL as a skill instead:
skillkit save https://react.dev/learn
Impact: The documented "smart URL fallback" feature (tries well-known → offers save) does not work. Users get confusing errors instead of actionable suggestions when installing from URLs without skills.
Prompt for agents
The well-known discovery block at lines 114-130 is dead code because detectProvider() at line 111 already returns WellKnownProvider for any non-github/gitlab/bitbucket HTTP URL, making the condition !providerAdapter always false.
To fix this, change the condition at line 115 to not rely on providerAdapter being null. Instead, check if providerAdapter is WellKnownProvider specifically and use discoverFromUrl in that case. For example:
1. At line 114-115, change the condition from:
const isUrl = this.source.startsWith('http://') || this.source.startsWith('https://');
if (isUrl && !this.provider && !providerAdapter) {
to:
const isUrl = this.source.startsWith('http://') || this.source.startsWith('https://');
if (isUrl && !this.provider && providerAdapter instanceof WellKnownProvider) {
2. This way, when detectProvider returns WellKnownProvider, the code enters the block and calls discoverFromUrl() which provides the Mintlify-first discovery and save fallback UX.
3. If discoverFromUrl succeeds, set result and continue. If it fails, show the save suggestion and return 1.
4. Remove or adjust the later if (!result) block at lines 146-156 accordingly, since for WellKnownProvider URLs the result would already be set by discoverFromUrl.
Was this helpful? React with 👍 or 👎 to provide feedback.
| `description: ${this.yamlEscape(description)}`, | ||
| tags.length > 0 ? `tags:\n${yamlTags}` : '', | ||
| 'metadata:', | ||
| source ? ` source: ${source}` : '', |
There was a problem hiding this comment.
🟡 Source URL/path not YAML-escaped in generated skill frontmatter
The source value (a URL or file path) is interpolated directly into YAML frontmatter without escaping, while the adjacent description field IS properly escaped via yamlEscape().
Root Cause
In packages/core/src/save/skill-generator.ts:99, the source value is placed directly into YAML:
source ? ` source: ${source}` : '',Compare this with how description is handled at line 96:
`description: ${this.yamlEscape(description)}`,The source value comes from content.sourceUrl ?? content.sourcePath ?? '' (line 34). When saving a local file via --file, sourcePath is a file path. File paths on Unix systems can contain YAML-special characters:
:(colon-space) is a YAML key-value separator#(space-hash) starts a YAML comment
For example, skillkit save --file "/home/user/project: notes/file.md" generates:
metadata:
source: /home/user/project: notes/file.mdYAML parsers would interpret source as having the value /home/user/project with notes/file.md as a separate element, producing invalid/incorrect frontmatter.
Impact: Skills saved from files with special characters in their paths produce invalid YAML frontmatter that cannot be correctly parsed.
| source ? ` source: ${source}` : '', | |
| source ? ` source: ${this.yamlEscape(source)}` : '', |
Was this helpful? React with 👍 or 👎 to provide feedback.
- wellknown.ts: drop multiline flag from frontmatter regex, extract skill name from URL path segments instead of basename on full URL - agents-md.ts: wrap init/sync in try-catch for user-friendly error messages - publish.ts: add path-traversal guard to mintlify branch - validate.ts: remove duplicate error/warning output in spec report - extractor.ts: add 30s AbortSignal timeout to all fetch calls - skill-generator.ts: fix .filter(Boolean) stripping blank line after frontmatter
Summary
skillkit save): Save any URL, text, or file as a reusable SKILL.md — content extraction via turndown, auto-tagging, frontmatter generation, long content split toreferences/skillkit agents init|sync|show): Generate and manage AGENTS.md files with managed section markers, auto-detect project context via ProjectDetector, auto-sync on install/removeskillkit validate --strict): SpecValidator with strict mode for agentskills.io compliance — name-directory match, token budget, frontmatter field placement.well-known/skills/default/skill.md,/skill.md), smart URL fallback in install (tries well-known → offers save)New Modules
packages/core/src/save/packages/core/src/agents-md/packages/core/src/validation/New CLI Commands
skillkit save <url>skillkit save --text "..."skillkit save --file ./notes.mdskillkit agents initskillkit agents syncskillkit agents showUpdated Files
packages/cli/src/commands/install.ts— smart URL fallbackpackages/cli/src/commands/remove.ts— auto-update AGENTS.mdpackages/cli/src/commands/validate.ts—--strictflagpackages/cli/src/commands/publish.ts—--format mintlifypackages/core/src/providers/wellknown.ts— Mintlify paths + stricter content detectionpackages/core/src/quality/index.ts— name-dir match + token budget checksschemas/skill.schema.json— 44 agents enumTest plan
pnpm build— All 12 packages compile (0 errors)pnpm test— All 900+ tests pass (0 regressions)skillkit save https://react.dev/learn— Fetches, extracts, generates valid SKILL.mdskillkit save --text "Always use strict mode"— Saves text as skillskillkit agents init— Generates AGENTS.md with project contextskillkit validate --strict— Shows spec compliance resultsSummary by CodeRabbit
Release Notes
New Features
savecommand to create skills from URLs, text, or local filesagentssubcommands:init(create AGENTS.md),sync(update), andshow(display)--formatoption to publish command supporting Mintlify output format--strictflag to validate command for enhanced spec validationImprovements