feat(plugins): version pinning (@ref + --pin) with sources in sync-state

[christso]

Summary

Reproducible version pinning end-to-end:

• CLI: allagents skills add --from owner/repo@<ref> and --pin <ref> are accepted (mutually exclusive — supplying both errors).
• Parsing: parseGitHubUrl / isGitHubUrl understand the owner/repo@<ref> shorthand. isPluginSpec rejects it so marketplace resolution stays correct.
• Workspace: PluginEntrySchema gains an optional pin: string; allagents update splices it into GitHub-shaped sources so existing fetch paths honour the pin.
• Sync state: a new optional sources block keys per owner/repo to { pluginSpec, resolvedRef, resolvedSha, pinnedRef? }. Field names are a strict subset of the gh-skill lockfile shape.

Test plan

• bun run build
• bun test (1195 pass / 0 fail) — includes new unit coverage for the @ref parsing branches.
• End-to-end verification from feat: version pinning for skills and plugins (@version suffix and --pin flag) #372 (substituting the stable public obra/superpowers@v3.1.0 for the example repo, per the issue's note):
  • allagents skills add brainstorming --from obra/superpowers@v3.1.0 → installs at v3.1.0.
  • allagents skills add brainstorming --from obra/superpowers --pin v3.1.0 → equivalent.
  • allagents skills add brainstorming --from obra/superpowers@v3.1.0 --pin v3.1.0 → exits non-zero with Cannot combine inline @version in --from with --pin.
  • jq '.sources["obra/superpowers"].pinnedRef' returns "v3.1.0"; resolvedSha is a 40-char commit SHA.
  • workspace.yaml with pin: v3.1.0 round-trips through allagents update and writes the same sources block.

Closes #372

[cloudflare-workers-and-pages]

Deploying allagents with    Cloudflare Pages

Latest commit:	e9de04a
Status:	✅  Deploy successful!
Preview URL:	https://bbd8b2a1.allagents.pages.dev
Branch Preview URL:	https://feat-372-version-pinning.allagents.pages.dev

View logs

[christso]

Code Review: ⚠️ APPROVE WITH SIGNIFICANT NON-BLOCKING CONCERNS

This is a substantial, well-thought-out feature. Schema design (sources block as a strict subset of gh-skill lockfile shape) is good forward-thinking. The verification gate from #372 is satisfied. Approving on functional merit; several real issues below for follow-up.

Verification gate (#372)

• (1) skills add --from owner/repo@ref ✓
• (2) skills add --from owner/repo --pin ref ✓
• (3) Inline + --pin rejected ✓
• (4) pinnedRef + 40-char resolvedSha in sync-state ✓
• (5) workspace.yaml pin: round-trips through update ✓

CLAUDE.md compliance

• Conventional commit ✓
• No Co-Authored-By ✓
• Test coverage: significantly short of one-per-distinct-path — see below.

---

Non-blocking issues

1. Test coverage is thin for the surface area added (CLAUDE.md: "one test per distinct code path")

The PR claims "new unit coverage for the @ref parsing branches" — and indeed parseGitHubUrl / isGitHubUrl got three new tests. But these are the smallest pieces of the feature. The following new code paths have zero unit tests:

• stripGitRef (src/utils/plugin-path.ts) — non-trivial: handles @ref/subpath, marketplace shorthand, empty cases.
• extractInlineRef (src/cli/commands/plugin-skills.ts) — duplicates logic from parseGitHubUrl; see fix: marketplace plugin architecture #4 below.
• recordSourceProvenance (src/cli/commands/plugin-skills.ts)
• buildSourcesProvenance (src/core/sync.ts)
• upsertSyncStateSource (src/core/sync-state.ts) — non-atomic read-modify-write, worth pinning.
• getPluginPin (src/models/workspace-config.ts) — string vs object entry handling.
• The mutex error path: --pin + inline @ref → "Cannot combine ..." (gate check 3 is essentially this; a unit test would lock the exact message).
• isPluginSpec's new branch — the beforeAt.includes('/') change is a behavior-affecting fork worth a test (e.g., owner/repo@v1.0 → false, plugin@market → true, plugin@owner/repo → true).
• getPluginName's @ref stripping (see refactor: move workspace.yaml to .allagents directory #5).

2. plugin install --pin is missing — the issue's title says "skills and plugins" and the implementation notes explicitly call for src/cli/commands/plugin.ts::installCmd to get the same --pin flag. The verification gate doesn't require it (all five checks use skills add), but the issue's scope does. Recommend either a follow-up PR or a noted out-of-scope in the merge commit.

3. Re-call of fetchPlugin for provenance relies on the fetch cache being warm.

In recordSourceProvenance and buildSourcesProvenance, the resolved SHA is read by calling fetchPlugin a second time after install/sync and trusting the in-process dedup cache to return the cached result. Comment acknowledges this: "The fetch ran during install so this hits the cache and returns the resolvedSha without another git operation." But:

• The cache is process-local; if resetFetchCache() ran between the install and the provenance call, the second fetchPlugin would actually re-clone.
• It's an invisible coupling — a future refactor that moves the cache reset could silently turn this into a duplicated network call.

Cleaner: thread the FetchResult (which already carries resolvedSha / resolvedRef) through the install path and write provenance directly from it, no second call.

4. Duplicated parsing & provenance logic

• extractInlineRef(spec) in plugin-skills.ts is functionally parseGitHubUrl(spec).branch. Two functions doing the same parse with slightly different rules invites drift.
• recordSourceProvenance (in addCmd path) and buildSourcesProvenance (in syncWorkspace path) both produce the same SyncStateSource shape from the same fetch results. Extracting one helper (buildSourceProvenanceFromFetch(parsed, fetchResult, pinnedRef)) and calling it from both sites would remove the duplication.

5. getPluginName semantic change is silent and untested.

The function previously returned basename(pluginPath) verbatim. It now strips @-suffixes. This is a behavior change for any caller — not just the new pinning paths — because the cache layout already produced owner-repo@<sanitized-branch> directories for /tree/branch URLs prior to this PR. The change is plausible (the PR's stated reason is workspace.yaml key alignment in setPluginSkillsMode), but:

• No test pins the new behavior.
• No grep audit in the PR description of every getPluginName(...) call site to confirm none of them wanted the literal basename.

Recommend adding a small test and a note explaining the rationale in the function's doc comment is fine — the unanswered question is whether any caller is now silently mismatched.

6. Asymmetric pin handling between CLI and workspace.yaml.

CLI install:

skills add --from owner/repo@v1.0 --pin v2.0   →  errors (mutex)

workspace.yaml in buildPluginSyncPlans:

- source: owner/repo@v1.0
  pin: v2.0

…silently ignores pin (the !rawSource.includes('@') guard). The CLI errors on this conflict but update accepts and silently picks one. Recommend either erroring on both paths or warning in update.

---

Suggested merge order

Approves as-is. If addressing the items above, items 1, 3, 4, and 5 can be a single follow-up PR (refactor: tighten pin tests and dedupe provenance helpers). Item 2 (plugin install --pin) probably wants its own issue.