feat(skills): the desk's procedures as versionable SKILL.md files - #3
Merged
Conversation
An agent profile is *who* works a ticket. A skill is *what this desk has decided to always do* about a class of problem — and until now that lived only in whatever an admin remembered to paste into four separate personas. A skill is `skills/<slug>/SKILL.md`: frontmatter (name, description, categories) plus the procedure, seeded non-destructively like `agents/*.md` and editable from the new Skills page. Four ship bundled: account lockouts, ops-database changes, shipping a code change, and when to escalate instead of resolving. The wiring is progressive disclosure, the way Claude Code loads skills: the resolver's system prompt carries only the catalogue — slug, scope, description — and the body costs one call to the new `read_skill` tool (LOW risk, no approval; it reads a procedure, it does not perform one). The catalogue is withheld entirely when an agent's allowlist has no `read_skill`, because naming procedures an agent cannot open is worse than saying nothing. Two things keep it honest. A skill never overrides an approval gate — the procedure says what to do, tool-policies.ts still decides whether the agent may. And QA is handed the skills that applied to the ticket along with which ones the run actually opened, so an agreed procedure that gets ignored is caught before the ticket closes. `read_skill` is deliberately not a core tool, so the MCP surface serves it: an agent outside Servo can follow the same procedures. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HtUAyrhtCCZsYv8taNHM7C
ricauts
force-pushed
the
feat/desk-skills
branch
from
August 14, 2026 13:26
f461817 to
397a13b
Compare
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
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.
A specialized agent is who works a ticket. A skill is what this desk has decided to always do about a class of problem — and until now that lived only in whatever an admin remembered to paste into four separate personas.
What was ported
Agent skills as versionable files, from Claude Code's own skills design and Paperclip's
skills/[slug]/SKILL.md+packages/skills-catalogprogressive-disclosure pattern.A skill is
skills/[slug]/SKILL.md— frontmatter (name,description,categories) plus the procedure — seeded into aSkilltable non-destructively, exactly likeagents/*.md, and editable from a new Skills page. Four ship bundled: account lockouts, ops-database changes, shipping a code change, and when to escalate instead of resolving.The load path is progressive disclosure, the way Claude Code loads skills:
slug (scope): description— ordered applicable-first and capped at 40, so a desk can hold dozens of procedures without bloating every prompt.read_skilltool (LOW risk, no approval — it reads a procedure, it does not perform one).categories: []makes a skill desk-wide; that is how policy like "when to escalate" is written.Why
It was the top "candidate for a future run" left by the desk-memory PR (#1), and it is the missing half of that feature: memory is what the desk did; a skill is what the desk decided to always do. It is also the cheapest way to make the core thesis enforceable — an admin writes "never reset an account for someone other than its owner" once, in a file under version control, instead of re-editing four agent personas.
Two things keep it aligned with the thesis rather than working around it:
tool-policies.tsand the engine still decide whether the agent may.read_skillmutates nothing, so it declares LOW/no-approval insrc/lib/ai/tool-policies.tsand passes the gate insrc/lib/ai/engine.tsunchanged.runQaReview()derives which applicable skills the run actually opened (from persistedTOOL_CALLsteps, so it survives pause/resume) and hands QA that list. An agreed procedure that gets ignored is caught before the ticket closes — which is the point of having agreed one.Two smaller decisions worth flagging for review:
read_skillis deliberately not a core tool. Core tools are excluded from the MCP surface as ticket-bound;read_skillneeds no ticket, so keeping it out ofCORE_TOOLSmeans external MCP clients can follow the desk's procedures too. The cost is one checkbox per already-customized specialist on upgrade (documented in the user guide, plus a troubleshooting row).read_skilltakes and the keysyncSkills()matches the bundled file on; letting a rename move it would make the next upgrade re-create the original alongside the renamed one. Renaming in frontmatter changes the display name only.Configuration
No new environment variables. Everything is managed from the existing UI: Skills in the sidebar (admins write and edit; agents can view), with the enable switch acting as retract —
read_skillrefuses a disabled skill with "must not be followed" rather than 404-ing, and the UI steers admins to the switch instead of Delete, since a bundled skill returns on the nextnpm run setup. Documented indocs/USER-GUIDE.md§5.How it was validated
There are no API keys in this environment, so everything ran through the deterministic mock provider.
npm ci,npm run setup,npm run typecheck,npm run build— all clean.npm test— 81 passing: 41 pre-existing (unchanged) + 40 new acrosstests/skill-format.test.ts,tests/skill-tools.test.ts,tests/skills-bundled.test.ts.read_skill{slug: "locked-out-account"}as its first call, then reset, commented and resolved; the QA section reportedlocked-out-account — READ by the run/when-to-escalate — NOT read by the run.npm run setupon a populated database with one skill edited and disabled — 0 new skills, edit and retraction both preserved.read_skill, the catalogue is correctly withheld and the run proceeds exactly as before.description), duplicate slug (409), rename (slug held), delete, and a REQUESTER403onGET /api/skills.read_skillconfirmed served bygetMcpTools(), including the refusal on a disabled skill.MockProviderparses the catalogue out of the system prompt it is handed and opens its script withread_skill, so the offline demo shows procedure-checking without a key — and a desk with no skills produces byte-for-byte the old script.resolverSystem(policies, "")is likewise identical to the old prompt, which is asserted in a test.What was rejected
SKILL.md. Servo skills are text on purpose: running admin-authored code out of the database would route around the tool policy layer entirely, which is the one thing this product must not do. A procedure that needs to act names a Servo tool, and that tool carries a risk level.PAPERCLIP_*env-var contract for skills. It assumes agents are external processes woken by heartbeats. Servo's agents run in-process inside the resolver loop and take their context from the run — and a new mandatory env var is out of bounds anyway.packages/adapters/*,packages/plugins/*,packages/db— re-confirmed as rejected in the ledger; all assume the pnpm monorepo, the Node+React split or its own database.Attribution
No upstream code was copied, so
THIRD-PARTY.mdis unchanged (it does not exist). The parser is Servo's own (gray-matter+ theCATEGORIESunion), and the catalogue ordering, applicability rule and QA review section were written against Servo's schema. The design was observed, not lifted.Ledger
docs/PORTING-LEDGER.mdis created in this PR (it currently exists only on the unmerged #1 branch, with the same header and rules, so the two should merge cleanly). It records this item, an In flight section naming PRs #1 and #2 so no run restarts branched work, the rejections above, and the next candidates —fetch_urlbehind an SSRF guard, "turn this run into a skill" from a resolved ticket, and per-agent skill scoping.