docs: add comprehensive documentation section to website#23
Conversation
- Create Documentation.tsx component with DeepWiki-sourced content - Add sections: Overview, Installation, Architecture, Agents, Commands - Add sections: Configuration, API Reference, Skill Format, Tech Stack - Include interactive sidebar navigation - Add Docs link to navbar - Reference DeepWiki as documentation source Source: https://deepwiki.com/rohitg00/skillkit
|
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. 📝 WalkthroughWalkthroughAdds a full FumaDocs-based documentation site (pages, MDX, Next.js config, styling, and UI components), a SkillGenerator UI with Gemini-backed generation and client-side caching, expands supported agents across core/adapters/TUI, and updates docs/UI copy and routing (header link, Vercel rewrites). Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser as "SkillGenerator UI"
participant Cache as "LocalStorage/Cache"
participant GeminiSvc as "geminiService (client)"
participant API as "Next API (/api/generate-skill)"
participant Gemini as "Gemini (external)"
participant Marketplace as "Marketplace / Raw URL"
User->>Browser: Enter topic / Click "Generate"
Browser->>Cache: lookupCachedSkill(topic)
alt cache hit
Cache-->>Browser: cached skill
Browser->>User: render cached skill
else cache miss
Browser->>GeminiSvc: generateAgentSkill(topic)
GeminiSvc->>API: POST /api/generate-skill { topic }
API->>Gemini: request structured JSON (schema)
Gemini-->>API: JSON response (AgentSkill)
API-->>GeminiSvc: AgentSkill
GeminiSvc-->>Browser: AgentSkill
Browser->>Cache: setCachedSkill(topic, AgentSkill)
Browser->>User: render generated skill (preview/download/submit)
end
User->>Browser: Open marketplace skill
Browser->>Marketplace: fetch(rawUrl)
Marketplace-->>Browser: return raw SKILL.md
Browser->>User: render marketplace content / enable download
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. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/skillkit/components/Documentation.tsx`:
- Around line 436-440: The anchor element in Documentation.tsx that renders the
external link (the <a ...> element with
href="https://deepwiki.com/rohitg00/skillkit" and className "inline-flex
items-center gap-2 mt-2 text-xs text-zinc-500 hover:text-zinc-300
transition-colors") should include "noopener" in its rel attribute for
tabnabbing protection; update the rel value to contain both "noreferrer" and
"noopener" (e.g., "noreferrer noopener") so the link uses both protections.
🧹 Nitpick comments (3)
docs/skillkit/components/Documentation.tsx (3)
9-13: ConsolidateCodeBlockto avoid divergence.
There’s already aCodeBlockindocs/skillkit/components/Terminal.tsx; consider extracting a shared component (or aligning props) so styles/behavior don’t drift. Also, either wirelanguageinto the markup (e.g.,data-language/class) or drop the unused prop.
38-422: Consider externalizing section content for maintainability.
This is a large, static content block; moving it to MDX/JSON (or a dedicated data module) will keep the component lean and make content updates easier (especially if it needs to stay aligned with DeepWiki).
450-464: Improve nav a11y and button semantics.
Addtype="button"to avoid accidental form submission if this is ever nested in a form, plusaria-currentfor the active item and a nav label for screen readers.Suggested tweak
- <nav className="lg:w-48 flex-shrink-0"> + <nav className="lg:w-48 flex-shrink-0" aria-label="Documentation sections"> <div className="sticky top-20 space-y-1"> {sections.map(section => ( <button key={section.id} + type="button" + aria-current={activeSection === section.id ? 'page' : undefined} onClick={() => setActiveSection(section.id)} className={`w-full text-left px-3 py-2 text-sm font-mono rounded transition-colors ${
| <a | ||
| href="https://deepwiki.com/rohitg00/skillkit" | ||
| target="_blank" | ||
| rel="noreferrer" | ||
| className="inline-flex items-center gap-2 mt-2 text-xs text-zinc-500 hover:text-zinc-300 transition-colors" |
There was a problem hiding this comment.
Add noopener for target=_blank safety.
noreferrer is good, but adding noopener is a safer default for tabnabbing protection.
Suggested fix
- rel="noreferrer"
+ rel="noreferrer noopener"📝 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.
| <a | |
| href="https://deepwiki.com/rohitg00/skillkit" | |
| target="_blank" | |
| rel="noreferrer" | |
| className="inline-flex items-center gap-2 mt-2 text-xs text-zinc-500 hover:text-zinc-300 transition-colors" | |
| <a | |
| href="https://deepwiki.com/rohitg00/skillkit" | |
| target="_blank" | |
| rel="noreferrer noopener" | |
| className="inline-flex items-center gap-2 mt-2 text-xs text-zinc-500 hover:text-zinc-300 transition-colors" |
🤖 Prompt for AI Agents
In `@docs/skillkit/components/Documentation.tsx` around lines 436 - 440, The
anchor element in Documentation.tsx that renders the external link (the <a ...>
element with href="https://deepwiki.com/rohitg00/skillkit" and className
"inline-flex items-center gap-2 mt-2 text-xs text-zinc-500 hover:text-zinc-300
transition-colors") should include "noopener" in its rel attribute for
tabnabbing protection; update the rel value to contain both "noreferrer" and
"noopener" (e.g., "noreferrer noopener") so the link uses both protections.
- Add Fumadocs-based documentation site at /docs
- Complete MDX documentation for all features
- TUI section with keyboard navigation
- Installation, quickstart, commands reference
- API reference and configuration guides
- Update SkillKit website (agentskills.com)
- Add 15 new agents (32 total): CodeBuddy, Continue, Crush,
Factory, MCPJam, Mux, Neovate, OpenHands, Pi, Qoder, Qwen,
Vercel, Zencoder, CommandCode
- Update agent count from 17 to 32 across all components
- Update command count from 30+ to 40+
- Remove inline Documentation section (now in Fumadocs)
- Link Docs to /docs route
- Add 15 new agent adapters to @skillkit/agents
- All use UniversalAdapter as base
- Added to detection order and adapter registry
- Configure Vercel rewrites for /docs proxy to Fumadocs
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| 'roo', | ||
| 'trae', | ||
| 'windsurf', | ||
| 'cline', | ||
| 'codebuddy', | ||
| 'commandcode', | ||
| 'continue', | ||
| 'crush', | ||
| 'factory', | ||
| 'mcpjam', | ||
| 'mux', | ||
| 'neovate', | ||
| 'openhands', | ||
| 'pi', | ||
| 'qoder', | ||
| 'qwen', | ||
| 'vercel', | ||
| 'zencoder', | ||
| 'universal', |
There was a problem hiding this comment.
🔴 New agent adapters all report type "universal", breaking detection and adapter lookup
Actual behavior: all newly added agents (cline, codebuddy, commandcode, etc.) are instantiated as new UniversalAdapter(), but UniversalAdapter.type is hard-coded to 'universal'. This means every one of those adapters reports the same type, so:
getAllAdapters()will return many entries whosetypeis'universal', causing incorrect UI/state (e.g., multiple rows for “universal” in the TUI).detectAgent()will mis-detect: when iteratingcheckOrder, theadapter.isDetected()check for e.g.clinewill actually beUniversalAdapter.isDetected()(which checks for.agentorAGENTS.md), so any repo containingAGENTS.mdcan be incorrectly detected ascline(or any earlier “new agent” in the list), instead of the correct agent.getAdapter('cline')returns an adapter whosetypeis still'universal', violating the implied API contract that adapter.type matches the key.
Expected behavior: each agent key should have an adapter instance whose type equals that agent and whose isDetected() logic checks that agent’s real directories.
Evidence
packages/agents/src/index.ts maps many distinct AgentType keys to new UniversalAdapter() (packages/agents/src/index.ts:61-75), and also adds those agent names into the detection order (packages/agents/src/index.ts:104-118).
(Refers to lines 61-119)
Recommendation: Implement real adapter classes (or a parameterized adapter) for each new agent so that adapter.type matches the agent key, and isDetected() checks that agent’s specific directories. At minimum, avoid using UniversalAdapter for non-universal agent keys.
Was this helpful? React with 👍 or 👎 to provide feedback.
- Remove createRelativeLink from docs page (not compatible with current fumadocs) - Remove redundant isLoading prop in SkillGenerator (status already narrowed) - Fix lucide-react icon type casting in Card component - Fix MDXComponents return type in mdx-components
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
README.md (1)
243-264: Inconsistency: Table still lists 17 agents while README claims 32.The README header, comparison table, and feature descriptions all reference 32 agents, but this section header says "Supported Agents (17)" and the table only lists 17 agents. The 15 new agents (cline, codebuddy, commandcode, continue, crush, factory, mcpjam, mux, neovate, openhands, pi, qoder, qwen, vercel, zencoder) added in
packages/core/src/translator/types.tsare missing here.Either update the table to include all 32 agents or clarify that only the 17 listed are "primary" agents with the others being community/experimental.
packages/agents/src/index.ts (1)
86-119: Detect order now misclassifies “universal” repos as “cline”.Because all new types share UniversalAdapter’s generic detection,
detectAgentwill return the first new type instead ofuniversalwhenever.agent/AGENTS.mdexists. Consider movinguniversalbefore these until you add distinct detection logic per agent.🛠️ Proposed fix (keep universal ahead of generic types)
const checkOrder: AgentType[] = [ 'claude-code', 'cursor', 'codex', 'gemini-cli', 'opencode', 'antigravity', 'amp', 'clawdbot', 'droid', 'github-copilot', 'goose', 'kilo', 'kiro-cli', 'roo', 'trae', 'windsurf', - 'cline', - 'codebuddy', - 'commandcode', - 'continue', - 'crush', - 'factory', - 'mcpjam', - 'mux', - 'neovate', - 'openhands', - 'pi', - 'qoder', - 'qwen', - 'vercel', - 'zencoder', 'universal', + 'cline', + 'codebuddy', + 'commandcode', + 'continue', + 'crush', + 'factory', + 'mcpjam', + 'mux', + 'neovate', + 'openhands', + 'pi', + 'qoder', + 'qwen', + 'vercel', + 'zencoder', ];docs/skillkit/components/Agents.tsx (1)
308-316: Keep the agent count in sync with the rendered list.Line 315 says “32”, but the AGENTS list currently has 31 entries. Consider deriving the number from
AGENTS.length(or adding the missing agent) to avoid drift.💡 Proposed fix (dynamic count)
- Works with 32 AI Agents + Works with {AGENTS.length} AI Agents
🤖 Fix all issues with AI agents
In `@docs/fumadocs/content/docs/agents.mdx`:
- Around line 22-24: The Clawdbot row in the table currently lists only
`skills/` which conflicts with the primary config `.clawdbot/skills`; update the
Clawdbot entry (the table row labelled "Clawdbot") to either use the primary
path `.clawdbot/skills/` or list both paths (e.g., `.clawdbot/skills/` and
`skills/`) so the docs match the core config and avoid confusion.
In `@docs/fumadocs/content/docs/cicd.mdx`:
- Around line 10-16: Update the docs snippet to use the implemented CLI command
form: replace the deprecated subcommands with the `skillkit cicd init` command
and add the `--provider` flag; specifically show examples such as `skillkit cicd
init --provider github`, `skillkit cicd init --provider gitlab`, and `skillkit
cicd init --provider circleci` so readers use `cicd init` and the `--provider`
options supported by the tool.
In `@docs/fumadocs/content/docs/commands.mdx`:
- Line 44: Update the provider names in the command docs table for the
`--provider` / `-p` option to use official capitalization: replace "github,
gitlab, bitbucket" with "GitHub, GitLab, Bitbucket" in the line that documents
the `--provider` (`-p`) flag so the brands are properly capitalized.
In `@docs/fumadocs/content/docs/quickstart.mdx`:
- Around line 158-166: Update the example JSON's "lastRefresh" value from
"2024-01-01T00:00:00Z" to a more recent placeholder (for example
"2025-01-01T00:00:00Z") in the quickstart documentation JSON block so the sample
feels current, keeping the ISO8601 Zulu format unchanged.
In `@docs/fumadocs/content/docs/skills.mdx`:
- Line 8: The sentence describing the skill format uses "markdown" in lowercase;
update the text so "Markdown" is capitalized as a proper noun (change "universal
markdown format (SKILL.md)" to "universal Markdown format (SKILL.md)") to
reflect the proper noun usage in the docs.
In `@docs/fumadocs/content/docs/testing.mdx`:
- Around line 2-51: Update the mismatch between the "14+ assertion types" claim
and the table: either change the sentence "SkillKit includes a testing framework
with 14+ assertion types." to an accurate count (e.g., "11 assertion types" or
"11+ assertion types") or add the missing assertion entries to the "Assertion
Types" table so the list includes 14+ items; locate the string "SkillKit
includes a testing framework with 14+ assertion types." and the table containing
`contains`, `not_contains`, `matches`, `starts_with`, `ends_with`,
`has_section`, `has_code_block`, `word_count_min`, `word_count_max`, `no_todos`,
and `has_frontmatter` and update one of them for consistency.
In `@docs/fumadocs/README.md`:
- Around line 23-32: Update the fenced code block that begins with
"docs/fumadocs/" in README.md to include a language identifier (e.g., add
```text instead of ``` ) so markdownlint rule MD040 is satisfied; locate the
block showing the directory tree and change its opening fence to include the
language token (text) to avoid the lint warning.
In `@docs/fumadocs/src/app/`(home)/layout.tsx:
- Around line 30-35: The Docs Link in layout.tsx is hidden on small screens via
the className "hidden sm:block" on the Link component, making docs inaccessible
on mobile; update the Link's className (the Link element that currently has
className="text-zinc-400 hover:text-white transition-colors text-sm font-mono
hidden sm:block") to remove the "hidden" and "sm:block" responsive classes so it
is visible on all screen sizes, or alternatively implement a mobile navigation
(hamburger/menu component) that includes this Link so mobile users can access
/docs.
In `@docs/fumadocs/src/components/Agents.tsx`:
- Around line 308-316: The header hard-codes "32" which can drift from the
AGENTS data; in the Agents component, derive the displayed count from the AGENTS
array (e.g., use AGENTS.length or duplicatedAgents.length if showing total)
instead of the literal "32", and also verify the AGENTS array contents (since
AGENTS currently appears to have 31 entries) to ensure no agent is missing
before updating the UI.
In `@docs/fumadocs/src/components/Button.tsx`:
- Around line 18-40: The Button component currently relies on the browser
default type and can cause accidental form submissions; update the Button
function signature to provide a default type by adding type = 'button' to the
destructured props in the Button({ ... }) parameters and ensure the rendered
<button> receives that type (it will be included via the existing {...props}
spread or set explicitly as type={type}); also update ButtonProps type if it
doesn't already include an optional type property so consumers can override it
with type="submit" when needed.
In `@docs/fumadocs/src/components/Documentation.tsx`:
- Around line 1-2: This file defines a React component that uses useState (the
Documentation component), so add the "use client" directive as the very first
line of the file to enable client-side rendering; ensure the string 'use client'
appears before any imports or code (above the import React, { useState } from
'react';) so the component and hooks like useState are allowed in this module.
In `@docs/fumadocs/src/components/SkillGenerator.tsx`:
- Around line 627-645: generatedSkill.references may contain non-URL strings and
is being used directly as href in the JSX map (generatedSkill.references.map),
risking broken links; update the render to validate each ref before creating an
<a> tag by checking URL validity (e.g., using the URL constructor or a utility
isValidUrl(ref)) and only render an <a href=... target="_blank"
rel="noreferrer"> for valid URLs, otherwise render the ref as plain text (same
styling but not a link); keep the mapping over generatedSkill.references and the
existing classNames/keys while swapping anchor vs. text rendering based on the
validation result.
In `@docs/fumadocs/src/components/SkillSubmitForm.tsx`:
- Around line 159-161: The onload handler for FileReader (reader.onload) may
call parseSkillContent with a null/undefined event.target?.result; update the
reader.onload implementation to first check that event?.target?.result is a
non-null string (or coerce to string only after verifying) and only call
parseSkillContent when a valid string is present, otherwise handle the empty
case (e.g., skip parsing or log/notify); reference reader.onload,
event.target?.result and parseSkillContent when making the change.
- Around line 156-163: processMarkdownFile is memoized with useCallback but
closes over parseSkillContent which is unstable, causing a stale closure; fix by
either wrapping parseSkillContent in useCallback and adding parseSkillContent to
processMarkdownFile's dependency array, or move the parsing/form-update logic
into processMarkdownFile (call setForm or updateField directly) so no external
unstable function is referenced—ensure references to parseSkillContent,
processMarkdownFile, updateField, and setForm are updated consistently and
include the correct dependencies in the hook arrays.
In `@docs/fumadocs/src/services/geminiService.ts`:
- Around line 6-15: The getAI function currently falls back to
NEXT_PUBLIC_GEMINI_API_KEY which exposes the key client-side; update getAI to
only read process.env.GEMINI_API_KEY (remove the NEXT_PUBLIC_GEMINI_API_KEY
fallback) and ensure this module (and the ai singleton) is used exclusively in
server-side code (API routes or server components); if client-side access is
required, instead implement a server-side proxy endpoint and have client code
call that endpoint rather than exposing the key in getAI.
In `@packages/agents/README.md`:
- Around line 31-32: The Factory agent is wired to UniversalAdapter while a
separate AGENT_CONFIG.factory exists but is never used; either create and wire a
dedicated FactoryAdapter (mirroring DroidAdapter) and switch the Factory mapping
from UniversalAdapter to FactoryAdapter so AGENT_CONFIG.factory is consumed, or
remove AGENT_CONFIG.factory entirely to avoid dead config; locate the Factory
registration where UniversalAdapter is referenced and update it to
FactoryAdapter (and implement FactoryAdapter to load `.factory/skills/`), or
delete the AGENT_CONFIG.factory entry if you choose the removal route.
In `@packages/core/src/agents/types.ts`:
- Line 239: The discovery path for the 'factory' agent type currently duplicates
the 'droid' path (both use ['.factory/agents']), causing attribution ambiguity;
update the 'factory' entry in the agent type map (the symbol 'factory' in
types.ts) to use a unique discovery path (for example ['.factoryai/agents'] or
['.factory-new/agents']) so files under .factory/agents are only attributed to
'droid' and the new path uniquely identifies 'factory'.
In `@packages/core/src/skills.ts`:
- Around line 78-104: parseStandaloneSkill currently returns .md file paths but
downstream helpers (readSkillContent, validateSkill and code in
recommend/fetcher and context/sync) expect a directory containing SKILL.md;
normalize handling by updating those helpers to accept either a direct .md file
or a skill directory: detect if the given path is a file ending with .md (or
exists as a file via fs.existsSync/stat), use that file as the SKILL content;
otherwise treat the input as a directory and resolve SKILL.md inside it.
Reference the functions parseStandaloneSkill, readSkillContent, validateSkill
and callers in recommend/fetcher and context/sync and ensure all places use the
normalized resolution logic so both standalone .md files and skill directories
work interchangeably.
In `@packages/tui/src/state/agents.ts`:
- Line 97: TOTAL_AGENTS was hard-coded to 32 and now mismatches the AgentType
enum and tests; change TOTAL_AGENTS to be derived from the actual adapter/agent
definitions instead of a magic number. Locate the TOTAL_AGENTS export in
agents.ts and replace the hard-coded 32 with a computed value (e.g., using the
adapters collection or the AgentType enum length) so it always reflects the real
count of agents/adapters; ensure the computation references the adapters object
or AgentType (defined in src/core/types.ts) so tests expecting 17 pass and the
UI shows correct statistics.
🧹 Nitpick comments (13)
packages/core/src/translator/types.ts (1)
7-11: Stale comment: agent count is outdated.The comment says "Category A: Standard SKILL.md (13 agents)" but with the 15 new additions, there are now 29 agents using
skill-md. Consider updating or removing the count to avoid confusion during future maintenance.Suggested fix
export type FormatCategory = - | 'skill-md' // Category A: Standard SKILL.md (13 agents) + | 'skill-md' // Category A: Standard SKILL.md format | 'cursor-mdc' // Category B: Cursor MDC format | 'markdown-rules' // Category C: Windsurf, GitHub Copilot | 'external'; // Category D: External systems (Devin, etc.)docs/fumadocs/src/components/Footer.tsx (1)
20-27: Consider addingnoopenerto the external link for consistency.The GitHub link on line 32 uses
rel="noopener noreferrer", but this link only usesrel="noreferrer". Whilenoreferrerimpliesnoopenerin modern browsers, adding both explicitly ensures consistency and clarity across the codebase.Suggested fix
<a href="https://agentskills.io" target="_blank" - rel="noreferrer" + rel="noopener noreferrer" className="text-zinc-600 hover:text-zinc-400 text-xs font-mono transition-colors" >docs/skillkit/App.tsx (1)
38-43: Prefer a relative/docslink for env portability.
Hardcoding the production domain bypasses rewrites and makes previews/staging point to prod docs.♻️ Suggested tweak
- href="https://agentskills.com/docs" + href="/docs"docs/fumadocs/src/components/Commands.tsx (1)
68-73: Set a non-submit button type defensively.♻️ Suggested tweak
<button key={g.name} + type="button" onClick={() => setActiveGroup(i)} className={`px-3 py-1.5 text-xs font-mono border transition-colors ${buttonClass}`} >docs/fumadocs/src/components/SkillSubmitForm.tsx (1)
104-109: GitHub API rate limiting may affect users.Unauthenticated GitHub API requests are limited to 60 per hour. Consider adding a note in the UI or handling 403 rate-limit responses specifically to inform users.
Optional: Add rate limit handling
const response = await fetch(apiUrl); if (!response.ok) { + if (response.status === 403) { + throw new Error('GitHub API rate limit exceeded. Please try again later.'); + } throw new Error('Repository not found or not public'); }docs/fumadocs/src/lib/layout.shared.tsx (1)
3-26: Consider using a constant instead of a function.Since
baseOptionsreturns a static object with no parameters or dynamic values, it could be a constant export for slightly better performance and clearer intent.Optional refactor
-export function baseOptions(): BaseLayoutProps { - return { +export const baseOptions: BaseLayoutProps = { nav: { title: 'SkillKit', }, links: [ { text: 'DeepWiki', url: 'https://deepwiki.com/rohitg00/skillkit', external: true, }, { text: 'GitHub', url: 'https://github.com/rohitg00/skillkit', external: true, }, { text: 'npm', url: 'https://www.npmjs.com/package/skillkit', external: true, }, ], - } -} +} as constThen update the usage in
layout.tsx:- {...baseOptions()} + {...baseOptions}docs/fumadocs/src/lib/components/Columns.tsx (1)
4-7: Restrictcolsto supported values (2–4).Only 2–4 map to classes; narrowing the type prevents invalid inputs from silently falling back to a 1‑column layout.
♻️ Suggested typing update
-type ColumnsProps = PropsWithChildren<{ - cols?: number +type ColumnsProps = PropsWithChildren<{ + cols?: 2 | 3 | 4 className?: string }>docs/fumadocs/content/docs/installation.mdx (1)
8-8: Avoid hard‑coding the current version in docs.
Line 8 will go stale after the next release. Consider sourcing from package metadata at build time or rephrasing to avoid a fixed version string.docs/fumadocs/src/components/Documentation.tsx (1)
439-493: Consider extracting a shared Documentation UI to avoid drift.
This component’s structure closely mirrorsdocs/skillkit/components/Documentation.tsx(lines 438–493). A shared component or shared data source would reduce duplication and future inconsistencies.docs/fumadocs/src/lib/components/Card.tsx (2)
14-16: Consider adding a fallback or warning for invalid icon names.The dynamic icon lookup will silently return
nullif the providediconstring doesn't match any Lucide icon export. This could lead to confusing behavior where an icon is expected but nothing renders.🔧 Optional: Add development warning for invalid icons
export const Card: React.FC<CardProps> = ({ title, description, href, icon, children, horizontal = false }) => { const IconComponent = icon ? (Icons as Record<string, React.ComponentType<{ className?: string }>>)[icon] : null + + if (process.env.NODE_ENV === 'development' && icon && !IconComponent) { + console.warn(`Card: Invalid icon name "${icon}". Check Lucide icon names.`) + } return (
1-34: Note: DuplicateCardcomponent exists in the codebase.There's another
Cardcomponent atdocs/fumadocs/src/components/Card.tsxwith a different interface (supportsonClick, drag handlers,noPadding, but nohref/icon/title/description). This component is insrc/lib/components/and uses Link-based navigation.Consider renaming one (e.g.,
LinkCardorNavCard) to avoid import confusion, or consolidate if the use cases can be merged.docs/fumadocs/src/services/geminiService.ts (2)
123-132: Improve error handling for JSON parsing and avoid logging potentially sensitive data.The JSON parsing at line 127 could fail with a generic error if Gemini returns malformed JSON. Additionally,
console.errorin production may log sensitive request/response data.🔧 Suggested improvements
if (!response.text) { throw new Error("No response from Gemini."); } - const data = JSON.parse(response.text) as AgentSkill; - return data; + try { + const data = JSON.parse(response.text) as AgentSkill; + return data; + } catch (parseError) { + throw new Error("Failed to parse Gemini response as valid JSON"); + } } catch (error) { - console.error("Gemini API Error:", error); + if (process.env.NODE_ENV === 'development') { + console.error("Gemini API Error:", error); + } throw error; } };
91-121: Consider adding a timeout for the Gemini API call.Long-running or hung API calls could block the request indefinitely. Adding a timeout (e.g., via
AbortController) would improve resilience.
| | Droid (Factory) | SKILL.md | `.factory/skills/` | - | | ||
| | Factory | SKILL.md | `.factory/skills/` | - | |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, read the README file to see the full context around lines 31-32
echo "=== README.md content around lines 31-32 ==="
cat -n packages/agents/README.md | sed -n '25,45p'
echo -e "\n=== Searching for Agent type definitions ==="
# Find the AgentType definition
rg "type AgentType|AgentType.*=" --type=ts packages/agents/src/ -A 35 | head -80
echo -e "\n=== Searching for Droid references ==="
rg "droid" --type=ts -i packages/agents/src/ -B 2 -A 2
echo -e "\n=== Searching for Factory agent implementation ==="
rg "factory" --type=ts -i packages/agents/src/ -B 2 -A 2Repository: rohitg00/skillkit
Length of output: 8898
🏁 Script executed:
#!/bin/bash
# Check AGENT_CONFIG for droid and factory configurations
echo "=== Searching for AGENT_CONFIG configuration ==="
rg "AGENT_CONFIG" --type=ts -A 3 packages/ | grep -A 100 "droid\|factory" | head -80
echo -e "\n=== Check UniversalAdapter implementation ==="
rg "class UniversalAdapter" --type=ts -A 20 packages/agents/src/
echo -e "\n=== Check if there's a Factory-specific adapter ==="
fd "factory" --type=f packages/agents/src/ -x cat {}Repository: rohitg00/skillkit
Length of output: 5876
🏁 Script executed:
#!/bin/bash
# Read the full universal.ts file
echo "=== Complete universal.ts ==="
cat -n packages/agents/src/universal.ts
echo -e "\n=== Search for AGENT_CONFIG definition ==="
rg "export.*AGENT_CONFIG|const.*AGENT_CONFIG" --type=ts packages/core/src/ -B 2 -A 40 | head -150Repository: rohitg00/skillkit
Length of output: 12519
🏁 Script executed:
#!/bin/bash
# Get the full droid and factory config entries
echo "=== Complete agent-config.ts to find droid and factory ==="
cat packages/core/src/agent-config.ts | grep -n "droid\|factory" -A 8 -B 1 | head -100
# Also check if there's a FACTORY type mapping
echo -e "\n=== Check index.ts mapping details ==="
cat -n packages/agents/src/index.ts | sed -n '1,150p'Repository: rohitg00/skillkit
Length of output: 5309
Fix Factory agent: it uses UniversalAdapter but has orphaned config.
Both "Droid (Factory)" and "Factory" correctly point to .factory/skills/ in the README. However, the Factory agent is mapped to UniversalAdapter which uses AGENT_CONFIG.universal instead of the defined AGENT_CONFIG.factory. This means the Factory configuration in the codebase is defined but never actually used. Either create a dedicated FactoryAdapter class (like DroidAdapter) or remove the unused AGENT_CONFIG.factory entry to avoid confusion.
🤖 Prompt for AI Agents
In `@packages/agents/README.md` around lines 31 - 32, The Factory agent is wired
to UniversalAdapter while a separate AGENT_CONFIG.factory exists but is never
used; either create and wire a dedicated FactoryAdapter (mirroring DroidAdapter)
and switch the Factory mapping from UniversalAdapter to FactoryAdapter so
AGENT_CONFIG.factory is consumed, or remove AGENT_CONFIG.factory entirely to
avoid dead config; locate the Factory registration where UniversalAdapter is
referenced and update it to FactoryAdapter (and implement FactoryAdapter to load
`.factory/skills/`), or delete the AGENT_CONFIG.factory entry if you choose the
removal route.
| 'commandcode': ['.commandcode/agents'], | ||
| 'continue': ['.continue/agents'], | ||
| 'crush': ['.crush/agents'], | ||
| 'factory': ['.factory/agents'], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/core/src/agents/types.ts | sed -n '220,250p'Repository: rohitg00/skillkit
Length of output: 1390
🏁 Script executed:
rg -n "'droid'" --type=ts -C2Repository: rohitg00/skillkit
Length of output: 8387
🏁 Script executed:
rg -n "'factory'" --type=ts -C2Repository: rohitg00/skillkit
Length of output: 2303
Duplicate discovery path prevents agent type attribution.
Both 'droid' (line 225) and 'factory' (line 239) share the same discovery path ['.factory/agents']. During agent discovery, files in .factory/agents cannot be attributed to either agent type, causing ambiguity or potential double-processing.
These are distinct agent types: 'droid' is an established agent with adapters and tests, while 'factory' is a new agent type. Assign a unique discovery path to 'factory', such as ['.factoryai/agents'] or ['.factory-new/agents'].
🤖 Prompt for AI Agents
In `@packages/core/src/agents/types.ts` at line 239, The discovery path for the
'factory' agent type currently duplicates the 'droid' path (both use
['.factory/agents']), causing attribution ambiguity; update the 'factory' entry
in the agent type map (the symbol 'factory' in types.ts) to use a unique
discovery path (for example ['.factoryai/agents'] or ['.factory-new/agents']) so
files under .factory/agents are only attributed to 'droid' and the new path
uniquely identifies 'factory'.
| function parseStandaloneSkill(filePath: string, location: SkillLocation = 'project'): Skill | null { | ||
| if (!existsSync(filePath)) { | ||
| return null; | ||
| } | ||
|
|
||
| try { | ||
| const content = readFileSync(filePath, 'utf-8'); | ||
| const frontmatter = extractFrontmatter(content); | ||
|
|
||
| if (!frontmatter) { | ||
| return null; | ||
| } | ||
|
|
||
| const name = (frontmatter.name as string) || basename(filePath, '.md'); | ||
| const description = (frontmatter.description as string) || 'No description available'; | ||
|
|
||
| if (!name || name.length === 0) { | ||
| return null; | ||
| } | ||
|
|
||
| return { | ||
| name, | ||
| description, | ||
| path: filePath, | ||
| location, | ||
| enabled: true, | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the structure and size of the skills.ts file
wc -l packages/core/src/skills.tsRepository: rohitg00/skillkit
Length of output: 92
🏁 Script executed:
# Find the readSkillContent and validateSkill function definitions
rg -n "function readSkillContent|export.*readSkillContent" packages/core/src/skills.ts
rg -n "function validateSkill|export.*validateSkill" packages/core/src/skills.tsRepository: rohitg00/skillkit
Length of output: 247
🏁 Script executed:
# Check how readSkillContent and validateSkill are used
rg -n "readSkillContent\(|validateSkill\(" --type=ts -C2Repository: rohitg00/skillkit
Length of output: 10581
🏁 Script executed:
# Check Skill type definition and path property usage
rg -n "interface Skill|type Skill" --type=ts -A10Repository: rohitg00/skillkit
Length of output: 39382
🏁 Script executed:
# Check downstream usage of .path property in context of skills
rg -n "skill\.path|\.path\s*=" --type=ts packages/core/src/ -C2Repository: rohitg00/skillkit
Length of output: 3208
🏁 Script executed:
sed -n '294,310p' packages/core/src/skills.tsRepository: rohitg00/skillkit
Length of output: 476
🏁 Script executed:
sed -n '343,380p' packages/core/src/skills.tsRepository: rohitg00/skillkit
Length of output: 1250
🏁 Script executed:
# Check parseStandaloneSkill to confirm it returns file path
sed -n '78,104p' packages/core/src/skills.tsRepository: rohitg00/skillkit
Length of output: 721
🏁 Script executed:
rg -n "parseStandaloneSkill" --type=ts packages/core/src/Repository: rohitg00/skillkit
Length of output: 375
🏁 Script executed:
# Check if there are other parsers that might create Skills with directory paths
rg -n "function.*parse.*[Ss]kill.*filePath|location.*project.*global" --type=ts packages/core/src/skills.ts -B2 -A10Repository: rohitg00/skillkit
Length of output: 1225
🏁 Script executed:
# Check how discovered skills are used - look at discoverSkills
sed -n '1,100p' packages/core/src/skills.ts | head -80Repository: rohitg00/skillkit
Length of output: 2014
🏁 Script executed:
# Check line 149 context to see second usage
sed -n '140,160p' packages/core/src/skills.tsRepository: rohitg00/skillkit
Length of output: 664
🏁 Script executed:
# Verify that discovered skills are actually used with readSkillContent or validateSkill
rg -n "discoverSkills|findAllSkills" --type=ts packages/core/src/ -A5 | head -60Repository: rohitg00/skillkit
Length of output: 3284
🏁 Script executed:
# Check the complete flow - where findAllSkills is used with readSkillContent/validateSkill
rg -n "findAllSkills\|discoverSkills" --type=ts -A10 packages/core/src/executor/skill-executor.ts packages/core/src/recommend/ packages/core/src/context/ | head -100Repository: rohitg00/skillkit
Length of output: 43
🏁 Script executed:
# Verify that executor passes discovered skills to readSkillContent
sed -n '100,135p' packages/core/src/executor/skill-executor.tsRepository: rohitg00/skillkit
Length of output: 1009
🏁 Script executed:
# Check if skill is found using parseStandaloneSkill in findSkill
sed -n '300,320p' packages/core/src/skills.tsRepository: rohitg00/skillkit
Length of output: 537
🏁 Script executed:
# Check what parseSkill does vs parseStandaloneSkill
sed -n '200,280p' packages/core/src/skills.ts | grep -A30 "function parseSkill"Repository: rohitg00/skillkit
Length of output: 933
🏁 Script executed:
# Verify recommend/fetcher actually uses discovered skills with standalone .md paths
sed -n '30,60p' packages/core/src/recommend/fetcher.tsRepository: rohitg00/skillkit
Length of output: 1276
🏁 Script executed:
# Check context/sync to see if it uses discovered skills
sed -n '195,210p' packages/core/src/context/sync.tsRepository: rohitg00/skillkit
Length of output: 592
Ensure helpers handle both file and directory paths for discovered standalone skills.
parseStandaloneSkill returns .md file paths, but readSkillContent, validateSkill, and downstream code in recommend/fetcher and context/sync assume directory paths with SKILL.md. This breaks standalone skill execution and syncing.
🛠️ Suggested fix (handle file paths in helpers)
export function readSkillContent(skillPath: string): string | null {
- const skillMdPath = join(skillPath, 'SKILL.md');
+ const skillMdPath = skillPath.endsWith('.md')
+ ? skillPath
+ : join(skillPath, 'SKILL.md');
if (!existsSync(skillMdPath)) {
return null;
} export function validateSkill(skillPath: string): { valid: boolean; errors: string[]; warnings?: string[] } {
const errors: string[] = [];
const warnings: string[] = [];
- const dirName = basename(skillPath);
-
- const skillMdPath = join(skillPath, 'SKILL.md');
+ const isStandalone = skillPath.endsWith('.md');
+ const dirName = isStandalone ? basename(skillPath, '.md') : basename(skillPath);
+ const skillMdPath = isStandalone ? skillPath : join(skillPath, 'SKILL.md');
if (!existsSync(skillMdPath)) {
errors.push('Missing SKILL.md file');
return { valid: false, errors };
}🤖 Prompt for AI Agents
In `@packages/core/src/skills.ts` around lines 78 - 104, parseStandaloneSkill
currently returns .md file paths but downstream helpers (readSkillContent,
validateSkill and code in recommend/fetcher and context/sync) expect a directory
containing SKILL.md; normalize handling by updating those helpers to accept
either a direct .md file or a skill directory: detect if the given path is a
file ending with .md (or exists as a file via fs.existsSync/stat), use that file
as the SKILL content; otherwise treat the input as a directory and resolve
SKILL.md inside it. Reference the functions parseStandaloneSkill,
readSkillContent, validateSkill and callers in recommend/fetcher and
context/sync and ensure all places use the normalized resolution logic so both
standalone .md files and skill directories work interchangeably.
| * Total number of supported agents | ||
| */ | ||
| export const TOTAL_AGENTS = 17; | ||
| export const TOTAL_AGENTS = 32; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "=== Agent Type Enum/Union ==="
rg "enum AgentType|type AgentType|export const AgentType" --type=ts -A 50 | head -100
echo ""
echo "=== TOTAL_AGENTS current usage ==="
rg "TOTAL_AGENTS" --type=ts -B 2 -A 2
echo ""
echo "=== Agent adapter files/classes ==="
fd -e ts -path "*/adapter*" | head -20
echo ""
echo "=== Agent config structure ==="
rg "AGENT_CONFIG\s*=" --type=ts -A 3 | head -50
echo ""
echo "=== getAllAdapters function ==="
rg "getAllAdapters" --type=ts -B 3 -A 10Repository: rohitg00/skillkit
Length of output: 25549
🏁 Script executed:
#!/bin/bash
# Verify the actual adapter count
python3 << 'EOF'
# Count from AgentType enum
agents = [
'claude-code', 'codex', 'cursor', 'antigravity', 'opencode',
'gemini-cli', 'amp', 'clawdbot', 'droid', 'github-copilot',
'goose', 'kilo', 'kiro-cli', 'roo', 'trae', 'windsurf', 'universal'
]
print(f"Actual AgentType enum count: {len(agents)}")
print(f"Expected TOTAL_AGENTS: 32")
print(f"Current mismatch: {32 - len(agents)} agents")
EOFRepository: rohitg00/skillkit
Length of output: 147
TOTAL_AGENTS = 32 does not match actual agent count; test expects 17 adapters.
The constant was updated to 32, but the AgentType enum in src/core/types.ts defines only 17 agents, and the test suite explicitly checks expect(adapters.length).toBe(17). This creates a critical inconsistency that will display incorrect statistics in the UI. Either correct TOTAL_AGENTS back to 17, or add 15 new agent types and update corresponding tests. Alternatively, derive the count programmatically to prevent future drift: export const TOTAL_AGENTS = Object.keys(adapters).length;
🤖 Prompt for AI Agents
In `@packages/tui/src/state/agents.ts` at line 97, TOTAL_AGENTS was hard-coded to
32 and now mismatches the AgentType enum and tests; change TOTAL_AGENTS to be
derived from the actual adapter/agent definitions instead of a magic number.
Locate the TOTAL_AGENTS export in agents.ts and replace the hard-coded 32 with a
computed value (e.g., using the adapters collection or the AgentType enum
length) so it always reflects the real count of agents/adapters; ensure the
computation references the adapters object or AgentType (defined in
src/core/types.ts) so tests expecting 17 pass and the UI shows correct
statistics.
- Handle undefined slug by defaulting to empty array for index page - Remove hidden class from Docs link so it shows on mobile
There was a problem hiding this comment.
Devin Review found 2 new potential issues.
🟡 1 issue in files not directly in the diff
🟡 Duplicate invalid source: "/" bug in Vite docs SkillSubmitForm (docs/skillkit/components/SkillSubmitForm.tsx:197-209)
The same issue exists in the Vite-based docs site version of SkillSubmitForm: for upload submissions (no repo detected), the generated JSON entry sets source to ${form.repoOwner}/${form.repoName}, which becomes "/".
This duplicates the broken marketplace entry generation and can mislead maintainers when triaging skill submissions.
Click to expand
Problematic line:
source: `${form.repoOwner}/${form.repoName}`,Recommendation: Apply the same fix as BUG-0001 in this duplicate implementation (or refactor to share a single implementation to prevent drift).
View issues and 17 additional flags in Devin Review.
| function generateIssueUrl(): string { | ||
| const skillId = form.repoOwner && form.repoName | ||
| ? `${form.repoOwner}/${form.repoName}/${form.slug}` | ||
| : `community/${form.authorGithub || 'anonymous'}/${form.slug}`; | ||
|
|
||
| const jsonEntry = JSON.stringify({ | ||
| id: skillId, | ||
| name: form.displayName, | ||
| description: form.description || '', | ||
| source: `${form.repoOwner}/${form.repoName}`, | ||
| tags: tagsArray, | ||
| author: form.authorGithub || form.repoOwner | ||
| }, null, 2); |
There was a problem hiding this comment.
🟡 Skill upload issues generate invalid marketplace JSON entry with source: "/"
When submitting a skill via Upload File mode, repoOwner/repoName are typically empty strings, but the generated marketplace JSON entry always sets source to ${form.repoOwner}/${form.repoName}.
Actual behavior: uploaded skills produce JSON like:
{
"source": "/"
}Expected behavior: uploaded skills should either (a) require a valid owner/repo source, or (b) omit source / use a valid placeholder consistent with the marketplace schema.
Impact: the generated issue content asks maintainers to copy/paste a JSON entry that violates the documented schema (source must match owner/repo), creating broken entries or extra manual cleanup.
Click to expand
Problematic line:
source: `${form.repoOwner}/${form.repoName}`,This occurs in the JSON snippet generation for issues.
Recommendation: In upload mode, either (1) add separate source input and validate it matches owner/repo, or (2) omit source for uploads and adjust the marketplace workflow/schema accordingly. At minimum, avoid emitting "/" by using source: form.repoOwner && form.repoName ? ${form.repoOwner}/${form.repoName} : undefined (and remove the field from JSON when undefined).
Was this helpful? React with 👍 or 👎 to provide feedback.
- Move Gemini API key to server-side route (security) - Add default type='button' to Button component - Use dynamic agent count instead of hardcoded 32 - Add 'use client' directive to Documentation - Fix stale closure in SkillSubmitForm with useCallback - Add URL validation for external references - Add noopener to external links for security - Fix Clawdbot path in agents.mdx - Update cicd.mdx with correct CLI commands - Capitalize proper nouns (Markdown, provider names) - Add text language to README code block - Update date example in quickstart.mdx
There was a problem hiding this comment.
Devin Review found 3 new potential issues.
🟡 1 issue in files not directly in the diff
🟡 Duplicate invalid source: "/" bug in Vite docs SkillSubmitForm (docs/skillkit/components/SkillSubmitForm.tsx:197-209)
The same issue exists in the Vite-based docs site version of SkillSubmitForm: for upload submissions (no repo detected), the generated JSON entry sets source to ${form.repoOwner}/${form.repoName}, which becomes "/".
This duplicates the broken marketplace entry generation and can mislead maintainers when triaging skill submissions.
Click to expand
Problematic line:
source: `${form.repoOwner}/${form.repoName}`,Recommendation: Apply the same fix as BUG-0001 in this duplicate implementation (or refactor to share a single implementation to prevent drift).
View issues and 16 additional flags in Devin Review.
There was a problem hiding this comment.
🟡 TUI agent list can include duplicates/incorrect agent identities after adding new agents
The TUI’s agent state construction uses adapter.type as the canonical identifier (packages/tui/src/state/agents.ts:50-55). Because the PR adds many new “agents” that are actually UniversalAdapter instances (see BUG-0001), getAllAdapters() will return multiple adapters whose .type is 'universal', causing duplicated agent IDs and incorrect detected/configured flags.
Actual vs expected:
- Expected: Each supported agent shows up once with a unique
typeand correct detection/config status. - Actual: Multiple entries can share the same
type('universal'), which can make the UI confusing and may break any logic that assumes unique agent IDs.
Click to expand
Problematic mapping logic:
const agents: AgentStatus[] = allAdapters.map((adapter) => ({
type: adapter.type,
name: adapter.name,
detected: adapter.type === detectedAgent,
configured: adapter.type === currentAgent,
}));(packages/tui/src/state/agents.ts:50-55)
(Refers to lines 50-55)
Recommendation: Once adapters are fixed to have correct unique types (BUG-0001), this resolves. As an additional safety measure, consider de-duplicating allAdapters by adapter.type before rendering state.
Was this helpful? React with 👍 or 👎 to provide feedback.
| cline: new UniversalAdapter(), | ||
| codebuddy: new UniversalAdapter(), | ||
| commandcode: new UniversalAdapter(), | ||
| continue: new UniversalAdapter(), | ||
| crush: new UniversalAdapter(), | ||
| factory: new UniversalAdapter(), | ||
| mcpjam: new UniversalAdapter(), | ||
| mux: new UniversalAdapter(), | ||
| neovate: new UniversalAdapter(), | ||
| openhands: new UniversalAdapter(), | ||
| pi: new UniversalAdapter(), | ||
| qoder: new UniversalAdapter(), | ||
| qwen: new UniversalAdapter(), | ||
| vercel: new UniversalAdapter(), | ||
| zencoder: new UniversalAdapter(), |
There was a problem hiding this comment.
🔴 New agent adapters all report type 'universal', breaking detection/configuration logic
packages/agents/src/index.ts registers many new agent keys (cline, codebuddy, commandcode, etc.) by instantiating new UniversalAdapter(), but UniversalAdapter has a fixed type: AgentType = 'universal'. As a result, every one of these adapters will report its type as 'universal'.
Actual vs expected:
- Expected:
getAdapter('cline').type === 'cline'(and similarly for other new agents), so UIs and logic can distinguish agents. - Actual:
getAdapter('cline').type === 'universal'because it’s aUniversalAdapterinstance.
Impact:
- Any code that relies on
adapter.typeto identify the agent will misbehave. For example, the TUI builds its agent list usingadapter.typeand then setsdetected/configuredflags based on that value (packages/tui/src/state/agents.ts:50-55in the PR shows this pattern). With multiple adapters reporting'universal', the list will contain duplicates/incorrect identities and the configured/detected flags can be wrong. detectAgent()will also be ineffective for these “new agents” because their adapters are universal detectors, not agent-specific.
Click to expand
The problematic registrations:
// packages/agents/src/index.ts
cline: new UniversalAdapter(),
codebuddy: new UniversalAdapter(),
commandcode: new UniversalAdapter(),
// ... many others(packages/agents/src/index.ts:61-75)
Downstream usage that expects a correct adapter.type:
const agents: AgentStatus[] = allAdapters.map((adapter) => ({
type: adapter.type,
detected: adapter.type === detectedAgent,
configured: adapter.type === currentAgent,
}));(packages/tui/src/state/agents.ts:50-55)
Recommendation: Create real adapter classes for each new agent (or a parameterized adapter) so that type, skillsDir, configFile, and isDetected() are correct for each agent. At minimum, don’t reuse UniversalAdapter instances for distinct agent keys unless the adapter can be constructed with the desired type.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/fumadocs/src/components/SkillGenerator.tsx`:
- Around line 208-226: In handleViewSkill, stop silently swallowing fetch
errors: catch the exception thrown by fetch(skill.rawUrl) and
setStatus(LoadingState.ERROR) (or a new error state) and optionally
setSkillContent(null) so the UI shows an error state; also log or include the
error message (e.g., via console.error or processLogger) when catching to aid
debugging — ensure the success path still sets LoadingState.SUCCESS only when
response.ok and content was successfully set.
🧹 Nitpick comments (1)
docs/fumadocs/src/mdx-components.tsx (1)
6-13: Consider preserving type safety instead of usinganyreturn type.The
anyreturn type bypasses TypeScript's type checking. While the eslint-disable comment acknowledges this, you could use a more precise type:💡 Suggested improvement
-// eslint-disable-next-line `@typescript-eslint/no-explicit-any` -export function getMDXComponents(components?: MDXComponents): any { +export function getMDXComponents(components?: MDXComponents): MDXComponents { return { ...defaultMdxComponents, ...components, Card, Columns, - } + } as MDXComponents }
| async function handleViewSkill(skill: IndexedSkill): Promise<void> { | ||
| setSelectedSkill(skill); | ||
| setSkillContent(null); | ||
| setViewMode('content'); | ||
|
|
||
| if (skill.rawUrl) { | ||
| setStatus(LoadingState.LOADING); | ||
| try { | ||
| const response = await fetch(skill.rawUrl); | ||
| if (response.ok) { | ||
| const content = await response.text(); | ||
| setSkillContent(content); | ||
| } | ||
| } catch { | ||
| // Silently handle fetch errors | ||
| } | ||
| setStatus(LoadingState.SUCCESS); | ||
| } | ||
| } |
There was a problem hiding this comment.
Fetch errors are silently ignored, leaving users without feedback.
When the fetch fails (line 221-223), the status is still set to SUCCESS and no error is shown. The user will see the "Skill content not available for direct preview" fallback without understanding why.
💡 Suggested improvement: surface fetch errors
if (skill.rawUrl) {
setStatus(LoadingState.LOADING);
try {
const response = await fetch(skill.rawUrl);
if (response.ok) {
const content = await response.text();
setSkillContent(content);
+ } else {
+ console.warn(`Failed to fetch skill content: ${response.status}`);
}
- } catch {
- // Silently handle fetch errors
+ } catch (err) {
+ console.warn('Failed to fetch skill content:', err);
}
setStatus(LoadingState.SUCCESS);
}This at least provides debugging information while maintaining the graceful fallback UI.
📝 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.
| async function handleViewSkill(skill: IndexedSkill): Promise<void> { | |
| setSelectedSkill(skill); | |
| setSkillContent(null); | |
| setViewMode('content'); | |
| if (skill.rawUrl) { | |
| setStatus(LoadingState.LOADING); | |
| try { | |
| const response = await fetch(skill.rawUrl); | |
| if (response.ok) { | |
| const content = await response.text(); | |
| setSkillContent(content); | |
| } | |
| } catch { | |
| // Silently handle fetch errors | |
| } | |
| setStatus(LoadingState.SUCCESS); | |
| } | |
| } | |
| async function handleViewSkill(skill: IndexedSkill): Promise<void> { | |
| setSelectedSkill(skill); | |
| setSkillContent(null); | |
| setViewMode('content'); | |
| if (skill.rawUrl) { | |
| setStatus(LoadingState.LOADING); | |
| try { | |
| const response = await fetch(skill.rawUrl); | |
| if (response.ok) { | |
| const content = await response.text(); | |
| setSkillContent(content); | |
| } else { | |
| console.warn(`Failed to fetch skill content: ${response.status}`); | |
| } | |
| } catch (err) { | |
| console.warn('Failed to fetch skill content:', err); | |
| } | |
| setStatus(LoadingState.SUCCESS); | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@docs/fumadocs/src/components/SkillGenerator.tsx` around lines 208 - 226, In
handleViewSkill, stop silently swallowing fetch errors: catch the exception
thrown by fetch(skill.rawUrl) and setStatus(LoadingState.ERROR) (or a new error
state) and optionally setSkillContent(null) so the UI shows an error state; also
log or include the error message (e.g., via console.error or processLogger) when
catching to aid debugging — ensure the success path still sets
LoadingState.SUCCESS only when response.ok and content was successfully set.
- Add noopener to rel attributes on external links for tabnabbing protection - Add aria-label to nav element for screen readers - Add type="button" and aria-current to nav buttons
There was a problem hiding this comment.
Devin Review found 1 new potential issue.
🟡 1 issue in files not directly in the diff
🟡 Duplicate invalid source: "/" bug in Vite docs SkillSubmitForm (docs/skillkit/components/SkillSubmitForm.tsx:197-209)
The same issue exists in the Vite-based docs site version of SkillSubmitForm: for upload submissions (no repo detected), the generated JSON entry sets source to ${form.repoOwner}/${form.repoName}, which becomes "/".
This duplicates the broken marketplace entry generation and can mislead maintainers when triaging skill submissions.
Click to expand
Problematic line:
source: `${form.repoOwner}/${form.repoName}`,Recommendation: Apply the same fix as BUG-0001 in this duplicate implementation (or refactor to share a single implementation to prevent drift).
View issue and 20 additional flags in Devin Review.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@docs/fumadocs/src/app/api/generate-skill/route.ts`:
- Around line 72-76: The public POST handler (export async function
POST(request: NextRequest) in route.ts) currently allows unauthenticated calls
that can trigger paid Gemini usage; add basic abuse protection by validating an
authenticated session or API token and enforcing a rate limit before calling
Gemini: check for a session/auth header or require a per-user API key and reject
unauthorized requests, and implement a lightweight rate limiter (in-memory for
dev or Redis/kv for production) keyed by user ID or IP to throttle excessive
POST requests; ensure the checks occur at the top of POST and return an
appropriate NextResponse (429 for rate limited, 401 for unauthorized) before
reading process.env.GEMINI_API_KEY or making external calls.
- Around line 79-83: The code currently assumes topic is a string before calling
topic?.trim(), which can throw if topic is not a string; modify the handler to
validate the type of topic returned from await request.json() (check typeof
topic === 'string') before trimming/slicing, and if it's missing or not a string
return NextResponse.json({ error: "Topic is required" }, { status: 400 }); then
compute sanitizedTopic = topic.trim().slice(0, 500); update any references to
sanitizedTopic accordingly.
In `@docs/fumadocs/src/components/SkillGenerator.tsx`:
- Around line 29-36: The frontmatter in generateSkillMd interpolates raw skill
fields which may contain quotes or newlines and break YAML; add an escape helper
(e.g., escapeYamlString) and use it when interpolating values: call it for
skill.name, skill.description, skill.version and for each tag and filePattern
before joining so quotes/newlines are escaped or the value is serialized safely;
update the tags and globs lines to map through that helper (e.g.,
skill.tags.map(t => `"${escapeYamlString(t)}"`)) and ensure empty arrays are
handled the same way.
- Around line 1-6: This file exports a React component that uses client-only
hooks and browser APIs (useState, useRef, useEffect, localStorage), so add the
Next.js App Router client directive by inserting "use client" as the very first
line of the module; ensure the string literal appears before any imports in
SkillGenerator.tsx (the module that contains the SkillGenerator component and
imports like generateAgentSkill, searchSkills, AgentSkill, LoadingState) so the
component is treated as a Client Component and runtime errors are avoided.
In `@docs/fumadocs/vercel.json`:
- Around line 3-5: Update the vercel config entry for installCommand so it uses
the repository's package manager by changing the "installCommand" value from
"npm install" to "pnpm install"; modify the key "installCommand" in the existing
JSON block alongside "buildCommand" and "outputDirectory" to ensure Vercel runs
pnpm and respects pnpm-lock.yaml during builds.
🧹 Nitpick comments (7)
docs/fumadocs/src/components/Documentation.tsx (2)
11-15: Unusedlanguageparameter inCodeBlock.The
languageparameter is defined but never used in the component. Either remove it or implement syntax highlighting based on the language value.Option 1: Remove unused parameter
-const CodeBlock = ({ children, language = 'bash' }: { children: string; language?: string }) => ( +const CodeBlock = ({ children }: { children: string }) => ( <pre className="bg-zinc-900 border border-zinc-800 rounded-lg p-4 overflow-x-auto text-sm font-mono"> <code className="text-zinc-300">{children}</code> </pre> );Option 2: Add data attribute for future styling/highlighting
const CodeBlock = ({ children, language = 'bash' }: { children: string; language?: string }) => ( - <pre className="bg-zinc-900 border border-zinc-800 rounded-lg p-4 overflow-x-auto text-sm font-mono"> + <pre className="bg-zinc-900 border border-zinc-800 rounded-lg p-4 overflow-x-auto text-sm font-mono" data-language={language}> <code className="text-zinc-300">{children}</code> </pre> );
11-15: Consider reusing the existingCodeBlockfromTerminal.tsx.A similar
CodeBlockcomponent exists indocs/fumadocs/src/components/Terminal.tsx. Consider extracting it to a shared location to reduce duplication and maintain consistency.docs/fumadocs/content/docs/agents.mdx (1)
24-24: Clarify relationship between Droid and Factory agents.Both "Droid (Factory)" on line 24 and "Factory" on line 35 map to
.factory/skills/. Consider either:
- Removing the duplicate "Factory" row since "Droid (Factory)" already covers it, or
- Adding a note explaining their relationship
Also applies to: 35-35
docs/fumadocs/src/components/SkillSubmitForm.tsx (3)
69-87:updateFieldis not in the dependency array ofparseSkillContent.The callback calls
updateFieldbut has an empty dependency array. While this works in practice becauseupdateFielduses the functional update pattern (setForm(prev => ...)), it violates the exhaustive-deps rule and could cause subtle issues ifupdateFieldis refactored later.♻️ Suggested fix: wrap updateField in useCallback
- function updateField(field: keyof SkillFormData, value: string): void { + const updateField = useCallback(function(field: keyof SkillFormData, value: string): void { setForm(prev => { const updated = { ...prev, [field]: value }; if (field === 'displayName' && !prev.slug) { updated.slug = value.toLowerCase().replace(/\s+/g, '-').replace(/[^a-z0-9-]/g, ''); } return updated; }); - } + }, []);Then add it to parseSkillContent's dependencies:
const parseSkillContent = useCallback(function(content: string): void { // ... existing logic - }, []); + }, [updateField]);
279-298: Consider addingtype="button"to mode toggle buttons for consistency.These buttons lack the explicit
type="button"attribute, while other buttons in the component (tag suggestions at line 434, submit at line 532) include it. While this doesn't cause issues since they're not inside a<form>, adding the attribute improves consistency and prevents potential issues if the component structure changes.🔧 Suggested fix
<button + type="button" onClick={() => setMode('import')} className={`px-6 py-3 font-mono text-sm transition-colors ${ mode === 'import' ? 'text-white border-b-2 border-white -mb-px' : 'text-zinc-500 hover:text-zinc-300' }`} > Import from GitHub </button> <button + type="button" onClick={() => setMode('upload')}
317-327: Same issue: Detect button lackstype="button".For consistency with other buttons in the component, consider adding the explicit type attribute.
🔧 Suggested fix
<button + type="button" onClick={handleDetect} disabled={isDetecting || !githubUrl}docs/fumadocs/src/app/api/generate-skill/route.ts (1)
117-122: Add error handling for JSON parsing to improve resilience.The code correctly uses
response.textper the@google/genaiAPI documentation. However, adding a try-catch guard aroundJSON.parseis good defensive programming. Even withresponseSchemaenforcement, unexpected model behavior or edge cases could cause parsing failures. The current code would crash and return a generic 500; explicit error handling returns a more appropriate error response.🔧 Suggested hardening
- if (!response.text) { + const raw = response.text; + if (!raw) { return NextResponse.json({ error: "No response from Gemini" }, { status: 500 }); } - const data = JSON.parse(response.text); - return NextResponse.json(data); + try { + const data = JSON.parse(raw); + return NextResponse.json(data); + } catch { + return NextResponse.json({ error: "Gemini returned invalid JSON" }, { status: 502 }); + }
| export async function POST(request: NextRequest) { | ||
| const apiKey = process.env.GEMINI_API_KEY; | ||
| if (!apiKey) { | ||
| return NextResponse.json({ error: "Gemini API key not configured" }, { status: 500 }); | ||
| } |
There was a problem hiding this comment.
Protect the generation endpoint from abuse.
This public POST route can trigger paid Gemini calls. Add basic abuse protection (rate limiting, auth/session gating, or CAPTCHA) to prevent quota drain and unexpected costs.
🤖 Prompt for AI Agents
In `@docs/fumadocs/src/app/api/generate-skill/route.ts` around lines 72 - 76, The
public POST handler (export async function POST(request: NextRequest) in
route.ts) currently allows unauthenticated calls that can trigger paid Gemini
usage; add basic abuse protection by validating an authenticated session or API
token and enforcing a rate limit before calling Gemini: check for a session/auth
header or require a per-user API key and reject unauthorized requests, and
implement a lightweight rate limiter (in-memory for dev or Redis/kv for
production) keyed by user ID or IP to throttle excessive POST requests; ensure
the checks occur at the top of POST and return an appropriate NextResponse (429
for rate limited, 401 for unauthorized) before reading
process.env.GEMINI_API_KEY or making external calls.
- Add rate limiting to API route (10 requests/min per IP) - Add proper type validation for topic parameter - Move API key check after input validation - Add 'use client' directive to SkillGenerator - Add YAML escape helper to prevent frontmatter injection - Change vercel.json to use pnpm instead of npm
There was a problem hiding this comment.
Devin Review found 7 new potential issues.
🟡 1 issue in files not directly in the diff
🟡 Duplicate invalid source: "/" bug in Vite docs SkillSubmitForm (docs/skillkit/components/SkillSubmitForm.tsx:197-209)
The same issue exists in the Vite-based docs site version of SkillSubmitForm: for upload submissions (no repo detected), the generated JSON entry sets source to ${form.repoOwner}/${form.repoName}, which becomes "/".
This duplicates the broken marketplace entry generation and can mislead maintainers when triaging skill submissions.
Click to expand
Problematic line:
source: `${form.repoOwner}/${form.repoName}`,Recommendation: Apply the same fix as BUG-0001 in this duplicate implementation (or refactor to share a single implementation to prevent drift).
View issues and 24 additional flags in Devin Review.
| cline: new UniversalAdapter(), | ||
| codebuddy: new UniversalAdapter(), | ||
| commandcode: new UniversalAdapter(), | ||
| continue: new UniversalAdapter(), | ||
| crush: new UniversalAdapter(), | ||
| factory: new UniversalAdapter(), | ||
| mcpjam: new UniversalAdapter(), | ||
| mux: new UniversalAdapter(), | ||
| neovate: new UniversalAdapter(), | ||
| openhands: new UniversalAdapter(), | ||
| pi: new UniversalAdapter(), | ||
| qoder: new UniversalAdapter(), | ||
| qwen: new UniversalAdapter(), | ||
| vercel: new UniversalAdapter(), | ||
| zencoder: new UniversalAdapter(), |
There was a problem hiding this comment.
🔴 New agents registered with UniversalAdapter report type='universal', breaking detection and per-agent state
packages/agents/src/index.ts registers many new agent keys (e.g. cline, codebuddy, etc.) to new UniversalAdapter() packages/agents/src/index.ts:61-75. But UniversalAdapter hardcodes readonly type: AgentType = 'universal' packages/agents/src/universal.ts:10-13.
Actual behavior:
detectAgent()iteratescheckOrderand callsadapter.isDetected(); forcline/etc it is actuallyUniversalAdapter.isDetected()(checks for.agentorAGENTS.md)packages/agents/src/universal.ts:95-100. If a project hasAGENTS.md(common for SkillKit), detection will return the first of these new agent names in the check order (currentlycline) rather than the real installed agent oruniversalpackages/agents/src/index.ts:86-120.getAllAdapters()returns many adapter objects whosetypeis'universal', so UIs that rely onadapter.typeto identify the agent will misbehave (e.g., TUI usesadapter.typeforAgentStatus.typeand to mark detected/configured)packages/tui/src/state/agents.ts:50-55.
Expected behavior:
- Each registered agent key should return an adapter whose
.typematches that agent key, and detection for that agent should check that agent’s actual directories (not.agent/AGENTS.md).
Impact:
- Wrong agent auto-detection and incorrect UI state for agents; can lead to generating/syncing to the wrong directories or showing incorrect “detected”/“configured” agent in the TUI.
Minimal repro
- In a repo with
AGENTS.mdpresent (common when SkillKit is initialized). - Call
detectAgent(). - It will likely return
'cline'(first of the newUniversalAdapter-backed entries) instead of'universal'or the actual agent.
Recommendation: Implement dedicated adapters for new agent types (or parameterize UniversalAdapter so type, skillsDir, detection paths, etc. are agent-specific). At minimum, do not map multiple AgentType keys to an adapter whose type is always 'universal'.
Was this helpful? React with 👍 or 👎 to provide feedback.
| cline: new UniversalAdapter(), | ||
| codebuddy: new UniversalAdapter(), | ||
| commandcode: new UniversalAdapter(), | ||
| continue: new UniversalAdapter(), | ||
| crush: new UniversalAdapter(), | ||
| factory: new UniversalAdapter(), | ||
| mcpjam: new UniversalAdapter(), | ||
| mux: new UniversalAdapter(), | ||
| neovate: new UniversalAdapter(), | ||
| openhands: new UniversalAdapter(), | ||
| pi: new UniversalAdapter(), | ||
| qoder: new UniversalAdapter(), | ||
| qwen: new UniversalAdapter(), | ||
| vercel: new UniversalAdapter(), | ||
| zencoder: new UniversalAdapter(), |
There was a problem hiding this comment.
🔴 New agents registered with UniversalAdapter report type='universal', breaking detection and per-agent state
packages/agents/src/index.ts registers many new agent keys (e.g. cline, codebuddy, etc.) to new UniversalAdapter() packages/agents/src/index.ts:61-75. But UniversalAdapter hardcodes readonly type: AgentType = 'universal' packages/agents/src/universal.ts:10-13.
Actual behavior:
detectAgent()iteratescheckOrderand callsadapter.isDetected(); forcline/etc it is actuallyUniversalAdapter.isDetected()(checks for.agentorAGENTS.md)packages/agents/src/universal.ts:95-100. If a project hasAGENTS.md(common for SkillKit), detection will return the first of these new agent names in the check order (currentlycline) rather than the real installed agent oruniversalpackages/agents/src/index.ts:86-120.getAllAdapters()returns many adapter objects whosetypeis'universal', so UIs that rely onadapter.typeto identify the agent will misbehave (e.g., TUI usesadapter.typeforAgentStatus.typeand to mark detected/configured)packages/tui/src/state/agents.ts:50-55.
Expected behavior:
- Each registered agent key should return an adapter whose
.typematches that agent key, and detection for that agent should check that agent’s actual directories (not.agent/AGENTS.md).
Impact:
- Wrong agent auto-detection and incorrect UI state for agents; can lead to generating/syncing to the wrong directories or showing incorrect “detected”/“configured” agent in the TUI.
Minimal repro
- In a repo with
AGENTS.mdpresent (common when SkillKit is initialized). - Call
detectAgent(). - It will likely return
'cline'(first of the newUniversalAdapter-backed entries) instead of'universal'or the actual agent.
Recommendation: Implement dedicated adapters for new agent types (or parameterize UniversalAdapter so type, skillsDir, detection paths, etc. are agent-specific). At minimum, do not map multiple AgentType keys to an adapter whose type is always 'universal'.
Was this helpful? React with 👍 or 👎 to provide feedback.
| function buildSkillIndex(): IndexedSkill[] { | ||
| return skillsData.skills.map(skill => { | ||
| const sourceParts = skill.source?.split('/') || []; | ||
| const owner = sourceParts[0] || 'unknown'; | ||
| const repo = sourceParts[1] || 'unknown'; | ||
| const skillSlug = skill.id.split('/').pop() || skill.name.toLowerCase().replace(/\s+/g, '-'); | ||
|
|
||
| return { | ||
| id: skill.id, | ||
| name: skill.name, | ||
| description: skill.description || '', | ||
| source: skill.source || '', | ||
| sourceUrl: owner !== 'unknown' ? `https://github.com/${skill.source}` : '', | ||
| rawUrl: owner !== 'unknown' && repo !== 'unknown' | ||
| ? `https://raw.githubusercontent.com/${owner}/${repo}/main/skills/${skillSlug}/SKILL.md` | ||
| : '', | ||
| tags: skill.tags || [], | ||
| }; |
There was a problem hiding this comment.
🟡 Marketplace preview rawUrl generation breaks for skill IDs with special characters and for collection entries
docs/fumadocs/src/data/skills-index.ts constructs a GitHub raw URL assuming:
- the skill lives at
/main/skills/<skillSlug>/SKILL.md, and <skillSlug>is safe to embed in a URL path.
But marketplace/skills.json includes entries that violate those assumptions:
- Collection entries (
idends with/collection) are not a skill folder and generally won’t haveskills/collection/SKILL.md, so previews will 404. - Some IDs contain characters like
:(e.g.google-labs-code/stitch-skills/react:components), producing an invalid/incorrect path segment.
Actual behavior: Skill preview fetches will frequently fail (404 / bad URL) even for valid marketplace entries.
Expected behavior: raw URLs should be derived from explicit metadata (e.g. a rawUrl field in the index) or should at least handle type: "collection" and URL-encode/sanitize slugs.
Click to expand
Raw URL construction: docs/fumadocs/src/data/skills-index.ts:14-31.
In marketplace data, examples include:
id: "anthropics/skills/collection"(collection)marketplace/skills.json:9-18id: "google-labs-code/stitch-skills/react:components"(colon)marketplace/skills.json:1536-1544
Recommendation: Skip preview/rawUrl generation for type: "collection" entries, and URL-encode the slug (or better: add an explicit per-skill rawUrl/path field in skills.json and use that).
Was this helpful? React with 👍 or 👎 to provide feedback.
| const rateLimitMap = new Map<string, { count: number; resetTime: number }>(); | ||
| const RATE_LIMIT_WINDOW_MS = 60 * 1000; | ||
| const RATE_LIMIT_MAX_REQUESTS = 10; | ||
|
|
||
| function getRateLimitKey(request: NextRequest): string { | ||
| const forwarded = request.headers.get("x-forwarded-for"); | ||
| const ip = forwarded ? forwarded.split(",")[0].trim() : "unknown"; | ||
| return ip; | ||
| } | ||
|
|
||
| function checkRateLimit(key: string): { allowed: boolean; remaining: number } { | ||
| const now = Date.now(); | ||
| const entry = rateLimitMap.get(key); | ||
|
|
||
| if (!entry || now > entry.resetTime) { | ||
| rateLimitMap.set(key, { count: 1, resetTime: now + RATE_LIMIT_WINDOW_MS }); | ||
| return { allowed: true, remaining: RATE_LIMIT_MAX_REQUESTS - 1 }; | ||
| } | ||
|
|
||
| if (entry.count >= RATE_LIMIT_MAX_REQUESTS) { | ||
| return { allowed: false, remaining: 0 }; | ||
| } | ||
|
|
||
| entry.count++; | ||
| return { allowed: true, remaining: RATE_LIMIT_MAX_REQUESTS - entry.count }; | ||
| } |
There was a problem hiding this comment.
🟡 In-memory rateLimitMap grows without bounds (memory leak / DoS risk)
The API route implements rate limiting with a module-level Map keyed by client IP (rateLimitMap). Entries are never pruned, only overwritten when the same key is reused after reset.
Actual behavior: a stream of requests with many distinct x-forwarded-for values will grow rateLimitMap indefinitely, increasing memory usage over time.
Expected behavior: rate limit storage should have eviction/pruning (e.g. delete expired entries), and ideally use a shared store suited for multi-instance/serverless deployments.
Click to expand
- Map declared:
docs/fumadocs/src/app/api/generate-skill/route.ts:4 - Only ever
set/increment; no deletion path:docs/fumadocs/src/app/api/generate-skill/route.ts:14-29
Recommendation: Prune expired entries in checkRateLimit (e.g. when now > resetTime, delete the old key before re-adding) and/or periodically clean the map. For production/serverless, use a central store (Redis/Vercel KV) with TTL.
Was this helpful? React with 👍 or 👎 to provide feedback.
| return skills; | ||
| } | ||
|
|
||
| function parseStandaloneSkill(filePath: string, location: SkillLocation = 'project'): Skill | null { | ||
| if (!existsSync(filePath)) { | ||
| return null; | ||
| } | ||
|
|
||
| try { | ||
| const content = readFileSync(filePath, 'utf-8'); | ||
| const frontmatter = extractFrontmatter(content); | ||
|
|
||
| if (!frontmatter) { | ||
| return null; | ||
| } | ||
|
|
||
| const name = (frontmatter.name as string) || basename(filePath, '.md'); | ||
| const description = (frontmatter.description as string) || 'No description available'; | ||
|
|
||
| if (!name || name.length === 0) { | ||
| return null; | ||
| } | ||
|
|
||
| return { | ||
| name, | ||
| description, | ||
| path: filePath, | ||
| location, | ||
| enabled: true, | ||
| }; | ||
| } catch { | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
🔴 Standalone .md skill discovery sets Skill.path to a file path, but rest of system assumes Skill.path is a directory
parseStandaloneSkill() returns Skill.path as the markdown file path (path: filePath), while existing code throughout the repo assumes skill.path is a directory containing SKILL.md and other assets.
Actual vs expected:
- Actual: For standalone skills discovered from
*.mdfiles,skill.pathis the file itself. - Expected:
skill.pathshould be a base directory (as it is for directory skills), or the rest of the code should be updated to support file-backed skills.
Impact:
- Multiple call sites do
join(skill.path, 'SKILL.md')orrmSync(skill.path, { recursive: true }), which will behave incorrectly for file-backed skills.- Example: recommendation/context code constructs
join(skill.path, 'SKILL.md')(packages/core/src/context/sync.ts:201per grep results) which becomes"/path/to/skill.md/SKILL.md"(invalid). - CLI update/remove paths may delete the file when expecting a folder.
- Example: recommendation/context code constructs
Where it happens
return {
name,
description,
path: filePath,
location,
enabled: true,
};packages/core/src/skills.ts:98-104
(Refers to lines 67-108)
Recommendation: Either (a) keep Skill.path as a directory by creating a synthetic directory representation, or (b) extend the Skill type to include a filePath vs dirPath distinction and update all consumers (read/translate/sync/delete) to handle file-backed skills safely.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if (skill.rawUrl) { | ||
| setStatus(LoadingState.LOADING); | ||
| try { | ||
| const response = await fetch(skill.rawUrl); | ||
| if (response.ok) { | ||
| const content = await response.text(); | ||
| setSkillContent(content); | ||
| } | ||
| } catch { | ||
| // Silently handle fetch errors | ||
| } | ||
| setStatus(LoadingState.SUCCESS); | ||
| } | ||
| } | ||
|
|
||
| async function handleGenerateWithAI(): Promise<void> { | ||
| setError(null); |
There was a problem hiding this comment.
🟡 SkillGenerator caches AI results keyed on raw input but normalizes only in storage, causing duplicate cache entries for equivalent queries
SkillGenerator normalizes cache keys via normalizeQuery() inside getCachedSkill()/setCachedSkill() docs/fumadocs/src/components/SkillGenerator.tsx:154-181, but handleGenerateWithAI() passes the un-normalized input directly to these helpers docs/fumadocs/src/components/SkillGenerator.tsx:241-257.
Actual behavior:
- Queries that differ only by surrounding whitespace / repeated spaces / casing can still create redundant cache entries across sessions because the cache stores
query: keybut the UI uses the rawinputfor display and can keep re-adding if the normalized key differs from the raw string at different times (e.g.,"React hooks"vs"react hooks").
Expected behavior:
- Cache should be effectively keyed on a canonical normalized query and the UI should use the same canonical value for both lookup and insertion.
Impact:
- Reduced cache hit rate and faster eviction of useful entries (max 50) due to duplicates.
Why this is an actual bug here
Even though setCachedSkill() normalizes internally, the component state isFromCache / and any future cache inspection uses the raw input, so users can easily generate duplicates by varying whitespace/case, harming eviction behavior.
Recommendation: Normalize input once (e.g., const queryKey = normalizeQuery(input)), use that for both cache lookup and insertion, and (optionally) store/display a separate original query string if needed.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
Documentation Sections
Source
Documentation content sourced from: https://deepwiki.com/rohitg00/skillkit
Test Plan
Summary by CodeRabbit
New Features
Documentation
Content / UI Updates
Chores
✏️ Tip: You can customize this high-level summary in your review settings.