feat(install): record commitSha and commitUrl in .openskills.json#90
Open
kinueng wants to merge 6 commits into
Open
feat(install): record commitSha and commitUrl in .openskills.json#90kinueng wants to merge 6 commits into
kinueng wants to merge 6 commits into
Conversation
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Splits install testing into its own integration file separate from the
shared e2e suite, so the install-related cases are easy to find and
extend as install gains features.
tests/integration/install.test.ts (new):
- Three local-path install cases moved here verbatim from
e2e.test.ts (absolute path, directory of skills, non-existent path
error) so all install behavior lives in one place.
- One end-to-end git install test: installs
anthropics/skills/skills/pdf against real github.com and asserts
.openskills.json records a 40-char hex commitSha plus a commitUrl
matching https://github.com/<owner>/<repo>/commit/<commitSha>.
This is the first test in the suite that actually exercises a git
clone end to end, and the only one that validates the new commit-
recording behavior under real conditions (real network, real
github URL format).
- Five pure-function tests for buildCommitUrl covering the
normalizations the helper has to handle (plain HTTPS, HTTPS with
.git suffix, SSH form, git:// scheme, unrecognized host returns
undefined).
tests/integration/e2e.test.ts:
- Removes the "openskills install (local paths)" describe block that
was just moved out. The remaining suite keeps covering list, read,
sync, and remove, which are not install-specific.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Swap the placeholder gitlab.com path for the actual `gitlab-org/ai/skills` repo, and rename the test to explicitly document that the assertion exists because gitlab support hasn't shipped yet. Two reasons: - The test stays meaningful when someone adds gitlab support later — they see a real repo URL they can verify against, plus a test name that maps directly to the change they're making. - Made-up placeholders (foo/bar, example.com) read as "fake test fixture" and obscure the design intent that this guard is the host filter doing its job for an unsupported real-world git host. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The plan was written before the implementation iterated through several design decisions; this updates it to match the shipped code instead of the original intent. Net effect: a future reader of the plan sees the real architecture, not a sketch of what was first imagined. Substantive updates: - JSON example uses real values from a smoke install — `source` and `subpath` reflect the actual nested layout of anthropics/skills (the earlier `pdf-editor` example didn't exist). - Documented the non-github behavior explicitly: commitSha still recorded, commitUrl omitted, yellow warning printed naming the file and field that wasn't populated. - Replaced the SHA-capture code snippet with the exact form used in install.ts (encoding + 'pipe' shorthand, no toString() roundtrip). - Rewrote "Building the commit URL" to reflect what was actually built: github-only registry today; COMMIT_URL_BUILDERS extension point with a code stub; URL class for parsing with the security wins it provides (userinfo, traversal, IDN, query/fragment); case-insensitive SSH prefix. - Split error handling into the two real failure modes — rev-parse aborts the install loudly, missing commitUrl just warns and proceeds. - Test descriptions match the actual file (one e2e github install, the normalization unit tests including case-insensitive, gitlab.com as the documented unsupported-host example). Removed the never-implemented HTTP 200 fetch assertion. - Smoke commands use the real install path; placeholder URLs use clear substitution markers instead of fake domains. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When `openskills install` clones a git-sourced skill, the resulting
metadata file now also records the commit SHA that was cloned and a
browser-clickable URL pointing at that commit on the upstream host.
A teammate cat'ing the file can answer "what version of this skill
is installed?" with one click; bug reports to upstream skill authors
become "we're on commit <sha> and seeing X" instead of vague.
src/utils/skill-metadata.ts:
- SkillSourceMetadata gains two optional fields, `commitSha` and
`commitUrl`. Optional so legacy files written by older versions
of openskills stay valid.
- New exported `buildCommitUrl(repoUrl, commitSha)` returns a
browser-clickable `https://` URL for github clone URLs, or
undefined for any other host.
- Per-host commit-URL builders live in a `COMMIT_URL_BUILDERS`
Record keyed by `URL.hostname`. github.com is the only entry
today; adding another host (e.g. gitlab) is a one-line addition
to that map with no other code touching.
- Internal `parseCloneUrl` uses the standard `URL` class for
parsing, which handles userinfo spoofing, path traversal, IDN
homoglyphs, and embedded query/fragment safely. A small
`sshToHttps` helper rewrites the SSH form (`git@host:owner/repo`)
to HTTPS first since SSH isn't a valid URI. SSH prefix detection
is case-insensitive.
- Trailing `.git` is stripped from the parsed pathname so the
derived commit URL is clean.
src/commands/install.ts:
- InstallSourceInfo gains `commitSha` and `commitUrl` so both
metadata-write sites (installSpecificSkill and installFromRepo)
pick them up through buildGitMetadata. The SHA is captured once,
right after the clone succeeds, via `git rev-parse HEAD` on the
shallow clone.
- When buildCommitUrl returns undefined (non-github host), the
install still succeeds — commitSha is recorded, commitUrl is
omitted from the JSON, and a yellow warning is printed naming
the file and field that wasn't written. No placeholder string is
written into the JSON: we deliberately avoid echoing the raw
repoUrl into the file to avoid auto-linkifier abuse risk if a
crafted URL is in play.
Behavior summary for the user:
- github install: .openskills.json gets commitSha + commitUrl,
no extra output.
- non-github install: .openskills.json gets commitSha only;
yellow warning printed; install still succeeds.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
What changes
openskills installnow records the upstream commit you cloned in.openskills.json:{ "source": "anthropics/skills/skills/pdf", ..., "commitSha": "f458cee31a7577a47ba0c9a101976fa599385174", "commitUrl": "https://github.com/anthropics/skills/commit/f458cee31a7577a47ba0c9a101976fa599385174" }commitUrlis a browser-clickable link —cat .openskills.json, click, and you're looking at the exact code that's installed.Motivation
f458ceeand seeing X" — actionable for upstream maintainers.git pullthat brings in a skill update now shows both the changedSKILL.mdand the new SHA in.openskills.json— the SHA bump explains the SKILL.md change.Behavior
commitShacommitUrlMigration
Existing
.openskills.jsonfiles keep working — both fields are optional. Old installs gain the fields when you next re-install those skills.