[common-skills migration] Add warp-skills install/remove wrappers + downstream lock refresh - #37
Draft
vkodithala wants to merge 6 commits into
Draft
[common-skills migration] Add warp-skills install/remove wrappers + downstream lock refresh#37vkodithala wants to merge 6 commits into
vkodithala wants to merge 6 commits into
Conversation
Add an update-warp-skills-lockfile job mirroring the existing common-skills lock job. It regenerates warp-skills-lock.json in the downstream consumers (warp, warp-server) from warpdotdev/warp-skills using the pinned skills CLI, opens a downstream PR when the lock changes, requests the source author as reviewer, and enables squash auto-merge. The refresh is best-effort: it skips repos that have no warp-skills-lock.json yet and skips on CLI failure without failing the workflow. The existing common-skills flow is left unchanged. Co-Authored-By: Oz <oz-agent@warp.dev>
vkodithala
marked this pull request as ready for review
June 30, 2026 22:17
Contributor
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
vkodithala
changed the title
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Overview
This PR adds a second downstream GitHub Actions job that refreshes warp-skills-lock.json for warpdotdev/warp and warpdotdev/warp-server using the pinned skills CLI, while leaving the existing common-skills lock refresh flow unchanged.
Concerns
- No blocking concerns found in the annotated diff.
- Supplemental security review found no security issues requiring comments.
- No approved or repository spec context was provided, so spec-drift review was not applicable.
Verdict
Found: 0 critical, 0 important, 0 suggestions
Approve
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
- Generalize scripts/update_common_skills_lock with --source/--lock-file (defaults preserved: warpdotdev/common-skills + skills-lock.json) so the warp-skills lock refresh is a one-liner instead of inlined logic. - Collapse the two jobs into a single matrixed job (2 repos x 2 lock variants) sharing resolve-source-PR, app token, checkout, create-PR, request-reviewer, and auto-merge steps. - Rename Actions var/secret COMMON_SKILLS_SYNC_APP_ID/_PRIVATE_KEY to SKILLS_SYNC_APP_ID/_PRIVATE_KEY (now syncs both locks). Co-Authored-By: Oz <oz-agent@warp.dev>
Parametrize install_common_skills and remove_common_skills with --source, --lock-file, and --label so one code path can drive any skill set, plus a --best-effort mode (friendly skip on missing lock / no access, no credential prompt, and bypass of the project-vs-global dedupe scoped to a single set). Add install_warp_skills and remove_warp_skills thin wrappers targeting warpdotdev/warp-skills (warp-skills-lock.json). Defaults keep the existing common-skills behavior unchanged. Co-Authored-By: Oz <oz-agent@warp.dev>
vkodithala
changed the title
vkodithala
commented
Jul 1, 2026
vkodithala
left a comment
vkodithala
left a comment
Contributor
Author
There was a problem hiding this comment.
Took a glance; few small comments that we should probably address.
- Remove the change-specific 'Renamed from ...' note from the README app-id section (recoverable via git blame). - Reword the best-effort exclusivity-bypass docs/comment to explain it as a within-set guard and note that warp-skills should avoid reusing common-skill names. Co-Authored-By: Oz <oz-agent@warp.dev>
Revert the app-id variable and private-key secret back to COMMON_SKILLS_SYNC_APP_ID / COMMON_SKILLS_SYNC_APP_PRIVATE_KEY so the existing repo secret and variable keep working without a rename (GitHub secrets are write-only and can't be renamed from the UI). Co-Authored-By: Oz <oz-agent@warp.dev>
- Remove the best-effort exclusivity bypass so the project-vs-global check runs for every set, surfacing any warp-skills/common-skills name collision instead of silently coexisting. - Add explanatory comments to the downstream workflow (missing-lock skip, lock diff check) and to install_common_skills (best-effort install failure, missing-lock adoption behavior). - Clarify the --best-effort usage text. - install_warp_skills: forward args as-is now that --skip-warp-skills exits early. Co-Authored-By: Oz <oz-agent@warp.dev>
vkodithala
marked this pull request as draft
July 31, 2026 15:11
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
2 participants
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
Extends the shared common-skills tooling so that the internal warp-skills repo is installed and kept in sync the same way common skills are.
This is part of a series of changes intended to make common-skills plug-and-play with any repo (which should be particularly helpful for enterprises we're working with, like Narwhal) and present a public-facing library of skills curated by Warp. The end-goal here is to make our common skills library akin to something like this, rather than a set of skills specific to our client/server. Here's some more context on the motivation behind this change: https://warpdev.slack.com/archives/C08KTPNQN65/p1781557027648009.
This PR lets
warpandwarp-serverdrop their vendoredinstall_warp_skillscopies and call a shared wrapper through the existingresolve_common_skillsresolver. Related client and server PRs:Changes
Installer
install_common_skillsgains--source,--lock-file,--label, and--best-effort, so one code path can drive any skill set. Defaults reproduce the current common-skills behavior exactly.skills-lock.json) beforeexperimental_install, then skill dirs are copied into the target — the same approach the global path already uses.--best-effortturns a missing lock or no repo access into a one-line notice + exit 0, setsGIT_TERMINAL_PROMPT=0so an inaccessible source fails fast without a credential prompt, and skips the interactive upstream-update check. It does not relax the naming guard below..agents/skillsand~/.agents/skillsfails the install rather than silently landing in two places. Since warp-skills and common skills use distinct names, this also surfaces any accidental naming collision between the two sets.install_warp_skills/remove_warp_skillsare thin wrappers that call the parametrized scripts with the warp-skills source/lock/label and--best-effort; they honor--skip-warp-skills/WARP_SKIP_WARP_SKILLS_INSTALL=1.common-skills-lock.hash,warp-skills-lock.hash) so the two sets don't collide.Downstream lock automation
update_common_skills_lockgains--source/--lock-file, and a single parametrized matrix job inupdate-downstream-skill-locks.ymlrefreshes bothskills-lock.jsonandwarp-skills-lock.jsoninwarp/warp-server, opening a lockfile-only PR per changed lock. A consumer that doesn't yet carry a given lock is skipped rather than failing the job. The workflow keeps using the existingCOMMON_SKILLS_SYNC_APP_IDvariable andCOMMON_SKILLS_SYNC_APP_PRIVATE_KEYsecret — no repo-settings changes required.Testing
Exercised the shared scripts via the resolver chain (
WARP_COMMON_SKILLS_SCRIPTS_DIR→ this branch'sscripts/) and the remote fetch path (WARP_COMMON_SKILLS_REF):install_common_skillswith defaults, in an isolatedHOME, installs all 20 common skills, verifies (… match skills-lock.json), and writescommon-skills-lock.hash. Project/global behavior is unchanged.… match warp-skills-lock.json), writeswarp-skills-lock.hash, and is idempotent under--if-needed.--skip-warp-skills,WARP_SKIP_WARP_SKILLS_INSTALL=1, and a missing lock all exit 0 with a clean notice; an inaccessible source fails fast (no credential prompt) and skips.remove_warp_skillsremoves the locked skills and the stamp.Note: the downstream
warp-skills-lock.jsoncurrently pinsref: main, but the warp-skills content is not onmainyet, so a real install no-ops there until the lock is regenerated against the finalized branch (see the client/server PRs).Follow-up
main, regeneratewarp-skills-lock.json(update_common_skills_lock --source warpdotdev/warp-skills --lock-file warp-skills-lock.json); the wrapper is lock-driven and needs no change.Plan: https://staging.warp.dev/drive/notebook/R4ucMiSKpclPGJXzpoFu4t
This PR was generated with _Oz._