Skip to content

feat(ai): add smart context generation with multi-provider support#45

Merged
rohitg00 merged 7 commits intomainfrom
feat/smart-context-generation
Feb 5, 2026
Merged

feat(ai): add smart context generation with multi-provider support#45
rohitg00 merged 7 commits intomainfrom
feat/smart-context-generation

Conversation

@rohitg00
Copy link
Owner

@rohitg00 rohitg00 commented Feb 5, 2026

Implements the SkillKit Smart Context Generation system:

  • Multi-Provider Support: 6 LLM providers (Anthropic, OpenAI, Google, Ollama, OpenRouter, Mock) with auto-detection via environment variables
  • Multi-Source Context Engine: 4 context sources (docs, codebase, skills, memory) with relevance weighting
  • Skill Composition Engine: Natural language search and intelligent skill merging
  • Agent Optimization: Agent-specific optimization for 10+ agents with compatibility scoring
  • Security Module: Trust scoring (0-10 scale) and prompt injection detection
  • Wizard Core: 6-step interactive wizard (expertise → context-sources → composition → clarification → review → install)
  • CLI Integration: skillkit generate command with @clack/prompts

New files:

  • packages/core/src/ai/providers/{types,anthropic,openai,google,ollama,openrouter,factory}.ts
  • packages/core/src/ai/context/{index,docs-source,codebase-source,skills-source,memory-source}.ts
  • packages/core/src/ai/composition/{index,analyzer,merger}.ts
  • packages/core/src/ai/agents/{optimizer,compatibility}.ts
  • packages/core/src/ai/security/{trust-score,injection-detect}.ts
  • packages/core/src/ai/wizard/{types,steps,clarification,index}.ts
  • packages/cli/src/commands/generate.ts
  • 6 test files for new modules

Open with Devin

Summary by CodeRabbit

  • New Features

    • Interactive AI skill generator (generate / gen) with multi-step wizard, review, and optional install/output.
    • CLI ai subcommands: wizard and providers (detects/configured LLMs, shows default and env hints).
    • Added broad LLM support and factory: Anthropic, OpenAI, Google Gemini, Ollama, OpenRouter, Mock.
    • New context & composition systems: docs, local codebase, marketplace skills, memory; skill analysis, merging, compatibility scoring, agent optimization, trust & injection checks.
  • Tests

    • Extensive unit tests covering composition, context sources, wizard logic, security (injection), trust scoring, and agent optimization.

Implements the SkillKit Smart Context Generation system:

- Multi-Provider Support: 6 LLM providers (Anthropic, OpenAI, Google, Ollama, OpenRouter, Mock) with auto-detection via environment variables
- Multi-Source Context Engine: 4 context sources (docs, codebase, skills, memory) with relevance weighting
- Skill Composition Engine: Natural language search and intelligent skill merging
- Agent Optimization: Agent-specific optimization for 10+ agents with compatibility scoring
- Security Module: Trust scoring (0-10 scale) and prompt injection detection
- Wizard Core: 6-step interactive wizard (expertise → context-sources → composition → clarification → review → install)
- CLI Integration: `skillkit generate` command with @clack/prompts

New files:
- packages/core/src/ai/providers/{types,anthropic,openai,google,ollama,openrouter,factory}.ts
- packages/core/src/ai/context/{index,docs-source,codebase-source,skills-source,memory-source}.ts
- packages/core/src/ai/composition/{index,analyzer,merger}.ts
- packages/core/src/ai/agents/{optimizer,compatibility}.ts
- packages/core/src/ai/security/{trust-score,injection-detect}.ts
- packages/core/src/ai/wizard/{types,steps,clarification,index}.ts
- packages/cli/src/commands/generate.ts
- 6 test files for new modules
@vercel
Copy link

vercel bot commented Feb 5, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
skillkit Ready Ready Preview, Comment Feb 5, 2026 3:39pm
skillkit-docs Ready Ready Preview, Comment Feb 5, 2026 3:39pm

@coderabbitai
Copy link

coderabbitai bot commented Feb 5, 2026

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Adds an interactive AI skill-generation wizard and CLI subcommands, multiple LLM providers with a provider factory, a ContextEngine with four context sources, composition/merging and agent optimization/compatibility tooling, injection-detection and trust scoring modules, many provider implementations, and extensive unit tests.

Changes

Cohort / File(s) Summary
CLI Commands
packages/cli/src/commands/ai.ts, packages/cli/src/commands/generate.ts, packages/cli/src/commands/index.ts
Added GenerateCommand (generate/gen) with an interactive multi-step wizard and new wizard / providers subcommands; ai.ts now lazily delegates wizard flow and prints provider status.
Provider Types & Factory
packages/core/src/ai/providers/types.ts, packages/core/src/ai/providers/factory.ts, packages/core/src/ai/providers/index.ts, packages/core/package.json
Introduced provider types, provider detection, default selection, createProvider/ProviderFactory, env var helpers and optional SDK deps in package.json; re-exported providers and factory utilities.
LLM Providers
packages/core/src/ai/providers/...
anthropic.ts, openai.ts, google.ts, ollama.ts, openrouter.ts, mock.ts
Implemented concrete provider classes adhering to LLMProvider interface (chat, generateSkill, generateClarifications, optimizeForAgent, search, generateFromExample) with provider-specific request/parsing logic.
Context Engine & Sources
packages/core/src/ai/context/index.ts, .../docs-source.ts, .../codebase-source.ts, .../skills-source.ts, .../memory-source.ts
Added ContextEngine coordinating sources; implemented DocsSource, CodebaseSource, SkillsSource, MemorySource with fetch/isAvailable, relevance scoring, caching, and snippet extraction.
Composition & Merging
packages/core/src/ai/composition/index.ts, .../analyzer.ts, .../merger.ts
Added SkillAnalyzer (parse/sections/patterns/complexity), SkillMerger (conflict strategies, dedupe, merge report), and SkillComposer (compose/composition-with-AI, naming/description heuristics).
Wizard Framework & Steps
packages/core/src/ai/wizard/types.ts, .../steps.ts, .../clarification.ts, .../index.ts
Introduced typed SkillWizard, step handlers (expertise, context-sources, composition, clarification, review, install), ClarificationGenerator, STEP_HANDLERS and public wizard API for interactive flows.
Agent Optimization & Compatibility
packages/core/src/ai/agents/optimizer.ts, packages/core/src/ai/agents/compatibility.ts
Added AgentOptimizer (agent constraints, truncation, MCP/tool stripping, formatting) and CompatibilityScorer (analyze requirements, score/grade agents, generate compatibility matrix).
Security & Trust
packages/core/src/ai/security/injection-detect.ts, packages/core/src/ai/security/trust-score.ts
Implemented InjectionDetector with pattern detection, sanitization and utilities; added TrustScorer and quickTrustScore producing trust breakdowns, warnings and recommendations.
Wizard CLI Integration & Provider Status
packages/cli/src/commands/ai.ts
New handleWizard() and handleProviders() methods: wizard delegates to GenerateCommand/SkillWizard; providers prints detected providers, marks default provider and shows env var hints (special-case mock default).
Exports & Tests
packages/core/src/ai/index.ts, packages/core/src/ai/__tests__/*
Re-exported new core AI modules/types from core index; added many unit tests covering wizard, context engine, composition, agents, injection detection, trust scoring, and composition utilities.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant User
    participant CLI as GenerateCommand
    participant Factory as ProviderFactory
    participant Wizard as SkillWizard
    participant Context as ContextEngine
    participant Provider as LLMProvider
    participant Optimizer as AgentOptimizer
    participant FS as Filesystem

    User->>CLI: run `skillkit generate`
    CLI->>Factory: detect/get default provider
    CLI->>Wizard: init(provider, projectPath)
    Wizard->>User: prompt expertise & options
    User->>Wizard: submit expertise
    Wizard->>Context: gather(expertise)
    Context-->>Wizard: aggregated context chunks
    Wizard->>Factory: getProvider(selected)
    Factory-->>Provider: instantiate
    Wizard->>Provider: generateSkill(context + expertise + clarifications)
    Provider-->>Wizard: generated skill
    Wizard->>Optimizer: optimizeForAgent(skill, agentId)
    Optimizer-->>Wizard: optimized variant
    Wizard->>FS: write/install skill
    FS-->>CLI: success
    CLI-->>User: finished
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

Poem

🐰 I hopped through docs and local code at night,

Asked many providers to craft skills just right,
A wizard stitched context, clarified each part,
Trimmed for each agent — neat, clever, and smart,
Now skills sprout like carrots — a rabbit's delight!

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'feat(ai): add smart context generation with multi-provider support' accurately summarizes the primary change: implementation of a multi-provider AI context generation system with provider support (Anthropic, OpenAI, Google, Ollama, OpenRouter, Mock) and context sources (docs, codebase, skills, memory).

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/smart-context-generation

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

View 6 additional findings in Devin Review.

Open in Devin Review

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 11

Note

Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/cli/src/commands/ai.ts (1)

414-427: ⚠️ Potential issue | 🟠 Major

Extend provider selection to the new providers.

getAIConfig() still only considers Anthropic/OpenAI, so Google/OpenRouter/Ollama configs are ignored and the CLI falls back to none. Align this with getDefaultProvider() so configured providers are actually used.

💡 Suggested update to support all providers
   private getAIConfig() {
-    const apiKey =
-      process.env.ANTHROPIC_API_KEY || process.env.OPENAI_API_KEY;
-
-    return {
-      provider:
-        process.env.ANTHROPIC_API_KEY
-          ? ('anthropic' as const)
-          : process.env.OPENAI_API_KEY
-            ? ('openai' as const)
-            : ('none' as const),
-      apiKey,
-      model: process.env.ANTHROPIC_API_KEY ? 'claude-sonnet-4-20250514' : undefined,
-      maxTokens: 4096,
-      temperature: 0.7,
-    };
+    const provider = getDefaultProvider();
+    const apiKey =
+      provider === 'anthropic'
+        ? process.env.ANTHROPIC_API_KEY
+        : provider === 'openai'
+          ? process.env.OPENAI_API_KEY
+          : provider === 'google'
+            ? (process.env.GOOGLE_AI_KEY || process.env.GEMINI_API_KEY)
+            : provider === 'openrouter'
+              ? process.env.OPENROUTER_API_KEY
+              : undefined;
+
+    return {
+      provider,
+      apiKey,
+      model: provider === 'anthropic' ? 'claude-sonnet-4-20250514' : undefined,
+      maxTokens: 4096,
+      temperature: 0.7,
+    };
   }
🤖 Fix all issues with AI agents
In `@packages/core/package.json`:
- Around line 61-64: Replace the deprecated dependency "@google/generative-ai"
in package.json with the supported "@google/genai" at the recommended version
(1.17.0); update the dependency string in the dependencies block (the entry
currently showing "@google/generative-ai") to "@google/genai": "1.17.0" and then
reinstall/update lockfiles (npm/yarn/pnpm) so the lockfile reflects the change.

In `@packages/core/src/ai/context/codebase-source.ts`:
- Around line 313-326: The calculateRelevance function currently constructs a
RegExp from raw user keywords (new RegExp(keyword, 'g')) which can throw or be
abused; fix calculateRelevance by skipping empty/whitespace tokens and escaping
each keyword before building the regex (create or reuse an escapeRegExp helper
that replaces metacharacters with escaped versions), then use the escaped
pattern in new RegExp(escapedKeyword, 'g') (or alternatively use a safe string
search like contentLower.indexOf for literal matches) to avoid invalid patterns
and ReDoS; ensure fileName checks still use the original token comparison.

In `@packages/core/src/ai/providers/factory.ts`:
- Around line 157-194: ProviderFactory.getProvider currently builds cacheKey
using only provider name and model, so different ProviderConfig values (apiKey,
baseUrl, maxTokens, temperature, etc.) can return the wrong cached LLMProvider;
update the cache key generation in ProviderFactory.getProvider to incorporate a
stable serialization of the entire ProviderConfig (e.g., JSON.stringify of
normalized config or selected config fields) so that providerCache keys uniquely
reflect config overrides before calling createProvider and storing/returning
instances.

In `@packages/core/src/ai/providers/google.ts`:
- Around line 107-131: In search(), validate each AI-returned item from parsed
(the array parsed in the try block) before using it to index skills: ensure
item.index is an integer within 1..skills.length and that item.relevance and
item.reasoning exist/are of expected types; skip (or filter out) any items with
out-of-range or invalid indices so that the mapped result in the return (where
you access skills[item.index - 1]) never produces undefined and only returns
well-formed SearchResult objects.

In `@packages/core/src/ai/providers/ollama.ts`:
- Around line 97-122: In search(query, skills) validate each parsed item.index
against the sliced limited array before mapping: use limited (not the original
skills) when resolving skill entries and ensure item.index is an integer within
1..limited.length; skip or filter out any parsed entries with missing/invalid
indices or non-number indices (and keep returning only valid SearchResult
objects with skill, relevance, reasoning) to avoid returning undefined skills to
downstream consumers.
- Around line 149-167: The makeRequest method currently calls fetch without a
timeout and may hang; wrap the fetch in an AbortController with a 15-second
timeout: create an AbortController, pass controller.signal to fetch, set a timer
to call controller.abort() after 15000ms, and clear the timer after the response
or on error; ensure you catch aborts and rethrow a clear Error (e.g., "Ollama
API request timed out") so callers of makeRequest get a deterministic failure;
update the existing makeRequest function that uses this.baseUrl, this.model,
messages and options.temperature to use the controller.signal.

In `@packages/core/src/ai/providers/openrouter.ts`:
- Around line 152-175: The fetch in makeRequest can hang indefinitely; add
AbortController-based timeout handling in the OpenRouter provider's makeRequest
method: create an AbortController, start a setTimeout that calls
controller.abort() after a configurable timeout (e.g., a new this.requestTimeout
or DEFAULT_TIMEOUT), pass controller.signal to fetch, clear the timer on
success, and catch the abort error to throw a clear timeout-specific Error
(e.g., "OpenRouter API request timed out"). Ensure to reference the existing
makeRequest function and use controller.signal with fetch and clearTimeout when
returning the response.
- Around line 100-124: The search method may map out-of-range indices from the
model into undefined skills; in the search function validate each parsed
item.index against the skills array bounds (1 ≤ index ≤ skills.length) before
creating SearchResult objects, e.g. filter or discard any parsed entries whose
index is not within that range and only map items that produce a defined
SearchableSkill, optionally logging or collecting parse/validation failures;
ensure the returned array only contains objects matching the SearchResult shape
(skill: SearchableSkill, relevance, reasoning) so no undefined skill is returned
from search.

In `@packages/core/src/ai/wizard/index.ts`:
- Around line 28-42: The constructor currently lets options.provider override an
explicit config.provider instance; change the initialization logic in the
constructor so an explicitly passed provider (this.provider / config.provider)
always wins. Concretely: if config.provider is provided, keep it and do not call
createProvider; otherwise if options.provider is present call
createProvider(options.provider, { model: this.options.model }); and finally
fall back to createProvider() when neither is set. Update the branches that
reference this.provider, this.options.provider, and createProvider to implement
this precedence.
- Around line 63-102: The code currently sets this.state.currentStep to
result.nextStep and then emits completion if this.state.currentStep ===
'install', which fires when merely entering the install step; instead capture
the step being executed (e.g., const executingStep = this.state.currentStep at
the start of executeStep), and change the completion condition to emitComplete
only when the executed step was 'install' and it succeeded (e.g., if
(executingStep === 'install' && result.success) this.emitComplete()); update
references in executeStep (handler, result, this.state.currentStep) accordingly
so completion fires after the install handler finishes, not when entering the
install step.

In `@packages/core/src/ai/wizard/steps.ts`:
- Around line 66-83: The code unconditionally calls
MemorySource.getMemoryPatterns and injects memoryPatterns even when memory is
disabled; guard this by checking state.memoryPersonalization and that the memory
source is enabled before calling getMemoryPatterns (e.g., create MemorySource as
now but only call memorySource.getMemoryPatterns(keywords) when
state.memoryPersonalization && memorySource.isEnabled() (or
memorySource.enabled) and otherwise set memoryPatterns = []). Apply the same
guard pattern for the other occurrence referenced (around the 191-202 block) so
memory lookups only happen when both the memory source is enabled and
state.memoryPersonalization is true.
🟡 Minor comments (11)
packages/core/src/ai/agents/optimizer.ts-115-119 (1)

115-119: ⚠️ Potential issue | 🟡 Minor

Inconsistent token-to-character ratio in estimation.

Line 115 uses content.length / 4 for token estimation, but line 190 uses maxTokens * 3 for the target character count. This inconsistency means content could be truncated to ~75% of what the token check expects.

🔧 Proposed fix for consistency
 private truncateContent(content: string, maxTokens: number): string {
-  const targetChars = maxTokens * 3;
+  const targetChars = maxTokens * 4; // Consistent with token estimation (length / 4)

Also applies to: 189-190

packages/core/src/ai/wizard/clarification.ts-48-51 (1)

48-51: ⚠️ Potential issue | 🟡 Minor

Add defensive check for expertise before calling toLowerCase().

If context.expertise is undefined or null, calling .toLowerCase() will throw. The WizardState interface shows expertise: string but the initial state sets it to an empty string, which is safe. However, WizardContext from providers/types.ts also defines expertise: string — verify this is always populated before this point in the flow.

🛡️ Proposed defensive fix
 private getFallbackQuestions(context: WizardContext, maxQuestions: number): ClarificationQuestion[] {
   const questions: ClarificationQuestion[] = [];
-  const expertise = context.expertise.toLowerCase();
+  const expertise = (context.expertise || '').toLowerCase();
packages/core/src/ai/providers/openai.ts-58-60 (1)

58-60: ⚠️ Potential issue | 🟡 Minor

Add null check for choices[0] to prevent potential runtime error.

If the OpenAI API returns an empty choices array (e.g., due to content filtering or an edge case), accessing choices[0]?.message.content is partially safe but choices[0] itself could be undefined. The current optional chaining handles message but consider a more explicit guard.

🛡️ Proposed fix
     const response = await this.makeRequest(openaiMessages);
-    return response.choices[0]?.message.content || '';
+    const choice = response.choices[0];
+    if (!choice) {
+      throw new Error('OpenAI API returned no choices');
+    }
+    return choice.message.content || '';
packages/core/src/ai/composition/analyzer.ts-107-129 (1)

107-129: ⚠️ Potential issue | 🟡 Minor

Deduplicate skill names when computing overlaps.

A single skill with duplicate section titles can create false overlaps.

🛠️ Suggested fix
-    const sectionMap: Map<string, string[]> = new Map();
+    const sectionMap: Map<string, Set<string>> = new Map();
@@
-        const existingSkills = sectionMap.get(normalizedTitle) || [];
-        existingSkills.push(skill.name);
-        sectionMap.set(normalizedTitle, existingSkills);
+        const existingSkills = sectionMap.get(normalizedTitle) ?? new Set<string>();
+        existingSkills.add(skill.name);
+        sectionMap.set(normalizedTitle, existingSkills);
@@
-    for (const [title, skillNames] of sectionMap) {
-      if (skillNames.length > 1) {
-        overlapping.set(title, skillNames);
+    for (const [title, skillNames] of sectionMap) {
+      if (skillNames.size > 1) {
+        overlapping.set(title, Array.from(skillNames));
       }
     }
packages/core/src/ai/composition/merger.ts-111-129 (1)

111-129: ⚠️ Potential issue | 🟡 Minor

Align section‑type priority mappings to avoid inconsistent ordering.

sortSectionGroups ranks types with a trigger‑first order, while getSectionTypePriority ranks rule highest. That can cause grouping/type promotion to disagree with final ordering. Use a single priority map for both.

🔧 Suggested fix
   private sortSectionGroups(groups: Map<string, SectionGroup>): SectionGroup[] {
     const sortedGroups = [...groups.values()];
-
-    const typePriority: Record<SkillSection['type'], number> = {
-      trigger: 1,
-      rule: 2,
-      instruction: 3,
-      example: 4,
-      metadata: 5,
-    };
 
     sortedGroups.sort((a, b) => {
-      const priorityDiff = typePriority[a.type] - typePriority[b.type];
+      const priorityDiff =
+        this.getSectionTypePriority(a.type) - this.getSectionTypePriority(b.type);
       if (priorityDiff !== 0) return priorityDiff;
 
       const importanceA = Math.max(...a.sections.map((s) => s.importance));
@@
   private getSectionTypePriority(type: SkillSection['type']): number {
     const priority: Record<SkillSection['type'], number> = {
-      rule: 5,
-      instruction: 4,
-      trigger: 3,
-      example: 2,
-      metadata: 1,
+      trigger: 1,
+      rule: 2,
+      instruction: 3,
+      example: 4,
+      metadata: 5,
     };
-    return priority[type] || 0;
+    return priority[type] ?? 0;
   }

Also applies to: 244-252

packages/core/src/ai/context/skills-source.ts-63-98 (1)

63-98: ⚠️ Potential issue | 🟡 Minor

Guard against empty keyword list to avoid NaN relevance.

When extractKeywords() returns an empty array, score / keywords.length becomes NaN, causing scores to be dropped unintentionally.

✅ Simple guard
   private calculateScore(skill: SkillSummary, keywords: string[]): number {
+    if (keywords.length === 0) return 0;
     let score = 0;
     const nameWords = skill.name.toLowerCase().split(/[-_\s]+/);
     const descWords = (skill.description || '').toLowerCase().split(/\s+/);
     const tagWords = (skill.tags || []).map((t) => t.toLowerCase());
packages/core/src/ai/context/index.ts-55-134 (1)

55-134: ⚠️ Potential issue | 🟡 Minor

Guard against zero/Infinity per-source chunk allocation.

When enabledSources.length is 0, chunksPerSource becomes Infinity; when enabled sources exceed maxTotalChunks, chunksPerSource becomes 0 and every source returns zero chunks. Consider an early return and a minimum of 1 chunk per source.

🛠️ Proposed fix
-    const chunksPerSource = Math.floor(this.maxTotalChunks / enabledSources.length);
+    if (enabledSources.length === 0) {
+      return { chunks: [], sources: [], totalTokensEstimate: 0 };
+    }
+    const chunksPerSource = Math.max(1, Math.floor(this.maxTotalChunks / enabledSources.length));
packages/core/src/ai/security/trust-score.ts-32-73 (1)

32-73: ⚠️ Potential issue | 🟡 Minor

Normalize weights and clamp final score to [0–10] range.

When a caller provides partial weights that don't sum to 1.0 (e.g., { weights: { clarity: 0.5 } }), the merged weights can exceed 1.0, causing weightedScore to exceed the maximum individual score of 10. The final score is never clamped, so scores above 10 can be returned. Normalize weights in the constructor and clamp finalScore to [0, 10] before rounding.

Proposed fix
  constructor(options: TrustScoreOptions = {}) {
-   this.weights = { ...DEFAULT_WEIGHTS, ...options.weights };
+   const merged = { ...DEFAULT_WEIGHTS, ...options.weights };
+   const sum = merged.clarity + merged.boundaries + merged.specificity + merged.safety;
+   this.weights = sum > 0
+     ? {
+         clarity: merged.clarity / sum,
+         boundaries: merged.boundaries / sum,
+         specificity: merged.specificity / sum,
+         safety: merged.safety / sum,
+       }
+     : DEFAULT_WEIGHTS;
    this.strictMode = options.strictMode ?? false;
  }
@@
-   const normalizedScore = Math.round(finalScore * 10) / 10;
+   const clamped = Math.max(0, Math.min(10, finalScore));
+   const normalizedScore = Math.round(clamped * 10) / 10;
packages/core/src/ai/security/injection-detect.ts-157-187 (1)

157-187: ⚠️ Potential issue | 🟡 Minor

Preserve custom regex flags when creating RegExp instances.

The code new RegExp(patternDef.pattern, 'gi') overwrites all flags from the original RegExp. If a custom pattern is added with flags like m (multiline), s (dotAll), or u (unicode), those flags are discarded, potentially causing incorrect detection behavior.

🛠️ Proposed fix
-      const matches = content.matchAll(new RegExp(patternDef.pattern, 'gi'));
+      const baseFlags = patternDef.pattern.flags;
+      const flags = baseFlags.includes('g') ? baseFlags : baseFlags + 'g';
+      const regex = new RegExp(patternDef.pattern.source, flags);
+      const matches = content.matchAll(regex);
packages/core/src/ai/context/memory-source.ts-214-231 (1)

214-231: ⚠️ Potential issue | 🟡 Minor

Normalize path separators in project memory directory key to handle Windows paths.

Line 226 uses replace(/\//g, '-') which only handles forward slashes. On Windows, basePath contains backslashes (from process.cwd() or path.join()), so the path won't be properly normalized, breaking project memory path resolution on Windows systems.

Proposed fix
-    const projectMemoryPath = join(homeDir, '.claude', 'projects', basePath.replace(/\//g, '-'), 'memory', 'MEMORY.md');
+    const projectKey = basePath.replace(/[\\/]/g, '-');
+    const projectMemoryPath = join(homeDir, '.claude', 'projects', projectKey, 'memory', 'MEMORY.md');
packages/core/src/ai/providers/anthropic.ts-225-239 (1)

225-239: ⚠️ Potential issue | 🟡 Minor

Missing bounds validation for skill indices.

If the LLM returns an invalid index (e.g., 0, negative, or exceeding skills.length), skills[item.index - 1] will be undefined, producing malformed AISearchResult objects with skill: undefined.

🛡️ Proposed fix to filter invalid indices
 private parseSearchResponse(
   response: string,
   skills: SearchableSkill[]
 ): AISearchResult[] {
   try {
     const parsed = JSON.parse(response);
-    return parsed.map((item: { index: number; relevance: number; reasoning: string }) => ({
-      skill: skills[item.index - 1],
-      relevance: item.relevance,
-      reasoning: item.reasoning,
-    }));
+    return parsed
+      .filter((item: { index: number }) => item.index >= 1 && item.index <= skills.length)
+      .map((item: { index: number; relevance: number; reasoning: string }) => ({
+        skill: skills[item.index - 1],
+        relevance: item.relevance,
+        reasoning: item.reasoning,
+      }));
   } catch {
     return [];
   }
 }
🧹 Nitpick comments (10)
packages/core/src/ai/wizard/clarification.ts (1)

22-30: Consider logging provider errors before falling back.

The error from generateClarifications is silently discarded. While falling back to predefined questions is the right UX choice, logging the error would aid debugging provider configuration issues.

💡 Suggested improvement
     } catch (error) {
+      // Log for debugging but don't expose to user
+      console.debug?.('Clarification generation failed, using fallbacks:', error);
       return this.getFallbackQuestions(context, maxQuestions);
     }
packages/core/src/ai/wizard/types.ts (1)

167-170: getStepNumber returns 0 for invalid steps, which could be misleading.

If an invalid WizardStep is passed, indexOf returns -1, so the function returns 0. This could be confused with a valid step number. Consider returning -1 or throwing for invalid input.

💡 Suggested fix to handle invalid steps explicitly
 export function getStepNumber(step: WizardStep): number {
   const order = getStepOrder();
-  return order.indexOf(step) + 1;
+  const index = order.indexOf(step);
+  return index === -1 ? -1 : index + 1;
 }
packages/core/src/ai/providers/types.ts (1)

108-113: ProviderCapabilities is defined but not exposed through LLMProvider.

The ProviderCapabilities interface is defined but there's no method or property on LLMProvider to retrieve capabilities. Consider adding a getCapabilities(): ProviderCapabilities method to the interface if providers need to expose their capabilities for runtime decisions.

💡 Suggested addition to LLMProvider interface
 export interface LLMProvider {
   readonly name: ProviderName;
   readonly displayName: string;
+  readonly capabilities: ProviderCapabilities;

   generateSkill(context: GenerationContext): Promise<GeneratedSkillResult>;
   // ... rest of methods
 }
packages/core/src/ai/providers/openai.ts (2)

175-196: Consider adding a timeout to prevent indefinite hangs.

The fetch call has no timeout configured. If the OpenAI API becomes unresponsive, this could block indefinitely. Consider using AbortController with a timeout.

💡 Suggested timeout implementation
 private async makeRequest(messages: OpenAIMessage[]): Promise<OpenAIResponse> {
+  const controller = new AbortController();
+  const timeoutId = setTimeout(() => controller.abort(), 60000); // 60s timeout
+
+  try {
     const response = await fetch(`${this.baseUrl}/chat/completions`, {
       method: 'POST',
       headers: {
         'Content-Type': 'application/json',
         Authorization: `Bearer ${this.apiKey}`,
       },
       body: JSON.stringify({
         model: this.model,
         messages,
         max_tokens: this.maxTokens,
         temperature: this.temperature,
       }),
+      signal: controller.signal,
     });

     if (!response.ok) {
       const error = await response.text();
       throw new Error(`OpenAI API error: ${response.status} - ${error}`);
     }

     return response.json() as Promise<OpenAIResponse>;
+  } finally {
+    clearTimeout(timeoutId);
+  }
 }

133-147: Inconsistent error handling between search() and generateFromExample().

search() returns an empty array on parse failure (lines 143-146), while generateFromExample() throws an error (line 172). Consider aligning the behavior — either both should throw, or both should return a safe default value based on the expected contract.

Also applies to: 156-173

packages/core/src/ai/agents/optimizer.ts (1)

335-347: The convertToCursorFormat regex replacements appear to be no-ops.

Lines 338, 340, and 342 replace headings with identical patterns (e.g., # $1# $1). The only effective change is adding newlines before ## and ### headings and normalizing multiple newlines.

💡 Simplified implementation
 private convertToCursorFormat(content: string): string {
   let result = content;

-  result = result.replace(/^# (.+)$/gm, '# $1');
-
-  result = result.replace(/^## (.+)$/gm, '\n## $1');
-
-  result = result.replace(/^### (.+)$/gm, '\n### $1');
+  // Ensure blank line before ## and ### headings
+  result = result.replace(/^(#{2,3}\s)/gm, '\n$1');

   result = result.replace(/\n{3,}/g, '\n\n');

   return result.trim();
 }
packages/core/src/ai/agents/compatibility.ts (1)

37-46: Avoid re-parsing requirements for every agent in generateMatrix.

scoreSkillForAgent re-analyzes the same content per agent. Computing requirements once avoids repeated regex/token work.

♻️ Suggested refactor
   generateMatrix(skillContent: string, agentIds?: string[]): CompatibilityMatrix {
     const agents = agentIds || this.optimizer.getSupportedAgents();
     const matrix: CompatibilityMatrix = {};
+    const requirements = this.analyzeSkillRequirements(skillContent);

     for (const agentId of agents) {
-      matrix[agentId] = this.scoreSkillForAgent(skillContent, agentId);
+      const constraints = this.optimizer.getConstraints(agentId);
+      matrix[agentId] = this.calculateScore(requirements, constraints, agentId);
     }

     return matrix;
   }
packages/core/src/ai/providers/ollama.ts (1)

39-41: Consider making isConfigured() reflect explicit opt-in.

Returning true unconditionally can cause Ollama to be selected even when no local server is running. It’s safer to return true only when a config/base URL or explicit env var is set.

🛠️ Suggested adjustment
-  isConfigured(): boolean {
-    return true;
-  }
+  isConfigured(): boolean {
+    return Boolean(process.env.OLLAMA_HOST);
+  }
packages/core/src/ai/context/docs-source.ts (1)

61-84: Consider de-duplicating resolved libraries.

Repeated keywords can add duplicate libraries, which wastes chunk budget and skews relevance distribution.

🛠️ Suggested adjustment
-      const libraries: Context7Library[] = [];
+      const libraries: Context7Library[] = [];
+      const seen = new Set<string>();
@@
-        if (knownLib) {
-          libraries.push(knownLib);
-        }
+        if (knownLib && !seen.has(knownLib.id)) {
+          libraries.push(knownLib);
+          seen.add(knownLib.id);
+        }
packages/core/src/ai/providers/anthropic.ts (1)

257-288: Consider adding a request timeout.

The fetch call has no timeout configured, which could cause the request to hang indefinitely if the Anthropic API becomes unresponsive. This could block wizard steps or cause poor user experience.

♻️ Proposed fix using AbortController
 private async makeRequest(
   messages: AnthropicMessage[],
-  system?: string
+  system?: string,
+  timeoutMs: number = 60000
 ): Promise<AnthropicResponse> {
+  const controller = new AbortController();
+  const timeoutId = setTimeout(() => controller.abort(), timeoutMs);
+
   const body: Record<string, unknown> = {
     model: this.model,
     max_tokens: this.maxTokens,
     temperature: this.temperature,
     messages,
   };

   if (system) {
     body.system = system;
   }

-  const response = await fetch('https://api.anthropic.com/v1/messages', {
-    method: 'POST',
-    headers: {
-      'Content-Type': 'application/json',
-      'x-api-key': this.apiKey,
-      'anthropic-version': '2023-06-01',
-    },
-    body: JSON.stringify(body),
-  });
+  try {
+    const response = await fetch('https://api.anthropic.com/v1/messages', {
+      method: 'POST',
+      headers: {
+        'Content-Type': 'application/json',
+        'x-api-key': this.apiKey,
+        'anthropic-version': '2023-06-01',
+      },
+      body: JSON.stringify(body),
+      signal: controller.signal,
+    });

-  if (!response.ok) {
-    const error = await response.text();
-    throw new Error(`Anthropic API error: ${response.status} - ${error}`);
-  }
+    if (!response.ok) {
+      const error = await response.text();
+      throw new Error(`Anthropic API error: ${response.status} - ${error}`);
+    }

-  return response.json() as Promise<AnthropicResponse>;
+    return response.json() as Promise<AnthropicResponse>;
+  } finally {
+    clearTimeout(timeoutId);
+  }
 }

Comment on lines 61 to 64
"@anthropic-ai/sdk": "^0.39.0",
"openai": "^4.0.0",
"@google/generative-ai": "^0.21.0",
"ollama": "^0.5.0",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash

echo "=== Checking latest versions on npm ==="
for pkg in "@anthropic-ai/sdk" "openai" "@google/generative-ai" "ollama"; do
  latest=$(npm view "$pkg" version 2>/dev/null)
  if [ -z "$latest" ]; then
    echo "$pkg: ERROR - package not found"
  else
    echo "$pkg: $latest"
  fi
done

echo -e "\n=== Checking package details ==="
npm view openai versions --json 2>/dev/null | tail -20

Repository: rohitg00/skillkit

Length of output: 435


🌐 Web query:

openai npm package latest version 2025 2026

💡 Result:

As of the most recent npm metadata I can see, the openai npm package is in the v5.x line:

  • The npm package page shows version 5.19.1 (listed as “Published 2 days ago” on that page). [1]
  • The npm “Versions” tab snapshots show the latest dist-tag pointing to 5.21.0 (at the time that page snapshot was crawled). [2]

To confirm the current latest on your machine right now, run:

npm view openai version
npm view openai dist-tags

Sources: [1] (npmjs.com) [2] (npmjs.com)

Citations:


🌐 Web query:

@anthropic-ai/sdk npm security vulnerabilities

💡 Result:

  • Known published CVEs / direct vulns in @anthropic-ai/sdk: None found in Snyk’s vulnerability database (Snyk reports “No direct vulnerabilities” and “No vulnerabilities found in the latest version”). [1]
  • Malware / tampering / known-vuln signals (supply-chain scanning): ReversingLabs’ community scan reports no known vulnerabilities detected and no evidence of malware/tampering for the package. [2]
  • Common security footgun called out by the package docs: Enabling browser usage via dangerouslyAllowBrowser can expose your Anthropic API key in client-side code. [3]

If you’re assessing risk for a real app, also scan transitive dependencies in your lockfile (e.g., npm audit / Snyk project scan), since “no direct vulnerabilities” doesn’t rule out vulnerable dependencies. [1]

Sources:
[1] Snyk package page for @anthropic-ai/sdk (version page) https://security.snyk.io/package/npm/%40anthropic-ai%2Fsdk/0.7.0
[2] ReversingLabs Spectra Assure Community package scan https://secure.software/npm/packages/%40anthropic-ai/sdk
[3] npm package documentation excerpt (browser warning) https://www.npmjs.com/package/%40anthropic-ai/sdk


🌐 Web query:

@google/generative-ai npm latest version

💡 Result:

@google/generative-ai latest npm version: 0.24.1 (published Apr 29, 2025). [1][2]

Note: this package is legacy/deprecated; Google’s recommended replacement is @google/genai (npm version 1.17.0 as of its latest publish). [3]

Install

npm i `@google/generative-ai`@latest
# or (recommended)
npm i `@google/genai`@latest

Replace deprecated @google/generative-ai with @google/genai.

The @google/generative-ai package is now deprecated by Google. Use the recommended replacement @google/genai (current version 1.17.0) instead. The other SDK versions are current and secure.

🤖 Prompt for AI Agents
In `@packages/core/package.json` around lines 61 - 64, Replace the deprecated
dependency "@google/generative-ai" in package.json with the supported
"@google/genai" at the recommended version (1.17.0); update the dependency
string in the dependencies block (the entry currently showing
"@google/generative-ai") to "@google/genai": "1.17.0" and then reinstall/update
lockfiles (npm/yarn/pnpm) so the lockfile reflects the change.

- Escape regex in codebase-source.ts to prevent ReDoS attacks
- Fix division by zero in skills-source.ts when no keywords present
- Include all config overrides in ProviderFactory cache key
- Validate AI-provided indices before indexing skills arrays
- Use limited array in ollama.ts search to avoid out-of-bounds access
- Add 120s timeout with AbortController to ollama/openrouter requests
- Fix provider precedence in wizard: config.provider > options.provider
- Fix emitComplete timing to fire after install step completes
- Guard memory lookups with memoryPersonalization opt-out check
The legacy @google/generative-ai package is deprecated as of Nov 2025.
Updated to the new unified @google/genai SDK (GA since May 2025).
Copy link

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 new potential issues.

View 11 additional findings in Devin Review.

Open in Devin Review

- Update AIConfig type to support all providers (google, ollama, openrouter)
- Update getAIConfig() in CLI to detect all configured providers
- Add defensive check for expertise in clarification fallback questions
- Add null check for OpenAI choices[0] response
- Validate AI-provided indices in OpenAI/Anthropic search methods
- Add 120s timeout with AbortController to OpenAI/Anthropic requests
- Guard against zero/Infinity per-source chunk allocation in ContextEngine
- Normalize trust score weights and clamp final score to [0-10]
- Fix Windows path handling in memory-source.ts (use [\\/] regex)
- Fix token-to-character ratio consistency (use 4 throughout)
- Target 95% of max tokens in truncation to stay under limit
- Update docs/skillkit/package.json version to 1.13.0
- Update docs/fumadocs/package.json version to 1.13.0
- Regenerate pnpm-lock.yaml with new @google/genai dependency
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

🤖 Fix all issues with AI agents
In `@packages/cli/src/commands/ai.ts`:
- Around line 388-409: The provider listing currently doesn't show a default
when getDefaultProvider() returns 'mock' because detectProviders() doesn't
include a mock entry; update handleProviders() to detect this case and display a
clear default entry: after calling detectProviders() and getDefaultProvider(),
if defaultProvider === 'mock' and no detected provider has provider === 'mock',
append or print a synthetic entry (displayName like 'Mock (default)', configured
false, envVar undefined) or print an explicit line such as "  Mock (default)\n  
○ Not configured\n    (mock provider is used as a fallback when no providers are
configured)"; ensure you use the same status, defaultBadge, and chalk styling as
the existing loop so the mock/default message appears consistently with other
providers and still guides users to use --provider.

In `@packages/core/src/ai/agents/optimizer.ts`:
- Around line 189-214: The truncation in truncateContent can exceed targetChars
because the '...[truncated]' marker is appended after slicing remaining bytes;
adjust the slice to reserve space for the marker by subtracting the marker
length from remaining before slicing (use the marker string length when
computing the slice size), only append the marker when there is enough reserved
room, and ensure currentLength accounting reflects the reserved marker; update
references in truncateContent (variables: targetChars, remaining, result,
section.content) accordingly.
- Around line 217-252: The splitIntoSections function currently ignores any text
before the first heading; update it to detect and capture leading pre-heading
content as its own section (e.g., title "Intro" or "Main") before iterating
matches: after building matches from headingRegex, if matches.length > 0 and
matches[0].index > 0, slice content from 0 to matches[0].index, trim it, compute
priority with getSectionPriority, and unshift/push that section into sections so
the preamble is preserved; ensure existing logic that handles the no-headings
case remains unchanged.

In `@packages/core/src/ai/providers/anthropic.ts`:
- Around line 225-241: The parseSearchResponse method currently calls JSON.parse
directly which fails when the model returns fenced JSON or extra text; update
parseSearchResponse to first extract the JSON payload using a tolerant parser
(reuse the project's tolerant JSON parser helper or implement extraction that
strips code fences and preamble, then finds the first JSON array token), attempt
to parse that payload, verify the parsed value is an array, and then map/filter
items by validating each item's shape (ensure item.index is a number within
1..skills.length and item.relevance/item.reasoning exist) before returning
AISearchResult[]; if extraction/parsing/validation fails, return an empty array
as before.

In `@packages/core/src/ai/providers/openai.ts`:
- Around line 127-153: The parsed AI results may contain non-integer indices
which pass the numeric range check but produce undefined skills; update the
filtering in search to ensure item.index is an integer and within
1..skills.length (e.g., use Number.isInteger(item.index) && item.index >= 1 &&
item.index <= skills.length) before mapping to skills[item.index - 1], so only
valid integer indices are used or skipped.
- Around line 155-179: The generateFromExample function can return a
GeneratedSkill with missing required fields if the model's JSON is partial;
after extracting parsed = JSON.parse(match[0]) validate that parsed.name,
parsed.description, and parsed.content are present and non-empty, ensure
parsed.tags is an array (default to []), coerce parsed.confidence to a number in
[0,1] (default 0.7 if invalid), and set parsed.reasoning to '' if missing; if
any required field is absent or invalid, throw an error (instead of returning a
partial skill) so callers never receive undefined/malformed properties. Use the
existing response.match/parsed flow and apply these checks before constructing
the returned GeneratedSkill.

In `@packages/core/src/ai/security/trust-score.ts`:
- Around line 86-109: In scoreClarity, the sentence-length averaging divides by
sentences.length which can be zero for empty content; update scoreClarity to
guard that division by checking if sentences.length > 0 before computing
avgSentenceLength (or default avgSentenceLength to a large number or skip the
short-sentence bonus when zero sentences) so you don't produce NaN and the
scoring remains stable; modify the block that computes sentences and
avgSentenceLength in scoreClarity accordingly to only add the 0.5 bonus when
sentences.length > 0 and avgSentenceLength < 25.
- Around line 21-44: The constructor of TrustScorer currently normalizes
whatever numbers come in via options.weights (TrustBreakdown) allowing negative
or non-finite values; before computing sum and normalizing, validate and clamp
each weight to a finite non-negative number (e.g., replace
NaN/Infinity/negatives with 0), compute the sum of these sanitized weights, and
only normalize when sum > 0 otherwise assign DEFAULT_WEIGHTS; update the logic
in TrustScorer (constructor) that builds merged from DEFAULT_WEIGHTS and
options.weights to perform this sanitization so strictMode assignment remains
unchanged.

In `@packages/core/src/ai/types.ts`:
- Around line 38-39: The AIConfig provider union is missing 'mock', causing type
errors when tests instantiate AIManager with provider: 'mock'; update the
AIConfig interface (symbol: AIConfig, property: provider) to include 'mock' in
its union of allowed provider string literals so the tests and any mock provider
implementations are typed correctly.
🧹 Nitpick comments (2)
packages/core/src/ai/context/index.ts (1)

162-170: Consider parallelizing availability checks.

The sequential await in the loop could be parallelized for slightly better performance, though with only 4 sources the impact is minimal.

♻️ Optional parallel implementation
 async checkSourceAvailability(): Promise<Record<string, boolean>> {
-  const result: Record<string, boolean> = {};
-
-  for (const [name, source] of this.sources) {
-    result[name] = await source.isAvailable();
-  }
-
-  return result;
+  const entries = await Promise.all(
+    Array.from(this.sources.entries()).map(async ([name, source]) => [
+      name,
+      await source.isAvailable(),
+    ])
+  );
+  return Object.fromEntries(entries);
 }
packages/core/src/ai/agents/optimizer.ts (1)

109-145: Consider moving truncation after MCP/tool stripping to preserve content.

Right now truncation runs before removals, so you may truncate content that would have been removed anyway. Reordering keeps more meaningful content within the same token budget.

♻️ Proposed reordering
-    const estimatedTokens = Math.ceil(content.length / 4);
-    if (estimatedTokens > constraints.maxContextLength * 0.9) {
-      optimized = this.truncateContent(optimized, constraints.maxContextLength);
-      changes.push(`Truncated to fit ${agentId} context limit (${constraints.maxContextLength} tokens)`);
-    }
-
     if (!constraints.supportsMCP) {
       const { content: noMcpContent, removed } = this.removeMCPReferences(optimized);
       if (removed > 0) {
         optimized = noMcpContent;
         changes.push(`Removed ${removed} MCP-specific references`);
       }
     }
@@
     if (agentId === 'codex' || agentId === 'github-copilot') {
       optimized = this.condenseContent(optimized);
       changes.push('Condensed for shorter context');
     }
+
+    const estimatedTokens = Math.ceil(optimized.length / 4);
+    if (estimatedTokens > constraints.maxContextLength * 0.9) {
+      optimized = this.truncateContent(optimized, constraints.maxContextLength);
+      changes.push(`Truncated to fit ${agentId} context limit (${constraints.maxContextLength} tokens)`);
+    }

Comment on lines 86 to 109
private scoreClarity(content: string): number {
let score = 5;

const hasHeadings = /^#{1,3}\s/m.test(content);
if (hasHeadings) score += 1;

const headingCount = (content.match(/^#{1,3}\s/gm) || []).length;
if (headingCount >= 3 && headingCount <= 10) score += 1;

const hasBulletPoints = /^[-*]\s/m.test(content);
if (hasBulletPoints) score += 0.5;

const hasExamples = /```[\s\S]*?```|example:|for example/i.test(content);
if (hasExamples) score += 1;

const sentences = content.split(/[.!?]+/).filter((s) => s.trim().length > 0);
const avgSentenceLength = sentences.reduce((sum, s) => sum + s.split(/\s+/).length, 0) / sentences.length;
if (avgSentenceLength < 25) score += 0.5;

const hasAmbiguousTerms = /\b(maybe|perhaps|sometimes|might|could be|possibly)\b/gi.test(content);
if (hasAmbiguousTerms) score -= 0.5;

return Math.max(0, Math.min(10, score));
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Guard sentence-length averaging for empty content.

Line 101–103 divides by sentences.length; when content has no sentences, this yields NaN. Guard the division to keep the score stable.

🛠️ Suggested fix
     const sentences = content.split(/[.!?]+/).filter((s) => s.trim().length > 0);
-    const avgSentenceLength = sentences.reduce((sum, s) => sum + s.split(/\s+/).length, 0) / sentences.length;
-    if (avgSentenceLength < 25) score += 0.5;
+    if (sentences.length > 0) {
+      const avgSentenceLength =
+        sentences.reduce((sum, s) => sum + s.split(/\s+/).length, 0) / sentences.length;
+      if (avgSentenceLength < 25) score += 0.5;
+    }
🤖 Prompt for AI Agents
In `@packages/core/src/ai/security/trust-score.ts` around lines 86 - 109, In
scoreClarity, the sentence-length averaging divides by sentences.length which
can be zero for empty content; update scoreClarity to guard that division by
checking if sentences.length > 0 before computing avgSentenceLength (or default
avgSentenceLength to a large number or skip the short-sentence bonus when zero
sentences) so you don't produce NaN and the scoring remains stable; modify the
block that computes sentences and avgSentenceLength in scoreClarity accordingly
to only add the 0.5 bonus when sentences.length > 0 and avgSentenceLength < 25.

Copy link

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

View 16 additional findings in Devin Review.

Open in Devin Review

Comment on lines +280 to +291
const installResults: InstallStepData['results'] = [];

for (const [agentId, optimized] of results) {
installResults.push({
agentId,
success: true,
path: `~/.${agentId}/skills/${state.generatedSkill.name}/`,
optimized: optimized.changes.length > 0,
changes: optimized.changes,
});
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 InstallStep does not actually write skill files to disk

The InstallStep.execute() method in the wizard claims to install skills to agents but never actually writes any files to disk. It creates installResults with success: true for each agent and a placeholder path like ~/.${agentId}/skills/${state.generatedSkill.name}/, but the skill content is never persisted.

Root Cause

The execute method only calls optimizer.optimizeForMultipleAgents() to generate optimized content variants, then constructs result objects with hardcoded success: true values:

for (const [agentId, optimized] of results) {
  installResults.push({
    agentId,
    success: true,  // Always true!
    path: `~/.${agentId}/skills/${state.generatedSkill.name}/`,  // Placeholder, never used
    optimized: optimized.changes.length > 0,
    changes: optimized.changes,
  });
}

The optimized skill content in optimized.content is never written to the filesystem. Users see "Installation complete" and success messages, but no files are created.

Impact: Users completing the wizard will believe their skills are installed across multiple agents when in reality nothing was persisted. This defeats the purpose of the entire installation step.

Prompt for agents
The InstallStep.execute() method needs to actually write the optimized skill content to the filesystem for each agent. For each entry in the results Map:

1. Resolve the actual installation path (expand ~ to home directory)
2. Create the directory if it doesn't exist using mkdirSync with recursive: true
3. Write the optimized content using writeFileSync
4. Wrap in try-catch and set success: false with error message if write fails
5. Only set success: true if the file was actually written

The path construction should use proper path resolution like:
const homedir = require('os').homedir();
const installPath = join(homedir, `.${agentId}`, 'skills', state.generatedSkill.name, 'SKILL.md');

Also consider that different agents may have different skill directory structures - the AgentOptimizer already has this info in AGENT_CONSTRAINTS.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

- Add 'mock' to AIConfig provider union type
- Show mock provider in handleProviders() when no API keys configured
- Reserve space for truncation marker in truncateContent
- Capture pre-heading content in splitIntoSections
- Add tolerant JSON parsing in anthropic parseSearchResponse
- Use Number.isInteger for index validation in openai search
- Validate all required fields in openai generateFromExample
- Guard against zero sentences in trust-score scoreClarity
- Sanitize weight values (non-negative, finite) in TrustScorer
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@packages/core/src/ai/agents/optimizer.ts`:
- Around line 147-161: After calling this.provider.optimizeForAgent(...)
re-validate the resulting optimized string against the agent's maximum allowed
size (the same truncation logic used before the provider call), and if it
exceeds the limit, truncate it back to the agent max, push a change like
'Truncated after AI optimization' onto changes, and recompute estimatedTokens
(Math.ceil(optimized.length/4)) before returning; apply this to the block around
optimizeForAgent, optimized, agentId, changes, and estimatedTokens so the
response can never exceed the agent's max after AI expansion.

In `@packages/core/src/ai/providers/anthropic.ts`:
- Around line 245-259: The parseGenerateResponse function currently does a raw
JSON.parse and can fail if the model wraps JSON in code fences and does not
validate required fields; update parseGenerateResponse to first extract a JSON
substring (strip surrounding markdown/code fences and use a regex to locate the
first JSON object/array) before parsing, then validate that the resulting object
contains required fields name, description, and content (throw a clear Error
mentioning which field is missing), coerce/normalize optional fields tags
(default []), confidence (default 0.7), and reasoning (default ''), and include
the original parse error message when throwing on malformed JSON; reference
parseGenerateResponse and the GeneratedSkill shape and the fields
name/description/content/tags/confidence/reasoning when making the changes.
🧹 Nitpick comments (3)
packages/core/src/ai/providers/openai.ts (1)

253-264: Consider logging when falling back to default values.

The fallback behavior is reasonable for graceful degradation, but silently using the raw response as content when JSON parsing fails could make debugging difficult. Consider adding a warning log when the default is used.

packages/core/src/ai/agents/optimizer.ts (1)

361-381: Preserve indentation inside fenced code blocks when condensing.

The global leading-whitespace strip can corrupt code samples (especially Python) and nested lists. Consider skipping whitespace trimming inside fenced blocks.

♻️ Suggested refactor
   private condenseContent(content: string): string {
     let result = content;
 
     result = result.replace(/^(#{1,3})\s+/gm, '$1 ');
-
-    result = result.replace(/^\s+/gm, '');
 
     result = result.replace(/\n{2,}/g, '\n\n');
 
     const lines = result.split('\n');
     const condensed: string[] = [];
+    let inFence = false;
 
-    for (const line of lines) {
-      if (line.trim().length > 0) {
-        condensed.push(line);
-      } else if (condensed.length > 0 && condensed[condensed.length - 1].trim().length > 0) {
-        condensed.push('');
-      }
-    }
+    for (let line of lines) {
+      if (/^\s*```/.test(line)) {
+        inFence = !inFence;
+        condensed.push(line.trimEnd());
+        continue;
+      }
+
+      if (!inFence) {
+        line = line.replace(/^\s+/, '');
+      }
+
+      if (line.trim().length > 0) {
+        condensed.push(line);
+      } else if (condensed.length > 0 && condensed[condensed.length - 1].trim().length > 0) {
+        condensed.push('');
+      }
+    }
 
     return condensed.join('\n').trim();
   }
packages/cli/src/commands/ai.ts (1)

83-116: Move wizard and providers early-return before AIManager instantiation to avoid unnecessary object creation and irrelevant mock provider warning.

The wizard and providers subcommands do not use AIManager. Currently, the manager is instantiated before the switch statement, which triggers the mock provider warning display even for these non-AI subcommands. Move their handling before manager creation for cleaner execution flow.

♻️ Suggested refactor (early-return for non-AI subcommands)
 async execute(): Promise<number> {
+  if (this.subcommand === 'wizard') {
+    return await this.handleWizard();
+  }
+  if (this.subcommand === 'providers') {
+    return this.handleProviders();
+  }
+
   const aiConfig = this.getAIConfig();
   const manager = new AIManager(aiConfig);
 
   if (!this.json) {
     const providerName = manager.getProviderName();
     if (providerName === 'mock') {
       console.log(
@@
   switch (this.subcommand) {
     case 'search':
       return await this.handleSearch(manager);
     case 'generate':
     case 'gen':
       return await this.handleGenerate(manager);
     case 'similar':
       return await this.handleSimilar(manager);
-    case 'wizard':
-      return await this.handleWizard();
-    case 'providers':
-      return this.handleProviders();
     default:
       console.error(
         chalk.red(`Unknown subcommand: ${this.subcommand}\n`)
       );

Comment on lines 147 to 161
if (this.provider && changes.length > 0) {
try {
optimized = await this.provider.optimizeForAgent(optimized, agentId);
changes.push('AI-enhanced optimization applied');
} catch {
// Use rule-based optimization if AI fails
}
}

return {
content: optimized,
agentId,
changes,
estimatedTokens: Math.ceil(optimized.length / 4),
};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Re-check size after AI optimization to avoid overruns.

optimizeForAgent can hand off to a provider that may expand content past the agent’s max; there’s no post-AI truncation guard. Consider re-validating length after the provider step and re-truncating if needed.

🛠️ Suggested fix
     if (this.provider && changes.length > 0) {
       try {
         optimized = await this.provider.optimizeForAgent(optimized, agentId);
         changes.push('AI-enhanced optimization applied');
       } catch {
         // Use rule-based optimization if AI fails
       }
     }
 
+    let finalTokens = Math.ceil(optimized.length / 4);
+    if (finalTokens > constraints.maxContextLength * 0.95) {
+      optimized = this.truncateContent(optimized, constraints.maxContextLength);
+      changes.push(
+        `Re-truncated after AI optimization to fit ${agentId} context limit (${constraints.maxContextLength} tokens)`
+      );
+      finalTokens = Math.ceil(optimized.length / 4);
+    }
+
     return {
       content: optimized,
       agentId,
       changes,
-      estimatedTokens: Math.ceil(optimized.length / 4),
+      estimatedTokens: finalTokens,
     };
📝 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.

Suggested change
if (this.provider && changes.length > 0) {
try {
optimized = await this.provider.optimizeForAgent(optimized, agentId);
changes.push('AI-enhanced optimization applied');
} catch {
// Use rule-based optimization if AI fails
}
}
return {
content: optimized,
agentId,
changes,
estimatedTokens: Math.ceil(optimized.length / 4),
};
if (this.provider && changes.length > 0) {
try {
optimized = await this.provider.optimizeForAgent(optimized, agentId);
changes.push('AI-enhanced optimization applied');
} catch {
// Use rule-based optimization if AI fails
}
}
let finalTokens = Math.ceil(optimized.length / 4);
if (finalTokens > constraints.maxContextLength * 0.95) {
optimized = this.truncateContent(optimized, constraints.maxContextLength);
changes.push(
`Re-truncated after AI optimization to fit ${agentId} context limit (${constraints.maxContextLength} tokens)`
);
finalTokens = Math.ceil(optimized.length / 4);
}
return {
content: optimized,
agentId,
changes,
estimatedTokens: finalTokens,
};
🤖 Prompt for AI Agents
In `@packages/core/src/ai/agents/optimizer.ts` around lines 147 - 161, After
calling this.provider.optimizeForAgent(...) re-validate the resulting optimized
string against the agent's maximum allowed size (the same truncation logic used
before the provider call), and if it exceeds the limit, truncate it back to the
agent max, push a change like 'Truncated after AI optimization' onto changes,
and recompute estimatedTokens (Math.ceil(optimized.length/4)) before returning;
apply this to the block around optimizeForAgent, optimized, agentId, changes,
and estimatedTokens so the response can never exceed the agent's max after AI
expansion.

…Response

- Re-truncate content after AI optimization if it exceeds agent max context
- Add JSON extraction from code fences in parseGenerateResponse
- Validate required fields (name, description, content) in parseGenerateResponse
- Clamp confidence to [0,1] range and properly type-check optional fields
@rohitg00 rohitg00 merged commit 2441f0d into main Feb 5, 2026
8 of 9 checks passed
@rohitg00 rohitg00 deleted the feat/smart-context-generation branch February 5, 2026 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant