feat(skills): prefer current upstream documents with embedded fallback - #64
Merged
Merged
Conversation
A binary's skill copy ages independently of the repository it describes. Installing that copy without saying where it came from leaves the agent applying old rules with no reason to question them. Fetch current documents on an explicit MCP install, but keep offline delivery a first-class path: the bare machine this feature exists for is also where a download is least likely to work. Reuse the CLI's ETag-aware upstream through a URL-taking trait. MCP URLs come from each manifest repository and document path; the CLI still fetches devup-ui and writes its existing HOME targets. A four-second budget covers all fetching in one MCP install, and every write still goes through the server's OutputPolicy and one OutputTransaction. If any reference fails, use the whole embedded skill rather than mixing documents from two copies. Report fetched or embedded for every installed skill, with a fallback reason. A 404 additionally warns that the manifest may point at a moved file. Fetched documents have no pinned commit, so their entry note reuses the CLI provenance fields: URL, ETag, fetched time and SHA-256 of upstream bytes. Keep that note after frontmatter so the skill loader still sees it. Own skills stay in the binary; external skills are never fetched or written. Read DEVUP_MCP_SKILLS_OFFLINE once and leave HTTP initialization on the install path, unreachable from self-check, status or resource reads. Run MCP integration tests in child processes with offline configuration set before startup. Injected upstreams cover fresh documents, every fetch failure, 404 warnings, nested references, the shared deadline, and rollback without opening a socket. Preserve all four CLI behavior tests.
owjs3901
added a commit
that referenced
this pull request
Sep 15, 2026
… file (#63) * feat(skills): carry devfive-frontend and let a skill be more than one file devup_figma_export returns one screen's TSX, but a project is a tree of routes, and knowing devup-ui does not tell an agent which file that TSX becomes. The gap is not hypothetical. A vinext project created from service-template was implemented as a Vite SPA - a main.tsx swapping screens on local state, with authored CSS files beside code that has globalCss() - by an agent that had been given the devup-ui skill and nothing else. devup-ui is a styling skill and correctly says nothing about project structure, so the rules that would have caught it live in devfive-frontend, which no bare machine had. devfive-frontend is five files, so carrying it meant a skill could no longer be one document. A third origin carries it. An `embedded` skill is copied from another DevFive repository and pins the commit it copied, so refresh-skills.mjs can move it forward. An `own` skill is authored here, has no second copy to drift from, and pins no commit - printing "at unknown" would read as a lost revision rather than one that never existed. Making that an origin rather than a special case keeps both on the same install path. The manifest now records documents[{path,bytes,sha256}] per skill instead of one digest per skill, and every origin uses skills/<name>/<relpath>, so a document's source path and its install path are the same string and cannot disagree. install() stages a skill's documents in one OutputTransaction: a SKILL.md that survived while its four references did not is the failure this exists to avoid, because it looks installed and its links go nowhere. Only the entry document is annotated, so the manifest digest stays true of every reference that lands on disk. Two things surfaced while building it. .gitattributes matched skills/*.md, and a git pattern containing a slash does not let * cross one. Moving the documents into per-skill directories silently dropped the -text attribute that the file's own comment says exists to stop a Windows checkout from failing the integrity check on that platform alone. It now matches skills/**/*.md, confirmed with git check-attr. refresh-skills.mjs skipped own skills entirely, which left no way to update a digest after editing a document this repository authors - the trap that makes someone write a SHA-256 by hand. For an own skill the direction reverses: the file on disk is the truth and the script reseals the manifest from it. The integration tests spelled out how many skills exist, so adding one failed four tests that had nothing to say about it. They derive the counts now. devfive-frontend also gained the two rules the SPA incident needed - that vite.config.ts does not mean Vite, because vinext runs Next App Router on Vite, and that no .css or .scss belongs in application source - plus the extraction rule from devup-ui issue 663, checked against the extractor itself: an inline object literal indexed at a style prop extracts to static classes, an external object referenced by name becomes a CSS variable, and an external object of css() results is neither, because css() already extracted at its own call site and className is never a style-extraction source. * style: apply cargo fmt * feat(skills): prefer current upstream documents with embedded fallback (#64) A binary's skill copy ages independently of the repository it describes. Installing that copy without saying where it came from leaves the agent applying old rules with no reason to question them. Fetch current documents on an explicit MCP install, but keep offline delivery a first-class path: the bare machine this feature exists for is also where a download is least likely to work. Reuse the CLI's ETag-aware upstream through a URL-taking trait. MCP URLs come from each manifest repository and document path; the CLI still fetches devup-ui and writes its existing HOME targets. A four-second budget covers all fetching in one MCP install, and every write still goes through the server's OutputPolicy and one OutputTransaction. If any reference fails, use the whole embedded skill rather than mixing documents from two copies. Report fetched or embedded for every installed skill, with a fallback reason. A 404 additionally warns that the manifest may point at a moved file. Fetched documents have no pinned commit, so their entry note reuses the CLI provenance fields: URL, ETag, fetched time and SHA-256 of upstream bytes. Keep that note after frontmatter so the skill loader still sees it. Own skills stay in the binary; external skills are never fetched or written. Read DEVUP_MCP_SKILLS_OFFLINE once and leave HTTP initialization on the install path, unreachable from self-check, status or resource reads. Run MCP integration tests in child processes with offline configuration set before startup. Injected upstreams cover fresh documents, every fetch failure, 404 warnings, nested references, the shared deadline, and rollback without opening a socket. Preserve all four CLI behavior tests.
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.
devup_skills installpreviously always wrote the copy compiled into the binary, even when the upstream skill had changed. It now prefers current documents from manifest-derived raw GitHub URLs and falls back to the complete embedded skill when any document cannot be fetched. Every installed skill reportssource: fetched | embeddedand a reason for using the binary; a 404 also warns that the manifest path may have moved.This PR stacks on #63 and targets
feat/multi-document-skills.The existing CLI ETag-aware fetcher now accepts a URL. MCP fetching has one four-second budget for the entire call, while writes retain the server's project-local OutputPolicy and atomic transaction. Fetched entry documents record URL, ETag, fetch time and SHA-256 instead of claiming the vendored commit, with the note placed after YAML frontmatter. Own skills remain binary-only; external skills are never fetched or written. The CLI's devup-ui/HOME install contract is unchanged.
DEVUP_MCP_SKILLS_OFFLINE=1disables fetching and is read once. Integration tests set it in child servers before startup; injected upstreams test fetched documents, all four error classes, 404 warnings, nested references, the shared deadline and rollback without network access. Self-check, status and resource reads cannot enter the fetch path. Includes the required changepack and README/tool-output updates.Local Windows verification, actual final exit codes:
cargo metadata --locked --format-version 1cargo fmt --all -- --checknode --test crates/devup-mcp-figma/tests/explore_script_behavior.mjscargo test --locked -p devup-mcp --test stdio_smokecargo clippy --locked --workspace --all-targets --all-features -- -D warningscargo test -p devup-mcp --all-featurescargo insta test --workspace --all-features --checkcargo build --locked --workspace --releasepython .github/scripts/test_release_lock.pyThe all-features devup-mcp run passed 579 tests; all four existing CLI tests remain meaningful and pass. The workspace snapshot check reported no snapshots to review. Independent code review found no critical or important issues.
Initial broad verification attempts exhausted local disk space (OS error 112). After cleaning only this run's Cargo artifacts, Clippy and the devup-mcp tests passed with
CARGO_PROFILE_DEV_DEBUG=0,CARGO_PROFILE_TEST_DEBUG=0, andCARGO_INCREMENTAL=0. The final workspace snapshot check and release build additionally usedRUSTFLAGS=-C link-arg=/DEBUG:NONEto avoid Windows linker symbol files. No tracked build configuration was changed. Linux-only CI steps that install packages were not run locally.