feat(skills): install the conventions instead of reciting them - #61
Merged
Merged
Conversation
The TSX an export returns is devup-ui code. On a machine that has devup-mcp and nothing else, the receiving agent has never seen devup-ui: it does not know the components are compile-time placeholders, that $token refers to devup.json, or that a style prop takes a responsive array. It guesses, and this server can neither see nor correct the guesses. Returning the rules in a response does not fix that - a document handed over once is read once. Every agent runtime already has a loader that reads SKILL.md from a directory and surfaces it on its own triggers, for the rest of that session and every session after it. So devup_skills reports install state, which is a concrete gap, and writes documents rather than prose. Two origins, handled differently on purpose. devup-ui, vespera and vespertide are ours, so their canonical SKILL.md is vendored and installs with no network - a bare machine is exactly where a download is least likely to work. vercel-react-best-practices and vercel-react-view-transitions are not vendored: vercel-labs/agent-skills publishes no LICENSE, so its content is not ours to redistribute, and they are multi-file anyway, about 350 KB. For those the publisher's command is handed over and never executed; a design-to-code server that shells out to a package installer turns one compromised registry entry into arbitrary execution on every machine that ever exported a screen. The provenance comment goes after the YAML frontmatter, not before it. A SKILL.md opens with --- at byte zero, and a comment in front of it leaves a file that is listed as installed and silently never loads - worse than no install, because nothing shows up to say so. Inducement stays narrow: one line in instructions, inside the existing 1,200-byte budget, and a skillGap on devup_ui_validate only when there are violations and the skill is genuinely absent.
related_nodes_reuse_one_explore_projection_without_changing_the_requested_anchor asserted ageSeconds == 0 on an artifact it had just created. That is whole seconds off the wall clock, so two calls straddling a second boundary report 1 while having done nothing different - which is how it failed on the ubuntu and windows runners and passed on the faster macos one. What the test is about is that the second call reused the first call's collection, and that is already asserted by reuseKind, avoidedFigmaToolCalls and figmaToolCalls == 0. Pre-existing; surfaced by an unrelated PR's CI run.
…nslation They are byte-exact copies of an upstream SKILL.md, embedded with include_str! and verified against a recorded SHA-256. A Windows checkout under the default core.autocrlf=true added one byte per line, so devup-ui.md arrived as 17,134 bytes against a recorded 16,629 and the integrity test failed on that platform while passing on Linux and macOS.
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.
What
Reported from a fresh machine: with only devup-mcp installed, the agent has no devup-ui skill and writes wrong code. It does — and handing it the rules in a response is not the fix.
The TSX
devup_figma_exportreturns is devup-ui code. An agent that has never seen devup-ui does not know its components are compile-time placeholders, that$tokenrefers todevup.json, or that a style prop takes a responsive array. It guesses, and this server can neither see nor correct the guesses.A document handed over once is read once. Every agent runtime already has a loader that reads
SKILL.mdfrom a directory and surfaces it on its own triggers — for the rest of that session and every session after. Sodevup_skillsreports install state, which is a concrete gap, and writes documents rather than prose.Two origins, handled differently
devup-ui,vespera,vespertideinstallwrites them with no networkvercel-react-best-practices,vercel-react-view-transitionsTwo reasons the vercel skills are not bundled:
vercel-labs/agent-skillspublishes no LICENSE file — root has none andGET /repos/.../license404s. All rights reserved; their content is not ours to redistribute.react-best-practicesisSKILL.md+ a 108 KBAGENTS.md+ ~70 rule files;react-view-transitionsisSKILL.md+ 59 KBAGENTS.md+ references. ~350 KB total — a thing to install, not a thing to hand over.devup-mcp never runs the install command. A design-to-code server that shells out to a package installer turns one compromised registry entry into arbitrary execution on every machine that ever exported a screen.
Where it installs
Project-local, preferring an existing
.claude/skills,.opencode/skillor.agents/skills. The project root is already the only place devup-mcp may write, so this needs no new permission, and the skills travel with the repository. Writes go through the sameOutputPolicyand oneOutputTransactionas every other file this server produces.The bug that would have made it pointless
The provenance comment originally went at the top of the installed file. A
SKILL.mdopens with---at byte zero and every loader reads that delimiter there — a comment in front of it leaves a file that is listed as installed and silently never loads, which is worse than not installing, because nothing shows up to say so. Caught by driving the release binary and reading the file back. The note now goes after the frontmatter block, andan_installed_document_still_opens_with_its_frontmatterholds it there.Inducement, kept narrow
instructions(rule 2b), inside the existing 1,200-byte budget — measured at 1,100 bytes.skillGapondevup_ui_validate, only when there are violations and devup-ui is genuinely absent. Telling someone who already has it to install it is the noise that teaches them to skip the field.Deliberately not added to the export response: rule 2b already fires at that moment, and a third hook would need a helper threaded through
complete_operationfor nothing.Staleness, stated rather than hidden
A vendored copy goes stale. The manifest records each embedded commit, SHA-256 and the current revision's URL; both the response and the installed file carry them.
node scripts/refresh-skills.mjsre-vendors,--checkreports drift without writing. An integrity test fails if a vendored document is edited without updating its record — the fix belongs upstream, not in the copy.Evidence
Release binary, real stdio, bare workspace,
DEVUP_FIGMA_BRIDGE_PORT=off, no Figma credential:Tests
skills_install.rs(6): the bare-workspace cycle end to end; external skills handed over as a command and never written; an existing root is the one used; resource read carries provenance; installing twice is not a second copy; an unknown name is refused with the known set.skills.rsunit (7): manifest/binary integrity per origin, pinned URL names the embedded commit, install state follows the disk across every root, frontmatter survives.Updated for the new tool and static resources:
stdio_tools,stdio_smoke,stdio_schema_compat_smoke,resource_delivery— the resource assertions now express the actual contract (generated outputs first and positional, statics behind) rather than a count that breaks whenever a skill is added.Gates
cargo fmt --all -- --checkcargo clippy --locked --workspace --all-targets --all-features -- -D warningscargo test --workspace --releasenode scripts/refresh-skills.mjs --check