feat(compat): model-invocable Skill tool — progressive disclosure via index block + on-demand body (#120)#172
Merged
Merged
Conversation
… index block + on-demand body (#120) Skills discovery (loadSkills) previously stopped at parsing; nothing let a subagent invoke a skill and no builder rendered a skill index. Add the two missing tiers of progressive disclosure plus the parser fixes they need. - skill-tool.ts (new): `skillIndexBlock(skills)` renders the always-visible tier — the blocking invocation contract plus one `<name>: <description>` line per skill; `skillTool(skills)` is the AI-SDK `Skill` tool that pulls a matched skill's body + absolute path into context on demand, rendered as plain text (toModelOutput) so it reads as instructions. Unknown names return an error naming the available skills — never throw. Skills with `disable-model-invocation: true` are excluded from both the index and the tool's matchable set. - frontmatter.ts: block-scalar support (`|` literal / `>` folded, with `-` chomping). Multi-line skill descriptions previously parsed as the bare marker and dropped their content. Stays hand-rolled, dependency-free. - skills-loader.ts: `SkillDefinition` gains `extra: Record<string, FrontmatterValue>` carrying every frontmatter key other than name/description, so callers can honor `allowed-tools`, `disable-model-invocation`, etc. - index.ts: export the new surface. Mechanism only: zero content, zero aitm wiring, so nothing changes at runtime when it lands. The caller passes the `skills` array, deciding what a role may mount; target-repo skills are untrusted (Worker-only) and their wiring is a separate aitm-side issue. Part of #100.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds YAML block scalar support to frontmatter parsing, preserves additional skill metadata, and introduces exported tooling for indexing and invoking model-accessible skills with exact-name lookup, disabled-skill filtering, argument echoing, and plain-text results. ChangesSkills frontmatter and invocation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Model
participant skillIndexBlock
participant skillTool
Model->>skillIndexBlock: Read eligible skill names and descriptions
skillIndexBlock-->>Model: Return invocation contract and skill index
Model->>skillTool: Invoke exact skill name with optional args
skillTool-->>Model: Return formatted skill instructions with body and path
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
Author
|
@coderabbitai full review |
✅ Action performedFull review 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: 2
🤖 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/ai-claude-compat/src/frontmatter.ts`:
- Around line 83-96: Update parseBlockScalar to remove trailing carriage returns
from each input line before collecting or slicing it, while preserving the
existing indentation and blank-line handling for LF input. Ensure both folded
and literal block-scalar assembly produces values without embedded CR characters
when parseBlock receives CRLF content.
- Around line 65-79: Restrict BLOCK_SCALAR to the documented chomping indicators
by removing “+” from its accepted pattern. Keep parseBlockScalar’s existing “-”
and default clip behavior unchanged, allowing headers such as >+ to follow the
plain-scalar/no-op path instead of being silently misparsed.
🪄 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: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: b170e067-70e4-4f31-99c7-58a22f07bf38
📒 Files selected for processing (7)
packages/ai-claude-compat/src/frontmatter.test.tspackages/ai-claude-compat/src/frontmatter.tspackages/ai-claude-compat/src/index.tspackages/ai-claude-compat/src/skill-tool.test.tspackages/ai-claude-compat/src/skill-tool.tspackages/ai-claude-compat/src/skills-loader.test.tspackages/ai-claude-compat/src/skills-loader.ts
…mping (#120) CR review on PR #172: - Major: CRLF line endings leaked `\r` into block-scalar values. Root cause is broader than the block scalar — `raw.split('\n')` left a trailing `\r` on every line, and the key-line regex `(.*)$` never matches a line ending in `\r`, so on a CRLF-authored SKILL.md NO field parsed at all. Fixed generally by splitting on `/\r?\n/` in parseBlock, which neutralizes CR for every value path. - Minor: `+` (keep) chomping was accepted by BLOCK_SCALAR but behaved like clip. Dropped it from the regex so `>+` falls through to the plain-scalar path; corrected the comment to document `-`-only support. Tests: CRLF folded/literal round-trips + `>+` plain-scalar fallback.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Closes #120. The skills subsystem stopped at discovery:
loadSkillsparsedSKILL.mdfiles but nothing let a subagent invoke a skill and no builder rendered a skill index for the system prompt. This ships the two missing tiers of progressive disclosure plus the parser fixes they depend on.Mechanism only — zero content, zero aitm wiring, so nothing changes at runtime when it lands (per the issue's scope note).
Changes
skill-tool.ts(new)skillIndexBlock(skills)— the always-visible tier: the blocking invocation contract as a preamble plus one<name>: <description>line per skill. Bodies never appear here; multi-line descriptions collapse to one line. Returns''when nothing is invocable.skillTool(skills)— theSkilltool a subagent calls to pull a matched skill's body + absolute path into context on demand, rendered as plain text (toModelOutput, per feat(compat): plain-text tool-result rendering — toModelOutput on the file/search/bash tools so the model stops seeing JSON-escaped file contents #127) so it reads as instructions rather than a JSON-escaped blob. Unknown names return an error result naming the available skills — never throws.disable-model-invocation: trueare excluded from both the index and the tool's matchable set.frontmatter.ts— block-scalar support (|literal />folded, with-chomping). Multi-line skill descriptions previously parsed as the bare marker character and silently dropped their content. Stays hand-rolled and dependency-free.skills-loader.ts—SkillDefinitiongainsextra: Record<string, FrontmatterValue>carrying every frontmatter key other thanname/description, so callers can honorallowed-tools,disable-model-invocation, etc.index.ts— export the new surface.Scope boundaries (from the issue)
The caller passes the
skillsarray, deciding what a role may mount. Target-repo.claude/skillsare untrusted input — Worker-only, behind an explicit opt-in, never Planner/Reviewer — and that mounting is a separate aitm-side issue this API deliberately does not preclude. Skipped here: plugin namespaces, the slash-command surface (no interactive surface in aitm), and directory-scoped resolution (blocked on #104's file-touch tracking).Tests / gate
skill-tool.test.ts(new): index preamble + one-line-per-skill +disable-model-invocationexclusion; valid lookup returns body/path and plain-text rendering; unknown name returns the available-names error without throwing; args echoed.frontmatter.test.ts:|/>/|-/>-, folded paragraph breaks, block ends at the next top-level key, andkey: >textstaying a plain scalar.skills-loader.test.ts:extrapassthrough + a folded description round-tripping throughloadSkills.typecheck(both packages) +typecheck:tests(compat) +test:node(247 pass / 0 fail) + biome all clean.Part of #100.
Summary by CodeRabbit
>) and literal (|) block scalars (including optional chomping) in skill frontmatter descriptions.extrafield on loaded skills.