docs(skills): document the real Vercel skill copy/review/PR workflow (#45) - #100
Merged
Merged
Conversation
…45) Adds the review workflow #44 left undocumented: discover a source read-only (demonstrated against a real repo, pstack, without selecting anything for the owner), copy an explicit selection with `sumctl skills install`, review the copy as untrusted third-party content, commit, and open an ordinary PR. Documents re-running the same explicit copy command to check for updates (idempotent when unchanged, a clean additive diff when not), backed by a real run against vercel-labs/agent-skills in a throwaway Git project. Also documents a real finding: the pinned CLI's own `update` subcommand relocates an `--agent`-scoped copy into a different agent directory instead of refreshing it in place, silently advancing skills-lock.json regardless -- so the workflow explicitly avoids it in favor of re-running the explicit copy. Docs only; no runtime behavior changes.
The map auditor flags backticked tokens that look like repository paths. The new skills.review-workflow row wrapped a GitHub slug in backticks and failed CI. Write the slug as plain text and add the scenario to the frozen manual inventory.
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.
Summary
Closes the remaining scope of #45 (roadmap #29 §2): documents the real workflow for reviewing third-party skill copies made through
sumctl skills install(delivered by #44), since #45's own acceptance explicitly called out that "the remaining workflow documentation and scoped proof still need delivery." Docs only — no runtime behavior changes; #44's wrapper and #65's activation/rollback mechanism are unchanged.Ran the real pinned Vercel Skills CLI (1.5.25, not a test fixture) end-to-end in an isolated dev checkout against a throwaway Git project, and used the results as documentation evidence:
.local/bin/skills add michael-denyer/pstack-claude --listlists that repository's real skills (architect,tdd,why, dozens more) without copying anything. Chosen deliberately per the issue's own guidance to "include pstack examples without selecting skills for the owner" — pstack illustrates discovery; nothing from it is installed.sumctl skills install --target ... --source vercel-labs/agent-skills --skill writing-guidelines --agent claude-codeagainst a throwaway Git project wrote exactly two project-local regular files (.claude/skills/writing-guidelines/SKILL.md,skills-lock.json) and nothing global.git status/git diff.vercel-composition-patterns) to the same project produced a clean additive diff — only the new skill's files plus a newskills-lock.jsonentry; the first skill's files were untouched.update -p -ydirectly (not throughsumctl, which doesn't wrapupdate) against the two-skill project relocated both copies out of.claude/skills/into a different.agents/skills/layout — silently dropping the originally requested--agent claude-codetarget — whileskills-lock.json's recorded hashes advanced regardless, with no warning in the CLI's own output. This confirms and sharpens the caution already in Review selected Vercel skill copies through ordinary pull requests #45's issue body ("the pinned CLI fixture showed its lock advancing while a Claude Code copy stayed stale"). Documented as: don't useupdate; re-run the explicitadd/sumctl skills installcommand instead, which is idempotent for an unchanged selection and reproduces the correct layout for a changed one.Changes
docs/sum-skills.md: new "Review workflow" section (discover → copy → review-as-untrusted-content → commit/PR → re-check) and a new "Do not use the pinned CLI's ownupdate" section with the concrete relocation finding above.docs/features/skills.md: newskills.review-workflowevidence row pointing at the manual steps indocs/ACCEPTANCE.md§11.docs/ACCEPTANCE.md§11: four new concrete manual steps (discovery-without-selection, real copy + diff + PR, unchanged/changed re-review, theupdategotcha) alongside the existing local-fixture acceptance steps from Install selected Git skills by repository, ref, and exact path with pinned resources #44.Scope boundaries respected
sumctl skills check --root(Sum's own namespace/projection validation) is unchanged and explicitly distinguished in the new docs from third-party content review..sum/dev/issue45-vercel-skills-review) against a throwaway scratch Git project outside the sum repository itself — never the live installation or a busy worker's discovery directory.Test plan
tests/test_skills_cli.py's fake binary) — see Summary above; each claim in the new docs was actually run and observed, not inferred.MISE_ENABLE_TOOLS=go,python,node python3 .agents/skills/verify/scripts/verify_run.py --base 81dbdda96dd7c0b31696634f9f943b28981f511b) was still running in the background when this PR was opened. Will update this line with the result once it completes.Review status
Independent review was not performed beyond the automated verification above — no separate reviewer pane was launched for this developer-checkout PR. Boss decides merge.