Repository navigation
feat(ai): unified agent skill system — discoverable, loadable, discardable capabilities - #2244
Conversation
…, requiredTools, eligibility) Adds the four domain skills (canvas-websites, spreadsheets, task-management, writing-documents) to BUILTIN_COMMANDS as kind:'skill' entries with hasAny requiredTools gates, exports BUILTIN_SKILLS/isSkillEligible/clipDescription, and threads kind through CommandSummary and loadAvailableCommands. Pure registry change — bodies, load tool, and catalog land in follow-up commits. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LqoE6ePM941BLDw6myo76T
…load paths Adds canvas-websites, spreadsheets, task-management, and writing-documents instruction packs (each source-verified against the renderer/tools it describes, guard-tested < 500 lines / < 20k chars), and refactors the command resolver: resolveBuiltinInjection injects skill bodies as dynamicContent (chip path works immediately), and the by-id DB half of resolveToken is extracted as resolveCommandInjectionById so the model load path shares the exact permission pipeline. Existing resolver tests pass untouched (refactor guard); available-commands expectations now derive from the registry. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LqoE6ePM941BLDw6myo76T
One model-initiated load path for built-in skills and user/drive commands: trigger-validated hostile input, precedence resolution via loadAvailableCommands, eligibility gating, and the existing permission-checked resolvers. The body returns as a tool result wrapped in <skill_instructions> framing (buildSkillLoadResult — same 60k cap and listed-not-loaded child manifest as the chip path). Registered as a core tool so it stays upfront in search exposure mode and read-only mode; workspace tool count 76 → 77 in doc copy. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LqoE6ePM941BLDw6myo76T
…corpus Discovery layer: built-in skills render as a stable SKILLS section in the system prompt (eligibility-gated before token cost, uniform across default and custom-prompt agents); the per-viewer user/drive command list rides the volatile turn context with an OpenClaw-style degradation ladder (full → 80-char → names-only, names never dropped within the cap) at a 2k-char budget — zero standing cost for users with no commands. tool_search now searches the whole capability corpus (deferred tools + builtin skills + sanitized command entries) returning kind-tagged results with load_skill pointers; output is byte-identical to before when no skills are passed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LqoE6ePM941BLDw6myo76T
…tion load_skill joins DEFAULT_ELIDABLE_TOOLS — skill bodies are deterministic re-fetchable reads, so stale loads collapse to the standard re-load stub. New ElisionOptions.protectMostRecentByArgs keeps the newest load per distinct input alive regardless of the boundary (agentskills.io: active skill instructions must survive pruning; superseded loads decay). Wired in context-assembly for load_skill. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LqoE6ePM941BLDw6myo76T
…obal-assistant guidance PAGE TYPES and TASK MANAGEMENT become tool-aware builders: with load_skill present the deep DOCUMENT/SHEET/CANVAS/TASK_LIST guidance shrinks to one-line skill pointers (the bodies cover it verbatim-or-better); agents without load_skill and the admin complete-preview keep the full fallback text. The Global Assistant's bespoke instruction block — which had drifted (claimed tasks create linked DOCUMENT pages; they create TASK_LIST children) and duplicated mention guidance — is replaced by the shared buildGlobalAssistantInstructions sections. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LqoE6ePM941BLDw6myo76T
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LqoE6ePM941BLDw6myo76T
📝 WalkthroughWalkthroughIntroduces built-in AI skills with server-shipped instruction bodies, a ChangesAgent skills and command discovery
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Agent
participant ChatRoute
participant ToolSearch
participant LoadSkill
participant CommandResolver
Agent->>ChatRoute: Send request with available tools
ChatRoute->>ToolSearch: Search tools, skills, and commands
ToolSearch-->>Agent: Return load_skill pointer
Agent->>LoadSkill: Request skill or command by name
LoadSkill->>CommandResolver: Resolve built-in or user command
CommandResolver-->>LoadSkill: Return instruction injection plan
LoadSkill-->>Agent: Return framed skill instructions
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba2bc43fe3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // The newest load of each skill is the agent's active instructions — | ||
| // protected from elision (superseded loads of the same skill decay). | ||
| protectMostRecentByArgs: new Set(['load_skill']), |
There was a problem hiding this comment.
When an admin conversation is compacted after its latest load_skill call falls before the cut, prepareConversationContext removes that turn before this MRU protection scans tailUIMessages, so the active instructions are still lost. The summarizer only receives the first 500 characters of each tool result (summarization-prompt.ts), meaning its summary cannot preserve these potentially 60k-character instructions either; long-running skill-guided work can therefore continue without the playbook. Protect or reattach the newest load per skill before compaction cuts the history, rather than only protecting loads remaining in the tail.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in f79f868, with a deliberately different shape than re-attachment: skill bodies are deterministic re-fetchable reads (that premise is what makes them elidable at all), so instead of carrying up-to-60k-char payloads across the compaction boundary — which would defeat the summary — prepareHistoryForModel now diffs the skills loaded anywhere in the sanitized history against those still present in the surviving tail, and appends a deterministic pointer to the summary text: [Skill instructions previously loaded … no longer in context: "x". Call load_skill again …]. The model re-loads on its next step in that domain rather than continuing without the playbook unknowingly. Three new tests: pointer appended when the load was compacted away, no pointer when the load survives in the tail, summaryText untouched when no compaction ran. If you think eager re-attachment (Claude-Code-style 5k/25k budgets) is worth the summary bloat here, happy to discuss — but the re-load pointer keeps the token cost O(names) and the behavior explicit.
| const kw = query.toLowerCase(); | ||
| return skills.filter( | ||
| (skill) => skill.name.includes(kw) || skill.description.toLowerCase().includes(kw) | ||
| ); |
There was a problem hiding this comment.
For users or drives with many commands, an empty query—or any broad keyword—returns every matching command because z.string() accepts "", includes("") matches every entry, and this filter has no result cap. loadAvailableCommands and commandsToSearchEntries are likewise unbounded, so a single model-generated search can append an arbitrarily large tool result and exceed the next model request's context limit, despite the separate command catalog being explicitly capped for that reason. Limit or paginate these matches and reject empty keyword searches.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f79f868: tool_search now rejects empty/whitespace queries outright (returning an error hint instead of the full corpus — includes("") matched every entry), and skill/command matches are capped at MAX_SKILL_MATCHES = 10 with an omission note pointing at a more specific keyword. Covered by two new tests (empty-query rejection incl. whitespace, 25-match corpus → 10 returned + "15 more" note). The tools half of the search was already bounded by the fixed workspace registry (~77 entries), so the cap targets the genuinely unbounded command corpus this thread flagged.
…-route expectations The renderer-coverage gate requires every workspace tool to have a rich renderer: load_skill renders a collapsed instruction preview (soft errors fall through to the generic renderer). Suggest-route tests now derive builtin expectations from BUILTIN_COMMANDS instead of hardcoding /help. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LqoE6ePM941BLDw6myo76T
…compaction Review feedback (PR #2244): (1) tool_search now rejects empty/whitespace queries (includes('') matched every corpus entry) and caps skill/command matches at 10 with an omission note — the command corpus is unbounded per drive and a broad keyword could append an arbitrarily large tool result. (2) When compaction cuts a conversation's most recent load_skill turn, the MRU elision protection can't see it and the summarizer truncates tool results far below body size — the summary now carries a deterministic re-load pointer for every skill loaded in the compacted head that no longer appears in the tail, keeping active instructions restorable instead of silently lost. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LqoE6ePM941BLDw6myo76T
…model corrections Adversarial audit of all four bodies against source found and fixed claims that would break agent interactions: - spreadsheets: IF does NOT short-circuit (IFERROR is the only lazy guard — the standard =IF(B2=0,"",A2/B2) pattern errors); #CYCLE never renders (everything displays #ERROR; CIRCULAR_REF lives in stored error.type); YEAR/MONTH/DAY UTC-parse/local-extract off-by-one; REPT errors past 10k; non-finite results store no error object. - canvas-websites: provision_form_target field constraints (type enum, 1-20 fields, name regex); in-app CSS url() stripping (expect blank preview backgrounds — don't 'fix'); unconditional publish CSP widening; canvasPageId; subdomain first-allocation semantics; unwrap only on <html>. - writing-documents: mentions section rewritten — documents take the editor HTML mention elements; @[..](..) is chat/channel syntax, fallback-parsed only in mention-free pages and literal text otherwise; required title field on read/edit tools; permission/notification claims aligned. - task-management: assigneeIds validation and get_assigned_tasks visibility claims aligned to the legacy-column reality; conditional status validation; linkedPageId naming. Model corrections (integration triage): requiredTools now gates DISCOVERY only — load_skill loads any skill by name regardless of allowlist (matches the prompt pointers that survive tool filtering); PAGE_TYPES/TASK_MANAGEMENT slim per-bullet only when that bullet's skill is actually eligible; the catalog omission note drops its list_commands pointer when the agent can't call it; chip injections (turn-volatile) now end with a load_skill re-load pointer; load_skill soft-degrades on missing auth. Spec updated (discovery vs load, chip volatility, trigger-namespace shadowing, invariant wording). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LqoE6ePM941BLDw6myo76T
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/web/src/lib/ai/core/command-processor.ts (1)
139-152: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract shared truncation/manifest logic to avoid drift between
buildCommandSystemPromptandbuildSkillLoadResult.Both functions duplicate the 60k truncation-note construction and the "children resources, NOT loaded" manifest list. The docstring on
buildSkillLoadResultexplicitly calls out that these two paths must share "the same 60k truncation contract" — extracting the logic into shared helpers makes that guarantee structural instead of a comment-enforced convention.♻️ Proposed refactor
+function truncateEntryPageContent(content: string, pageId: string): string { + if (content.length <= COMMAND_CONTENT_CHAR_LIMIT) return content; + return ( + content.slice(0, COMMAND_CONTENT_CHAR_LIMIT) + + `\n\n[Content truncated — the page is unusually large. Use read_page with pageId "${pageId}" to read the rest on demand.]` + ); +} + +function buildChildManifestLines( + children: CommandChildResource[], + intro: string, +): string[] { + if (children.length === 0) return []; + return ['', intro, ...children.map((child) => `- "${child.title}" (pageId: ${child.id})`)]; +} + export function buildCommandSystemPrompt(injection: CommandInjection): string { ... if (injection.entryPage) { - let content = injection.entryPage.serializedContent; - let truncationNote = ''; - if (content.length > COMMAND_CONTENT_CHAR_LIMIT) { - content = content.slice(0, COMMAND_CONTENT_CHAR_LIMIT); - truncationNote = `\n\n[Content truncated — the page is unusually large. Use read_page with pageId "${injection.entryPage.id}" to read the rest on demand.]`; - } + const content = truncateEntryPageContent(injection.entryPage.serializedContent, injection.entryPage.id); lines.push( `Follow the instructions in its page "${injection.entryPage.title}" (pageId: ${injection.entryPage.id}) below:`, '', '<command_instructions>', - content + truncationNote, + content, '</command_instructions>' ); }Also applies to: 187-219
🤖 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 `@apps/web/src/lib/ai/core/command-processor.ts` around lines 139 - 152, Extract the shared 60k content-truncation behavior and children-resource manifest construction from buildCommandSystemPrompt and buildSkillLoadResult into reusable helpers. Update both functions to use those helpers, preserving the existing truncation note, page identifiers, and “children resources, NOT loaded” wording so both paths maintain the same contract.apps/web/src/lib/ai/tools/__tests__/tool-search-tool.test.ts (1)
29-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename the module-level constant to UPPER_SNAKE_CASE.
Use
SKILL_CORPUSand update its references. As per coding guidelines, use UPPER_SNAKE_CASE for constants.🤖 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 `@apps/web/src/lib/ai/tools/__tests__/tool-search-tool.test.ts` around lines 29 - 32, Rename the module-level constant skillCorpus to SKILL_CORPUS and update every reference to it in the test module, preserving its contents and behavior.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@apps/web/src/lib/ai/core/command-processor.ts`:
- Around line 139-152: Extract the shared 60k content-truncation behavior and
children-resource manifest construction from buildCommandSystemPrompt and
buildSkillLoadResult into reusable helpers. Update both functions to use those
helpers, preserving the existing truncation note, page identifiers, and
“children resources, NOT loaded” wording so both paths maintain the same
contract.
In `@apps/web/src/lib/ai/tools/__tests__/tool-search-tool.test.ts`:
- Around line 29-32: Rename the module-level constant skillCorpus to
SKILL_CORPUS and update every reference to it in the test module, preserving its
contents and behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7f415a4a-93a8-4c3d-b207-920aa3cb260b
📒 Files selected for processing (40)
README.mdapps/marketing/src/app/docs/getting-started/page.tsxapps/web/src/app/api/ai/chat/route.tsapps/web/src/app/api/ai/global/[id]/messages/route.tsapps/web/src/app/api/commands/__tests__/suggest-route.test.tsapps/web/src/components/ai/shared/chat/tool-calls/registry.tsxapps/web/src/lib/ai/core/__tests__/ai-tools.test.tsapps/web/src/lib/ai/core/__tests__/context-assembly.test.tsapps/web/src/lib/ai/core/__tests__/inline-instructions.test.tsapps/web/src/lib/ai/core/__tests__/skill-catalog.test.tsapps/web/src/lib/ai/core/__tests__/stub-tools.test.tsapps/web/src/lib/ai/core/__tests__/tool-filtering.test.tsapps/web/src/lib/ai/core/ai-tools.tsapps/web/src/lib/ai/core/command-processor.tsapps/web/src/lib/ai/core/command-resolver.tsapps/web/src/lib/ai/core/context-assembly.tsapps/web/src/lib/ai/core/inline-instructions.tsapps/web/src/lib/ai/core/prompt-assembly.tsapps/web/src/lib/ai/core/skill-catalog.tsapps/web/src/lib/ai/core/stub-tools.tsapps/web/src/lib/ai/skills/__tests__/skill-bodies.test.tsapps/web/src/lib/ai/skills/bodies/canvas-websites.tsapps/web/src/lib/ai/skills/bodies/spreadsheets.tsapps/web/src/lib/ai/skills/bodies/task-management.tsapps/web/src/lib/ai/skills/bodies/writing-documents.tsapps/web/src/lib/ai/skills/skill-bodies.tsapps/web/src/lib/ai/tools/__tests__/skill-tools.test.tsapps/web/src/lib/ai/tools/__tests__/tool-exposure.test.tsapps/web/src/lib/ai/tools/__tests__/tool-search-tool.test.tsapps/web/src/lib/ai/tools/skill-tools.tsapps/web/src/lib/ai/tools/tool-exposure.tsapps/web/src/lib/ai/tools/tool-search-tool.tsapps/web/src/lib/commands/__tests__/available-commands.test.tsapps/web/src/lib/commands/available-commands.tsapps/web/src/lib/commands/command-catalog-loader.tsdocs/specs/agent-skills.mdpackages/lib/src/ai/__tests__/tool-result-eliding.test.tspackages/lib/src/ai/tool-result-eliding.tspackages/lib/src/commands/__tests__/command-core.test.tspackages/lib/src/commands/command-core.ts
What
One unified capability primitive for agents — tools, built-in skills, and user/drive commands share
{name, kind, description}with three verbs (discover / load / act) — built by transforming the existing seams, not adding parallel systems:BUILTIN_COMMANDS(command-core) gainskind: 'skill'+requiredTools; picker,/help, chips, catalog, and loads all consume one precedence-resolved listresolveToken's DB half extracted asresolveCommandInjectionById— chip path and model path are permission-identical by constructionload_skillreturns the body as a tool result (streamText's only mid-stream channel; append-only, prefix-cache-safe) with<skill_instructions>framingload_skilljoinsDEFAULT_ELIDABLE_TOOLS; newprotectMostRecentByArgskeeps the newest load per skill alive (Agent Skills standard: protect active instructions)tool_searchsearches the whole corpus (deferred tools + skills + sanitized command entries), kind-tagged resultsThe four built-in skills
Code-shipped, source-verified bodies (guard-tested <500 lines / <20k chars):
canvas-websites(sandbox/CSP model, theme bridge, link conventions, publish rewriting, verbatim formHtml),spreadsheets(SheetDoc format, full formula table, cross-sheet refs),task-management(linked TASK_LIST children, SUBTASKS_INCOMPLETE, triggers),writing-documents(content modes, line-numbered editing, mention syntax).Discovery economics
load_skill+ intersectrequiredTools) before any token cost; ~40 tok/skill.universal-commands-palette-readiness.md§2 with that memo's recommended shape.load_skillis present (full text remains the fallback); the Global Assistant's drifted bespoke block (claimed tasks create DOCUMENT children — they create TASK_LIST) is replaced by the shared sections.Notes for review
select trigger from commands where trigger in ('canvas-websites','spreadsheets','task-management','writing-documents').commandPrompt— pre-existing gap, capped at 2k chars, not widened here.tool-result-eliding.ts).docs/specs/agent-skills.md: catalogs for consult/headless/workflow surfaces, search-tier past ~100 skills, proposal→approve governance for agent-authored skills, per-skill trigger evals.Gates: monorepo typecheck ✓, lint ✓, full unit suite ✓, security suite ✓, knip clean for changed files. Rebased onto master post-#2241 (global-tool-exposure) —
tool_searchcomposes withexcludeAlwaysUpfront+splitToolsForExposure.Post-review hardening (review fixes + adversarial accuracy triage)
Review-thread fixes (f79f868):
tool_searchrejects empty queries and caps skill/command matches at 10 with an omission note; compaction summaries carry a deterministicload_skillre-load pointer for skills whose loads were compacted away (design-consistent with re-fetchable bodies rather than re-attaching 60k payloads).Adversarial source audit of all four skill bodies + integration seams (6708090) — three independent audits verified every claim against source; all findings fixed:
IFdoes not short-circuit (onlyIFERRORis lazy — the standard divide-guard errors);#CYCLEnever renders (all errors display#ERROR,CIRCULAR_REFis stored-only);YEAR/MONTH/DAYUTC/local off-by-one caveat;REPTerrors past 10k; non-finite results store no error object.url()stripping (blank preview backgrounds are expected — don't "fix"); unconditional publish CSP widening;canvasPageId; subdomain first-allocation semantics; unwrap only on<html>.@[..](..)is chat/channel syntax (fallback-parsed only in mention-free pages, literal text otherwise); requiredtitlefield documented; notification claims aligned.assigneeIdsvalidation andget_assigned_tasksvisibility aligned to the legacy-column reality; conditional status validation;linkedPageIdnaming.requiredToolsgates discovery only —load_skillloads any skill by name regardless of allowlist; prompt bullets slim per-bullet only when that skill is eligible (no more prompt-instructs/runtime-refuses contradiction); catalog omission note drops itslist_commandspointer when uncallable; chip injections (turn-volatile) end with a re-load pointer;load_skillsoft-degrades on missing auth.🤖 Generated with Claude Code
https://claude.ai/code/session_01LqoE6ePM941BLDw6myo76T
Summary by CodeRabbit
New Features
Documentation