feat(kit): add core skill plugin capabilities#368
Conversation
|
✅ Preview build completed successfully!
|
📦 Package Previewpnpm add https://pkg.pr.new/@opentiny/tiny-robot@1514455 pnpm add https://pkg.pr.new/@opentiny/tiny-robot-kit@1514455 pnpm add https://pkg.pr.new/@opentiny/tiny-robot-svgs@1514455 commit: 1514455 |
WalkthroughAdds a new ChangesSkill Plugin and Capabilities
Sequence Diagram(s)sequenceDiagram
participant Engine as Message Engine
participant Plugin as skillPlugin
participant Model
participant Tools as Runtime Tools
Engine->>Plugin: onTurnStart(selection options)
alt manual
Plugin->>Plugin: resolve skills by name
else auto
Plugin->>Plugin: normalize candidates, store selecting phase
else none
Plugin->>Plugin: store empty skill context
end
Engine->>Plugin: onBeforeRequest
alt auto & selecting
Plugin->>Model: append select_skills instructions
Model->>Tools: call select_skills
Tools->>Plugin: resolve skill names, update context
end
Plugin->>Model: append skill + resource instructions
Model->>Tools: call list_skill_files / read_skill_file
Tools->>Model: return file summaries / content
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![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
🧹 Nitpick comments (2)
packages/kit/src/skills/capabilities/selection.ts (1)
10-37: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd an empty-candidates guard for consistency with
createSkillResourceInstructionsMessage.
createSkillSelectionRuntimeToolsalready short-circuits whencandidates.length === 0(no tool is exposed), butcreateSkillSelectionInstructionsMessagehas no equivalent guard — it will still instruct the model to "Use the select_skills tool before answering" even when no tool is actually registered. The resource capability module (resources.ts) applies the same guard (hasSkillResources) symmetrically to both its instructions and runtime-tools functions; this module doesn't.If callers in
skillPlugin.tsalways supply non-empty candidates before invoking this function, this is currently unreachable, but adding the guard removes the fragile cross-file assumption.♻️ Suggested guard
export const createSkillSelectionInstructionsMessage = ({ candidates, preferredSkillNames, }: { candidates: SkillCandidate[] preferredSkillNames?: string[] }): ChatCompletionSystemMessageParam => { + if (candidates.length === 0) { + return undefined + } + const lines = [Note: this would also require updating the return type to
ChatCompletionSystemMessageParam | undefined, matchingcreateSkillResourceInstructionsMessage's signature.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/kit/src/skills/capabilities/selection.ts` around lines 10 - 37, `createSkillSelectionInstructionsMessage` should mirror the empty-candidates guard used by `createSkillSelectionRuntimeTools` and `createSkillResourceInstructionsMessage`. Add a check for `candidates.length === 0` at the start of the function, return `undefined` in that case, and update the return type accordingly to `ChatCompletionSystemMessageParam | undefined`. Keep the rest of the message-building logic in `selection.ts` unchanged for non-empty candidates.packages/kit/src/skills/capabilities/resources.ts (1)
11-51: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider restricting
skillNameto the active skill names viaenum, mirroringselection.ts.
select_skills's parameter schema constrainsskillNamesviaenum: candidateNames(selection.ts:74), butlist_skill_files/read_skill_file'sskillNameparameter here has no such constraint, even though only names present in the currentskillsarray are ever valid (seefindSkill). Since these tools are already built per-call insidecreateSkillResourceRuntimeTools(skills), the schema could be generated dynamically withenum: skills.map((skill) => skill.name)to reduce hallucinated skill names, consistent with the guidance already established inselection.ts.Handler-side validation (
skill_not_found) already covers correctness, so this is purely a model-steering improvement, not a functional gap.Also applies to: 93-108
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/kit/src/skills/capabilities/resources.ts` around lines 11 - 51, The `skillName` parameter in `skillResourceTools` is currently unconstrained, which can lead the model to invent invalid names. Update `createSkillResourceRuntimeTools(skills)` so the schemas for `listSkillFiles` and `readSkillFile` set `skillName` to an `enum` built from `skills.map((skill) => skill.name)`, matching the pattern used in `selection.ts` for `select_skills`. Keep the existing `findSkill`/`skill_not_found` runtime validation unchanged, and apply the same schema change anywhere the same tool definitions are duplicated.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/kit/src/message/plugins/skillPlugin.ts`:
- Around line 234-258: In resolveSkillsByNames, avoid marking a skill as
unresolved when getSkillByName returns a valid SkillDefinition whose skill.name
differs from the requested name; track resolution status from the actual lookup
result rather than only the requested name. Update the unresolvedSkillNames
bookkeeping so aliases/normalized names don’t create contradictory state, and
ensure onSkillsResolved/SkillRequestContext only receive truly unresolved
entries. Also distinguish resolver failures in the try/catch from “not found”
cases by preserving the failure path in the results object instead of collapsing
every exception into a generic failed flag.
---
Nitpick comments:
In `@packages/kit/src/skills/capabilities/resources.ts`:
- Around line 11-51: The `skillName` parameter in `skillResourceTools` is
currently unconstrained, which can lead the model to invent invalid names.
Update `createSkillResourceRuntimeTools(skills)` so the schemas for
`listSkillFiles` and `readSkillFile` set `skillName` to an `enum` built from
`skills.map((skill) => skill.name)`, matching the pattern used in `selection.ts`
for `select_skills`. Keep the existing `findSkill`/`skill_not_found` runtime
validation unchanged, and apply the same schema change anywhere the same tool
definitions are duplicated.
In `@packages/kit/src/skills/capabilities/selection.ts`:
- Around line 10-37: `createSkillSelectionInstructionsMessage` should mirror the
empty-candidates guard used by `createSkillSelectionRuntimeTools` and
`createSkillResourceInstructionsMessage`. Add a check for `candidates.length ===
0` at the start of the function, return `undefined` in that case, and update the
return type accordingly to `ChatCompletionSystemMessageParam | undefined`. Keep
the rest of the message-building logic in `selection.ts` unchanged for non-empty
candidates.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: db793450-50c7-4d95-8bf0-0c15f4acbbce
📒 Files selected for processing (10)
packages/kit/src/message/plugins/index.tspackages/kit/src/message/plugins/skillPlugin.tspackages/kit/src/message/utils.tspackages/kit/src/skills/capabilities/commands.tspackages/kit/src/skills/capabilities/resources.tspackages/kit/src/skills/capabilities/selection.tspackages/kit/src/skills/capabilities/utils.tspackages/kit/src/skills/test/resourceCapability.test.tspackages/kit/src/skills/test/skillPlugin.test.tspackages/kit/src/utils.ts
| const resolveSkillsByNames = async ( | ||
| skillNames: string[], | ||
| getSkillByName: SkillResolver['getSkillByName'], | ||
| context: BasePluginContext, | ||
| ) => { | ||
| const results = await Promise.all( | ||
| skillNames.map(async (name) => { | ||
| try { | ||
| return { name, skill: await getSkillByName(name, context) } | ||
| } catch { | ||
| return { name, failed: true } | ||
| } | ||
| }), | ||
| ) | ||
|
|
||
| const skills = results.map((result) => result.skill).filter((skill): skill is SkillDefinition => Boolean(skill)) | ||
| const resolvedSkillNameSet = new Set(skills.map((skill) => skill.name)) | ||
|
|
||
| return { | ||
| skills, | ||
| unresolvedSkillNames: results | ||
| .filter((result) => result.failed || !result.skill || !resolvedSkillNameSet.has(result.name)) | ||
| .map((result) => result.name), | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Possible double-bookkeeping when getSkillByName returns a skill with a different .name than requested.
unresolvedSkillNames is derived via resolvedSkillNameSet.has(result.name) (the requested name), while skills is populated from result.skill regardless of whether skill.name matches the requested name. If a resolver ever normalizes/aliases names (e.g. case-insensitive lookup), the same skill would end up both enabled (skills) and reported as unresolved (unresolvedSkillNames) — contradictory state surfaced to onSkillsResolved/SkillRequestContext.
Separately, the catch block (lines 241-245) treats genuine resolver failures (e.g. storage unavailable, per the test at skillPlugin.test.ts:178-180) identically to "skill not found" with no error detail preserved — this can mask real operational issues from anything relying on unresolvedSkillNames alone.
🐛 Proposed fix for the name-mismatch bookkeeping issue
const skills = results.map((result) => result.skill).filter((skill): skill is SkillDefinition => Boolean(skill))
- const resolvedSkillNameSet = new Set(skills.map((skill) => skill.name))
+ const resolvedNameByRequestedName = new Map(
+ results.filter((result) => result.skill).map((result) => [result.name, result.skill]),
+ )
return {
- skills,
+ skills: [...resolvedNameByRequestedName.values()] as SkillDefinition[],
unresolvedSkillNames: results
- .filter((result) => result.failed || !result.skill || !resolvedSkillNameSet.has(result.name))
+ .filter((result) => result.failed || !result.skill)
.map((result) => result.name),
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/kit/src/message/plugins/skillPlugin.ts` around lines 234 - 258, In
resolveSkillsByNames, avoid marking a skill as unresolved when getSkillByName
returns a valid SkillDefinition whose skill.name differs from the requested
name; track resolution status from the actual lookup result rather than only the
requested name. Update the unresolvedSkillNames bookkeeping so
aliases/normalized names don’t create contradictory state, and ensure
onSkillsResolved/SkillRequestContext only receive truly unresolved entries. Also
distinguish resolver failures in the try/catch from “not found” cases by
preserving the failure path in the results object instead of collapsing every
exception into a generic failed flag.
1 participant
PR 描述
新增 core
skillPlugin与 skill capabilities，将已选择的 skills 转换为 system instructions 和 runtime tools，接入 message engine。主要改动：
message/plugins/skillPlugin.ts。manual/auto/none三种 request-level skill selection。manual模式支持直接传入完整SkillDefinition[]，或通过skillNames + getSkillByName解析。auto模式支持候选 skill summaries、偏好 skill names、select_skillsruntime tool，以及选择后解析完整 skill。SkillRequestContext，记录skills、skillNames、requestedSkillNames、unresolvedSkillNames、runtimeTools和 selection 状态。list_skill_files/read_skill_fileruntime tools，并在 system instruction 中提示先 list 再 read。execute_skill_command。getUniqueStringArray通用工具函数。验证：
pnpm -F @opentiny/tiny-robot-kit test -- --run src/skills/test/resourceCapability.test.ts src/skills/test/skillPlugin.test.ts src/message/test/toolPlugin.test.ts pnpm buildSummary by CodeRabbit
New Features
Bug Fixes
Tests