Add automated per-plugin versioning (NBGV) with /version-bump + weekly backstop#813
Open
AbhitejJohn wants to merge 6 commits into
Open
Add automated per-plugin versioning (NBGV) with /version-bump + weekly backstop#813AbhitejJohn wants to merge 6 commits into
AbhitejJohn wants to merge 6 commits into
Conversation
…y backstop WHY Tools that surface skills (Copilot CLI, Claude Code, Codex, Cursor) read a plugin's version directly from its checked-in manifest. With no versioning discipline, a plugin's behavior can change while its advertised version stays flat, so clients never learn to re-pull, and there is no human-readable signal of what changed. We want correct, current versions in the repo with minimal manual work and without bloating the marketplace clone. WHAT - Per-plugin semantic versioning via Nerdbank.GitVersioning (NBGV). Each plugin owns a version.json whose pathFilters exclude the generated manifests and the version.json itself, so version height tracks real content changes only. - The computed version is materialized into the checked-in manifests (plugin.json and .codex-plugin/plugin.json) so every consumer reads a current value with no build step on their side. - eng/version/Sync-PluginVersions.ps1 is the single workhorse. It resolves the set of changed plugins from a git diff, computes each version with nbgv (predicting the squash-merge height for PRs), and either reports or stamps. AUTOMATIONS (two, low-touch by design) - /version-bump: an admin/maintainer comments the command on a PR and the affected plugins are stamped on the PR branch. Gated on collaborator permission (admin/write/maintain); forks are rejected before any privileged step. No other PRs are auto-modified. - weekly-version-sync: a Monday backstop (and workflow_dispatch) that stamps any drift on main, opens/updates a single bot PR, and explains the per-plugin reason. This self-heals anything that merged without a bump. We deliberately did NOT auto-edit contributor PRs or add a noisy advisory comment bot; maintainers stay in control and the signal stays clean. SECURITY (multi-model adversarial review: GPT-5.5 + Gemini 3.1 Pro) - Supply chain (High, both models): dotnet tool restore would have honored a nuget.config authored in the PR tree, letting an attacker remap the nbgv package source to a malicious feed and run code in the privileged contents:write context. Mitigated with a trusted eng/version/nuget.config (clear + nuget.org-only + packageSourceMapping), overlaid from main and used via --configfile so PR-supplied configs are ignored. No nuget.config is tracked in the repo today, so this path was genuinely exploitable. - TOCTOU (Medium): /version-bump now checks out the authorized head SHA rather than the mutable branch name; a racing push fails non-fast-forward, which is the safe outcome. - Injection: Set-ManifestVersion uses a MatchEvaluator (not a replacement string) so a "$"-bearing version cannot re-expand, plus a strict major.minor.patch guard that throws on a malformed base, leaving manifests untouched. - A base-only version.json bump (0.1 -> 0.2) is correctly detected and stamped. VERIFIED End-to-end against a real NBGV git harness: content-scoped predict, base-only bump -> x.y.0, docs-only -> [], weekly drift stamping, malformed-base guard, and --configfile restore (exit 0). actionlint passes on both workflows. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Skill Coverage Report
Uncovered:
|
Collaborator
Author
|
Curious to hear thoughts on this versioning strategy. Goal was to keep this with as less ceremony as possible. At the moment Claude code users do not really get updates if they've already pulled down a plugin earlier because we haven't changed our versions. |
AbhitejJohn
requested review from
a team,
JanKrivanek,
ManishJayaswal,
Redth,
ViktorHofer,
YuliiaKovalova,
jfversluis and
webreidi
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces automated, per-plugin semantic versioning for the plugins/* tree using Nerdbank.GitVersioning (NBGV), and adds two GitHub Actions automations to materialize the computed versions into the checked-in plugin manifests (so downstream consumers read current versions directly from the repo).
Changes:
- Add per-plugin
plugins/<plugin>/version.jsonfiles to scope NBGV version height to each plugin subtree. - Add
eng/version/Sync-PluginVersions.ps1plus a locked-downeng/version/nuget.configand a local tool manifest fornbgv. - Add two workflows:
/version-bump(issue_comment-driven, same-repo PRs) and a scheduled weekly backstop PR that stamps any drift onmain.
Show a summary per file
| File | Description |
|---|---|
| plugins/dotnet11/version.json | Adds NBGV per-plugin version configuration for dotnet11. |
| plugins/dotnet/version.json | Adds NBGV per-plugin version configuration for dotnet. |
| plugins/dotnet-upgrade/version.json | Adds NBGV per-plugin version configuration for dotnet-upgrade. |
| plugins/dotnet-test/version.json | Adds NBGV per-plugin version configuration for dotnet-test. |
| plugins/dotnet-template-engine/version.json | Adds NBGV per-plugin version configuration for dotnet-template-engine. |
| plugins/dotnet-nuget/version.json | Adds NBGV per-plugin version configuration for dotnet-nuget. |
| plugins/dotnet-msbuild/version.json | Adds NBGV per-plugin version configuration for dotnet-msbuild. |
| plugins/dotnet-maui/version.json | Adds NBGV per-plugin version configuration for dotnet-maui. |
| plugins/dotnet-experimental/version.json | Adds NBGV per-plugin version configuration for dotnet-experimental. |
| plugins/dotnet-diag/version.json | Adds NBGV per-plugin version configuration for dotnet-diag. |
| plugins/dotnet-data/version.json | Adds NBGV per-plugin version configuration for dotnet-data. |
| plugins/dotnet-blazor/version.json | Adds NBGV per-plugin version configuration for dotnet-blazor. |
| plugins/dotnet-aspnetcore/version.json | Adds NBGV per-plugin version configuration for dotnet-aspnetcore. |
| plugins/dotnet-ai/version.json | Adds NBGV per-plugin version configuration for dotnet-ai. |
| eng/version/Sync-PluginVersions.ps1 | Core PowerShell implementation to compute/stamp versions and emit a JSON report. |
| eng/version/nuget.config | Locked-down NuGet config used by workflows to prevent PR-controlled feed remapping during tool restore. |
| .config/dotnet-tools.json | Adds local nbgv tool dependency for consistent version computation in CI. |
| .github/workflows/version-bump-command.yml | Implements the maintainer-triggered /version-bump workflow. |
| .github/workflows/weekly-version-sync.yml | Implements the scheduled backstop that opens/updates a single “weekly sync” PR on drift. |
| CONTRIBUTING.md | Documents the new per-plugin versioning model and contributor expectations. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 20/20 changed files
- Comments generated: 4
Contributor
|
👋 @AbhitejJohn — this PR has 4 unresolved review thread(s). When you're ready, please address the feedback and push an update; the triage bot will pick up the next state automatically. (Add the |
- Add missing plugins/dotnet-test-migration/version.json so it participates in versioning (it was the only plugin without one; manifests are at 0.1.0). - CONTRIBUTING: the two manifests are not byte-identical; say the version is duplicated across two manifest files instead. - weekly-version-sync: include version.json in commit attribution so a base-only bump is explained rather than showing 'no attributable commits'. - Get-NbgvInfo: capture nbgv stderr and include it in the thrown error so CI failures are diagnosable, while keeping stdout clean for JSON parsing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
github-actions
Bot
added
waiting-on-review
Contributor
|
✅ Evaluation passed for |
Evangelink
reviewed
Jun 25, 2026
Evangelink
left a comment
Evangelink
left a comment
Member
There was a problem hiding this comment.
Review: per-plugin NBGV versioning
Really like the overall design — per-plugin pathFilters, materializing into the checked-in manifests, the opt-in /version-bump + weekly backstop split, and the security hardening on the privileged workflow (SHA-pinned actions, fail-closed permission gate, fork rejection before any privileged step, --configfile source-mapped NuGet, trusted-tooling overlay from main, injection-safe MatchEvaluator). The security posture of version-bump-command.yml looks sound to me.
My concern is that the privileged workflow's git plumbing and the tool runtime pin look like they'll prevent /version-bump (and likely the weekly sync) from actually running. Because both workflows are issue_comment/schedule-triggered they run from main and get no CI signal on this PR, so these can only be caught by review. Two of the inline findings below were reproduced in a scratch repo.
This was a combined pass (two independent reviewers); we converged on the same set with no contradictions. Inline comments have details + suggested fixes. Summary:
Blocking / CI-breaking
git fetch --depth=1 origin mainmakes the whole repo shallow → NBGV refuses to compute height. Reproduced: a depth-limited fetch writes.git/shallowand flips the entire repo to shallow even though the head was checked out withfetch-depth: 0, so everynbgv get-versionin the ordinary-content path fails.git merge-base $BASE_SHA $headreturns empty when the PR is behindmain. Reproduced (PR 3 commits behind → empty merge-base, exit 1 → script throws-PredictSquashMerge requires -BaseCommit). Fixed by the same full-fetch change, plus an empty-merge-base guard.nbgv(net8) underrollForward: falseon a net11-preview-only runtime.setup-dotnetinstalls only theglobal.jsonruntime; a net8-targeted tool won't run, and evenrollForward: truewon't pick a preview runtime withoutDOTNET_ROLL_FORWARD_TO_PRERELEASE=1. Best fix: explicitly install an 8.0.x runtime in both workflows.
Correctness
- Squash prediction over-bumps on
version.json-only (non-base) edits, then the weekly backstop computes the true (lower) height and corrects downward — a non-monotonic version regression visible to consumers.
Minor
- Format guard validates the computed value but not the 2-part base shape in
version.json(a malformed 3-part base slips through the weekly path). - Concurrent
/version-bumpon the same plugin can stamp colliding patch numbers until the weekly sync reconciles — worth a doc note. - PR description says
version.json×14; there are 15 (dotnet-test-migrationwas added in commit 2).
github-actions
Bot
added
waiting-on-author
Contributor
|
👋 @AbhitejJohn — this PR has 5 unresolved review thread(s). When you're ready, please address the feedback and push an update; the triage bot will pick up the next state automatically. (Add the |
Address @Evangelink's review on the per-plugin versioning workflows. These issue_comment/schedule workflows run from main and get no CI signal on the PR, so each fix is validated with a local NBGV git harness instead. Workflow runtime/git plumbing (version-bump-command.yml, weekly-version-sync.yml): - Drop --depth=1 from the "pin trusted tooling from main" fetch. A shallow fetch writes .git/shallow and flips the whole repo shallow (proven locally), and NBGV refuses to compute height on a shallow repo. fetch-depth: 0 already fetched main, so the full fetch is cheap and keeps the supply-chain pin intact. - Add a guard that throws if `git merge-base BASE_SHA head` is empty, instead of letting nbgv fail with an opaque "-PredictSquashMerge requires -BaseCommit". - setup-dotnet: install dotnet-version 8.0.x alongside global-json-file. nbgv targets net8.0; global.json pins an SDK that ships only a net11 preview runtime, onto which a release-targeted tool will not roll forward. setup-dotnet installs both inputs, giving nbgv a runtime to run on. Prediction correctness (Sync-PluginVersions.ps1): - Only add +1 to the predicted height when a height-bearing file actually changed. A version.json-only non-base edit is height-neutral, so predicting +1 over-bumps and the weekly sync would later correct it downward — a visible version regression. New Test-HeightBearingChange checks the diff with the canonical exclusions. - Validate the version.json base is major.minor; a 3-part base (e.g. 0.1.0) was silently normalized by NBGV into a passing SimpleVersion on the weekly path. - Reject non-canonical pathFilters (raised in cross-model review). The predict height math assumes version.json and the two stamped manifests are excluded and that the filter set is stable between base and head; a hand-edited pathFilters would silently diverge the post-merge height from the prediction. The height-excluded file list is now a single source of truth shared by the guard and Test-HeightBearingChange. CONTRIBUTING.md: note that concurrently bumped PRs can predict the same patch and that the weekly sync reconciles the collision. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
Author
|
@Evangelink thanks for the thorough pass — all five findings are addressed in 22133dc, and since these
One extra hardening came out of a cross-model adversarial review of the over-bump fix: a non-canonical |
Clarify in CONTRIBUTING that the two generated manifests and version.json are excluded from NBGV height, so editing only manifest metadata won't change the patch number or be picked up by /version-bump or the weekly sync. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
👋 @AbhitejJohn — this PR has 7 unresolved review thread(s). When you're ready, please address the feedback and push an update; the triage bot will pick up the next state automatically. (Add the |
github-actions
Bot
added
waiting-on-review
Contributor
|
✅ Evaluation passed for |
Comment on lines
+2
to
+4
| "$schema": "https://raw.githubusercontent.com/dotnet/Nerdbank.GitVersioning/main/src/NerdBank.GitVersioning/version.schema.json", | ||
| "version": "0.1", | ||
| "pathFilters": [ |
Comment on lines
+2
to
+4
| "$schema": "https://raw.githubusercontent.com/dotnet/Nerdbank.GitVersioning/main/src/NerdBank.GitVersioning/version.schema.json", | ||
| "version": "0.1", | ||
| "pathFilters": [ |
Comment on lines
+66
to
+71
| if ($PredictSquashMerge -and -not $BaseCommit) { | ||
| throw '-PredictSquashMerge requires -BaseCommit (the PR merge base).' | ||
| } | ||
| if ($HeadCommit -and -not $BaseCommit) { | ||
| throw '-HeadCommit requires -BaseCommit (the diff is BaseCommit..HeadCommit).' | ||
| } |
Comment on lines
+73
to
+75
| $repoRoot = (Resolve-Path (Join-Path $PSScriptRoot '..' '..')).Path | ||
| $pluginsRoot = Join-Path $repoRoot 'plugins' | ||
|
|
Comment on lines
+179
to
+183
| $Plugins = Get-ChildItem -Path $pluginsRoot -Directory | | ||
| Where-Object { Test-Path (Join-Path $_.FullName 'version.json') } | | ||
| Select-Object -ExpandProperty Name | | ||
| Sort-Object | ||
| } |
Comment on lines
+78
to
+81
| Versioning is automated with [Nerdbank.GitVersioning](https://github.com/dotnet/Nerdbank.GitVersioning). | ||
| A per-plugin `plugins/<plugin>/version.json` scopes the git height to that plugin's subtree, so the | ||
| **patch** number is derived from history — you do not edit it by hand. The two generated manifests | ||
| (`plugin.json`, `.codex-plugin/plugin.json`) and `version.json` itself are excluded from that height |
github-actions
Bot
added
waiting-on-author
Contributor
|
👋 @AbhitejJohn — this PR has 6 unresolved review thread(s). When you're ready, please address the feedback and push an update; the triage bot will pick up the next state automatically. (Add the |
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.
Why
Every client that surfaces these skills — Copilot CLI, Claude Code, Codex, Cursor — reads a plugin's version directly from its checked‑in manifest (
plugin.json/.codex-plugin/plugin.json). Today those versions are static, so:We want correct, current versions in the repo with minimal manual work, without bloating the marketplace clone and without taking on third‑party/external plugins (out of scope for now).
What this adds
version.jsonwhosepathFiltersexclude the generated manifests andversion.jsonitself, so the computed version height tracks real content changes only (editing a manifest or the version file doesn't inflate the height).eng/version/Sync-PluginVersions.ps1— resolves the changed‑plugin set from a git diff, computes each version withnbgv(predicting the squash‑merge height for PRs), and either reports or stamps.Automations (two, low‑touch by design)
/version-bump/version-bumpon a PRweekly-version-syncworkflow_dispatchmain, opens/updates a single bot PR, and explains the per‑plugin reason. Self‑heals anything merged without a bump.We deliberately did not auto‑edit contributor PRs or add a noisy advisory‑comment bot — maintainers stay in control and the signal stays clean. (No official skills repo we surveyed auto‑mutates contributor PRs for versioning; the opt‑in command + scheduled backstop pattern is the conservative norm.)
Security — multi‑model adversarial review (GPT‑5.5 + Gemini 3.1 Pro)
dotnet tool restorewould have honored anuget.configauthored in the PR tree, letting an attacker remap thenbgvpackage source to a malicious feed and execute code in the privilegedcontents: writecontext. This was genuinely reachable — nonuget.configis tracked anywhere in the repo today, so a PR could add one. Fix: a trustedeng/version/nuget.config(<clear/>+ nuget.org‑only +packageSourceMapping), overlaid frommainand consumed via--configfileso PR‑supplied configs are ignored entirely./version-bumpnow checks out the authorized head SHA rather than the mutable branch name; if the branch advances after authorization, the push fails non‑fast‑forward — the safe outcome.Set-ManifestVersionuses aMatchEvaluator(not a replacement string), so a$‑bearing version can't re‑expand, plus a strict^\d+\.\d+\.\d+$guard that throws on a malformed base and leaves manifests untouched.version.jsonbump (0.1 → 0.2) is now detected and stamped.Verification
Exercised end‑to‑end against a real NBGV git harness (the repo
global.jsonpins an SDK that isn't installable locally, so testing uses an isolated harness):x.y.0;[](no bump);-OnlyChanged→ stamps drift authoritatively;dotnet tool restore --configfile→ exit 0.actionlintpasses on both workflows.Files
eng/version/Sync-PluginVersions.ps1,eng/version/nuget.config,.config/dotnet-tools.json.github/workflows/version-bump-command.yml,.github/workflows/weekly-version-sync.ymlplugins/*/version.json(×15),CONTRIBUTING.mdNotes for reviewers
issue_comment/scheduled workflows run the YAML frommain, so/version-bumpand the weekly job only take effect once this is merged — expected, not a bootstrap bug.Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com