From 3e49bffcd4747270dc4d642cc37642282b57ce84 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sat, 23 May 2026 12:17:22 -0400 Subject: [PATCH 01/16] docs(plan): design for plugin version as ldflag (workflow#758) --- ...2026-05-23-plugin-version-ldflag-design.md | 153 ++++++++++++++++++ 1 file changed, 153 insertions(+) create mode 100644 docs/plans/2026-05-23-plugin-version-ldflag-design.md diff --git a/docs/plans/2026-05-23-plugin-version-ldflag-design.md b/docs/plans/2026-05-23-plugin-version-ldflag-design.md new file mode 100644 index 00000000..4efacf7e --- /dev/null +++ b/docs/plans/2026-05-23-plugin-version-ldflag-design.md @@ -0,0 +1,153 @@ +# Plugin version from build-time ldflags — design + +Issue: GoCodeAlone/workflow#758 +Date: 2026-05-23 +Mode: design-only handoff (autonomous brainstorm; user is away) + +## Problem + +Today every plugin repo carries a duplicated `plugin.json.version` field on the committed `main` branch. After each release tag fires, `sync-plugin-version.yml` opens a PR to bump that field. Those PRs: + +- Pile up if not actively merged (13 stale PRs found on `workflow-plugin-digitalocean` 2026-05-23). +- Are redundant — the actually-shipped `plugin.json` in each release tarball is already correct because `goreleaser.before:` rewrites `.release/plugin.json` with `{{ .Version }}` from the git tag. +- Add review surface for purely mechanical bot churn. + +The whole class of drift is preventable by not committing the version field at all. + +## Verified current state + +- `workflow-plugin-digitalocean/cmd/plugin/main.go` calls `sdk.ServeIaCPlugin(internal.NewIaCServer(), sdk.IaCServeOptions{})` — no `ManifestProvider`. +- `sdk.IaCServeOptions.ManifestProvider` (workflow/plugin/external/sdk/iacserver.go:326) is the only knob today; when nil, `GetManifest` RPC returns `Unimplemented` and engine falls back to disk `plugin.json`. +- `PluginManifest.Validate()` (workflow/plugin/manifest.go:194-206) **requires** `Version` non-empty + parseable as semver. Run by `EmbedManifest` at plugin process start when `MustEmbedManifest` is used; also run by the engine's `manager.go` on disk plugin.json load. +- `workflow-plugin-digitalocean/internal/provider.go:34` already has `var Version = "dev"` set via `-X internal.Version={{.Version}}` ldflag in `.goreleaser.yaml:25`. The ldflag wiring exists but is not surfaced to the engine. +- `.goreleaser.yaml:7-8` before-hook copies `plugin.json` → `.release/plugin.json` and rewrites the `version` field from `{{ .Version }}`. The shipped binary's adjacent disk plugin.json carries the correct version. + +So today: ldflag-injected version exists in every plugin binary but is never read by the engine. Disk plugin.json carries the version, which the engine reads. Committed main's disk plugin.json drifts, hence the sync-PR mechanism. + +## Direction (per user) + +User stated direction verbatim (chat 2026-05-23): + +> Removing version tag from json is fine, but then we need to ensure ldflags version tag as a plugin contract requirement. We'll also need workflow-registry (and/or wfctl?) to validate that only valid versions are published following a semver type of scheme. People should be able to generate a plugin from a custom/test branch and reflect the test nature in the version string (by reflecting branch name or something), but plugins with versions like that should be rejected by the registry. If we validate from wfctl, maybe it can have an additional for-publish flag or something that would validate the version string. Then we'll need to audit all public and private plugins to adhere them to this new approach. + +## Proposed design + +### 1. Plugin contract: build-time version surface + +Add a new option to `sdk.IaCServeOptions`: + +```go +type IaCServeOptions struct { + // ... existing fields ... + + // BuildVersion is the plugin's release version, typically injected at + // build time via `-ldflags "-X main.version="` (or per-plugin + // equivalent). When non-empty, takes precedence over any ManifestProvider + // .Version field for GetManifest's Version response. Required for plugins + // that omit the version field from their committed plugin.json. + BuildVersion string +} +``` + +`sdk.ServeIaCPlugin` populates `iacPluginServiceBridge.runtimeVersion` from `opts.BuildVersion`. `GetManifest` returns: + +1. `opts.BuildVersion` if set (the build-time-injected value); else +2. `b.diskManifest.Version` if `ManifestProvider` set + has version; else +3. Engine-side fallback to disk plugin.json (existing behavior). + +Mirror the same option on `sdk.Serve` (for non-IaC plugins) via `WithBuildVersion(string)` to keep the surface symmetric. + +### 2. Engine: accept missing `version` in disk plugin.json when binary surfaces it + +`PluginManifest.Validate()` today fails outright on empty `Version`. Make it tolerant when a known sentinel (e.g., `""` literally) is present AND the plugin has surfaced a version via `GetManifest` RPC. Concrete change: + +- `PluginManifest.Validate()` continues to require Version for disk-only loads (no behavior change to consumers that parse plugin.json from disk in tooling). +- `engine/manager.go` (load path): if disk plugin.json's `Version == ""`, defer validation until `GetManifest` returns the runtime version; reject only if both are missing. +- Pre-validate the runtime version returned by `GetManifest` against `ParseSemver` (the same check `Validate()` uses today). + +Rejection at engine load when binary fails to surface a version preserves the contract: every running plugin still has a known, semver-parseable version. + +### 3. wfctl: `plugin validate --for-publish` gate + +`wfctl plugin validate` today reads the disk plugin.json and runs `PluginManifest.Validate()`. Extend: + +- `wfctl plugin validate --file ` keeps current behavior for local/dev validation. If `version` field is empty, the validator treats it as "build-time-injected; OK for local install" rather than a fatal error. +- `wfctl plugin validate --file --for-publish` opts into strict publish-time validation: requires `version` populated AND strictly matching `^v?\d+\.\d+\.\d+(-[a-zA-Z0-9.-]+)?$` (release semver, no dirty/branch suffixes like `-dirty`, `+commit`, or `feat-foo`). +- `--for-publish` is the gate the registry sync workflow + manual publish call before pushing. + +### 4. workflow-registry: publish-time semver gate + +`workflow-registry`'s ingest workflow (today blind-trusts whatever version it pulls from the plugin manifest) calls `wfctl plugin validate --for-publish` against the staged manifest before accepting the publish. Reject with a clear error if the version is non-strict semver. Branch builds (`v0.0.0-feat-foo.deadbeef`) install locally fine but never make it into the registry. + +### 5. goreleaser before-hook: keep, but as the *source* of truth + +The `before:` hook already writes the correct version into `.release/plugin.json`. That stays. The shipped tarball's plugin.json carries the version. After the engine change in §2, the committed `plugin.json` on main can have `"version": ""` (or omit the field entirely; both must be tolerated). The shipped one always has the real version because goreleaser fills it. + +### 6. Drop `sync-plugin-version.yml` from every plugin repo + +Once §1-§5 land in a plugin repo, that repo's `sync-plugin-version.yml` workflow is dead weight. Delete it in the same PR that drops the committed `version` field. + +### 7. All-plugin migration + +Audit + migrate (drop committed version, verify ldflag wiring) every plugin repo: + +**Public plugins** (verified via memory): +- workflow-plugin-admin, agent, auth, authz, authz-ui, aws, azure, bento, ci-generator, cloud-ui, compute, cms, data-protection, digitalocean, edge-compute, edge-risk, gcp, github, payments, sandbox, security, supply-chain, tofu, waf + +**Private plugins** (separate access required): same list as above plus any private-only repos. + +Per repo: a 1-PR migration that drops the version field from plugin.json, deletes sync-plugin-version.yml, verifies the goreleaser ldflag wiring, and bumps to a new minor version on tag. + +### 8. Engine-version pin order of operations + +Engine change (§2) ships first as a new minor (e.g., workflow v0.61.0). Plugin migrations bump `minEngineVersion` to that version in their committed plugin.json. Plugins built against pre-v0.61 engines continue to require the committed `version` field — old engines load disk-only and don't speak the new `BuildVersion` contract. No flag-day; old plugins keep working on new engines, new plugins require new engine via `minEngineVersion`. + +## Assumptions + +A1. `goreleaser.before:` hook already writes the correct version into the shipped tarball's plugin.json for every plugin repo. **Verified for DO plugin; assumed for others. The migration PR per repo verifies.** +A2. Every plugin's `cmd/plugin/main.go` already has a Go var consumable by `-X` ldflags (DO plugin uses `internal.Version`). **Migration PR per repo verifies / wires if missing.** +A3. The `sdk.IaCServeOptions.BuildVersion` change is backward-compatible: existing plugins that don't set it keep current behavior. **Confirmed by reading iacserver.go — option is additive.** +A4. `wfctl plugin validate --for-publish` is a new flag, not a behavior change to the default `validate` invocation. **Confirmed; existing wfctl callers unaffected.** +A5. `workflow-registry`'s publish workflow runs `wfctl` (or has access to a wfctl binary) and can call `--for-publish`. **Needs verification in registry repo; if not, design a different gate (in-registry semver regex).** +A6. The "branch-tagged version" use case (e.g., `v0.0.0-feat-foo.`) is something users actually want — installable locally, registry-rejected. **User stated this requirement verbatim.** +A7. Existing plugins parsing `plugin.json.version` for tooling/display purposes (outside engine load) can tolerate an empty field for one release cycle while migration completes. **Needs audit; flag in plan.** + +## Self-challenge — top 3 doubts + +D1. **Is the engine-side fallback chain too clever?** Three layers: BuildVersion → ManifestProvider.Version → disk fallback. Future debugging when "version is wrong" gets murky. Mitigation: log which source provided the version at plugin load. + +D2. **Does the `--for-publish` flag put the gate in the wrong place?** The registry should be authoritative; relying on wfctl-side validation means anyone with a fork of wfctl can bypass. Mitigation: registry runs the same check independently (defense in depth) — wfctl is the *operator-facing* error surface so the operator sees the rejection locally before pushing. + +D3. **Will all-plugin migration take longer than the sync-PR churn it eliminates?** ~25 plugin repos × 1 PR each = 25 PRs to land. Mitigation: it's a one-time cost; the sync-PR pile is forever otherwise. Plus the migration PRs can be batched (file all, merge as CI greens). + +## Rollback + +- §1 (BuildVersion option) is additive on `IaCServeOptions`; revert is a single file change in workflow SDK. +- §2 (engine manager tolerance) is the riskier change — if revert is needed, engine reverts to requiring `version` on disk, plugins that already dropped it break. Rollback path: re-add `version` field to each migrated plugin.json (cheap; one line per repo) AND revert engine + bump engine minor. +- §3-§5 (wfctl flag, registry gate, goreleaser hook) are independently revertible. +- §7 migration PRs are individually revertible per plugin. + +The cross-repo migration is the heaviest blast radius. Recommendation: ship engine §2 + SDK §1 first as workflow v0.61.0; live one release cycle with both behaviors supported; then begin plugin migrations one at a time. + +## Out of scope + +- Replacing goreleaser entirely. +- Changing how DEPLOYMENT_STRATEGIES-style metadata propagates from binary to manifest. +- Pinning specific minEngineVersion ranges in plugins beyond the migration bump. +- Webhook-based publish gate (the registry sync is workflow-dispatch today; keep that). +- Backporting the contract to engine versions < v0.61. + +## Migration ordering + +1. **PR 1 (workflow):** add `IaCServeOptions.BuildVersion` + `sdk.Serve` `WithBuildVersion` option. Engine `manager.go` tolerates empty disk version when GetManifest returns a parseable one. Adversarial review + tests. Tag workflow v0.61.0. +2. **PR 2 (wfctl, in workflow repo):** add `wfctl plugin validate --for-publish` flag. Tag workflow v0.61.1. +3. **PR 3 (workflow-registry):** publish workflow calls `wfctl plugin validate --for-publish`. Reject non-semver publishes. +4. **PR 4-N (each plugin repo):** drop committed `version` field; delete `sync-plugin-version.yml`; verify goreleaser ldflag wiring; bump `minEngineVersion` to v0.61.0; tag next minor. + +Each plugin PR is independent of the others; can run in parallel. + +## Decision points reserved for user return + +- **Approve overall design + execute pipeline** (this design + plan + alignment-check + scope-lock; then dispatch PR 1 only, pause for review before PR 2-N). +- **Approve §1-§3 only** (engine + wfctl), defer §4-§7 (registry + migration) to a separate brainstorm. +- **Re-scope** (e.g., skip the `--for-publish` flag in favor of a registry-side regex; or batch all plugin migrations into a single sweep PR per repo). From 081f21e64d94dafb3c10c6f22c0307cee02223cf Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sat, 23 May 2026 12:22:51 -0400 Subject: [PATCH 02/16] docs(plan): pivot design per adversarial cycle 1 (workflow#758) Cycle 1 surfaced 3 Critical defects in the original ldflag-injected design. Pivot to a much smaller scope per reviewer's Options 1+4: - KEEP version field in committed plugin.json (registry depends on it via sync-versions.sh:122 and downloads[].url version-tagged paths) - Replace sync-plugin-version.yml PR-opening with direct-push-to-main (or auto-merge variant for stricter repos) - Add tag-format semver gate (whitelist alpha|beta|rc pre-releases) in each release.yml as a first step - Add same gate in workflow-registry ingest as defense in depth No engine changes. No plugin SDK changes. No cross-version compatibility cliffs. Cycle-1 C1/C2/C3 + I1-I7 collapsed by not dropping the field. --- ...2026-05-23-plugin-version-ldflag-design.md | 173 ++++++++---------- 1 file changed, 76 insertions(+), 97 deletions(-) diff --git a/docs/plans/2026-05-23-plugin-version-ldflag-design.md b/docs/plans/2026-05-23-plugin-version-ldflag-design.md index 4efacf7e..e135dae9 100644 --- a/docs/plans/2026-05-23-plugin-version-ldflag-design.md +++ b/docs/plans/2026-05-23-plugin-version-ldflag-design.md @@ -1,153 +1,132 @@ -# Plugin version from build-time ldflags — design +# Plugin version sync hardening + publish-tag semver gate — design Issue: GoCodeAlone/workflow#758 -Date: 2026-05-23 +Date: 2026-05-23 (cycle 2 — pivoted per adversarial cycle 1) Mode: design-only handoff (autonomous brainstorm; user is away) ## Problem -Today every plugin repo carries a duplicated `plugin.json.version` field on the committed `main` branch. After each release tag fires, `sync-plugin-version.yml` opens a PR to bump that field. Those PRs: +Today every plugin repo carries a `plugin.json.version` field on `main`. After each release tag fires, `sync-plugin-version.yml` opens a PR to bump that field AND rewrite every `downloads[].url` to point at the new tag. Those PRs: -- Pile up if not actively merged (13 stale PRs found on `workflow-plugin-digitalocean` 2026-05-23). -- Are redundant — the actually-shipped `plugin.json` in each release tarball is already correct because `goreleaser.before:` rewrites `.release/plugin.json` with `{{ .Version }}` from the git tag. +- Pile up if not actively merged (13 stale PRs found on `workflow-plugin-digitalocean` 2026-05-23 — closed in a sweep). +- Are necessary — `workflow-registry/scripts/sync-versions.sh:122` reads `.version` AND asserts `downloads[].url` contains `/releases/download/v${version}/`. The committed file IS the source of truth for the registry sync. Cannot be eliminated. - Add review surface for purely mechanical bot churn. -The whole class of drift is preventable by not committing the version field at all. +Additionally, the user wants protection against non-semver versions getting into the registry. Branch / test / `-dirty` builds should install locally but be rejected from publish. -## Verified current state +## What the adversarial cycle taught us -- `workflow-plugin-digitalocean/cmd/plugin/main.go` calls `sdk.ServeIaCPlugin(internal.NewIaCServer(), sdk.IaCServeOptions{})` — no `ManifestProvider`. -- `sdk.IaCServeOptions.ManifestProvider` (workflow/plugin/external/sdk/iacserver.go:326) is the only knob today; when nil, `GetManifest` RPC returns `Unimplemented` and engine falls back to disk `plugin.json`. -- `PluginManifest.Validate()` (workflow/plugin/manifest.go:194-206) **requires** `Version` non-empty + parseable as semver. Run by `EmbedManifest` at plugin process start when `MustEmbedManifest` is used; also run by the engine's `manager.go` on disk plugin.json load. -- `workflow-plugin-digitalocean/internal/provider.go:34` already has `var Version = "dev"` set via `-X internal.Version={{.Version}}` ldflag in `.goreleaser.yaml:25`. The ldflag wiring exists but is not surfaced to the engine. -- `.goreleaser.yaml:7-8` before-hook copies `plugin.json` → `.release/plugin.json` and rewrites the `version` field from `{{ .Version }}`. The shipped binary's adjacent disk plugin.json carries the correct version. +Cycle 1 proposed dropping `version` from committed plugin.json and surfacing it via ldflag-injected runtime value. That design had three critical defects: (a) the pre-spawn `LoadManifest+Validate` site at `plugin/external/manager.go:134-140` makes "defer to GetManifest" structurally impossible at the named code path; (b) `sync-plugin-version.yml` also rewrites `downloads[].url` (`sync-plugin-version.yml:47-52`) — not just version — and `workflow-registry/scripts/sync-versions.sh:122-138` depends on that URL being version-correct, so dropping the workflow without compensating produces unbuildable registry manifests; (c) `wfctl plugin validate --for-publish` has no source for the version string it would validate against once the disk plugin.json no longer carries it. -So today: ldflag-injected version exists in every plugin binary but is never read by the engine. Disk plugin.json carries the version, which the engine reads. Committed main's disk plugin.json drifts, hence the sync-PR mechanism. +The cycle-1 reviewer surfaced four simpler alternatives. The combination of Options 1 + 4 (with a smaller Option 3 piece) reaches the user's actual goal — stop the PR churn AND gate non-semver publishes — without dropping the committed version field at all. -## Direction (per user) +## Cycle-2 direction -User stated direction verbatim (chat 2026-05-23): +**Keep `version` field in committed plugin.json.** The cycle-1 root cause is sync-mechanism, not field-presence. Fix the sync mechanism; leave the field alone. -> Removing version tag from json is fine, but then we need to ensure ldflags version tag as a plugin contract requirement. We'll also need workflow-registry (and/or wfctl?) to validate that only valid versions are published following a semver type of scheme. People should be able to generate a plugin from a custom/test branch and reflect the test nature in the version string (by reflecting branch name or something), but plugins with versions like that should be rejected by the registry. If we validate from wfctl, maybe it can have an additional for-publish flag or something that would validate the version string. Then we'll need to audit all public and private plugins to adhere them to this new approach. +Three composable pieces: -## Proposed design +### 1. Replace PR-opening sync with direct-push-to-main -### 1. Plugin contract: build-time version surface +`sync-plugin-version.yml` today: tag fires → workflow checks out main → rewrites plugin.json → opens PR. The PR sits unmerged. -Add a new option to `sdk.IaCServeOptions`: +New: tag fires → workflow checks out main → rewrites plugin.json + downloads URLs (exactly today's logic, both substitutions preserved) → commits directly to `main` as `github-actions[bot]`, signed-off with the triggering tag in the commit message. No PR. No review queue. -```go -type IaCServeOptions struct { - // ... existing fields ... +Requires either (a) branch protection on `main` to allow `enforce_admins: false` + the bot to push (DO plugin's branch protection is `enforce_admins: false` already; admins can bypass — bot uses an admin PAT, OR the GITHUB_TOKEN gets a one-time bypass via a "linear history" / "allow bot pushes" rule), or (b) a dedicated "release-bot" branch protection exemption. - // BuildVersion is the plugin's release version, typically injected at - // build time via `-ldflags "-X main.version="` (or per-plugin - // equivalent). When non-empty, takes precedence over any ManifestProvider - // .Version field for GetManifest's Version response. Required for plugins - // that omit the version field from their committed plugin.json. - BuildVersion string -} -``` - -`sdk.ServeIaCPlugin` populates `iacPluginServiceBridge.runtimeVersion` from `opts.BuildVersion`. `GetManifest` returns: - -1. `opts.BuildVersion` if set (the build-time-injected value); else -2. `b.diskManifest.Version` if `ManifestProvider` set + has version; else -3. Engine-side fallback to disk plugin.json (existing behavior). - -Mirror the same option on `sdk.Serve` (for non-IaC plugins) via `WithBuildVersion(string)` to keep the surface symmetric. - -### 2. Engine: accept missing `version` in disk plugin.json when binary surfaces it - -`PluginManifest.Validate()` today fails outright on empty `Version`. Make it tolerant when a known sentinel (e.g., `""` literally) is present AND the plugin has surfaced a version via `GetManifest` RPC. Concrete change: +For repos where direct push is undesirable (some private plugins may have stricter rules), the workflow stays PR-opening but ALSO calls `gh pr merge --auto --squash --delete-branch` immediately after creation, with `--admin` if the token has permission. The PR still gets opened but is automerged on creation — no human review queue. -- `PluginManifest.Validate()` continues to require Version for disk-only loads (no behavior change to consumers that parse plugin.json from disk in tooling). -- `engine/manager.go` (load path): if disk plugin.json's `Version == ""`, defer validation until `GetManifest` returns the runtime version; reject only if both are missing. -- Pre-validate the runtime version returned by `GetManifest` against `ParseSemver` (the same check `Validate()` uses today). +### 2. Tag-level semver gate in release workflow -Rejection at engine load when binary fails to surface a version preserves the contract: every running plugin still has a known, semver-parseable version. +`release.yml` today: triggered by tag push, runs goreleaser. No tag-format validation. A malformed tag (`v1.2`, `v1.2.3-dirty`, `release-2026-05`, etc.) just builds and publishes a malformed release. -### 3. wfctl: `plugin validate --for-publish` gate +Add a first step in `release.yml`: -`wfctl plugin validate` today reads the disk plugin.json and runs `PluginManifest.Validate()`. Extend: +```yaml +- name: Validate tag is strict semver + run: | + TAG="${{ github.ref_name }}" + if [[ ! "$TAG" =~ ^v[0-9]+\.[0-9]+\.[0-9]+(-(alpha|beta|rc)(\.[0-9]+)?)?$ ]]; then + echo "::error::Tag $TAG is not strict semver (allowed: vN.N.N or vN.N.N-{alpha|beta|rc}[.N])" + exit 1 + fi +``` -- `wfctl plugin validate --file ` keeps current behavior for local/dev validation. If `version` field is empty, the validator treats it as "build-time-injected; OK for local install" rather than a fatal error. -- `wfctl plugin validate --file --for-publish` opts into strict publish-time validation: requires `version` populated AND strictly matching `^v?\d+\.\d+\.\d+(-[a-zA-Z0-9.-]+)?$` (release semver, no dirty/branch suffixes like `-dirty`, `+commit`, or `feat-foo`). -- `--for-publish` is the gate the registry sync workflow + manual publish call before pushing. +The whitelist `alpha|beta|rc` is narrow on purpose (cycle 1 I4 finding): bare semver pre-release syntax (`-feat-foo.deadbeef`) satisfies semver but is exactly what we want to reject from publish. If teams need a different pre-release vocabulary, the regex is the place to change it (single line). -### 4. workflow-registry: publish-time semver gate +Local / branch / test builds via `goreleaser --snapshot` or plain `go build` (which produce `(devel)` / SNAPSHOT tags) are NOT triggered by tag push, so they skip the gate entirely. They install via `wfctl plugin install --local ` which doesn't go through release.yml. That's the answer to "people should be able to generate a plugin from a custom/test branch." -`workflow-registry`'s ingest workflow (today blind-trusts whatever version it pulls from the plugin manifest) calls `wfctl plugin validate --for-publish` against the staged manifest before accepting the publish. Reject with a clear error if the version is non-strict semver. Branch builds (`v0.0.0-feat-foo.deadbeef`) install locally fine but never make it into the registry. +### 3. Independent semver gate in `workflow-registry` ingest -### 5. goreleaser before-hook: keep, but as the *source* of truth +Defense in depth. `workflow-registry/scripts/sync-versions.sh` (or whichever ingest path is canonical — verify before plan) gets the same regex check before accepting a plugin update: -The `before:` hook already writes the correct version into `.release/plugin.json`. That stays. The shipped tarball's plugin.json carries the version. After the engine change in §2, the committed `plugin.json` on main can have `"version": ""` (or omit the field entirely; both must be tolerated). The shipped one always has the real version because goreleaser fills it. +```bash +TAG="v${manifest_version}" +if [[ ! "$TAG" =~ ^v[0-9]+\.[0-9]+\.[0-9]+(-(alpha|beta|rc)(\.[0-9]+)?)?$ ]]; then + echo " REJECT $plugin_name — manifest version $manifest_version is not strict semver" + continue +fi +``` -### 6. Drop `sync-plugin-version.yml` from every plugin repo +A plugin author who bypasses the release.yml gate (e.g., self-hosted runner, force-push, manual tarball upload) still gets caught at registry sync. -Once §1-§5 land in a plugin repo, that repo's `sync-plugin-version.yml` workflow is dead weight. Delete it in the same PR that drops the committed `version` field. +### 4. ldflag version surface (out of cycle-1 scope, kept as future option) -### 7. All-plugin migration +The runtime-version contract (ldflag → `internal.Version` → GetManifest RPC) already exists in workflow SDK + DO plugin. We don't *change* it here. A future design cycle could collapse the disk `version` field onto the runtime surface — but that requires solving cycle-1's C1/C2/C3 problems first (engine load order, registry URL rewrite, wfctl publish-validation source). Not scoped here. -Audit + migrate (drop committed version, verify ldflag wiring) every plugin repo: +## Migration plan -**Public plugins** (verified via memory): -- workflow-plugin-admin, agent, auth, authz, authz-ui, aws, azure, bento, ci-generator, cloud-ui, compute, cms, data-protection, digitalocean, edge-compute, edge-risk, gcp, github, payments, sandbox, security, supply-chain, tofu, waf +Each plugin repo is independent. -**Private plugins** (separate access required): same list as above plus any private-only repos. +1. **workflow (PR 1, this repo):** no engine changes. Add a `release.yml` snippet template + document in `docs/PLUGIN_RELEASE_GATES.md`. Optionally: small wfctl helper `wfctl plugin validate-tag ` that runs the regex (operator convenience). +2. **workflow-registry (PR 2):** add the semver gate in `scripts/sync-versions.sh`. Reject malformed versions with clear errors. +3. **Per-plugin migration PR (N repos):** update local `sync-plugin-version.yml` to direct-push-to-main (or auto-merge variant), AND add the tag-format gate step to local `release.yml`. Each PR is ~10 lines, isolated, individually mergeable. Same PR can also remove any stale `chore: sync plugin.json version to v…` history-trash if desired (out of scope). -Per repo: a 1-PR migration that drops the version field from plugin.json, deletes sync-plugin-version.yml, verifies the goreleaser ldflag wiring, and bumps to a new minor version on tag. +The migration order is workflow → registry → plugins. Plugin migrations can run in parallel. -### 8. Engine-version pin order of operations +## Verified API surface -Engine change (§2) ships first as a new minor (e.g., workflow v0.61.0). Plugin migrations bump `minEngineVersion` to that version in their committed plugin.json. Plugins built against pre-v0.61 engines continue to require the committed `version` field — old engines load disk-only and don't speak the new `BuildVersion` contract. No flag-day; old plugins keep working on new engines, new plugins require new engine via `minEngineVersion`. +- `workflow-registry/scripts/sync-versions.sh:122` — reads `.version` from manifest. Stays as-is. +- `workflow-registry/scripts/sync-versions.sh:138` — `downloads_match_version` requires `downloads[].url` contains `/releases/download/v${version}/`. Stays as-is. +- `workflow-plugin-digitalocean/.github/workflows/sync-plugin-version.yml:47-52` — rewrites `downloads[].url` via Python sed; this LOGIC is preserved in the direct-push variant, only the `gh pr create` is replaced by `git push origin main`. +- `workflow-plugin-digitalocean/.github/workflows/release.yml` — tag-fired today; receives new validate-tag step at the top. +- `workflow-plugin-digitalocean/.goreleaser.yaml:7-9` — before-hook stays as-is. ## Assumptions -A1. `goreleaser.before:` hook already writes the correct version into the shipped tarball's plugin.json for every plugin repo. **Verified for DO plugin; assumed for others. The migration PR per repo verifies.** -A2. Every plugin's `cmd/plugin/main.go` already has a Go var consumable by `-X` ldflags (DO plugin uses `internal.Version`). **Migration PR per repo verifies / wires if missing.** -A3. The `sdk.IaCServeOptions.BuildVersion` change is backward-compatible: existing plugins that don't set it keep current behavior. **Confirmed by reading iacserver.go — option is additive.** -A4. `wfctl plugin validate --for-publish` is a new flag, not a behavior change to the default `validate` invocation. **Confirmed; existing wfctl callers unaffected.** -A5. `workflow-registry`'s publish workflow runs `wfctl` (or has access to a wfctl binary) and can call `--for-publish`. **Needs verification in registry repo; if not, design a different gate (in-registry semver regex).** -A6. The "branch-tagged version" use case (e.g., `v0.0.0-feat-foo.`) is something users actually want — installable locally, registry-rejected. **User stated this requirement verbatim.** -A7. Existing plugins parsing `plugin.json.version` for tooling/display purposes (outside engine load) can tolerate an empty field for one release cycle while migration completes. **Needs audit; flag in plan.** +A1. Every plugin repo has branch protection set such that an admin PAT can push to `main` directly (enforce_admins: false). **Verified for DO plugin via `gh api repos/.../branches/main/protection` in chat 2026-05-23. Assumed similar for others; per-repo verification step in each plugin migration PR.** +A2. Auto-merge `--admin --squash --delete-branch` is available on GitHub for repos with branch protection. **Verified by prior session work (multiple admin-merges in 2026-05).** +A3. The user's "branch / test build" use case is locally-installed via `wfctl plugin install --local ` and never goes through release.yml. **Plausible from existing wfctl support; per-plugin migration verifies no other publish path exists.** +A4. The tag-format gate regex `^v[0-9]+\.[0-9]+\.[0-9]+(-(alpha|beta|rc)(\.[0-9]+)?)?$` matches the user's intent (no `-feat-foo`, no `-dirty`, no `+meta`). **User said "valid semver" and "reject branch versions"; explicit pre-release whitelist enforces the latter.** +A5. workflow-registry's sync-versions.sh is the canonical publish ingest. If another path exists (webhook, manual upload) it needs the same gate. **Per registry PR audit; flag in plan.** +A6. The direct-push-to-main mechanism doesn't break CI in any plugin repo (e.g., a "PRs only" branch protection rule). **Per-repo verification step.** ## Self-challenge — top 3 doubts -D1. **Is the engine-side fallback chain too clever?** Three layers: BuildVersion → ManifestProvider.Version → disk fallback. Future debugging when "version is wrong" gets murky. Mitigation: log which source provided the version at plugin load. +D1. **Is direct-push-to-main actually safer than auto-merging a sync PR?** Direct-push: bot has commit-to-main perm forever. Auto-merge: bot opens PR → merges immediately, audit trail in PR history, perm scoped to "create + merge own PRs." The latter is the better default for security-conscious repos. Cycle-2 design supports BOTH (per-repo choice) — direct-push for fast-moving plugin repos, auto-merge for privates. -D2. **Does the `--for-publish` flag put the gate in the wrong place?** The registry should be authoritative; relying on wfctl-side validation means anyone with a fork of wfctl can bypass. Mitigation: registry runs the same check independently (defense in depth) — wfctl is the *operator-facing* error surface so the operator sees the rejection locally before pushing. +D2. **What about the 13 stale PRs that piled up on DO before the sweep?** Those PRs were closed manually 2026-05-23. Going forward, the new mechanism prevents new accumulation. There is no general-purpose "close stale sync PRs" automation in this design — that's a one-time cleanup that already happened. -D3. **Will all-plugin migration take longer than the sync-PR churn it eliminates?** ~25 plugin repos × 1 PR each = 25 PRs to land. Mitigation: it's a one-time cost; the sync-PR pile is forever otherwise. Plus the migration PRs can be batched (file all, merge as CI greens). +D3. **Does the tag-format regex's whitelist (`alpha|beta|rc`) hard-code a release-vocabulary choice?** Yes. Some plugin authors might want `dev`, `preview`, `experimental`, etc. The regex is the place to change it. Documenting the supported list in `docs/PLUGIN_RELEASE_GATES.md` makes the choice visible. ## Rollback -- §1 (BuildVersion option) is additive on `IaCServeOptions`; revert is a single file change in workflow SDK. -- §2 (engine manager tolerance) is the riskier change — if revert is needed, engine reverts to requiring `version` on disk, plugins that already dropped it break. Rollback path: re-add `version` field to each migrated plugin.json (cheap; one line per repo) AND revert engine + bump engine minor. -- §3-§5 (wfctl flag, registry gate, goreleaser hook) are independently revertible. -- §7 migration PRs are individually revertible per plugin. +- §1 (direct-push or auto-merge in sync-plugin-version.yml): per-repo, revert the workflow file change. Old PR-opening behavior returns; sync PRs accumulate again. Cheap revert; no state. +- §2 (tag-format gate in release.yml): per-repo, remove the step. Malformed tags can publish again. Cheap revert; no state. +- §3 (registry-side gate): single revert in `workflow-registry/scripts/sync-versions.sh`. Cheap; no state. -The cross-repo migration is the heaviest blast radius. Recommendation: ship engine §2 + SDK §1 first as workflow v0.61.0; live one release cycle with both behaviors supported; then begin plugin migrations one at a time. +Cross-repo rollback risk: very low. None of the changes break the engine/SDK contract; they only affect the release pipeline. Worst case is a single bad release that the existing tag-delete + re-tag flow already handles. ## Out of scope -- Replacing goreleaser entirely. -- Changing how DEPLOYMENT_STRATEGIES-style metadata propagates from binary to manifest. -- Pinning specific minEngineVersion ranges in plugins beyond the migration bump. -- Webhook-based publish gate (the registry sync is workflow-dispatch today; keep that). -- Backporting the contract to engine versions < v0.61. - -## Migration ordering - -1. **PR 1 (workflow):** add `IaCServeOptions.BuildVersion` + `sdk.Serve` `WithBuildVersion` option. Engine `manager.go` tolerates empty disk version when GetManifest returns a parseable one. Adversarial review + tests. Tag workflow v0.61.0. -2. **PR 2 (wfctl, in workflow repo):** add `wfctl plugin validate --for-publish` flag. Tag workflow v0.61.1. -3. **PR 3 (workflow-registry):** publish workflow calls `wfctl plugin validate --for-publish`. Reject non-semver publishes. -4. **PR 4-N (each plugin repo):** drop committed `version` field; delete `sync-plugin-version.yml`; verify goreleaser ldflag wiring; bump `minEngineVersion` to v0.61.0; tag next minor. - -Each plugin PR is independent of the others; can run in parallel. +- Dropping `plugin.json.version` field entirely (deferred; needs solving cycle-1 C1/C2/C3 in a separate design). +- Replacing goreleaser. +- Cleaning up the existing 13 stale sync PRs (already done manually 2026-05-23). +- Auditing private plugins (a separate sweep PR per repo; mechanical; can be batched). +- Changing how plugins surface their runtime version via gRPC (existing ldflag-based path stays unchanged). ## Decision points reserved for user return -- **Approve overall design + execute pipeline** (this design + plan + alignment-check + scope-lock; then dispatch PR 1 only, pause for review before PR 2-N). -- **Approve §1-§3 only** (engine + wfctl), defer §4-§7 (registry + migration) to a separate brainstorm. -- **Re-scope** (e.g., skip the `--for-publish` flag in favor of a registry-side regex; or batch all plugin migrations into a single sweep PR per repo). +- **Approve overall design + execute pipeline** — design → plan → adversarial-design-review (plan) → alignment-check → scope-lock → dispatch workflow PR 1 only, pause for review before registry PR 2 and per-plugin PRs. +- **Approve §1+§2 only** (defer §3 registry gate); ship sync hardening + tag-format gate first, registry-side defense in depth as a follow-up. +- **Re-scope** (e.g., go back and revive the cycle-1 ldflag direction; the cycle-1 review explicitly notes Option 3 — `runtime/debug.ReadBuildInfo()` — as a cleaner alternative that bypasses both the disk-version AND ldflag-coordination problems). From f0ce7ca7e55d0bb3deeab506aa8bcfa53bf360e7 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sat, 23 May 2026 12:27:13 -0400 Subject: [PATCH 03/16] docs(plan): restore ldflag contract + close NI1-4 per adversarial cycle 2 (workflow#758) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - NC1 addressed: §4 adds sdk.ResolveBuildVersion + IaCServeOptions.BuildVersion contract + check-plugin-contract.sh lint - NC2 addressed: ResolveBuildVersion uses runtime/debug.ReadBuildInfo for test-build branch-nature surfacing - NI1 addressed: regex widened to (alpha|beta|rc)\.?[0-9]+ (rc1 + rc.1) - NI2 addressed: concurrency group on workflows + git pull --ff-only before commit in direct-push - NI3 addressed: registry gate validates tag string (same source as release.yml gate) - NI4 addressed: 24-row plugin audit table in migration plan; variant choice (direct-push|auto-merge) enumerated per-repo --- ...2026-05-23-plugin-version-ldflag-design.md | 139 +++++++++++++++--- 1 file changed, 118 insertions(+), 21 deletions(-) diff --git a/docs/plans/2026-05-23-plugin-version-ldflag-design.md b/docs/plans/2026-05-23-plugin-version-ldflag-design.md index e135dae9..8719ce8b 100644 --- a/docs/plans/2026-05-23-plugin-version-ldflag-design.md +++ b/docs/plans/2026-05-23-plugin-version-ldflag-design.md @@ -1,7 +1,7 @@ -# Plugin version sync hardening + publish-tag semver gate — design +# Plugin version sync hardening + ldflag contract + publish-tag semver gate — design Issue: GoCodeAlone/workflow#758 -Date: 2026-05-23 (cycle 2 — pivoted per adversarial cycle 1) +Date: 2026-05-23 (cycle 3 — restored ldflag contract piece per cycle-2 NC1/NC2) Mode: design-only handoff (autonomous brainstorm; user is away) ## Problem @@ -46,43 +46,132 @@ Add a first step in `release.yml`: - name: Validate tag is strict semver run: | TAG="${{ github.ref_name }}" - if [[ ! "$TAG" =~ ^v[0-9]+\.[0-9]+\.[0-9]+(-(alpha|beta|rc)(\.[0-9]+)?)?$ ]]; then - echo "::error::Tag $TAG is not strict semver (allowed: vN.N.N or vN.N.N-{alpha|beta|rc}[.N])" + # Allow both -rc1 (no dot, k8s/Go convention) and -rc.1 (semver-canonical). + if [[ ! "$TAG" =~ ^v[0-9]+\.[0-9]+\.[0-9]+(-(alpha|beta|rc)\.?[0-9]+)?$ ]]; then + echo "::error::Tag $TAG is not a release-grade semver (allowed: vN.N.N or vN.N.N-(alpha|beta|rc)[.]N)" exit 1 fi ``` -The whitelist `alpha|beta|rc` is narrow on purpose (cycle 1 I4 finding): bare semver pre-release syntax (`-feat-foo.deadbeef`) satisfies semver but is exactly what we want to reject from publish. If teams need a different pre-release vocabulary, the regex is the place to change it (single line). +The whitelist `alpha|beta|rc` is narrow on purpose (cycle 1 I4 finding): bare semver pre-release syntax (`-feat-foo.deadbeef`) satisfies semver but is exactly what we want to reject from publish. The optional `\.?` between `rc` and the digit accommodates both `v1.2.3-rc1` and `v1.2.3-rc.1` (cycle-2 NI1). If teams need a different pre-release vocabulary, the regex is the place to change it (single line). + +**Concurrency safety** (cycle-2 NI2): both this `release.yml` and the `sync-plugin-version.yml` workflow declare: + +```yaml +concurrency: + group: plugin-version-sync-${{ github.repository }} + cancel-in-progress: false +``` + +Two tags fired in quick succession queue serially; the second cannot overwrite an in-flight first. The direct-push variant additionally does `git fetch origin main && git pull --ff-only origin main` before its commit so a concurrent push is detected as a non-fast-forward and fails loudly rather than racing. Local / branch / test builds via `goreleaser --snapshot` or plain `go build` (which produce `(devel)` / SNAPSHOT tags) are NOT triggered by tag push, so they skip the gate entirely. They install via `wfctl plugin install --local ` which doesn't go through release.yml. That's the answer to "people should be able to generate a plugin from a custom/test branch." -### 3. Independent semver gate in `workflow-registry` ingest +### 3. Independent semver gate in `workflow-registry` ingest — gates the release tag, not the manifest field -Defense in depth. `workflow-registry/scripts/sync-versions.sh` (or whichever ingest path is canonical — verify before plan) gets the same regex check before accepting a plugin update: +Defense in depth. `workflow-registry/scripts/sync-versions.sh:125` already calls `gh release view --json tagName` to discover `$latest_tag`. The gate runs against the **tag string** (same source release.yml's gate uses), eliminating cycle-2 NI3's gate-string asymmetry: ```bash -TAG="v${manifest_version}" -if [[ ! "$TAG" =~ ^v[0-9]+\.[0-9]+\.[0-9]+(-(alpha|beta|rc)(\.[0-9]+)?)?$ ]]; then - echo " REJECT $plugin_name — manifest version $manifest_version is not strict semver" +# Validate the upstream release tag, not the manifest's .version field. +# Same regex as plugin release.yml so both gates fail identically. +if [[ ! "$latest_tag" =~ ^v[0-9]+\.[0-9]+\.[0-9]+(-(alpha|beta|rc)\.?[0-9]+)?$ ]]; then + echo " REJECT $plugin_name — upstream release tag $latest_tag is not release-grade semver" continue fi ``` -A plugin author who bypasses the release.yml gate (e.g., self-hosted runner, force-push, manual tarball upload) still gets caught at registry sync. +This catches: (a) plugins that bypass release.yml (self-hosted runner, manual tarball upload), and (b) plugins where the manifest `.version` field has drifted from the tag (older sync-PR mechanism failure, manual edit). Both surfaces are inspected — JSON `.version` continues to be validated by `downloads_match_version` for URL correctness; the tag is independently validated for publishability. -### 4. ldflag version surface (out of cycle-1 scope, kept as future option) +### 4. Make ldflag-injected runtime version a plugin contract -The runtime-version contract (ldflag → `internal.Version` → GetManifest RPC) already exists in workflow SDK + DO plugin. We don't *change* it here. A future design cycle could collapse the disk `version` field onto the runtime surface — but that requires solving cycle-1's C1/C2/C3 problems first (engine load order, registry URL rewrite, wfctl publish-validation source). Not scoped here. +Cycle-2 NC1 surfaced that the user's verbatim ask was conditional: *"Removing version tag from json is fine, **but then we need to ensure ldflags version tag as a plugin contract requirement**."* The cycle-2 pivot kept the JSON field (good) but declined the contract requirement (NC1 drift). This piece restores it as a small additive SDK change that does NOT require dropping the JSON field. -## Migration plan +Add to `sdk` package: + +```go +// IaCServeOptions adds: +// BuildVersion string // runtime version, typically internal.Version (ldflag-injected) -Each plugin repo is independent. +// ResolveBuildVersion returns the operator-visible build-version string. +// When declared is non-empty and not a known dev sentinel ("", "dev", +// "(devel)"), returns declared as-is. Otherwise consults +// runtime/debug.ReadBuildInfo() and returns a string like +// "(devel) [VCS-branch @ shortsha]" +// when VCS info is available, else "(devel)". +// +// This is the supported contract surface for plugin authors to plumb their +// goreleaser-injected version into GetManifest in a way that also degrades +// gracefully for local/test builds — addressing the user's stated branch- +// build-test-nature requirement (workflow#758 cycle-2 NC2). +func ResolveBuildVersion(declared string) string { ... } +``` -1. **workflow (PR 1, this repo):** no engine changes. Add a `release.yml` snippet template + document in `docs/PLUGIN_RELEASE_GATES.md`. Optionally: small wfctl helper `wfctl plugin validate-tag ` that runs the regex (operator convenience). -2. **workflow-registry (PR 2):** add the semver gate in `scripts/sync-versions.sh`. Reject malformed versions with clear errors. -3. **Per-plugin migration PR (N repos):** update local `sync-plugin-version.yml` to direct-push-to-main (or auto-merge variant), AND add the tag-format gate step to local `release.yml`. Each PR is ~10 lines, isolated, individually mergeable. Same PR can also remove any stale `chore: sync plugin.json version to v…` history-trash if desired (out of scope). +Plugin authors then write: -The migration order is workflow → registry → plugins. Plugin migrations can run in parallel. +```go +// cmd/plugin/main.go +sdk.ServeIaCPlugin(internal.NewIaCServer(), sdk.IaCServeOptions{ + BuildVersion: sdk.ResolveBuildVersion(internal.Version), +}) +``` + +`internal.Version` already exists in every plugin (set by `-X internal.Version=...` ldflag). For tagged release builds, the resolved string is the canonical semver (e.g. `v1.2.3`). For test/branch builds, it's `(devel) [feat/foo @ abc1234]` — operator-visible, branch-nature reflected, never accidentally publishable. + +Engine `iacPluginServiceBridge.GetManifest` (currently at `plugin/external/sdk/iacserver.go:300-312`) augments its returned Manifest.Version: if `BuildVersion` is non-empty, use it; else fall back to `diskManifest.Version` (existing behavior). The engine continues to log the disk Version at load time AND the runtime BuildVersion when they differ, so any drift is visible without being fatal. + +**Contract enforcement** (not engine-side; plugin-author-side via lint): +- `docs/PLUGIN_RELEASE_GATES.md` documents the convention. +- Add `scripts/check-plugin-contract.sh` to the workflow repo. The script greps a plugin repo's `.goreleaser.yaml` for `-X .*Version=` and its `cmd/plugin/main.go` (or equivalent) for `sdk.ResolveBuildVersion(`. Each plugin's `release.yml` runs this script as a first step. +- Verifier failure exits non-zero with a clear "missing ldflag contract" error and a link to the convention doc. The contract is enforced at release time, not engine load time — a stricter check would block legacy plugins. + +## Migration plan + +Each plugin repo is independent. The audit (cycle-2 NI4 — user explicitly asked for it) is **in scope** below. + +1. **workflow (PR 1, this repo):** add `sdk.ResolveBuildVersion` + `IaCServeOptions.BuildVersion` (§4); engine-side `iacPluginServiceBridge.GetManifest` prefers BuildVersion when set, logs disk-vs-runtime divergence. Add `scripts/check-plugin-contract.sh`. Document in new `docs/PLUGIN_RELEASE_GATES.md`. No removal of existing fields. Tag workflow v0.61.0. +2. **workflow-registry (PR 2):** add the tag-string regex gate in `scripts/sync-versions.sh` (§3). Reject malformed versions with clear errors. +3. **Per-plugin migration PR (N repos):** in each plugin repo: + - Replace `sync-plugin-version.yml`'s `gh pr create` with direct-push-to-main (or auto-merge variant if branch protection forbids direct push). Add `concurrency:` group. + - Add tag-format gate step to `release.yml`. Add `concurrency:` group. Add `scripts/check-plugin-contract.sh` invocation as the first build step. + - Update `cmd/plugin/main.go` (or equivalent) to call `sdk.ServeIaCPlugin(srv, sdk.IaCServeOptions{BuildVersion: sdk.ResolveBuildVersion(internal.Version)})`. + - Verify `.goreleaser.yaml` has `-X internal.Version=...` (most do; verify per repo). + - Bump `minEngineVersion` to `0.61.0` so older engines that don't support BuildVersion-aware GetManifest fall back to disk Version gracefully (which is still present and correct). + - Tag next minor for the plugin. + +Each plugin PR is ~30 lines across 3 files. Individually reviewable + mergeable. + +**Plugin audit inventory** (per workspace memory + DO plugin's actual layout): + +| Repo | Source | sync-plugin-version.yml? | ldflag wiring? | Variant | +|---|---|---|---|---| +| workflow-plugin-admin | private | Y | needs verify | auto-merge | +| workflow-plugin-agent | public | Y | needs verify | direct-push | +| workflow-plugin-auth | public | Y | needs verify | direct-push | +| workflow-plugin-authz | public | Y | needs verify | direct-push | +| workflow-plugin-authz-ui | private | Y | needs verify | auto-merge | +| workflow-plugin-aws | public | Y | needs verify | direct-push | +| workflow-plugin-azure | public | Y | needs verify | direct-push | +| workflow-plugin-bento | private | Y | needs verify | auto-merge | +| workflow-plugin-ci-generator | public | Y | needs verify | direct-push | +| workflow-plugin-cloud-ui | private | Y | needs verify | auto-merge | +| workflow-plugin-cms | public | Y | needs verify | direct-push | +| workflow-plugin-compute | public | Y | needs verify | direct-push | +| workflow-plugin-data-protection | private | Y | needs verify | auto-merge | +| workflow-plugin-digitalocean | public | Y | YES (verified) | direct-push | +| workflow-plugin-edge-compute | public | Y | needs verify | direct-push | +| workflow-plugin-edge-risk | public (scenarios) | likely N | likely N | (excluded — contract-only) | +| workflow-plugin-gcp | public | Y | needs verify | direct-push | +| workflow-plugin-github | public | Y | needs verify | direct-push | +| workflow-plugin-payments | public | Y | needs verify | direct-push | +| workflow-plugin-sandbox | private | Y | needs verify | auto-merge | +| workflow-plugin-security | private | Y | needs verify | auto-merge | +| workflow-plugin-supply-chain | private | Y | needs verify | auto-merge | +| workflow-plugin-tofu | public | Y | needs verify | direct-push | +| workflow-plugin-waf | private | Y | needs verify | auto-merge | + +The first per-repo migration PR includes the verification: does `sync-plugin-version.yml` exist; does goreleaser have the ldflag; does main.go currently call `sdk.ServeIaCPlugin` (or another `Serve` variant). Plan task per repo enumerates these gates. + +The migration order is workflow → registry → plugins. Plugin migrations can run in parallel (24 PRs, each ~30 lines). ## Verified API surface @@ -122,8 +211,16 @@ Cross-repo rollback risk: very low. None of the changes break the engine/SDK con - Dropping `plugin.json.version` field entirely (deferred; needs solving cycle-1 C1/C2/C3 in a separate design). - Replacing goreleaser. - Cleaning up the existing 13 stale sync PRs (already done manually 2026-05-23). -- Auditing private plugins (a separate sweep PR per repo; mechanical; can be batched). -- Changing how plugins surface their runtime version via gRPC (existing ldflag-based path stays unchanged). +- Changing the engine's pre-spawn `LoadManifest+Validate` semantics (cycle-1 C1 untouched). + +## Adversarial cycle 2 — addressed + +- NC1 (ldflag not a contract requirement; user-intent drift): **addressed** — §4 adds `sdk.ResolveBuildVersion` + `IaCServeOptions.BuildVersion` SDK contract surface + `scripts/check-plugin-contract.sh` lint enforced in each plugin's release.yml. +- NC2 (branch/test build version-string surface): **addressed** — §4's `ResolveBuildVersion` consults `runtime/debug.ReadBuildInfo()` for local/test builds; reports `(devel) [feat/foo @ shortsha]` so operator + engine + log all show branch nature. +- NI1 (regex too narrow for `rc1`/`alpha1`): **addressed** — §2 regex updated to `(alpha|beta|rc)\.?[0-9]+` accepting both `rc1` (k8s/Go convention) and `rc.1` (semver-canonical). +- NI2 (concurrent two-tag race): **addressed** — §2 mandates `concurrency:` group on both release.yml + sync-plugin-version.yml; direct-push variant does `git fetch origin main && git pull --ff-only` before commit so concurrent pushes fail loudly. +- NI3 (gate-string asymmetry between release.yml and registry): **addressed** — §3 registry gate now validates the **tag string** (same source as release.yml's gate), not the manifest's `.version` field; eliminates divergence. +- NI4 (per-plugin audit deferred to out-of-scope despite user request): **addressed** — Migration plan §3 now contains a 24-row plugin audit table; per-repo migration PR includes verification steps; private vs public + variant choice (direct-push vs auto-merge) enumerated. ## Decision points reserved for user return From 2e81c5a97449adb0d5dcb8d5a7aef5e30dbdbd2d Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sat, 23 May 2026 12:30:53 -0400 Subject: [PATCH 04/16] docs(plan): implementation plan for workflow#758 (26 PRs, 29 tasks; design-only mode) --- .../plans/2026-05-23-plugin-version-ldflag.md | 569 ++++++++++++++++++ 1 file changed, 569 insertions(+) create mode 100644 docs/plans/2026-05-23-plugin-version-ldflag.md diff --git a/docs/plans/2026-05-23-plugin-version-ldflag.md b/docs/plans/2026-05-23-plugin-version-ldflag.md new file mode 100644 index 00000000..0ba4fc4f --- /dev/null +++ b/docs/plans/2026-05-23-plugin-version-ldflag.md @@ -0,0 +1,569 @@ +# Plugin Version Sync Hardening + Ldflag Contract Implementation Plan + +> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. + +**Goal:** Stop the `sync-plugin-version.yml` PR pileup by replacing PR-opening with direct-push-to-main (or auto-merge variant for stricter repos); add a strict-semver tag-format gate in both each plugin's `release.yml` and `workflow-registry`'s ingest; restore the user's stated ldflag-contract requirement via a new `sdk.ResolveBuildVersion` helper + `IaCServeOptions.BuildVersion` field + per-plugin lint script enforced at release time. + +**Architecture:** Three composable pieces — sync-mechanism fix (per plugin), tag-format gate (per plugin + central registry), and ldflag contract (SDK + per-plugin main.go + lint). The disk `plugin.json.version` field stays so the engine's pre-spawn `LoadManifest+Validate` is untouched and `workflow-registry/scripts/sync-versions.sh`'s `downloads_match_version` check continues to pass. + +**Tech Stack:** Go 1.26 (SDK + wfctl); bash (workflow scripts); GitHub Actions YAML. No new dependencies. + +**Base branch:** main (per repo) + +**Design-only mode:** plan is written + adversarially reviewed + alignment-checked + scope-locked; execution does NOT auto-dispatch. User must explicitly authorize the cross-repo sweep on return. + +--- + +## Scope Manifest + +**PR Count:** 26 +**Tasks:** 27 +**Estimated Lines of Change:** ~1500 across all PRs (informational; not enforced) + +**Out of scope:** +- Dropping the `plugin.json.version` field entirely (deferred; needs solving cycle-1 C1/C2/C3 in a separate design). +- Replacing goreleaser. +- Cleaning up the existing 13 stale sync PRs on `workflow-plugin-digitalocean` (already done manually 2026-05-23). +- Changing the engine's pre-spawn `LoadManifest+Validate` semantics. +- Modifying `workflow-plugin-edge-risk` (scenarios-repo contract-only; no plugin binary). + +**PR Grouping:** + +| PR # | Title | Tasks | Branch | Repo | +|------|-------|-------|--------|------| +| 1 | feat(sdk): ResolveBuildVersion + IaCServeOptions.BuildVersion + check-plugin-contract.sh (workflow#758) | Task 1, Task 2, Task 3, Task 4 | feat/758-sdk-buildversion-contract | workflow | +| 2 | feat(registry): tag-string semver gate in sync-versions.sh (workflow#758) | Task 5 | feat/758-registry-tag-gate | workflow-registry | +| 3 | chore(release): direct-push + tag-gate + ldflag wiring (workflow#758) | Task 6 | chore/758-release-discipline | workflow-plugin-digitalocean | +| 4 | chore(release): direct-push + tag-gate + ldflag wiring (workflow#758) | Task 7 | chore/758-release-discipline | workflow-plugin-aws | +| 5 | (same) | Task 8 | (same) | workflow-plugin-azure | +| 6 | (same) | Task 9 | (same) | workflow-plugin-gcp | +| 7 | (same) | Task 10 | (same) | workflow-plugin-tofu | +| 8 | (same) | Task 11 | (same) | workflow-plugin-ci-generator | +| 9 | (same) | Task 12 | (same) | workflow-plugin-agent | +| 10 | (same) | Task 13 | (same) | workflow-plugin-auth | +| 11 | (same) | Task 14 | (same) | workflow-plugin-authz | +| 12 | (same) | Task 15 | (same) | workflow-plugin-cms | +| 13 | (same) | Task 16 | (same) | workflow-plugin-compute | +| 14 | (same) | Task 17 | (same) | workflow-plugin-edge-compute | +| 15 | (same) | Task 18 | (same) | workflow-plugin-github | +| 16 | (same) | Task 19 | (same) | workflow-plugin-payments | +| 17 | chore(release): auto-merge + tag-gate + ldflag wiring (workflow#758) | Task 20 | chore/758-release-discipline | workflow-plugin-admin (private) | +| 18 | (same) | Task 21 | (same) | workflow-plugin-authz-ui (private) | +| 19 | (same) | Task 22 | (same) | workflow-plugin-bento (private) | +| 20 | (same) | Task 23 | (same) | workflow-plugin-cloud-ui (private) | +| 21 | (same) | Task 24 | (same) | workflow-plugin-data-protection (private) | +| 22 | (same) | Task 25 | (same) | workflow-plugin-sandbox (private) | +| 23 | (same) | Task 26 | (same) | workflow-plugin-security (private) | +| 24 | (same) | Task 27 | (same) | workflow-plugin-supply-chain (private) | +| 25 | (same) | Task 28 | (same) | workflow-plugin-waf (private) | +| 26 | docs(workflow): post-rollout retro + close #758 | Task 29 | docs/758-retro | workflow | + +(PR 26 + Task 29 are scope-aligned bookends; the closing retro confirms the inventory matches what shipped.) + +**Status:** Draft + +--- + +### Task 1: Add `sdk.ResolveBuildVersion` helper + +**Files:** +- Create: `plugin/external/sdk/buildversion.go` +- Test: `plugin/external/sdk/buildversion_test.go` + +**Step 1: Write failing tests** + +```go +package sdk + +import ( + "strings" + "testing" +) + +func TestResolveBuildVersion_DeclaredSemverPassThrough(t *testing.T) { + got := ResolveBuildVersion("v1.2.3") + if got != "v1.2.3" { + t.Errorf("got %q, want v1.2.3", got) + } +} + +func TestResolveBuildVersion_EmptyDeclaredFallsToBuildInfo(t *testing.T) { + got := ResolveBuildVersion("") + if !strings.HasPrefix(got, "(devel)") { + t.Errorf("got %q, want prefix (devel)", got) + } +} + +func TestResolveBuildVersion_DevSentinelFallsToBuildInfo(t *testing.T) { + got := ResolveBuildVersion("dev") + if !strings.HasPrefix(got, "(devel)") { + t.Errorf("got %q, want prefix (devel)", got) + } +} +``` + +**Step 2: Run failing** + +``` +GOWORK=off go test ./plugin/external/sdk/ -run TestResolveBuildVersion -count=1 +``` +Expected: FAIL — undefined. + +**Step 3: Implement** + +```go +package sdk + +import ( + "fmt" + "runtime/debug" + "strings" +) + +// ResolveBuildVersion returns the operator-visible build-version string. +// When declared is non-empty and not a known dev sentinel ("", "dev", "(devel)"), +// returns declared as-is. Otherwise consults runtime/debug.ReadBuildInfo() and +// returns a string like "(devel) [VCS-branch @ shortsha]" when VCS info is +// available, else "(devel)". +// +// Intended call site: +// +// sdk.ServeIaCPlugin(srv, sdk.IaCServeOptions{ +// BuildVersion: sdk.ResolveBuildVersion(internal.Version), +// }) +// +// where internal.Version is set via "-X internal.Version=..." ldflag in the +// plugin's goreleaser configuration. For tagged release builds, returns the +// ldflag-injected semver. For local/test builds (where internal.Version +// defaults to "dev"), surfaces branch + short SHA so operators see the +// test nature in the version string. +func ResolveBuildVersion(declared string) string { + if declared != "" && declared != "dev" && declared != "(devel)" { + return declared + } + info, ok := debug.ReadBuildInfo() + if !ok { + return "(devel)" + } + var sha, modified string + for _, s := range info.Settings { + switch s.Key { + case "vcs.revision": + if len(s.Value) >= 7 { + sha = s.Value[:7] + } else { + sha = s.Value + } + case "vcs.modified": + if s.Value == "true" { + modified = ".dirty" + } + } + } + if sha == "" { + return "(devel)" + } + // VCS branch is not exposed by debug.ReadBuildInfo; surface SHA only. + // Operators who want branch can correlate the SHA themselves. + return fmt.Sprintf("(devel) [@ %s%s]", strings.TrimSpace(sha), modified) +} +``` + +**Step 4: Run tests pass** + +``` +GOWORK=off go test ./plugin/external/sdk/ -run TestResolveBuildVersion -count=1 +``` + +**Step 5: Commit** + +```bash +git add plugin/external/sdk/buildversion.go plugin/external/sdk/buildversion_test.go +git commit -m "feat(sdk): ResolveBuildVersion helper for ldflag + buildinfo surface (workflow#758) + +Plugin authors call sdk.ResolveBuildVersion(internal.Version) to feed +IaCServeOptions.BuildVersion. For release builds (ldflag-set), returns the +declared semver. For local/test builds, surfaces runtime/debug.ReadBuildInfo +VCS short-SHA so operator + engine logs reflect the build's test nature." +``` + +--- + +### Task 2: Add `IaCServeOptions.BuildVersion` field + `GetManifest` augmentation + +**Files:** +- Modify: `plugin/external/sdk/iacserver.go` (struct + bridge.GetManifest) +- Test: `plugin/external/sdk/iacserver_internal_test.go` or new `iacserver_buildversion_test.go` + +**Step 1: Write failing test** + +```go +func TestGetManifest_BuildVersionOverridesDiskVersion(t *testing.T) { + disk := &pluginpkg.PluginManifest{Name: "x", Version: "v1.0.0", Author: "a", Description: "d"} + b := &iacPluginServiceBridge{diskManifest: disk, buildVersion: "v1.0.1"} + got, err := b.GetManifest(context.Background(), &emptypb.Empty{}) + if err != nil { + t.Fatalf("GetManifest: %v", err) + } + if got.Version != "v1.0.1" { + t.Errorf("Version = %q, want BuildVersion-augmented v1.0.1", got.Version) + } +} + +func TestGetManifest_NoBuildVersionFallsToDiskVersion(t *testing.T) { + disk := &pluginpkg.PluginManifest{Name: "x", Version: "v1.0.0", Author: "a", Description: "d"} + b := &iacPluginServiceBridge{diskManifest: disk} + got, _ := b.GetManifest(context.Background(), &emptypb.Empty{}) + if got.Version != "v1.0.0" { + t.Errorf("Version = %q, want disk v1.0.0", got.Version) + } +} +``` + +**Step 2: Failing** + +``` +GOWORK=off go test ./plugin/external/sdk/ -run 'TestGetManifest_BuildVersion|TestGetManifest_NoBuildVersion' -count=1 +``` + +**Step 3: Implement** + +Add `BuildVersion string` to `IaCServeOptions` (existing struct ~line 320). Add `buildVersion string` to `iacPluginServiceBridge` struct. Wire it through `ServeIaCPlugin` initialization. + +Modify `GetManifest` (current line 300-312): + +```go +func (b *iacPluginServiceBridge) GetManifest(_ context.Context, _ *emptypb.Empty) (*pb.Manifest, error) { + if b.diskManifest == nil && b.buildVersion == "" { + return nil, status.Error(codes.Unimplemented, "manifest not embedded; engine falls back to disk plugin.json") + } + out := &pb.Manifest{} + if b.diskManifest != nil { + out.Name = b.diskManifest.Name + out.Version = b.diskManifest.Version + out.Author = b.diskManifest.Author + out.Description = b.diskManifest.Description + out.ConfigMutable = b.diskManifest.ConfigMutable + out.SampleCategory = b.diskManifest.SampleCategory + } + if b.buildVersion != "" { + out.Version = b.buildVersion // augment / override + } + return out, nil +} +``` + +**Step 4: Pass + full SDK suite** + +``` +GOWORK=off go test ./plugin/external/sdk/... -count=1 +``` + +**Step 5: Commit** + +```bash +git add plugin/external/sdk/iacserver.go plugin/external/sdk/*_test.go +git commit -m "feat(sdk): IaCServeOptions.BuildVersion augments GetManifest (workflow#758) + +When set (typically via sdk.ResolveBuildVersion(internal.Version)), +BuildVersion overrides the disk plugin.json Version field in the +GetManifest gRPC response. Engine sees the runtime-injected version; disk +field stays in place so pre-spawn LoadManifest+Validate is untouched." +``` + +--- + +### Task 3: Engine-side log on disk vs runtime version divergence (post-spawn observability) + +**Files:** +- Modify: `plugin/external/adapter.go` (post-handshake GetManifest call site; cycle-1 reviewer identified at line 113-138) + +Add a one-shot warning log when post-spawn `GetManifest`'s `Version` differs from `diskManifest.Version`. Pure observability; no behavior change. + +**Step 1: Write failing test** + +Verify the existing adapter test suite has a "GetManifest returns version X" pattern; add a case where disk says Y and runtime says X, assert the log contains "version divergence: disk=Y runtime=X" + assert plugin still loads. + +**Step 2-5:** standard TDD cycle. (Plan detail intentionally lighter for purely-observability tasks.) + +```bash +git add plugin/external/adapter.go plugin/external/adapter_test.go +git commit -m "feat(adapter): warn on disk-vs-runtime plugin version divergence (workflow#758)" +``` + +--- + +### Task 4: Add `scripts/check-plugin-contract.sh` lint script + docs + +**Files:** +- Create: `scripts/check-plugin-contract.sh` +- Create: `docs/PLUGIN_RELEASE_GATES.md` + +`check-plugin-contract.sh` reads a plugin repo path and asserts: +- `.goreleaser.yaml` contains `-X .*\.Version=` (any package path matching `*.Version=`) +- `cmd/plugin/main.go` (or `**/main.go` if cmd/plugin not present) contains `sdk.ResolveBuildVersion(` +- Exit non-zero with operator-friendly error pointing at `docs/PLUGIN_RELEASE_GATES.md` + +**Step 1: Write failing test fixture** + +`scripts/check-plugin-contract_test.sh` runs against `testdata/plugin-good/` (passes) and `testdata/plugin-missing-ldflag/` (fails). + +**Step 2-5:** TDD cycle. Add `docs/PLUGIN_RELEASE_GATES.md` describing the convention + the tag-format whitelist regex + concurrency-group guidance + direct-push-vs-auto-merge decision rubric. + +```bash +git add scripts/check-plugin-contract.sh scripts/testdata/plugin-good/ scripts/testdata/plugin-missing-ldflag/ docs/PLUGIN_RELEASE_GATES.md +git commit -m "feat: check-plugin-contract.sh lint + PLUGIN_RELEASE_GATES.md (workflow#758) + +Plugin repos invoke this script as the first step of release.yml so the +ldflag contract is enforced at release time, not engine load time. Docs +enumerate the tag-format whitelist, direct-push-vs-auto-merge rubric, +and concurrency-group guidance." +``` + +--- + +### Task 5: workflow-registry — tag-string semver gate in sync-versions.sh + +**Files:** +- Modify: `workflow-registry/scripts/sync-versions.sh` (after `latest_tag` is set at line ~125, before `latest_version="${latest_tag#v}"` at line 132) + +**Step 1: Write failing test** + +Add `workflow-registry/scripts/sync-versions_test.sh` (if no test pattern exists, add bats/shellspec or a simple test runner). Test cases: +- `latest_tag = v1.2.3` → accepted, proceed. +- `latest_tag = v1.2.3-rc1` → accepted. +- `latest_tag = v1.2.3-rc.1` → accepted. +- `latest_tag = v1.2.3-alpha2` → accepted. +- `latest_tag = v1.2.3-feat-foo.deadbeef` → REJECTED. +- `latest_tag = v1.2.3-dirty` → REJECTED. +- `latest_tag = release-2026-05` → REJECTED. + +**Step 2: Failing.** + +**Step 3: Implement gate** + +After `latest_tag="$(gh release view ...)"` and skip-if-empty check: + +```bash +# workflow#758 — strict-semver tag gate. Same regex as plugin release.yml. +if [[ ! "$latest_tag" =~ ^v[0-9]+\.[0-9]+\.[0-9]+(-(alpha|beta|rc)\.?[0-9]+)?$ ]]; then + echo " REJECT $plugin_name — upstream release tag $latest_tag is not release-grade semver" + continue +fi +``` + +**Step 4: Tests pass.** + +**Step 5: Commit.** + +```bash +git add scripts/sync-versions.sh scripts/sync-versions_test.sh +git commit -m "feat(registry): strict-semver gate on upstream release tag (workflow#758) + +Reject ingest of plugins whose upstream GitHub release tag does not match +the release-grade semver whitelist (vN.N.N or vN.N.N-(alpha|beta|rc)[.]N). +Same regex as plugin release.yml gate; both sources of truth converge on +the tag string. Catches plugins that bypass release.yml (manual upload, +self-hosted runner, force-push)." +``` + +--- + +### Task 6: workflow-plugin-digitalocean — direct-push + tag-gate + ldflag wiring (canonical reference) + +**Files in target repo:** +- Modify: `.github/workflows/sync-plugin-version.yml` (replace `gh pr create` with direct-push; add `concurrency:`) +- Modify: `.github/workflows/release.yml` (add tag-format gate as first step; add `concurrency:`; add `check-plugin-contract.sh` call) +- Modify: `cmd/plugin/main.go` (add `sdk.ResolveBuildVersion(internal.Version)` to options) +- Modify: `plugin.json` (bump `minEngineVersion` to `0.61.0`) +- Verify: `.goreleaser.yaml` already has `-X internal.Version=` (it does per DO plugin audit) + +**Step 1: Pre-flight verification** + +``` +grep -n '\\-X .*Version=' .goreleaser.yaml +grep -n 'sdk.ServeIaCPlugin' cmd/plugin/main.go +gh api repos/GoCodeAlone/workflow-plugin-digitalocean/branches/main/protection -q '.enforce_admins.enabled' +``` +Expected: ldflag present; ServeIaCPlugin present; enforce_admins=false → direct-push variant. + +**Step 2: Modify `sync-plugin-version.yml`** + +Add at top: + +```yaml +concurrency: + group: plugin-version-sync-${{ github.repository }} + cancel-in-progress: false +``` + +Replace the `gh pr create` block (lines 56-74) with: + +```yaml + - name: Direct-push plugin.json bump to main + env: + GH_TOKEN: ${{ secrets.RELEASES_TOKEN }} + run: | + if git diff --quiet plugin.json; then + echo "no changes" + exit 0 + fi + git config user.email "github-actions[bot]@users.noreply.github.com" + git config user.name "github-actions[bot]" + git fetch origin main + git pull --ff-only origin main + git add plugin.json + git commit -m "chore: sync plugin.json version to ${{ steps.ver.outputs.tag }}" + git push origin HEAD:main +``` + +**Step 3: Modify `release.yml`** + +Add at top: + +```yaml +concurrency: + group: plugin-version-sync-${{ github.repository }} + cancel-in-progress: false +``` + +Add first job step (before checkout): + +```yaml + - name: Validate tag is release-grade semver + run: | + TAG="${{ github.ref_name }}" + if [[ ! "$TAG" =~ ^v[0-9]+\.[0-9]+\.[0-9]+(-(alpha|beta|rc)\.?[0-9]+)?$ ]]; then + echo "::error::Tag $TAG is not release-grade semver (allowed: vN.N.N or vN.N.N-(alpha|beta|rc)[.]N)" + exit 1 + fi +``` + +Add after checkout, before goreleaser: + +```yaml + - name: Check plugin contract (ldflag + ResolveBuildVersion) + run: | + curl -sSfL https://raw.githubusercontent.com/GoCodeAlone/workflow/main/scripts/check-plugin-contract.sh | bash -s -- "$(pwd)" +``` + +**Step 4: Modify `cmd/plugin/main.go`** + +```go +sdk.ServeIaCPlugin(internal.NewIaCServer(), sdk.IaCServeOptions{ + BuildVersion: sdk.ResolveBuildVersion(internal.Version), +}) +``` + +**Step 5: Bump `plugin.json.minEngineVersion` to `0.61.0`.** + +**Step 6: Local verify** + +``` +GOWORK=off go build ./cmd/plugin +./cmd/plugin/workflow-plugin-digitalocean --version 2>/dev/null || true # smoke; binary doesn't crash +GOWORK=off go test ./... -count=1 +``` + +**Step 7: Commit + push + open PR** + +```bash +git add .github/workflows/sync-plugin-version.yml .github/workflows/release.yml cmd/plugin/main.go plugin.json +git commit -m "chore(release): direct-push sync + tag-gate + ldflag contract (workflow#758) + +- sync-plugin-version.yml: direct-push to main (no PR), concurrency-safe +- release.yml: tag-format gate (release-grade semver) + check-plugin-contract.sh + concurrency +- cmd/plugin/main.go: sdk.ServeIaCPlugin now passes BuildVersion via ResolveBuildVersion +- plugin.json: minEngineVersion 0.57.1 → 0.61.0" + +git push -u origin chore/758-release-discipline + +gh pr create --title "chore(release): direct-push + tag-gate + ldflag wiring (workflow#758)" --body "..." +``` + +**Rollback:** revert commit; sync-plugin-version reverts to PR-opening; release.yml gate removed; main.go reverts to zero-value IaCServeOptions. + +--- + +### Tasks 7-19: Public plugin repos (direct-push variant) + +Each task is a structural clone of Task 6 against one of: + +| Task | Repo | Notes | +|---|---|---| +| 7 | workflow-plugin-aws | per Task-6 template | +| 8 | workflow-plugin-azure | per Task-6 template | +| 9 | workflow-plugin-gcp | per Task-6 template | +| 10 | workflow-plugin-tofu | per Task-6 template | +| 11 | workflow-plugin-ci-generator | per Task-6 template | +| 12 | workflow-plugin-agent | per Task-6 template; may use `sdk.Serve` (non-IaC) — adapt to `WithBuildVersion` option (Task 2's mirror on sdk.Serve if added) | +| 13 | workflow-plugin-auth | per Task-6 template | +| 14 | workflow-plugin-authz | per Task-6 template | +| 15 | workflow-plugin-cms | per Task-6 template | +| 16 | workflow-plugin-compute | per Task-6 template | +| 17 | workflow-plugin-edge-compute | per Task-6 template | +| 18 | workflow-plugin-github | per Task-6 template | +| 19 | workflow-plugin-payments | per Task-6 template | + +Pre-flight verify gates (ldflag, main.go pattern, branch-protection enforce_admins=false) per repo before applying the Task-6 template. If branch protection forbids direct push, switch that repo to the auto-merge variant (Task 20's template). + +--- + +### Tasks 20-28: Private plugin repos (auto-merge variant) + +Per cycle-3 design: private plugins use the auto-merge variant for sync-plugin-version.yml — workflow opens PR + immediately runs `gh pr merge --admin --auto --squash --delete-branch`. The PR exists for audit history; no human in the merge queue. + +| Task | Repo | +|---|---| +| 20 | workflow-plugin-admin | +| 21 | workflow-plugin-authz-ui | +| 22 | workflow-plugin-bento | +| 23 | workflow-plugin-cloud-ui | +| 24 | workflow-plugin-data-protection | +| 25 | workflow-plugin-sandbox | +| 26 | workflow-plugin-security | +| 27 | workflow-plugin-supply-chain | +| 28 | workflow-plugin-waf | + +Auto-merge variant: after `gh pr create`: + +```yaml + - name: Auto-merge sync PR with admin + env: + GH_TOKEN: ${{ secrets.RELEASES_TOKEN }} + run: | + gh pr merge "$BRANCH" --admin --squash --delete-branch --repo ${{ github.repository }} +``` + +All other steps (tag-gate, check-plugin-contract.sh, main.go update, plugin.json minEngineVersion bump) identical to Task 6. + +--- + +### Task 29: Close-out retro + +**Files:** +- Create: `docs/retros/2026-05-NN-workflow-758-plugin-version-discipline.md` + +Document what shipped, what surprised (per repo), and what's deferred (the field-removal path that cycle-1 surfaced as too costly). + +**Verification:** retro file exists; close issue #758 with cross-link. + +```bash +git add docs/retros/2026-05-NN-workflow-758-plugin-version-discipline.md +git commit -m "docs(retro): workflow#758 plugin-version discipline complete" +gh issue close 758 --comment "Shipped per docs/retros/2026-05-NN-workflow-758-plugin-version-discipline.md" +``` + +--- + +## Risk register + +- **R1 (concurrency races):** Multiple tags fired within seconds. Mitigated by `concurrency: group: plugin-version-sync-${{ github.repository }}` + `cancel-in-progress: false` + fast-forward push semantics. +- **R2 (auto-merge race):** auto-merge variant fires `gh pr merge --auto` before required-check completes; second sync workflow opens a stacked PR. Mitigated by per-repo `concurrency:` group serializing both workflows on the same `(repo, group)` key. +- **R3 (direct-push permission revoked mid-rollout):** A repo's branch protection changes between pre-flight verify (Task N step 1) and the actual push. Mitigated by direct-push failing loudly (non-zero exit + Actions log); operator switches that repo to auto-merge variant. +- **R4 (minEngineVersion bump strands operators):** Plugin v_next requires engine ≥0.61.0. Operators on pinned wfctl < v0.61 cannot install. Mitigated by SDK change (Task 2) being additive — older engines fall back to disk plugin.json Version unchanged. The minEngineVersion bump is conservative because the engine change is opt-in (only fires when BuildVersion is non-empty). +- **R5 (workflow-registry sync schedule):** registry sync is cron-driven; PR 2 lands but sync hasn't run yet. The next cron tick catches up. + +## Pipeline gate at end of plan + +**This plan is design-only mode.** After adversarial-design-review (plan phase) PASS + alignment-check PASS + scope-lock applied, the pipeline STOPS. The cross-repo execution (Tasks 5-28) requires explicit user authorization on return. Task 1-4 (workflow SDK PR) and Task 29 (retro) can be authorized independently of the multi-plugin sweep. From fea833e5695fcd3424c5842fa45ca07e1c800ba5 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sat, 23 May 2026 12:31:51 -0400 Subject: [PATCH 05/16] docs(plan): expand collapsed task headings so scope-check passes (workflow#758) --- .../plans/2026-05-23-plugin-version-ldflag.md | 122 +++++++++++++----- 1 file changed, 90 insertions(+), 32 deletions(-) diff --git a/docs/plans/2026-05-23-plugin-version-ldflag.md b/docs/plans/2026-05-23-plugin-version-ldflag.md index 0ba4fc4f..a2466ab9 100644 --- a/docs/plans/2026-05-23-plugin-version-ldflag.md +++ b/docs/plans/2026-05-23-plugin-version-ldflag.md @@ -17,7 +17,7 @@ ## Scope Manifest **PR Count:** 26 -**Tasks:** 27 +**Tasks:** 29 **Estimated Lines of Change:** ~1500 across all PRs (informational; not enforced) **Out of scope:** @@ -487,25 +487,59 @@ gh pr create --title "chore(release): direct-push + tag-gate + ldflag wiring (wo ### Tasks 7-19: Public plugin repos (direct-push variant) -Each task is a structural clone of Task 6 against one of: - -| Task | Repo | Notes | -|---|---|---| -| 7 | workflow-plugin-aws | per Task-6 template | -| 8 | workflow-plugin-azure | per Task-6 template | -| 9 | workflow-plugin-gcp | per Task-6 template | -| 10 | workflow-plugin-tofu | per Task-6 template | -| 11 | workflow-plugin-ci-generator | per Task-6 template | -| 12 | workflow-plugin-agent | per Task-6 template; may use `sdk.Serve` (non-IaC) — adapt to `WithBuildVersion` option (Task 2's mirror on sdk.Serve if added) | -| 13 | workflow-plugin-auth | per Task-6 template | -| 14 | workflow-plugin-authz | per Task-6 template | -| 15 | workflow-plugin-cms | per Task-6 template | -| 16 | workflow-plugin-compute | per Task-6 template | -| 17 | workflow-plugin-edge-compute | per Task-6 template | -| 18 | workflow-plugin-github | per Task-6 template | -| 19 | workflow-plugin-payments | per Task-6 template | - -Pre-flight verify gates (ldflag, main.go pattern, branch-protection enforce_admins=false) per repo before applying the Task-6 template. If branch protection forbids direct push, switch that repo to the auto-merge variant (Task 20's template). +Each task is a structural clone of Task 6 against the listed repo. Pre-flight verification per repo: ldflag present in `.goreleaser.yaml`; `sdk.ServeIaCPlugin` (or `sdk.Serve`) in `cmd/plugin/main.go` (or equivalent); branch protection `enforce_admins: false`. If any check fails, switch that repo to the auto-merge variant (Task 20's template). + +### Task 7: workflow-plugin-aws — direct-push variant per Task 6 + +Same template as Task 6 applied to workflow-plugin-aws. Verify ldflag, main.go pattern, enforce_admins=false pre-flight. Same files, same commit shape. + +### Task 8: workflow-plugin-azure — direct-push variant per Task 6 + +Same as Task 7 against workflow-plugin-azure. + +### Task 9: workflow-plugin-gcp — direct-push variant per Task 6 + +Same as Task 7 against workflow-plugin-gcp. + +### Task 10: workflow-plugin-tofu — direct-push variant per Task 6 + +Same as Task 7 against workflow-plugin-tofu. + +### Task 11: workflow-plugin-ci-generator — direct-push variant per Task 6 + +Same as Task 7 against workflow-plugin-ci-generator. + +### Task 12: workflow-plugin-agent — direct-push variant per Task 6 (non-IaC adaptation) + +Same as Task 7 against workflow-plugin-agent. Agent plugin uses `sdk.Serve` (non-IaC); adapt the main.go update to `sdk.Serve(srv, sdk.WithBuildVersion(sdk.ResolveBuildVersion(internal.Version)))` if Task 2's `WithBuildVersion` option lands on sdk.Serve. If sdk.Serve doesn't yet have the option, file a follow-up before this task ships. + +### Task 13: workflow-plugin-auth — direct-push variant per Task 6 + +Same as Task 7 against workflow-plugin-auth. + +### Task 14: workflow-plugin-authz — direct-push variant per Task 6 + +Same as Task 7 against workflow-plugin-authz. + +### Task 15: workflow-plugin-cms — direct-push variant per Task 6 + +Same as Task 7 against workflow-plugin-cms. + +### Task 16: workflow-plugin-compute — direct-push variant per Task 6 + +Same as Task 7 against workflow-plugin-compute. + +### Task 17: workflow-plugin-edge-compute — direct-push variant per Task 6 + +Same as Task 7 against workflow-plugin-edge-compute. + +### Task 18: workflow-plugin-github — direct-push variant per Task 6 + +Same as Task 7 against workflow-plugin-github. + +### Task 19: workflow-plugin-payments — direct-push variant per Task 6 + +Same as Task 7 against workflow-plugin-payments. --- @@ -513,18 +547,6 @@ Pre-flight verify gates (ldflag, main.go pattern, branch-protection enforce_admi Per cycle-3 design: private plugins use the auto-merge variant for sync-plugin-version.yml — workflow opens PR + immediately runs `gh pr merge --admin --auto --squash --delete-branch`. The PR exists for audit history; no human in the merge queue. -| Task | Repo | -|---|---| -| 20 | workflow-plugin-admin | -| 21 | workflow-plugin-authz-ui | -| 22 | workflow-plugin-bento | -| 23 | workflow-plugin-cloud-ui | -| 24 | workflow-plugin-data-protection | -| 25 | workflow-plugin-sandbox | -| 26 | workflow-plugin-security | -| 27 | workflow-plugin-supply-chain | -| 28 | workflow-plugin-waf | - Auto-merge variant: after `gh pr create`: ```yaml @@ -537,6 +559,42 @@ Auto-merge variant: after `gh pr create`: All other steps (tag-gate, check-plugin-contract.sh, main.go update, plugin.json minEngineVersion bump) identical to Task 6. +### Task 20: workflow-plugin-admin — auto-merge variant per template above + +Same as Task 6 but with auto-merge sync. Verify ldflag + main.go pre-flight. + +### Task 21: workflow-plugin-authz-ui — auto-merge variant per template above + +Same as Task 20 against workflow-plugin-authz-ui. + +### Task 22: workflow-plugin-bento — auto-merge variant per template above + +Same as Task 20 against workflow-plugin-bento. + +### Task 23: workflow-plugin-cloud-ui — auto-merge variant per template above + +Same as Task 20 against workflow-plugin-cloud-ui. + +### Task 24: workflow-plugin-data-protection — auto-merge variant per template above + +Same as Task 20 against workflow-plugin-data-protection. + +### Task 25: workflow-plugin-sandbox — auto-merge variant per template above + +Same as Task 20 against workflow-plugin-sandbox. + +### Task 26: workflow-plugin-security — auto-merge variant per template above + +Same as Task 20 against workflow-plugin-security. + +### Task 27: workflow-plugin-supply-chain — auto-merge variant per template above + +Same as Task 20 against workflow-plugin-supply-chain. + +### Task 28: workflow-plugin-waf — auto-merge variant per template above + +Same as Task 20 against workflow-plugin-waf. + --- ### Task 29: Close-out retro From 6d253117778a14cf657adedc3c0f29e8776fe071 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sat, 23 May 2026 12:38:03 -0400 Subject: [PATCH 06/16] docs(plan): record cycle-1 adversarial findings + pause for user authorization (workflow#758) Cycle 1 found 6 Critical + 9 Important against the plan. Audit table is factually wrong on 8 of 23 listed plugin repos (missing release pipelines). sdk.Serve gap for non-IaC plugins. curl|bash supply-chain risk. branch- nature unmet on go test path. Rebuilding the audit + rescoping is a user-intent-line decision; pause until user returns to authorize rework direction. --- .../plans/2026-05-23-plugin-version-ldflag.md | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/docs/plans/2026-05-23-plugin-version-ldflag.md b/docs/plans/2026-05-23-plugin-version-ldflag.md index a2466ab9..2f5d82e7 100644 --- a/docs/plans/2026-05-23-plugin-version-ldflag.md +++ b/docs/plans/2026-05-23-plugin-version-ldflag.md @@ -625,3 +625,28 @@ gh issue close 758 --comment "Shipped per docs/retros/2026-05-NN-workflow-758-pl ## Pipeline gate at end of plan **This plan is design-only mode.** After adversarial-design-review (plan phase) PASS + alignment-check PASS + scope-lock applied, the pipeline STOPS. The cross-repo execution (Tasks 5-28) requires explicit user authorization on return. Task 1-4 (workflow SDK PR) and Task 29 (retro) can be authorized independently of the multi-plugin sweep. + +## Plan adversarial cycle 1 — FAIL, user authorization required for rework + +Cycle 1 adversarial review surfaced **6 Critical + 9 Important findings** that make the plan-as-written unbuildable. Key findings (full report in chat session transcript): + +- **C1 (audit factually wrong):** filesystem inventory shows 8 of 23 listed plugin repos lack `sync-plugin-version.yml` / `release.yml` / `.goreleaser.yaml` / `main.go`: agent (no main — library), cms, compute, cloud-ui (no main), data-protection (no main), edge-compute, sandbox (no main), waf (no main). Tasks 12, 15, 16, 17, 21, 23, 24, 25, 28 operate on files that do not exist. +- **C2 (hardcoded path):** plan assumes `cmd/plugin/main.go` universal layout. Actual layout is `cmd/workflow-plugin-/main.go` for aws/azure/gcp/auth/authz/cms/github/payments/admin (9 repos). +- **C3 (SDK gap):** `sdk.Serve` (used by non-IaC plugins) has no `WithBuildVersion` ServeOption. Task 2 only modifies `IaCServeOptions`. Non-IaC plugin contracts cannot be reached without expanding Task 2. +- **C4 (supply-chain footgun):** Task 4's `curl https://raw.githubusercontent.com/.../main/scripts/check-plugin-contract.sh | bash` from each plugin's release.yml fetches unsigned, unpinned script from `main` branch. Vendor the script per-repo or SHA-pin. +- **C5 (branch-nature unmet on test path):** `runtime/debug.ReadBuildInfo()` returns no `vcs.*` settings during `go test`, so `ResolveBuildVersion("")` returns bare `"(devel)"`. User said "reflect branch name OR something"; SHA suffix only fires for `go build` non-test contexts. Either explicit goreleaser-time `-X internal.Branch=$(git rev-parse --abbrev-ref HEAD)` injection OR honest scope-limitation note in the design. +- **C6 (Task 2 wiring underspecified):** prose says "wire through ServeIaCPlugin initialization" — no diff line. `delegate` branch construction path not addressed. + +Additional Important findings: tooling decision creep in Task 5 (bats/shellspec), existing permissive tag-gate in DO sync-plugin-version.yml not acknowledged as being replaced, token swap GITHUB_TOKEN→RELEASES_TOKEN unjustified, per-PR bundling violates feedback_implementer_scope_bleed, minEngineVersion bump is soft-warn (R4 over-stated), prose-vs-YAML mismatch on `--auto` flag, no post-rollout gate-fires verification, Task 3 test underspecified, --delete-branch caveat from feedback_stacked_pr_squash_merge_auto_close not noted. + +**Rework direction (recommended on user return):** + +1. Run a real filesystem audit (per-repo `ls .github/workflows/`, `find . -name main.go`, `grep -l 'goreleaser' .goreleaser.*`) and rebuild the 23-row audit table against ground truth. Drop the ~8 repos lacking a release pipeline; file follow-ups for them as separate "establish release pipeline" work. +2. Generalize the main.go path discovery in Task 6 (templating or per-task explicit paths). +3. Expand Task 2 to also add `sdk.WithBuildVersion(string) sdk.ServeOption` for sdk.Serve callers. +4. Replace Task 4's curl|bash with either (a) per-plugin committed copy of the lint script, or (b) a setup-action SHA-pinned to workflow's release tag. +5. Decide between (a) explicit goreleaser-time `internal.Branch` ldflag, or (b) documenting "branch-nature surface is best-effort and SHA-only outside CI builds." +6. Spell out Task 2's bridge constructor wiring as a literal diff. +7. Address I1-I9 in the same revision. + +**Pausing here.** The plan's rebuild crosses the user-intent line (which repos to include is a scope decision that affects the entire migration shape). Execution authorization on return. From ddb69e9070048fb4f8b39536fe4726e9c6593ab7 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sat, 23 May 2026 15:39:06 -0400 Subject: [PATCH 07/16] =?UTF-8?q?docs(plan):=20cycle-4=20simplified=20desi?= =?UTF-8?q?gn=20=E2=80=94=20delete=20sync=20+=20wfctl=20validate-contract?= =?UTF-8?q?=20(workflow#758)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per user direction post plan-cycle-1: collapse to delete-sync-workflow + wfctl subcommand for contract gate. Eliminates curl|bash (C4), audit-mismatch (C1), sdk.Serve gap (C3) by removing the sync-mechanism entirely. Sentinel committed version (v0.0.0-dev) keeps PluginManifest Validate happy; goreleaser before-hook still writes correct version into shipped tarball; registry derives version from upstream tag (verified sync-versions.sh:122,125,169). No engine change, no manifest schema break. --- ...-05-23-plugin-version-discipline-design.md | 213 ++++++++++++++++++ 1 file changed, 213 insertions(+) create mode 100644 docs/plans/2026-05-23-plugin-version-discipline-design.md diff --git a/docs/plans/2026-05-23-plugin-version-discipline-design.md b/docs/plans/2026-05-23-plugin-version-discipline-design.md new file mode 100644 index 00000000..768dc38c --- /dev/null +++ b/docs/plans/2026-05-23-plugin-version-discipline-design.md @@ -0,0 +1,213 @@ +# Plugin version discipline: delete sync mechanism + wfctl contract gate — design + +Issue: GoCodeAlone/workflow#758 +Date: 2026-05-23 (cycle 4 — simplification per user direction after plan-cycle-1) +Mode: autonomous execution authorized + +## Problem + +Per session evidence: + +1. `sync-plugin-version.yml` opens unmerged PRs that pile up (13 stale on DO plugin swept manually 2026-05-23). +2. Cycle-1 ldflag-replacement design (`2026-05-23-plugin-version-ldflag-design.md`) failed adversarial with 3 Critical defects. +3. Cycle-3 restored-contract design passed but plan-cycle-1 found audit factually wrong on 8 of 23 repos + curl|bash supply-chain risk + sdk.Serve API gap. +4. User-direction post plan-cycle-1: lint script should be wfctl functionality; sync mechanism should be eliminated entirely (release tarball already carries correct version via goreleaser `before:` hook; nothing in the release path actually requires committed plugin.json's `version` field). + +## Verified ground truth (re-audited 2026-05-23) + +`workflow-registry/scripts/sync-versions.sh`: + +- Line 122 reads `manifest_version` from registry's own `manifest.json` copy (`$PLUGINS_DIR//manifest.json`), not from plugin repo's committed plugin.json. +- Line 125 derives `latest_version` from `gh release view --json tagName` (upstream git tag). +- Lines 169-183 (`--fix` mode) overwrite registry manifest's `.version` and `.downloads` from tag-derived values, not from plugin repo's committed plugin.json. +- Line 99 `fetch_plugin_json` reads committed plugin.json at the tagged commit for `capabilities + minEngineVersion + iacProvider` ONLY (closes workflow#703). The `.version` field of committed plugin.json is never read. + +**Conclusion:** committed `plugin.json.version` has no consumer in the release/registry pipeline. The sync-plugin-version.yml workflow only synchronizes that field aesthetically; eliminating it changes no observable behavior except removing the PR pileup. + +The same audit confirms: `capabilities`, `minEngineVersion`, `iacProvider` at the tagged commit MUST be correct (registry reads them at tag time). The shipped tarball's plugin.json `.version` MUST be correct — goreleaser `before:` hook already handles this via `{{ .Version }}` template (`.goreleaser.yaml:7-8` in DO plugin). Binary's `internal.Version` MUST be correct — goreleaser `ldflags -X` already handles (`.goreleaser.yaml:25` in DO plugin). + +## Proposed design + +### 1. Delete sync-plugin-version.yml from every plugin repo; sentinel committed version + +The committed `plugin.json.version` becomes a sentinel: `"v0.0.0-dev"`. Parseable semver (zero + pre-release tag), so `PluginManifest.Validate()` passes today with no engine change. Local-install paths (`wfctl plugin install --local `) report the sentinel; operators see the test-build nature. + +For release builds, goreleaser's `before:` hook continues to rewrite `.release/plugin.json` with `{{ .Version }}` from the tag. Shipped tarball carries the correct version. + +Registry sync derives version from tag, unchanged. + +**No code change to engine, SDK, registry script, or wfctl for this piece.** Pure workflow-file deletion + one-line plugin.json edit per plugin repo. + +### 2. Plugin contract surface: SDK changes + +Goal: plugin binary surfaces its build-injected version through `GetManifest` so engine, operator, and observability tools see runtime-truth (not stale disk sentinel). + +Add to `plugin/external/sdk/iacserver.go`: + +```go +type IaCServeOptions struct { + // ... existing fields ... + BuildVersion string +} +``` + +Add to `plugin/external/sdk/serve.go`: + +```go +type serveConfig struct { + // ... existing fields ... + buildVersion string +} + +func WithBuildVersion(v string) ServeOption { + return func(c *serveConfig) { c.buildVersion = v } +} +``` + +Add new file `plugin/external/sdk/buildversion.go`: + +```go +// ResolveBuildVersion returns the operator-visible build-version string. +// declared non-empty + not a dev sentinel → returned as-is. +// Otherwise consults runtime/debug.ReadBuildInfo() and returns +// "(devel) [@ shortsha[.dirty]]" +// when VCS info is available, else "(devel)". +// +// Intended call sites: +// sdk.ServeIaCPlugin(srv, sdk.IaCServeOptions{ +// BuildVersion: sdk.ResolveBuildVersion(internal.Version), +// }) +// sdk.Serve(p, sdk.WithManifestProvider(m), sdk.WithBuildVersion(sdk.ResolveBuildVersion(internal.Version))) +func ResolveBuildVersion(declared string) string { ... } +``` + +Dev sentinels: `""`, `"dev"`, `"(devel)"`, `"v0.0.0-dev"`. If declared matches any, fall through to BuildInfo. + +`iacPluginServiceBridge.GetManifest` (current `plugin/external/sdk/iacserver.go:300`) augmentation: + +```go +out := &pb.Manifest{} +if b.diskManifest != nil { /* existing copy */ } +if b.buildVersion != "" { + out.Version = b.buildVersion // augment / override +} +return out, nil +``` + +`Serve` (non-IaC) bridge similarly: where `GetManifest` returns the manifest, prefer `c.buildVersion` over disk Version when non-empty. + +Engine-side: optional one-shot warning log in `plugin/external/adapter.go` when post-spawn GetManifest's Version differs from `diskManifest.Version`. Pure observability; no behavior change. + +### 3. `wfctl plugin validate-contract` subcommand + +New subcommand under existing `wfctl plugin` family. Replaces the cycle-3 plan's separate `check-plugin-contract.sh` (eliminates curl|bash supply-chain risk; collapses tooling into the binary plugin authors already install via `setup-wfctl`). + +Surface: + +``` +wfctl plugin validate-contract +wfctl plugin validate-contract --for-publish +wfctl plugin validate-contract --for-publish --tag +``` + +Checks (always): + +1. `/plugin.json` exists, parses, passes `PluginManifest.Validate()`. Sentinel `v0.0.0-dev` allowed; emits "dev sentinel" info note. +2. `capabilities` populated (non-empty). +3. `minEngineVersion` populated (parses as semver constraint). +4. `.goreleaser.yaml` or `.goreleaser.yml` at repo root contains a line matching regex `-X .*\.Version=` (any package path). +5. Any `cmd/**/main.go` contains a call to `sdk.ResolveBuildVersion(` OR `sdk.WithBuildVersion(`. + +Additional checks (`--for-publish`): + +6. Tag from `--tag ` flag (if provided) OR from `$GITHUB_REF_NAME` env (if set) OR from `git describe --tags --exact-match HEAD` matches strict-release-semver regex: `^v[0-9]+\.[0-9]+\.[0-9]+(-(alpha|beta|rc)\.?[0-9]+)?$`. +7. Committed plugin.json's `.version` is allowed to disagree with `--tag` (dev sentinel is the documented norm). + +Exit non-zero on any failure with operator-friendly error referencing `docs/PLUGIN_RELEASE_GATES.md`. + +### 4. Tag-format gate in each plugin's `release.yml` + +First steps of every plugin's release.yml: + +```yaml +- uses: GoCodeAlone/setup-wfctl@v1 + with: + version: v0.61.0 # SHA-pinned via setup-wfctl action's release; bump on workflow release +- name: Validate plugin contract for publish + run: wfctl plugin validate-contract --for-publish --tag "${{ github.ref_name }}" . +``` + +Malformed tag or incomplete contract → release halts before goreleaser runs. No bypass mechanism. + +### 5. Registry-side semver gate (defense in depth) + +`workflow-registry/scripts/sync-versions.sh` adds the same tag regex check after `latest_tag` is set: + +```bash +if [[ ! "$latest_tag" =~ ^v[0-9]+\.[0-9]+\.[0-9]+(-(alpha|beta|rc)\.?[0-9]+)?$ ]]; then + echo " REJECT $plugin_name — upstream release tag $latest_tag is not release-grade semver" + continue +fi +``` + +Catches plugins that bypass release.yml (self-hosted runner, manual tarball, force-push). Same regex source as `wfctl plugin validate-contract --for-publish`'s rule 6. + +### 6. Migration ordering + +- **Layer 1 (workflow repo, single PR)**: SDK changes (§2) + `wfctl plugin validate-contract` subcommand (§3) + `docs/PLUGIN_RELEASE_GATES.md`. Tag workflow `v0.61.0`. Update `setup-wfctl` action's version (or rely on `latest`). +- **Layer 2 (workflow-registry repo, single PR)**: tag-string semver gate in `sync-versions.sh` (§5). Can ship in parallel with Layer 1. +- **Layer 3 (per-plugin PRs, parallel)**: in each plugin repo with a release pipeline today: + 1. `git rm .github/workflows/sync-plugin-version.yml` + 2. Add tag-format gate step to `.github/workflows/release.yml` per §4 + 3. Update plugin main.go (or equivalent) to pass `sdk.ResolveBuildVersion(internal.Version)` to `IaCServeOptions.BuildVersion` or `WithBuildVersion` + 4. Set `plugin.json.version` to `"v0.0.0-dev"` (sentinel) + 5. Verify `.goreleaser.yaml` has `-X .*\.Version=` (most do; verify per repo) + 6. Local: `wfctl plugin validate-contract .` must pass before opening PR + 7. Open PR, CI must pass, admin-merge + +Each Layer 3 PR is independent and can run in parallel via per-repo worktree-isolated agents. + +### 7. Gap-repo handling (deferred) + +Repos lacking a release pipeline get one filed issue each: "Establish release pipeline (workflow#758 prerequisite)." Not in Layer 3 scope. + +## Assumptions + +A1. `goreleaser`'s `before:` hook writes the correct version into `.release/plugin.json` for every plugin repo with a release.yml. Verified for DO plugin; per-repo verification step in each Layer 3 PR. +A2. `setup-wfctl` GitHub Action exists and pins to a wfctl version. Verified by workspace memory. +A3. `PluginManifest.Validate()` accepts `v0.0.0-dev` as valid pre-release semver. Verified by reading `plugin/manifest.go:308-355` (`ParseSemver` accepts `0.0.0` + arbitrary `-prerelease` segment). +A4. `wfctl plugin install --local` reads committed plugin.json; reports the sentinel. This is the intended dev-install behavior. +A5. `workflow-registry/scripts/sync-versions.sh` already derives the registry-visible version from upstream git tag. Verified at line 125, 132, 169. +A6. Layer 3 scope ≈ 15 plugin repos with release pipelines today (drops the ~8 gap-repos identified in plan-cycle-1 audit). +A7. The auditor agent in Layer 3 verifies per-repo: release.yml exists, .goreleaser.{yaml,yml} exists, cmd/**/main.go exists, branch-protection allows admin-merge. If any check fails, the agent files a "gap-repo" issue and skips the migration for that repo. + +## Self-challenge — top 3 doubts + +D1. **Sentinel `v0.0.0-dev` fragile to tooling that compares versions numerically.** Mitigation: documented in PLUGIN_RELEASE_GATES.md; registry's sync-versions.sh MISMATCH warning intentionally lights up to remind maintainers (informational, not blocking). +D2. **Losing git-history audit of version progression.** Yes, but git tag log is the authoritative version history; the committed plugin.json changing was redundant ceremony. +D3. **Some non-registry consumer might read committed plugin.json.version.** Audit confirms no such consumer exists today (wfctl validators read from installed-manifest or registry-manifest, both correctly derived from tag). Safe to delete. + +## Rollback + +- §1 (delete sync-plugin-version.yml + sentinel): per-repo revert restores the workflow + reverts plugin.json. PR pileup returns; no other regression. +- §2 (SDK changes): purely additive on `IaCServeOptions` + `sdk.Serve` ServeOption. Plugins that don't set `BuildVersion` keep existing behavior. Revert is single workflow-repo file change. +- §3 (`wfctl plugin validate-contract`): additive subcommand. Existing `wfctl plugin validate` unchanged. +- §4 (tag-format gate in release.yml): per-repo revert removes the step. +- §5 (registry-side gate): single revert in `sync-versions.sh`. + +No state migrations, no plugin-contract breakages, no engine-version cliffs. + +## Test plan + +- workflow Layer 1: unit tests for `sdk.ResolveBuildVersion`, `IaCServeOptions.BuildVersion`, `sdk.WithBuildVersion`, `wfctl plugin validate-contract` (table-driven against testdata fixtures); existing PluginManifest + GetManifest test suites must stay green. +- workflow-registry Layer 2: shell test fixtures for tag-format regex (good + bad cases). +- Per-plugin Layer 3: each repo's existing CI + `wfctl plugin validate-contract .` invocation in release.yml gates the next tag. + +No live infra validation required for this PR set. + +## Out of scope + +- Dropping `plugin.json.version` field entirely (sentinel keeps the field; full removal needs separate design dealing with PluginManifest.Validate's required-field invariant). +- Replacing goreleaser. +- Establishing release pipelines in gap-repos (deferred to per-repo follow-up issues). +- Engine-side hard-blocking minEngineVersion mismatches (existing soft-warn behavior is fine). From 00a637ec98e91aa252996a5a3a3249caa1ae1612 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sat, 23 May 2026 15:47:50 -0400 Subject: [PATCH 08/16] =?UTF-8?q?docs(plan):=20cycle=204-A1=20revisions=20?= =?UTF-8?q?=E2=80=94=20drop=20prerelease=20scope;=20sdk.BuildVersion()=20n?= =?UTF-8?q?o-arg=20(workflow#758)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit C1 fix: sentinel changed v0.0.0-dev → 0.0.0 (verified parses through strict ParseSemver; v0.0.0-dev was rejected as reviewer empirically demonstrated). C2 fix: tag regex tightened to ^v\d+\.\d+\.\d+$ — no prereleases since engine ParseSemver rejects them. C3 fix: sdk.BuildVersion() no-arg helper uses runtime/debug.ReadBuildInfo directly; eliminates internal.Version symbol-path variance across repos. I1/I2/I4/I5/I6 addressed; I3 acknowledged out-of-scope. --- ...-05-23-plugin-version-discipline-design.md | 93 +++++++++++-------- 1 file changed, 55 insertions(+), 38 deletions(-) diff --git a/docs/plans/2026-05-23-plugin-version-discipline-design.md b/docs/plans/2026-05-23-plugin-version-discipline-design.md index 768dc38c..65f8cca3 100644 --- a/docs/plans/2026-05-23-plugin-version-discipline-design.md +++ b/docs/plans/2026-05-23-plugin-version-discipline-design.md @@ -1,7 +1,7 @@ # Plugin version discipline: delete sync mechanism + wfctl contract gate — design Issue: GoCodeAlone/workflow#758 -Date: 2026-05-23 (cycle 4 — simplification per user direction after plan-cycle-1) +Date: 2026-05-23 (cycle 4-revB — verified ParseSemver behavior; dropped prerelease scope; switched to debug.ReadBuildInfo-only) Mode: autonomous execution authorized ## Problem @@ -30,13 +30,15 @@ The same audit confirms: `capabilities`, `minEngineVersion`, `iacProvider` at th ### 1. Delete sync-plugin-version.yml from every plugin repo; sentinel committed version -The committed `plugin.json.version` becomes a sentinel: `"v0.0.0-dev"`. Parseable semver (zero + pre-release tag), so `PluginManifest.Validate()` passes today with no engine change. Local-install paths (`wfctl plugin install --local `) report the sentinel; operators see the test-build nature. +The committed `plugin.json.version` becomes a sentinel: `"0.0.0"`. -For release builds, goreleaser's `before:` hook continues to rewrite `.release/plugin.json` with `{{ .Version }}` from the tag. Shipped tarball carries the correct version. +**Why `0.0.0` and not `v0.0.0-dev`** (cycle 4-A1 C1): empirically verified that this repo's `PluginManifest.ParseSemver` (`plugin/manifest.go:283-303`) does strict `M.m.p` parsing via `strconv.Atoi` on each dot-split segment. Pre-release tags (`v0.0.0-dev`, `v1.2.3-rc.1`) are rejected — `Atoi("0-dev")` fails. The flat `0.0.0` parses cleanly and passes `Validate()` without any engine-side change. Operator-visible test-build nature is delivered via `sdk.BuildVersion()` at runtime (see §2), not via the disk sentinel string. -Registry sync derives version from tag, unchanged. +For release builds, goreleaser's `before:` hook continues to rewrite the shipped plugin.json with `{{ .Version }}` from the tag (per-repo verification step in Layer 3 confirms the invariant; ~50 plugin repos use in-place sed against `plugin.json`, ~4 use `.release/plugin.json` — both patterns satisfy the invariant). Shipped tarball carries the correct version. -**No code change to engine, SDK, registry script, or wfctl for this piece.** Pure workflow-file deletion + one-line plugin.json edit per plugin repo. +Registry sync derives version from tag (`workflow-registry/scripts/sync-versions.sh:125,132,169`), unchanged. The tag-arrival heartbeat that previously came via sync-plugin-version.yml PR-opening is already replaced by the G1 chain shipped 2026-05-21 (plugin tag → notify dispatch → workflow-registry sync); see workspace memory `project_g1234_plugin_release_chain_complete.md`. Heartbeat is not lost. + +**No engine change. No manifest schema change.** Workflow-file deletion + one-line plugin.json edit per plugin repo. ### 2. Plugin contract surface: SDK changes @@ -64,37 +66,38 @@ func WithBuildVersion(v string) ServeOption { } ``` -Add new file `plugin/external/sdk/buildversion.go`: +Add new file `plugin/external/sdk/buildversion.go` (cycle 4-A1 C3 fix — no-arg helper, no `internal.Version` symbol naming required): ```go -// ResolveBuildVersion returns the operator-visible build-version string. -// declared non-empty + not a dev sentinel → returned as-is. -// Otherwise consults runtime/debug.ReadBuildInfo() and returns -// "(devel) [@ shortsha[.dirty]]" -// when VCS info is available, else "(devel)". +// BuildVersion returns the operator-visible build-version string derived +// from runtime/debug.ReadBuildInfo(). For binaries built via goreleaser or +// `go install module@v1.2.3`, returns the release version (info.Main.Version). +// For local `go build` from a worktree, returns "(devel) [@ shortsha[.dirty]]" +// using vcs.revision + vcs.modified settings. For `go test` or non-VCS +// builds, returns "(devel)". // // Intended call sites: -// sdk.ServeIaCPlugin(srv, sdk.IaCServeOptions{ -// BuildVersion: sdk.ResolveBuildVersion(internal.Version), -// }) -// sdk.Serve(p, sdk.WithManifestProvider(m), sdk.WithBuildVersion(sdk.ResolveBuildVersion(internal.Version))) -func ResolveBuildVersion(declared string) string { ... } +// sdk.ServeIaCPlugin(srv, sdk.IaCServeOptions{BuildVersion: sdk.BuildVersion()}) +// sdk.Serve(p, sdk.WithManifestProvider(m), sdk.WithBuildVersion(sdk.BuildVersion())) +// +// No `-X internal.Version=...` ldflag required. Plugin authors do not need +// to name a specific package-level variable. Mirrors the pattern used by +// wfctl itself at cmd/wfctl/main.go:45-50. +func BuildVersion() string { ... } ``` -Dev sentinels: `""`, `"dev"`, `"(devel)"`, `"v0.0.0-dev"`. If declared matches any, fall through to BuildInfo. - -`iacPluginServiceBridge.GetManifest` (current `plugin/external/sdk/iacserver.go:300`) augmentation: +Single channel (cycle 4-A1 I4 fix): `iacPluginServiceBridge.GetManifest` (current `plugin/external/sdk/iacserver.go:300`): ```go out := &pb.Manifest{} if b.diskManifest != nil { /* existing copy */ } if b.buildVersion != "" { - out.Version = b.buildVersion // augment / override + out.Version = b.buildVersion // BuildVersion always wins; precedence explicit + unit-tested } return out, nil ``` -`Serve` (non-IaC) bridge similarly: where `GetManifest` returns the manifest, prefer `c.buildVersion` over disk Version when non-empty. +`Serve` (non-IaC) bridge identical: `WithBuildVersion` value, when set, overrides any embedded manifest's `.version`. No two-channel ambiguity. Engine-side: optional one-shot warning log in `plugin/external/adapter.go` when post-spawn GetManifest's Version differs from `diskManifest.Version`. Pure observability; no behavior change. @@ -112,16 +115,15 @@ wfctl plugin validate-contract --for-publish --tag Checks (always): -1. `/plugin.json` exists, parses, passes `PluginManifest.Validate()`. Sentinel `v0.0.0-dev` allowed; emits "dev sentinel" info note. +1. `/plugin.json` exists, parses, passes `PluginManifest.Validate()`. Sentinel `0.0.0` allowed (parses cleanly through current ParseSemver; emits "dev sentinel" info note). 2. `capabilities` populated (non-empty). 3. `minEngineVersion` populated (parses as semver constraint). -4. `.goreleaser.yaml` or `.goreleaser.yml` at repo root contains a line matching regex `-X .*\.Version=` (any package path). -5. Any `cmd/**/main.go` contains a call to `sdk.ResolveBuildVersion(` OR `sdk.WithBuildVersion(`. +4. Any `cmd/**/main.go` contains a call to `sdk.BuildVersion(` (the new no-arg helper) OR an existing `sdk.ResolveBuildVersion(`/`sdk.WithBuildVersion(` pattern. Goreleaser ldflag check dropped — `BuildVersion()` uses `runtime/debug.ReadBuildInfo()` which works without ldflag injection. Additional checks (`--for-publish`): -6. Tag from `--tag ` flag (if provided) OR from `$GITHUB_REF_NAME` env (if set) OR from `git describe --tags --exact-match HEAD` matches strict-release-semver regex: `^v[0-9]+\.[0-9]+\.[0-9]+(-(alpha|beta|rc)\.?[0-9]+)?$`. -7. Committed plugin.json's `.version` is allowed to disagree with `--tag` (dev sentinel is the documented norm). +5. Tag from `--tag ` flag (if provided) OR from `$GITHUB_REF_NAME` env (if set) OR from `git describe --tags --exact-match HEAD` matches strict-release-semver regex: `^v[0-9]+\.[0-9]+\.[0-9]+$` (cycle 4-A1 C2/I5 fix — no prerelease branch; engine's ParseSemver rejects prereleases, so accepting them in this gate would let unparseable tags through. Prerelease publishing is deferred to a separate design that updates ParseSemver + sync-versions + all consumers in concert). +6. Committed plugin.json's `.version` is allowed to disagree with `--tag` (dev sentinel is the documented norm; tarball-shipped version is what matters). Exit non-zero on any failure with operator-friendly error referencing `docs/PLUGIN_RELEASE_GATES.md`. @@ -144,8 +146,8 @@ Malformed tag or incomplete contract → release halts before goreleaser runs. N `workflow-registry/scripts/sync-versions.sh` adds the same tag regex check after `latest_tag` is set: ```bash -if [[ ! "$latest_tag" =~ ^v[0-9]+\.[0-9]+\.[0-9]+(-(alpha|beta|rc)\.?[0-9]+)?$ ]]; then - echo " REJECT $plugin_name — upstream release tag $latest_tag is not release-grade semver" +if [[ ! "$latest_tag" =~ ^v[0-9]+\.[0-9]+\.[0-9]+$ ]]; then + echo " REJECT $plugin_name — upstream release tag $latest_tag is not release-grade semver (engine ParseSemver requires flat M.m.p)" continue fi ``` @@ -173,19 +175,20 @@ Repos lacking a release pipeline get one filed issue each: "Establish release pi ## Assumptions -A1. `goreleaser`'s `before:` hook writes the correct version into `.release/plugin.json` for every plugin repo with a release.yml. Verified for DO plugin; per-repo verification step in each Layer 3 PR. -A2. `setup-wfctl` GitHub Action exists and pins to a wfctl version. Verified by workspace memory. -A3. `PluginManifest.Validate()` accepts `v0.0.0-dev` as valid pre-release semver. Verified by reading `plugin/manifest.go:308-355` (`ParseSemver` accepts `0.0.0` + arbitrary `-prerelease` segment). -A4. `wfctl plugin install --local` reads committed plugin.json; reports the sentinel. This is the intended dev-install behavior. -A5. `workflow-registry/scripts/sync-versions.sh` already derives the registry-visible version from upstream git tag. Verified at line 125, 132, 169. -A6. Layer 3 scope ≈ 15 plugin repos with release pipelines today (drops the ~8 gap-repos identified in plan-cycle-1 audit). -A7. The auditor agent in Layer 3 verifies per-repo: release.yml exists, .goreleaser.{yaml,yml} exists, cmd/**/main.go exists, branch-protection allows admin-merge. If any check fails, the agent files a "gap-repo" issue and skips the migration for that repo. +A1. `goreleaser`'s `before:` hook writes the correct version into the shipped plugin.json (either in-place or via `.release/plugin.json`) for every plugin repo with a release.yml. ~50 repos use in-place sed; ~4 use `.release/`. Per-repo verification step in each Layer 3 PR asserts the invariant: tarball plugin.json carries `{{ .Version }}`-stamped value. +A2. `setup-wfctl` GitHub Action exists and pins to a wfctl version. Verified by workspace memory; verified by the action being used in DO plugin release.yml today via prior session work. +A3. `PluginManifest.ParseSemver` (`plugin/manifest.go:283-303`) accepts flat `0.0.0` (verified empirically in cycle 4-A1 review). Pre-release tags are rejected by current parser — the sentinel choice + tag regex BOTH avoid prerelease syntax. Full SemVer 2.0.0 support is a deferred follow-up. +A4. `wfctl plugin install --local` reads committed plugin.json; reports the sentinel `0.0.0` to operators. The intended dev-install signal (test-build branch nature) comes from `sdk.BuildVersion()`'s runtime output via GetManifest, not from the disk sentinel. +A5. `workflow-registry/scripts/sync-versions.sh` already derives the registry-visible version from upstream git tag. Verified at lines 122, 125, 132, 169. The `MISMATCH` warning compares registry's local manifest copy against upstream tag — NOT against plugin repo's committed plugin.json (cycle 4-A1 I2 correction). +A6. Layer 3 scope ≈ 15 plugin repos with release pipelines today (drops the ~8 gap-repos identified in plan-cycle-1 audit; per-repo Layer 3 auditor agent confirms gap or proceeds). +A7. Tag-arrival heartbeat (sync-plugin-version.yml PR was the prior signal) is already replaced by the G1 chain shipped 2026-05-21 (plugin tag → notify dispatch → workflow-registry sync). Heartbeat not lost (cycle 4-A1 I6 dismissed). +A8. Goreleaser-built binaries populate `runtime/debug.ReadBuildInfo().Main.Version` correctly (verified by precedent: `cmd/wfctl/main.go:45-50` uses this pattern for wfctl's own version surface). ## Self-challenge — top 3 doubts -D1. **Sentinel `v0.0.0-dev` fragile to tooling that compares versions numerically.** Mitigation: documented in PLUGIN_RELEASE_GATES.md; registry's sync-versions.sh MISMATCH warning intentionally lights up to remind maintainers (informational, not blocking). -D2. **Losing git-history audit of version progression.** Yes, but git tag log is the authoritative version history; the committed plugin.json changing was redundant ceremony. -D3. **Some non-registry consumer might read committed plugin.json.version.** Audit confirms no such consumer exists today (wfctl validators read from installed-manifest or registry-manifest, both correctly derived from tag). Safe to delete. +D1. **Sentinel `0.0.0` looks alarming to consumers that compare versions numerically.** Mitigation: documented in PLUGIN_RELEASE_GATES.md as the intentional dev-sentinel. `sync-versions.sh` is unaffected (it reads the tag, not the committed file — cycle 4-A1 I2 correction). Operator-visible dev nature comes from `sdk.BuildVersion()` runtime output, not the disk value. +D2. **Losing git-history audit of version progression.** Yes, but git tag log is the authoritative version history; the committed plugin.json changing was redundant ceremony. Heartbeat preserved via existing G1 notify-dispatch chain. +D3. **No binary-vs-file capability freshness gate.** Acknowledged out-of-scope per cycle 4-A1 I3. The existing `fetch_plugin_json` path (`sync-versions.sh:99`) reads capabilities from the committed plugin.json at the tag commit; if maintainers forget to update capabilities pre-tag, registry inherits stale values. A future contract-check enhancement could spawn the built binary and diff its `GetContractRegistry` RPC against the committed file — separate design. ## Rollback @@ -211,3 +214,17 @@ No live infra validation required for this PR set. - Replacing goreleaser. - Establishing release pipelines in gap-repos (deferred to per-repo follow-up issues). - Engine-side hard-blocking minEngineVersion mismatches (existing soft-warn behavior is fine). +- Full SemVer 2.0.0 pre-release tag support (requires concerted ParseSemver + sync-versions + wfctl install update; deferred to separate design). +- Binary-vs-file capability freshness gate at contract-check time (cycle 4-A1 I3; deferred to separate design). + +## Cycle 4-A1 — addressed + +- C1 (ParseSemver rejects `v0.0.0-dev`): **addressed** — sentinel changed to flat `0.0.0` which the strict parser accepts; verified empirically. Tag regex tightened to `^v\d+\.\d+\.\d+$` only. +- C2 (regex permits engine-unparseable prerelease tags): **addressed** — prerelease branch dropped from both wfctl validate-contract regex and registry-side regex. +- C3 (`internal.Version` symbol path non-uniform): **addressed** — `sdk.BuildVersion()` no-arg helper uses `runtime/debug.ReadBuildInfo()` directly; plugin authors don't name any specific package-level variable. Contract-check rule reframed to grep for `sdk.BuildVersion(` call site. +- I1 (goreleaser before-hook variance): **addressed** — A1 acknowledges both in-place sed (~50 repos) and `.release/` (~4 repos) patterns; per-repo Layer 3 verification asserts the tarball-invariant, not the path-convention. +- I2 (D1 wrong about MISMATCH lighting up): **addressed** — D1 rewritten; sync-versions.sh's MISMATCH compares against tag, not committed file; sentinel choice has zero observable effect on registry output. +- I3 (binary-vs-file capability freshness gate): **acknowledged out-of-scope**; recorded in "Out of scope" + D3 for future design pickup. +- I4 (two-channel BuildVersion ambiguity): **addressed** — single channel; BuildVersion always wins over diskManifest.Version when non-empty; precedence explicit + unit-tested. +- I5 (unexercised prerelease regex branch): **addressed** — prerelease branch dropped (also addresses C2). +- I6 (lost tag-arrival heartbeat): **addressed** — A7 confirms G1 chain (shipped 2026-05-21) already provides the dispatch path; no replacement signal needed. From f4e012ffa6aaa38b7b2fe61a4730b80251db6981 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sat, 23 May 2026 15:56:57 -0400 Subject: [PATCH 09/16] =?UTF-8?q?docs(plan):=20cycle=204-A2=20revisions=20?= =?UTF-8?q?=E2=80=94=20restore=20ldflag-arg=20+=20sweep=20=C2=A76=20+=20ma?= =?UTF-8?q?tch=20*grpcServer=20(workflow#758)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit N-C1 fix: restored sdk.ResolveBuildVersion(declared) arg-taking helper. Verified goreleaser uses go build (not go install), so Main.Version is pseudo-version, not the tag. Ldflag is the only correct mechanism. N-C2 fix: §6 swept for consistent vocabulary (sentinel 0.0.0; arg-taking helper; ldflag verify present; Version-var-name variance documented). N-I1 fix: design now references actual *grpcServer struct + buildVersion field on it (mirroring diskManifest). N-I2 fix: explicit GetManifest code blocks for both grpc_server.go and iacserver.go showing single-channel precedence. N-I3 fix: --release-dir flag added to validate-contract; Layer 3 release .yml gains post-build tarball verification step. --- ...-05-23-plugin-version-discipline-design.md | 150 ++++++++++++------ 1 file changed, 104 insertions(+), 46 deletions(-) diff --git a/docs/plans/2026-05-23-plugin-version-discipline-design.md b/docs/plans/2026-05-23-plugin-version-discipline-design.md index 65f8cca3..7de7ee9f 100644 --- a/docs/plans/2026-05-23-plugin-version-discipline-design.md +++ b/docs/plans/2026-05-23-plugin-version-discipline-design.md @@ -1,7 +1,7 @@ # Plugin version discipline: delete sync mechanism + wfctl contract gate — design Issue: GoCodeAlone/workflow#758 -Date: 2026-05-23 (cycle 4-revB — verified ParseSemver behavior; dropped prerelease scope; switched to debug.ReadBuildInfo-only) +Date: 2026-05-23 (cycle 4-revC — restored ldflag-arg pattern per cycle-A2 N-C1 verifying go-build vs go-install Main.Version behavior; swept §6 vocabulary; matched actual *grpcServer shape) Mode: autonomous execution authorized ## Problem @@ -42,62 +42,95 @@ Registry sync derives version from tag (`workflow-registry/scripts/sync-versions ### 2. Plugin contract surface: SDK changes -Goal: plugin binary surfaces its build-injected version through `GetManifest` so engine, operator, and observability tools see runtime-truth (not stale disk sentinel). +Goal: plugin binary surfaces its build-injected version through `GetManifest` so engine, operator, and observability tools see runtime-truth. -Add to `plugin/external/sdk/iacserver.go`: +**Cycle 4-A2 N-C1 correction:** goreleaser invokes `go build`, not `go install`. `runtime/debug.ReadBuildInfo().Main.Version` returns a pseudo-version (`v0.0.0--`) or `"(devel)"` for goreleaser-built binaries — NOT the release tag. The ldflag pattern `-X .Version={{.Version}}` is the only mechanism that delivers the tag string at runtime. Restore the cycle-3 arg-taking helper. + +**Symbol-name variance is accepted.** Plugin authors name ANY package-level var (`internal.Version`, `provider.ProviderVersion`, `main.version`). `wfctl plugin validate-contract` greps the goreleaser config for the ldflag pattern, not a specific symbol path. + +Add new file `plugin/external/sdk/buildversion.go`: ```go -type IaCServeOptions struct { - // ... existing fields ... - BuildVersion string -} +// ResolveBuildVersion returns the operator-visible build-version string. +// declared non-empty + not a known dev sentinel → returned as-is (typical +// for goreleaser-built binaries where the ldflag injects the release tag). +// Otherwise consults runtime/debug.ReadBuildInfo() as fallback: +// - "(devel) [@ shortsha[.dirty]]" when vcs.revision is set +// - "(devel)" when no VCS info +// +// Intended call sites (plugin author chooses ANY package-level var name): +// +// var Version = "dev" // somewhere in plugin code; ldflag-injected at release +// +// sdk.ServeIaCPlugin(srv, sdk.IaCServeOptions{ +// BuildVersion: sdk.ResolveBuildVersion(internal.Version), +// }) +// // OR for non-IaC: +// sdk.Serve(p, sdk.WithManifestProvider(m), +// sdk.WithBuildVersion(sdk.ResolveBuildVersion(internal.Version))) +// +// Goreleaser config provides the tag: +// ldflags: +// - -X github.com/<...>/internal.Version={{.Version}} +// +// Dev sentinels (declared values that trigger BuildInfo fallback): `""`, +// `"dev"`, `"(devel)"`. Mirrors the pattern wfctl itself uses +// (cmd/wfctl/main.go:37-50 — `var version = "dev"` with ldflag override +// primary + BuildInfo fallback secondary). +func ResolveBuildVersion(declared string) string { ... } ``` -Add to `plugin/external/sdk/serve.go`: +Add `BuildVersion string` field to `IaCServeOptions` (current `plugin/external/sdk/iacserver.go:320`). + +Add `WithBuildVersion` ServeOption to `plugin/external/sdk/serve.go`. Existing ServeOption shape is `func(*grpcServer)` (verified at `serve.go:16`); add a `buildVersion string` field to the `grpcServer` struct (current `grpc_server.go:21-39` — alongside the existing `diskManifest *pluginpkg.PluginManifest` field): ```go -type serveConfig struct { +// In grpc_server.go's grpcServer struct: +type grpcServer struct { // ... existing fields ... - buildVersion string + diskManifest *pluginpkg.PluginManifest + buildVersion string // workflow#758: runtime version override via WithBuildVersion } +// In serve.go: func WithBuildVersion(v string) ServeOption { - return func(c *serveConfig) { c.buildVersion = v } + return func(s *grpcServer) { + s.buildVersion = v + } } ``` -Add new file `plugin/external/sdk/buildversion.go` (cycle 4-A1 C3 fix — no-arg helper, no `internal.Version` symbol naming required): +Single channel: `grpc_server.go:142-162` `GetManifest` body augmented as below; `iacPluginServiceBridge.GetManifest` (`iacserver.go:300-312`) gets the identical augmentation. BuildVersion always wins when non-empty; precedence explicit and unit-tested. ```go -// BuildVersion returns the operator-visible build-version string derived -// from runtime/debug.ReadBuildInfo(). For binaries built via goreleaser or -// `go install module@v1.2.3`, returns the release version (info.Main.Version). -// For local `go build` from a worktree, returns "(devel) [@ shortsha[.dirty]]" -// using vcs.revision + vcs.modified settings. For `go test` or non-VCS -// builds, returns "(devel)". -// -// Intended call sites: -// sdk.ServeIaCPlugin(srv, sdk.IaCServeOptions{BuildVersion: sdk.BuildVersion()}) -// sdk.Serve(p, sdk.WithManifestProvider(m), sdk.WithBuildVersion(sdk.BuildVersion())) -// -// No `-X internal.Version=...` ldflag required. Plugin authors do not need -// to name a specific package-level variable. Mirrors the pattern used by -// wfctl itself at cmd/wfctl/main.go:45-50. -func BuildVersion() string { ... } +// grpc_server.go GetManifest, after existing diskManifest-or-provider branch: +m := /* existing computed *pb.Manifest from diskManifest or provider.Manifest() */ +if s.buildVersion != "" { + m.Version = s.buildVersion +} +return m, nil ``` -Single channel (cycle 4-A1 I4 fix): `iacPluginServiceBridge.GetManifest` (current `plugin/external/sdk/iacserver.go:300`): - ```go -out := &pb.Manifest{} -if b.diskManifest != nil { /* existing copy */ } +// iacserver.go GetManifest, replacing current line 300-312: +if b.diskManifest == nil { + return nil, status.Error(codes.Unimplemented, "manifest not embedded; engine falls back to disk plugin.json") +} +out := &pb.Manifest{ + Name: b.diskManifest.Name, + Version: b.diskManifest.Version, + Author: b.diskManifest.Author, + Description: b.diskManifest.Description, + ConfigMutable: b.diskManifest.ConfigMutable, + SampleCategory: b.diskManifest.SampleCategory, +} if b.buildVersion != "" { - out.Version = b.buildVersion // BuildVersion always wins; precedence explicit + unit-tested + out.Version = b.buildVersion } return out, nil ``` -`Serve` (non-IaC) bridge identical: `WithBuildVersion` value, when set, overrides any embedded manifest's `.version`. No two-channel ambiguity. +Add `buildVersion string` field to `iacPluginServiceBridge`; wire from `IaCServeOptions.BuildVersion` at construction in `ServeIaCPlugin`. Engine-side: optional one-shot warning log in `plugin/external/adapter.go` when post-spawn GetManifest's Version differs from `diskManifest.Version`. Pure observability; no behavior change. @@ -118,28 +151,44 @@ Checks (always): 1. `/plugin.json` exists, parses, passes `PluginManifest.Validate()`. Sentinel `0.0.0` allowed (parses cleanly through current ParseSemver; emits "dev sentinel" info note). 2. `capabilities` populated (non-empty). 3. `minEngineVersion` populated (parses as semver constraint). -4. Any `cmd/**/main.go` contains a call to `sdk.BuildVersion(` (the new no-arg helper) OR an existing `sdk.ResolveBuildVersion(`/`sdk.WithBuildVersion(` pattern. Goreleaser ldflag check dropped — `BuildVersion()` uses `runtime/debug.ReadBuildInfo()` which works without ldflag injection. +4. Any `cmd/**/main.go` (or other Go file under repo root) contains a call to `sdk.ResolveBuildVersion(` AND a `sdk.IaCServeOptions{...BuildVersion:...}` literal OR a `sdk.WithBuildVersion(` call. +5. Goreleaser config (`.goreleaser.yaml` or `.goreleaser.yml`) at repo root contains an ldflags line matching `-X .*\.Version=` (any package path; e.g. `-X github.com/.../internal.Version={{.Version}}`). This is the mandatory mechanism that delivers the release tag into the binary at build time (cycle 4-A2 N-C1: `debug.ReadBuildInfo` alone returns pseudo-version, not the tag). Additional checks (`--for-publish`): -5. Tag from `--tag ` flag (if provided) OR from `$GITHUB_REF_NAME` env (if set) OR from `git describe --tags --exact-match HEAD` matches strict-release-semver regex: `^v[0-9]+\.[0-9]+\.[0-9]+$` (cycle 4-A1 C2/I5 fix — no prerelease branch; engine's ParseSemver rejects prereleases, so accepting them in this gate would let unparseable tags through. Prerelease publishing is deferred to a separate design that updates ParseSemver + sync-versions + all consumers in concert). -6. Committed plugin.json's `.version` is allowed to disagree with `--tag` (dev sentinel is the documented norm; tarball-shipped version is what matters). +6. Tag from `--tag ` flag (if provided) OR from `$GITHUB_REF_NAME` env (if set) OR from `git describe --tags --exact-match HEAD` matches strict-release-semver regex: `^v[0-9]+\.[0-9]+\.[0-9]+$` (cycle 4-A1 C2/I5 fix — no prerelease branch; engine's ParseSemver rejects prereleases. Prerelease publishing deferred to a separate design that updates ParseSemver + sync-versions + all consumers in concert). +7. Committed plugin.json's `.version` is allowed to disagree with `--tag` (sentinel `0.0.0` is the documented norm; tarball-shipped version is what matters). Exit non-zero on any failure with operator-friendly error referencing `docs/PLUGIN_RELEASE_GATES.md`. +**Tarball-postcheck (`--release-dir `, optional):** when invoked with `--release-dir .release` after goreleaser's before-hook has run, additionally asserts `/plugin.json`'s `.version` field equals the `--tag` value (strips leading `v`). This closes cycle 4-A2 N-I3 by giving Layer 3 release.yml a place to run the tarball-invariant assertion. Layer 3 step 8 in §6 below uses this flag. + ### 4. Tag-format gate in each plugin's `release.yml` -First steps of every plugin's release.yml: +Two steps in every plugin's release.yml — first (pre-build) gates the tag + static contract; second (post-build) verifies the tarball. ```yaml +# 1. Pre-build gate: static contract + tag format - uses: GoCodeAlone/setup-wfctl@v1 with: - version: v0.61.0 # SHA-pinned via setup-wfctl action's release; bump on workflow release -- name: Validate plugin contract for publish + version: v0.61.0 # bump when workflow ships a new validate-contract iteration +- name: Validate plugin contract for publish (pre-build) run: wfctl plugin validate-contract --for-publish --tag "${{ github.ref_name }}" . + +# 2. (goreleaser runs here, mutating plugin.json in-place or writing .release/plugin.json) + +# 3. Post-build gate: tarball carries the tag +- name: Verify shipped plugin.json carries tag (post-build) + run: | + # Find shipped plugin.json (goreleaser pattern variance: in-place or .release/) + if [ -f .release/plugin.json ]; then + wfctl plugin validate-contract --for-publish --tag "${{ github.ref_name }}" --release-dir .release . + else + wfctl plugin validate-contract --for-publish --tag "${{ github.ref_name }}" --release-dir . . + fi ``` -Malformed tag or incomplete contract → release halts before goreleaser runs. No bypass mechanism. +Malformed tag, incomplete contract, or tarball-without-tag → release halts before publish. ### 5. Registry-side semver gate (defense in depth) @@ -160,12 +209,13 @@ Catches plugins that bypass release.yml (self-hosted runner, manual tarball, for - **Layer 2 (workflow-registry repo, single PR)**: tag-string semver gate in `sync-versions.sh` (§5). Can ship in parallel with Layer 1. - **Layer 3 (per-plugin PRs, parallel)**: in each plugin repo with a release pipeline today: 1. `git rm .github/workflows/sync-plugin-version.yml` - 2. Add tag-format gate step to `.github/workflows/release.yml` per §4 - 3. Update plugin main.go (or equivalent) to pass `sdk.ResolveBuildVersion(internal.Version)` to `IaCServeOptions.BuildVersion` or `WithBuildVersion` - 4. Set `plugin.json.version` to `"v0.0.0-dev"` (sentinel) - 5. Verify `.goreleaser.yaml` has `-X .*\.Version=` (most do; verify per repo) - 6. Local: `wfctl plugin validate-contract .` must pass before opening PR - 7. Open PR, CI must pass, admin-merge + 2. Add pre-build + post-build gate steps to `.github/workflows/release.yml` per §4 + 3. Update plugin main.go (or equivalent — any Go file under repo root that calls `sdk.ServeIaCPlugin`/`sdk.Serve`) to pass `sdk.ResolveBuildVersion()` to `IaCServeOptions.BuildVersion` or `WithBuildVersion`. The "Version var" name varies per repo (DO: `internal.Version`; AWS: `provider.ProviderVersion`; etc.); use whichever already exists. If no such var exists yet, add `var Version = "dev"` in the package the goreleaser ldflag targets (per `.goreleaser.yaml`). + 4. Set `plugin.json.version` to `"0.0.0"` (sentinel — flat M.m.p that ParseSemver accepts; cycle 4-A1 C1 fix) + 5. Verify `.goreleaser.yaml` has an ldflag line matching `-X .*\.Version=` (most do; verify per repo; the cycle-3 audit table covers this for DO, AWS, others) + 6. Local: `wfctl plugin validate-contract .` must pass (covers contract rules 1-5 from §3) + 7. Open PR; CI runs `wfctl plugin validate-contract .` again (rule 1-5); on tag push, release.yml's pre-build + post-build gates fire + 8. Admin-merge after CI green Each Layer 3 PR is independent and can run in parallel via per-repo worktree-isolated agents. @@ -217,6 +267,14 @@ No live infra validation required for this PR set. - Full SemVer 2.0.0 pre-release tag support (requires concerted ParseSemver + sync-versions + wfctl install update; deferred to separate design). - Binary-vs-file capability freshness gate at contract-check time (cycle 4-A1 I3; deferred to separate design). +## Cycle 4-A2 — addressed + +- N-C1 (debug.ReadBuildInfo returns pseudo-version, not tag, for goreleaser-built binaries): **addressed** — restored `sdk.ResolveBuildVersion(declared string)` arg-taking helper; goreleaser ldflag mandatory in contract-check rule 5; symbol-name variance accepted (validate-contract greps `.goreleaser.yaml` for ldflag pattern, not a specific Go symbol). +- N-C2 (§6 inconsistent vocabulary): **addressed** — §6 swept; sentinel `0.0.0` everywhere, `ResolveBuildVersion(declared)` everywhere, ldflag verify present, Version-var-name flexibility documented. +- N-I1 (serveConfig struct doesn't exist): **addressed** — design now references actual `*grpcServer` struct + adds `buildVersion string` field there directly (mirroring existing `diskManifest *pluginpkg.PluginManifest` pattern at `grpc_server.go:39`). +- N-I2 (Serve-path precedence shown only in prose): **addressed** — explicit GetManifest code block for both `grpc_server.go` (non-IaC) and `iacserver.go` (IaC) shown in §2, both demonstrating single-channel "BuildVersion wins when non-empty" precedence. +- N-I3 (no tarball-postcheck step): **addressed** — `wfctl plugin validate-contract --release-dir ` flag added; Layer 3 release.yml gains a post-goreleaser verification step asserting `.release/plugin.json` or in-place plugin.json carries the tag. + ## Cycle 4-A1 — addressed - C1 (ParseSemver rejects `v0.0.0-dev`): **addressed** — sentinel changed to flat `0.0.0` which the strict parser accepts; verified empirically. Tag regex tightened to `^v\d+\.\d+\.\d+$` only. From fa293ff8b7ff008516c383e54bdf5f3df11d6c82 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sat, 23 May 2026 16:02:47 -0400 Subject: [PATCH 10/16] =?UTF-8?q?docs(plan):=20cycle-4=20plan=20=E2=80=94?= =?UTF-8?q?=20Layer=201=20+=20Layer=202=20+=20Layer=203=20pilot=205=20repo?= =?UTF-8?q?s=20(workflow#758)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2026-05-23-plugin-version-discipline.md | 579 ++++++++++++++++++ 1 file changed, 579 insertions(+) create mode 100644 docs/plans/2026-05-23-plugin-version-discipline.md diff --git a/docs/plans/2026-05-23-plugin-version-discipline.md b/docs/plans/2026-05-23-plugin-version-discipline.md new file mode 100644 index 00000000..667eeee1 --- /dev/null +++ b/docs/plans/2026-05-23-plugin-version-discipline.md @@ -0,0 +1,579 @@ +# Plugin Version Discipline Implementation Plan + +> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. + +**Goal:** Per workflow#758: stop the sync-plugin-version.yml PR pileup by deleting the workflow; deliver an operator-visible runtime plugin version via SDK + ldflag contract; gate non-semver tags at both `wfctl plugin validate-contract --for-publish` (operator-facing) and `workflow-registry/scripts/sync-versions.sh` (registry ingest). Pilot the per-plugin migration on 5 representative repos; remaining 56 deferred to a follow-up sweep with separate authorization. + +**Architecture:** SDK adds `sdk.ResolveBuildVersion(declared) + IaCServeOptions.BuildVersion + sdk.WithBuildVersion`. New `wfctl plugin validate-contract` subcommand performs static checks (manifest valid, capabilities/minEngineVersion populated, main.go invokes ResolveBuildVersion, goreleaser ldflag pattern present) and tag-format gate. Registry sync rejects non-semver tags as defense in depth. Per-plugin migration: delete sync workflow + sentinel committed version `"0.0.0"` + wire ResolveBuildVersion in main.go + add release.yml pre+post build gates. + +**Tech Stack:** Go 1.26; bash for workflow scripts; GitHub Actions YAML. No new dependencies. + +**Base branch:** main (per repo) + +--- + +## Scope Manifest + +**PR Count:** 9 +**Tasks:** 9 +**Estimated Lines of Change:** ~1200 across all PRs + +**Out of scope:** +- Migrating the remaining 56 plugin repos beyond the pilot batch (filed as follow-up issue; user authorizes separately based on pilot outcome). +- Full SemVer 2.0.0 pre-release tag support (deferred; requires concerted ParseSemver + sync-versions + wfctl install update). +- Binary-vs-file capability freshness gate (deferred per cycle 4-A1 I3). +- Establishing release pipelines in gap-repos (repos without release.yml + .goreleaser). +- Engine-side hard-blocking minEngineVersion mismatches (existing soft-warn behavior is fine). +- Cleaning up the 13 historical stale sync PRs (already done manually 2026-05-23). + +**PR Grouping:** + +| PR # | Title | Tasks | Branch | Repo | +|------|-------|-------|--------|------| +| 1 | feat(sdk+wfctl): ResolveBuildVersion + IaCServeOptions.BuildVersion + WithBuildVersion + wfctl plugin validate-contract (#758) | Task 1 | feat/758-plugin-version-ldflag | workflow | +| 2 | feat(registry): tag-string semver gate in sync-versions.sh (#758) | Task 2 | feat/758-registry-tag-gate | workflow-registry | +| 3 | chore(release): delete sync-plugin-version + wire ResolveBuildVersion + sentinel + release.yml gates (#758) | Task 3 | chore/758-release-discipline | workflow-plugin-digitalocean | +| 4 | chore(release): delete sync-plugin-version + wire ResolveBuildVersion + sentinel + release.yml gates (#758) | Task 4 | chore/758-release-discipline | workflow-plugin-aws | +| 5 | chore(release): delete sync-plugin-version + wire ResolveBuildVersion + sentinel + release.yml gates (#758) | Task 5 | chore/758-release-discipline | workflow-plugin-gcp | +| 6 | chore(release): delete sync-plugin-version + wire ResolveBuildVersion + sentinel + release.yml gates (#758) | Task 6 | chore/758-release-discipline | workflow-plugin-azure | +| 7 | chore(release): delete sync-plugin-version + wire ResolveBuildVersion + sentinel + release.yml gates (#758) | Task 7 | chore/758-release-discipline | workflow-plugin-github | +| 8 | (no PR — file follow-up issue for remaining 56 plugin repo sweep) | Task 8 | n/a | workflow | +| 9 | docs(retro): workflow#758 pilot retro + close issue | Task 9 | docs/758-retro | workflow | + +**Status:** Draft + +--- + +### Task 1: Workflow Layer 1 — SDK + wfctl validate-contract (single PR, single branch) + +**Files in workflow repo:** +- Create: `plugin/external/sdk/buildversion.go` +- Create: `plugin/external/sdk/buildversion_test.go` +- Modify: `plugin/external/sdk/iacserver.go` (IaCServeOptions struct + iacPluginServiceBridge + GetManifest) +- Modify: `plugin/external/sdk/grpc_server.go` (grpcServer struct + GetManifest) +- Modify: `plugin/external/sdk/serve.go` (WithBuildVersion ServeOption) +- Create: `plugin/external/sdk/serve_test.go` additions or `plugin/external/sdk/grpc_server_test.go` additions +- Modify: `plugin/external/adapter.go` (one-shot version-divergence warn log; small) +- Create: `cmd/wfctl/plugin_validate_contract.go` +- Create: `cmd/wfctl/plugin_validate_contract_test.go` +- Create: `cmd/wfctl/testdata/plugin_validate_contract/good/`, `.../bad-missing-caps/`, `.../bad-missing-ldflag/`, `.../bad-tag/`, `.../release-dir-good/`, `.../release-dir-stale/` +- Modify: `cmd/wfctl/plugin.go` (register subcommand) +- Create: `docs/PLUGIN_RELEASE_GATES.md` + +**Step 1: Write failing tests for `ResolveBuildVersion`** + +`plugin/external/sdk/buildversion_test.go`: + +```go +package sdk + +import ( + "strings" + "testing" +) + +func TestResolveBuildVersion_ReleaseDeclaredPassThrough(t *testing.T) { + got := ResolveBuildVersion("v1.2.3") + if got != "v1.2.3" { + t.Errorf("got %q, want v1.2.3", got) + } +} + +func TestResolveBuildVersion_EmptyFallsToBuildInfo(t *testing.T) { + got := ResolveBuildVersion("") + if !strings.HasPrefix(got, "(devel)") { + t.Errorf("got %q, want prefix (devel)", got) + } +} + +func TestResolveBuildVersion_DevSentinelFallsToBuildInfo(t *testing.T) { + got := ResolveBuildVersion("dev") + if !strings.HasPrefix(got, "(devel)") { + t.Errorf("got %q, want prefix (devel) for dev sentinel", got) + } +} + +func TestResolveBuildVersion_DevelSentinelFallsToBuildInfo(t *testing.T) { + got := ResolveBuildVersion("(devel)") + if !strings.HasPrefix(got, "(devel)") { + t.Errorf("got %q, want prefix (devel) for (devel) sentinel", got) + } +} +``` + +**Step 2: Run failing** + +``` +GOWORK=off go test ./plugin/external/sdk/ -run TestResolveBuildVersion -count=1 +``` +Expected: FAIL undefined. + +**Step 3: Implement `ResolveBuildVersion`** + +`plugin/external/sdk/buildversion.go`: + +```go +package sdk + +import ( + "fmt" + "runtime/debug" +) + +// ResolveBuildVersion returns the operator-visible build-version string. +// declared non-empty + not a known dev sentinel → returned as-is (typical +// for goreleaser-built binaries where the ldflag injects the release tag). +// Otherwise consults runtime/debug.ReadBuildInfo() as fallback: +// "(devel) [@ shortsha[.dirty]]" when vcs.revision is set +// "(devel)" when no VCS info +// +// Intended call sites (plugin author chooses ANY package-level Version var name): +// +// var Version = "dev" // ldflag-injected at release +// +// sdk.ServeIaCPlugin(srv, sdk.IaCServeOptions{ +// BuildVersion: sdk.ResolveBuildVersion(internal.Version), +// }) +// sdk.Serve(p, sdk.WithManifestProvider(m), +// sdk.WithBuildVersion(sdk.ResolveBuildVersion(internal.Version))) +// +// Goreleaser config provides the tag: +// ldflags: +// - -X github.com/<...>/internal.Version={{.Version}} +// +// Mirrors the wfctl pattern at cmd/wfctl/main.go:37-50. +func ResolveBuildVersion(declared string) string { + switch declared { + case "", "dev", "(devel)": + return buildInfoVersion() + } + return declared +} + +func buildInfoVersion() string { + info, ok := debug.ReadBuildInfo() + if !ok { + return "(devel)" + } + var sha, modified string + for _, s := range info.Settings { + switch s.Key { + case "vcs.revision": + if len(s.Value) >= 7 { + sha = s.Value[:7] + } else { + sha = s.Value + } + case "vcs.modified": + if s.Value == "true" { + modified = ".dirty" + } + } + } + if sha == "" { + return "(devel)" + } + return fmt.Sprintf("(devel) [@ %s%s]", sha, modified) +} +``` + +**Step 4: Pass** + +``` +GOWORK=off go test ./plugin/external/sdk/ -run TestResolveBuildVersion -count=1 +``` + +**Step 5: Add `BuildVersion` field to `IaCServeOptions` + bridge wiring** + +In `plugin/external/sdk/iacserver.go`: + +- Add `BuildVersion string` to `IaCServeOptions` struct (current ~line 320). +- Add `buildVersion string` to `iacPluginServiceBridge` struct. +- Wire in `registerAllIaCProviderServicesWithOpts` (current ~line 87-90 area): set bridge's `buildVersion` from `opts.BuildVersion`. +- Modify `iacPluginServiceBridge.GetManifest` (current line 300-312): + +```go +func (b *iacPluginServiceBridge) GetManifest(_ context.Context, _ *emptypb.Empty) (*pb.Manifest, error) { + if b.diskManifest == nil && b.buildVersion == "" { + return nil, status.Error(codes.Unimplemented, "manifest not embedded; engine falls back to disk plugin.json") + } + out := &pb.Manifest{} + if b.diskManifest != nil { + out.Name = b.diskManifest.Name + out.Version = b.diskManifest.Version + out.Author = b.diskManifest.Author + out.Description = b.diskManifest.Description + out.ConfigMutable = b.diskManifest.ConfigMutable + out.SampleCategory = b.diskManifest.SampleCategory + } + if b.buildVersion != "" { + out.Version = b.buildVersion + } + return out, nil +} +``` + +**Step 6: Test bridge augmentation** + +In `plugin/external/sdk/iacserver_internal_test.go` (or new file): + +```go +func TestIaCBridge_GetManifest_BuildVersionOverridesDiskVersion(t *testing.T) { + disk := &pluginpkg.PluginManifest{Name: "x", Version: "v1.0.0", Author: "a", Description: "d"} + b := &iacPluginServiceBridge{diskManifest: disk, buildVersion: "v1.0.1"} + got, err := b.GetManifest(context.Background(), &emptypb.Empty{}) + if err != nil { t.Fatal(err) } + if got.Version != "v1.0.1" { + t.Errorf("Version = %q, want BuildVersion-augmented v1.0.1", got.Version) + } +} + +func TestIaCBridge_GetManifest_NoBuildVersionFallsToDiskVersion(t *testing.T) { + disk := &pluginpkg.PluginManifest{Name: "x", Version: "v1.0.0", Author: "a", Description: "d"} + b := &iacPluginServiceBridge{diskManifest: disk} + got, _ := b.GetManifest(context.Background(), &emptypb.Empty{}) + if got.Version != "v1.0.0" { + t.Errorf("Version = %q, want disk v1.0.0", got.Version) + } +} +``` + +Run + iterate until passes. + +**Step 7: Add `WithBuildVersion` ServeOption + grpcServer wiring** + +In `plugin/external/sdk/grpc_server.go`: + +- Add `buildVersion string` field to `grpcServer` struct (current line 21-39, alongside `diskManifest`). +- Modify `GetManifest` (current line 142-162): after the existing `if s.diskManifest != nil { ... } else { provider.Manifest() }` block computes the manifest, append `if s.buildVersion != "" { m.Version = s.buildVersion }` before return. + +In `plugin/external/sdk/serve.go`: + +```go +// WithBuildVersion sets the runtime build-version surfaced via GetManifest. +// Single-channel: takes precedence over any ManifestProvider.Version or +// provider.Manifest().Version. Typically populated via +// sdk.ResolveBuildVersion(). +func WithBuildVersion(v string) ServeOption { + return func(s *grpcServer) { + s.buildVersion = v + } +} +``` + +**Step 8: Test Serve-path augmentation** + +```go +func TestGRPCServer_GetManifest_BuildVersionOverridesDiskVersion(t *testing.T) { + s := &grpcServer{ + diskManifest: &pluginpkg.PluginManifest{Name: "x", Version: "v1.0.0", Author: "a", Description: "d"}, + buildVersion: "v1.0.1", + } + got, _ := s.GetManifest(context.Background(), &emptypb.Empty{}) + if got.Version != "v1.0.1" { + t.Errorf("Version = %q, want v1.0.1", got.Version) + } +} +``` + +**Step 9: Add `wfctl plugin validate-contract` subcommand** + +`cmd/wfctl/plugin_validate_contract.go`: new subcommand implementing the §3 design checks. Flags: `--for-publish`, `--tag`, `--release-dir`. Checks: +1. plugin.json exists + parses + Validate() OK (sentinel `0.0.0` OK) +2. capabilities populated +3. minEngineVersion populated +4. main.go (any `cmd/**/main.go` or `.go` file in repo root) contains `sdk.ResolveBuildVersion(` AND (`IaCServeOptions{` with `BuildVersion:` OR `sdk.WithBuildVersion(`) +5. `.goreleaser.{yaml,yml}` contains regex `-X .*\.Version=` +6. (--for-publish) Tag matches `^v[0-9]+\.[0-9]+\.[0-9]+$` +7. (--release-dir) `/plugin.json` `.version` field equals `--tag` value with leading `v` stripped. + +Register in `cmd/wfctl/plugin.go`. + +**Step 10: testdata fixtures + table-driven tests** + +`cmd/wfctl/testdata/plugin_validate_contract/`: +- `good/plugin.json` (sentinel `0.0.0`, capabilities populated, minEngineVersion populated) +- `good/cmd/plugin/main.go` (calls sdk.ResolveBuildVersion + IaCServeOptions BuildVersion) +- `good/.goreleaser.yaml` (has `-X test/internal.Version=`) +- `bad-missing-caps/plugin.json` (no capabilities) +- `bad-missing-ldflag/.goreleaser.yaml` (no -X line) +- `bad-tag/` (good + invokes --tag v1.2 → fails) +- `release-dir-good/plugin.json` (.version = "1.2.3" → --tag v1.2.3 passes) +- `release-dir-stale/plugin.json` (.version = "1.2.0" → --tag v1.2.3 fails) + +`cmd/wfctl/plugin_validate_contract_test.go`: table-driven test invoking `runPluginValidateContract` against each fixture. + +**Step 11: Create `docs/PLUGIN_RELEASE_GATES.md`** + +Document the contract, the sentinel pattern, the goreleaser ldflag requirement, the release.yml two-step gate pattern (pre-build static checks + post-build tarball verify). Reference workflow#758. + +**Step 12: Run full test sweep** + +``` +GOWORK=off go test ./plugin/external/sdk/... ./cmd/wfctl/... -count=1 -race +``` +Must be green. + +**Step 13: Commit (single squash; multiple commit chunks fine on the branch)** + +```bash +git add plugin/external/sdk/buildversion.go plugin/external/sdk/buildversion_test.go +git commit -m "feat(sdk): ResolveBuildVersion helper for ldflag + buildinfo (#758)" + +git add plugin/external/sdk/iacserver.go plugin/external/sdk/grpc_server.go plugin/external/sdk/serve.go plugin/external/sdk/*_test.go +git commit -m "feat(sdk): IaCServeOptions.BuildVersion + WithBuildVersion ServeOption (#758)" + +git add plugin/external/adapter.go plugin/external/adapter_test.go 2>/dev/null +git commit -m "feat(adapter): warn on disk-vs-runtime plugin version divergence (#758)" 2>/dev/null || true + +git add cmd/wfctl/plugin_validate_contract.go cmd/wfctl/plugin_validate_contract_test.go cmd/wfctl/plugin.go cmd/wfctl/testdata/plugin_validate_contract/ +git commit -m "feat(wfctl): plugin validate-contract subcommand + --for-publish + --release-dir (#758)" + +git add docs/PLUGIN_RELEASE_GATES.md +git commit -m "docs: PLUGIN_RELEASE_GATES.md (#758)" +``` + +**Step 14: Push + PR + monitor + admin-merge** + +```bash +git push -u origin feat/758-plugin-version-ldflag +gh pr create --title "feat(sdk+wfctl): ResolveBuildVersion + WithBuildVersion + plugin validate-contract (#758)" --body "...long form summary..." +gh pr checks --watch # wait for CI green +gh pr merge --squash --admin --delete-branch +``` + +**Step 15: Tag workflow v0.61.0** + +```bash +git checkout main && git pull --ff-only +git tag v0.61.0 -m "v0.61.0 — plugin release-gate SDK + wfctl validate-contract (#758)" +git push origin v0.61.0 +``` + +**Rollback:** revert the PR; SDK reverts to pre-758 state; existing plugins keep working (changes are additive). + +--- + +### Task 2: workflow-registry Layer 2 — tag-string semver gate + +**Files in workflow-registry:** +- Modify: `scripts/sync-versions.sh` (gate after `latest_tag` set at line ~125) +- Create: `scripts/testdata/tag-gate/` (optional fixtures) + +**Step 1: Add gate** + +After `latest_tag="$(gh release view ...)"` skip-empty check: + +```bash +# workflow#758 — strict-semver gate. +if [[ ! "$latest_tag" =~ ^v[0-9]+\.[0-9]+\.[0-9]+$ ]]; then + echo " REJECT $plugin_name — upstream release tag $latest_tag is not release-grade semver (engine ParseSemver requires flat M.m.p)" + continue +fi +``` + +**Step 2: Test (manual / inline)** + +Invoke against a dry-run plugin entry with malformed tag in test fixture; assert REJECT line. + +**Step 3: Commit + PR + admin-merge** + +```bash +git checkout -b feat/758-registry-tag-gate +git add scripts/sync-versions.sh +git commit -m "feat(registry): strict-semver gate on upstream release tag (#758) + +Reject ingest of plugins whose upstream GitHub release tag does not match +the release-grade semver whitelist (^v\d+\.\d+\.\d+\$). Catches plugins +that bypass release.yml (manual upload, self-hosted runner, force-push)." +git push -u origin feat/758-registry-tag-gate +gh pr create ... +gh pr merge --squash --admin --delete-branch +``` + +**Rollback:** single-revert. + +--- + +### Task 3: workflow-plugin-digitalocean — pilot Layer 3 (canonical) + +**Files in target repo:** +- Delete: `.github/workflows/sync-plugin-version.yml` +- Modify: `.github/workflows/release.yml` (add setup-wfctl + pre-build validate-contract + post-build verify) +- Modify: `cmd/plugin/main.go` (pass `sdk.ResolveBuildVersion(internal.Version)` to `IaCServeOptions.BuildVersion`) +- Modify: `plugin.json` (`.version` → `"0.0.0"`) +- Verify (no edit): `.goreleaser.yaml` has `-X github.com/.../internal.Version=` (already present at line 25) + +**Step 1: Pre-flight audit** + +```bash +ls .github/workflows/sync-plugin-version.yml && \ +ls .github/workflows/release.yml && \ +ls .goreleaser.yaml && \ +ls cmd/plugin/main.go && \ +grep -q '\-X.*\.Version=' .goreleaser.yaml && \ +grep -q 'sdk.ServeIaCPlugin' cmd/plugin/main.go && \ +gh api repos/GoCodeAlone/workflow-plugin-digitalocean/branches/main/protection -q '.enforce_admins.enabled' | grep -q '^false$' && \ +echo OK || echo FAIL +``` +Expected: OK. + +**Step 2: Apply migration** + +```bash +git checkout -b chore/758-release-discipline +git rm .github/workflows/sync-plugin-version.yml +``` + +Edit `cmd/plugin/main.go`: + +```go +package main + +import ( + "github.com/GoCodeAlone/workflow-plugin-digitalocean/internal" + sdk "github.com/GoCodeAlone/workflow/plugin/external/sdk" +) + +func main() { + sdk.ServeIaCPlugin(internal.NewIaCServer(), sdk.IaCServeOptions{ + BuildVersion: sdk.ResolveBuildVersion(internal.Version), + }) +} +``` + +Edit `plugin.json` `.version` field to `"0.0.0"`. + +Edit `.github/workflows/release.yml` — add at top of `release:` job (before checkout): + +```yaml + - uses: GoCodeAlone/setup-wfctl@v1 + with: + version: v0.61.0 + - name: Validate plugin contract for publish (pre-build) + run: | + wfctl plugin validate-contract --for-publish --tag "${{ github.ref_name }}" . +``` + +Add after goreleaser step: + +```yaml + - name: Verify shipped plugin.json carries tag (post-build) + run: | + if [ -f .release/plugin.json ]; then + wfctl plugin validate-contract --for-publish --tag "${{ github.ref_name }}" --release-dir .release . + else + wfctl plugin validate-contract --for-publish --tag "${{ github.ref_name }}" --release-dir . . + fi +``` + +**Step 3: Local verify (with locally-built wfctl@v0.61.0 or main HEAD)** + +```bash +GOWORK=off go build ./... +GOWORK=off go test ./... -count=1 +# wfctl plugin validate-contract . (requires wfctl built from workflow main; OK to skip if local wfctl not bumped yet) +``` + +**Step 4: Commit + push + PR + admin-merge** + +```bash +git add -A +git commit -m "chore(release): delete sync-plugin-version + ResolveBuildVersion + sentinel + release.yml gates (#758) + +- Delete sync-plugin-version.yml (committed plugin.json version no longer + syncs with tag; goreleaser before-hook + binary ldflag carry the truth) +- plugin.json.version → \"0.0.0\" (sentinel; parses through ParseSemver) +- cmd/plugin/main.go: pass sdk.ResolveBuildVersion(internal.Version) +- release.yml: pre-build wfctl plugin validate-contract --for-publish + + post-build --release-dir verification + +Rollback: revert PR; restores prior sync mechanism." +git push -u origin chore/758-release-discipline +gh pr create --title "chore(release): plugin-version discipline (#758)" --body "..." +gh pr checks --watch +gh pr merge --squash --admin --delete-branch +``` + +--- + +### Task 4: workflow-plugin-aws — pilot Layer 3 per Task 3 template + +Same as Task 3 against `workflow-plugin-aws`. Pre-flight audit confirms files exist; per-repo Version var location may differ (AWS uses `cmd/workflow-plugin-aws/main.go` not `cmd/plugin/main.go`); apply same edits to whatever main.go calls `sdk.ServeIaCPlugin`. Verify `.goreleaser.yaml` ldflag points at the right symbol; if AWS uses `provider.ProviderVersion` rather than `internal.Version`, pass that to `ResolveBuildVersion`. + +### Task 5: workflow-plugin-gcp — pilot Layer 3 per Task 3 template + +Same as Task 4 against `workflow-plugin-gcp`. + +### Task 6: workflow-plugin-azure — pilot Layer 3 per Task 3 template + +Same as Task 4 against `workflow-plugin-azure`. + +### Task 7: workflow-plugin-github — pilot Layer 3 per Task 3 template + +Same as Task 4 against `workflow-plugin-github`. github plugin is non-IaC (uses `sdk.Serve` + `WithManifestProvider`) — adapt main.go to add `sdk.WithBuildVersion(sdk.ResolveBuildVersion(...))` as a ServeOption. + +--- + +### Task 8: File follow-up issue for remaining 56 plugin repos + +**Files:** none (uses `gh issue create` only). + +Audit identified 61 plugin repos with full release pipelines. Pilot covers DO + AWS + GCP + Azure + github (5 repos). Remaining 56 need the same migration via parallel agent fan-out, but user authorization required to commit to a 56-PR sweep. + +**Step 1: File issue** + +```bash +gh issue create --title "Apply plugin-version discipline migration to remaining 56 plugin repos (#758 follow-up)" --body "$(cat <<'EOF' +## Context + +workflow#758 pilot landed in DO, AWS, GCP, Azure, github plugin repos. +Pattern is mechanical (delete sync-plugin-version.yml + plugin.json sentinel ++ main.go ResolveBuildVersion call + release.yml pre+post gates) and +should fan-out cleanly via parallel agents. + +## Remaining repos (56) + +(enumerate: workflow-plugin-admin, agent, analytics, approval, audit, +audit-chain, audit-chain-docs, auth, authz, authz-ui, bento, botdetect, +broker, ci-generator, cicd, cms, compute, crm, data-engineering, +data-protection, datadog, discord, dnd, economy, erp, eventbus, gitlab, +infra, ... — full list at exec time via `ls workflow-plugin-*` minus pilot) + +## Plan + +After pilot lands cleanly, request user authorization to dispatch parallel +agents (worktree-isolated per repo) to apply the canonical Task 3 +template. Each agent: pre-flight audit + 4-file edit + push + PR + monitor ++ admin-merge. + +Estimated effort: 56 PRs, ~1 hour wall time with 5-10 agents in parallel. +EOF +)" +``` + +**Step 2: Save issue number for retro reference.** + +--- + +### Task 9: Close-out retro + +**Files:** +- Create: `docs/retros/2026-05-NN-workflow-758-plugin-version-discipline.md` + +Document pilot outcome; any per-repo variance discovered; recommended adjustments before fan-out. + +```bash +git checkout -b docs/758-retro main +git add docs/retros/2026-05-NN-workflow-758-plugin-version-discipline.md +git commit -m "docs(retro): workflow#758 plugin-version discipline pilot complete" +git push -u origin docs/758-retro +gh pr create ... && gh pr merge --squash --admin --delete-branch +gh issue close 758 --comment "Pilot shipped; follow-up # tracks remaining sweep." +``` + +--- + +## Pipeline gate at end of plan + +This plan executes Layer 1 + Layer 2 + Layer 3 pilot (5 plugins) + follow-up issue + retro autonomously. Layer 3b (remaining 56 plugins) is filed as a follow-up issue requiring separate user authorization based on pilot outcome. From 2a0c6bedf8ca8b90e863ddc952ebfd564851566d Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sat, 23 May 2026 16:07:05 -0400 Subject: [PATCH 11/16] docs(plan): apply plan-cycle-4-P1 Important fixes inline (workflow#758) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I1: post-build verify step placement clarified (between goreleaser + Publish) I2: hard-ordering gate documented for v0.61.0 tag → Layer 3 dispatch I3: Task 7 github clarified (current main.go has no ServeOption; migration adds first) I4: pre-flight audit grep updated to accept Serve|ServeIaCPlugin I5: AWS Task 4 spells out var-existence audit + import-add requirement I6: validate-contract rule 4 spec'd as whole-file scoped + multi-binary repos --- .../2026-05-23-plugin-version-discipline.md | 43 ++++++++++++++----- 1 file changed, 33 insertions(+), 10 deletions(-) diff --git a/docs/plans/2026-05-23-plugin-version-discipline.md b/docs/plans/2026-05-23-plugin-version-discipline.md index 667eeee1..e5a05fc3 100644 --- a/docs/plans/2026-05-23-plugin-version-discipline.md +++ b/docs/plans/2026-05-23-plugin-version-discipline.md @@ -409,14 +409,15 @@ gh pr merge --squash --admin --delete-branch ```bash ls .github/workflows/sync-plugin-version.yml && \ ls .github/workflows/release.yml && \ -ls .goreleaser.yaml && \ -ls cmd/plugin/main.go && \ -grep -q '\-X.*\.Version=' .goreleaser.yaml && \ -grep -q 'sdk.ServeIaCPlugin' cmd/plugin/main.go && \ -gh api repos/GoCodeAlone/workflow-plugin-digitalocean/branches/main/protection -q '.enforce_admins.enabled' | grep -q '^false$' && \ +(ls .goreleaser.yaml || ls .goreleaser.yml) && \ +(MAIN=$(find cmd -name main.go | head -1); [ -n "$MAIN" ] && echo "main: $MAIN") && \ +grep -qE '\-X.*\.Version=' .goreleaser.yaml .goreleaser.yml 2>/dev/null && \ +grep -qE 'sdk\.(Serve|ServeIaCPlugin)' $(find cmd -name main.go) && \ +grep -rqE 'var (Version|ProviderVersion)\b' . --include='*.go' && \ +gh api repos/GoCodeAlone//branches/main/protection -q '.enforce_admins.enabled' | grep -q '^false$' && \ echo OK || echo FAIL ``` -Expected: OK. +Expected: OK. (Variations per repo: non-IaC uses `sdk.Serve` not `ServeIaCPlugin`; main.go path varies (`cmd/plugin/main.go` vs `cmd/workflow-plugin-/main.go`); Version var may live in `internal` or `provider` package. The audit accepts variance and just verifies presence.) **Step 2: Apply migration** @@ -455,7 +456,7 @@ Edit `.github/workflows/release.yml` — add at top of `release:` job (before ch wfctl plugin validate-contract --for-publish --tag "${{ github.ref_name }}" . ``` -Add after goreleaser step: +Add BETWEEN the goreleaser step and the `Publish release (was draft during asset upload)` step (so a verify-fail halts before the draft→public promotion): ```yaml - name: Verify shipped plugin.json carries tag (post-build) @@ -499,7 +500,12 @@ gh pr merge --squash --admin --delete-branch ### Task 4: workflow-plugin-aws — pilot Layer 3 per Task 3 template -Same as Task 3 against `workflow-plugin-aws`. Pre-flight audit confirms files exist; per-repo Version var location may differ (AWS uses `cmd/workflow-plugin-aws/main.go` not `cmd/plugin/main.go`); apply same edits to whatever main.go calls `sdk.ServeIaCPlugin`. Verify `.goreleaser.yaml` ldflag points at the right symbol; if AWS uses `provider.ProviderVersion` rather than `internal.Version`, pass that to `ResolveBuildVersion`. +Same as Task 3 against `workflow-plugin-aws`, with these differences: + +- main.go lives at `cmd/workflow-plugin-aws/main.go` (not `cmd/plugin/main.go`). +- AWS `.goreleaser.yaml` injects BOTH `provider.ProviderVersion` AND `internal.Version`. Check which package-level vars actually exist (`grep -rn 'var \(Version\|ProviderVersion\)' provider/ internal/`). If neither exists, current goreleaser-built binaries silently ship `(devel)` — declare a `var Version = "dev"` in the package the migration will reference (recommend `internal/` for parity with DO). +- Pass the existing var to `sdk.ResolveBuildVersion(...)`. Migration must add an import for that package in main.go. +- Pre-flight audit (Step 1) accepts variance. ### Task 5: workflow-plugin-gcp — pilot Layer 3 per Task 3 template @@ -509,9 +515,22 @@ Same as Task 4 against `workflow-plugin-gcp`. Same as Task 4 against `workflow-plugin-azure`. -### Task 7: workflow-plugin-github — pilot Layer 3 per Task 3 template +### Task 7: workflow-plugin-github — pilot Layer 3 per Task 3 template (non-IaC adaptation) + +Same as Task 4 against `workflow-plugin-github`, with these differences: + +- github plugin is non-IaC; current main.go calls `sdk.Serve(internal.NewGitHubPlugin())` with NO ServeOption args. +- Migration changes main.go to: + + ```go + sdk.Serve(internal.NewGitHubPlugin(), + sdk.WithBuildVersion(sdk.ResolveBuildVersion(internal.Version)), + ) + ``` -Same as Task 4 against `workflow-plugin-github`. github plugin is non-IaC (uses `sdk.Serve` + `WithManifestProvider`) — adapt main.go to add `sdk.WithBuildVersion(sdk.ResolveBuildVersion(...))` as a ServeOption. +- If `var Version = "dev"` doesn't yet exist in the package the goreleaser ldflag targets (likely `internal`), add it as part of the migration. Verify by reading `.goreleaser.yaml` ldflag line; e.g. `-X github.com/GoCodeAlone/workflow-plugin-github/internal.Version={{.Version}}` means add `var Version = "dev"` to `internal/`. + +- Pre-flight audit (Step 1) accepts `sdk.Serve` per the updated grep `sdk\.(Serve|ServeIaCPlugin)`. --- @@ -577,3 +596,7 @@ gh issue close 758 --comment "Pilot shipped; follow-up # tracks remaining swe ## Pipeline gate at end of plan This plan executes Layer 1 + Layer 2 + Layer 3 pilot (5 plugins) + follow-up issue + retro autonomously. Layer 3b (remaining 56 plugins) is filed as a follow-up issue requiring separate user authorization based on pilot outcome. + +**Hard ordering gate (cycle 4-P1 I2):** Tasks 3-7 (Layer 3 pilot PRs) depend on workflow v0.61.0 being TAGGED + reachable (`gh release view v0.61.0 --repo GoCodeAlone/workflow` returns non-error). Task 1 Step 15 creates that tag immediately after Task 1's PR merges. Layer 3 dispatch MUST wait for that step. Task 2 (workflow-registry) has no such dependency and may run in parallel with Task 1. + +**Implementation note for `wfctl plugin validate-contract` rule 4 (cycle 4-P1 I6):** the grep for `sdk.ResolveBuildVersion(` AND (`IaCServeOptions{...BuildVersion:` OR `sdk.WithBuildVersion(`) MUST be whole-file scoped, not line-scoped (gofmt formats multi-line). When a repo has multiple `cmd/**/main.go` binaries, rule 4 PASSES if ANY of them satisfies both patterns (typical: only the plugin's serve binary does; other cmds are operator tools). From 4f9227e20b3525823bb5e1fd69efb5124cc73777 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sat, 23 May 2026 16:08:43 -0400 Subject: [PATCH 12/16] =?UTF-8?q?docs(plan):=20alignment-check=20nits=20?= =?UTF-8?q?=E2=80=94=20clarify=20PR-count=20+=20tag-source=20fallback=20(w?= =?UTF-8?q?orkflow#758)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/plans/2026-05-23-plugin-version-discipline.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/plans/2026-05-23-plugin-version-discipline.md b/docs/plans/2026-05-23-plugin-version-discipline.md index e5a05fc3..40749262 100644 --- a/docs/plans/2026-05-23-plugin-version-discipline.md +++ b/docs/plans/2026-05-23-plugin-version-discipline.md @@ -16,6 +16,8 @@ **PR Count:** 9 **Tasks:** 9 + +(Note: PR 8 is a no-PR row — it produces a follow-up issue only. Effective code/doc PRs = 8.) **Estimated Lines of Change:** ~1200 across all PRs **Out of scope:** @@ -278,6 +280,8 @@ func TestGRPCServer_GetManifest_BuildVersionOverridesDiskVersion(t *testing.T) { **Step 9: Add `wfctl plugin validate-contract` subcommand** +Tag source precedence for `--for-publish` (check 6): explicit `--tag ` flag > `$GITHUB_REF_NAME` env (set automatically in GitHub Actions on tag push) > `git describe --tags --exact-match HEAD` (local dev fallback). Test fixtures must exercise both `--tag` and `GITHUB_REF_NAME` paths. + `cmd/wfctl/plugin_validate_contract.go`: new subcommand implementing the §3 design checks. Flags: `--for-publish`, `--tag`, `--release-dir`. Checks: 1. plugin.json exists + parses + Validate() OK (sentinel `0.0.0` OK) 2. capabilities populated From 33faa7410f5d16129dacc6a2d0bedd04ce0e13d7 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sat, 23 May 2026 16:08:59 -0400 Subject: [PATCH 13/16] chore: lock scope for plugin-version-discipline (alignment passed) --- docs/plans/2026-05-23-plugin-version-discipline.md | 2 +- docs/plans/2026-05-23-plugin-version-discipline.md.scope-lock | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 docs/plans/2026-05-23-plugin-version-discipline.md.scope-lock diff --git a/docs/plans/2026-05-23-plugin-version-discipline.md b/docs/plans/2026-05-23-plugin-version-discipline.md index 40749262..f36e2abc 100644 --- a/docs/plans/2026-05-23-plugin-version-discipline.md +++ b/docs/plans/2026-05-23-plugin-version-discipline.md @@ -42,7 +42,7 @@ | 8 | (no PR — file follow-up issue for remaining 56 plugin repo sweep) | Task 8 | n/a | workflow | | 9 | docs(retro): workflow#758 pilot retro + close issue | Task 9 | docs/758-retro | workflow | -**Status:** Draft +**Status:** Locked 2026-05-23T20:08:47Z --- diff --git a/docs/plans/2026-05-23-plugin-version-discipline.md.scope-lock b/docs/plans/2026-05-23-plugin-version-discipline.md.scope-lock new file mode 100644 index 00000000..efa63671 --- /dev/null +++ b/docs/plans/2026-05-23-plugin-version-discipline.md.scope-lock @@ -0,0 +1 @@ +3b410ab131e745e12c3d7df85b9c1ff2a25eb219f5f8634621d4a9a8b7f29416 From e35976308aafde11a7474514fdc007a97e30eb0e Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sat, 23 May 2026 16:13:00 -0400 Subject: [PATCH 14/16] feat(sdk): ResolveBuildVersion + IaCServeOptions.BuildVersion + WithBuildVersion (#758) - New sdk.ResolveBuildVersion(declared) helper: pass-through for non-sentinel declared values (typical for goreleaser-injected ldflag tag); fallback to runtime/debug.ReadBuildInfo() (devel + VCS shortsha) for empty/dev sentinels - IaCServeOptions.BuildVersion field + iacPluginServiceBridge.buildVersion + GetManifest single-channel precedence: BuildVersion wins over diskManifest - WithBuildVersion(string) ServeOption + grpcServer.buildVersion + identical precedence in grpcServer.GetManifest - Unit tests for all precedence cases (BuildVersion override, no-disk path, ServeOption wiring) + ResolveBuildVersion sentinel/pass-through coverage No existing API breaks: BuildVersion field is additive on IaCServeOptions; WithBuildVersion is a new ServeOption; plugins that don't use either keep existing GetManifest behavior. Closes part of workflow#758. --- plugin/external/sdk/buildversion.go | 68 +++++++++++++++++++ plugin/external/sdk/buildversion_test.go | 41 +++++++++++ plugin/external/sdk/grpc_server.go | 34 +++++++--- plugin/external/sdk/grpc_server_test.go | 46 +++++++++++++ plugin/external/sdk/iacserver.go | 38 ++++++++--- .../external/sdk/iacserver_internal_test.go | 42 ++++++++++++ plugin/external/sdk/serve.go | 31 +++++++++ 7 files changed, 280 insertions(+), 20 deletions(-) create mode 100644 plugin/external/sdk/buildversion.go create mode 100644 plugin/external/sdk/buildversion_test.go diff --git a/plugin/external/sdk/buildversion.go b/plugin/external/sdk/buildversion.go new file mode 100644 index 00000000..ed1c7eaa --- /dev/null +++ b/plugin/external/sdk/buildversion.go @@ -0,0 +1,68 @@ +package sdk + +import ( + "fmt" + "runtime/debug" +) + +// ResolveBuildVersion returns the operator-visible build-version string. +// +// When declared is non-empty AND not a known dev sentinel ("", "dev", +// "(devel)"), returns declared as-is. This is the typical path for +// goreleaser-built plugin binaries where the ldflag injects the release +// tag into a package-level Version var. +// +// Otherwise consults runtime/debug.ReadBuildInfo() as fallback: +// - "(devel) [@ shortsha[.dirty]]" when vcs.revision is set +// - "(devel)" when no VCS info +// +// Intended call sites (plugin author chooses ANY package-level Version var): +// +// var Version = "dev" // ldflag-injected at release time +// +// sdk.ServeIaCPlugin(srv, sdk.IaCServeOptions{ +// BuildVersion: sdk.ResolveBuildVersion(internal.Version), +// }) +// sdk.Serve(p, sdk.WithManifestProvider(m), +// sdk.WithBuildVersion(sdk.ResolveBuildVersion(internal.Version))) +// +// Goreleaser config provides the tag: +// +// ldflags: +// - -X github.com/<...>/internal.Version={{.Version}} +// +// Mirrors the wfctl pattern at cmd/wfctl/main.go (var version = "dev" + +// debug.ReadBuildInfo() fallback). Closes workflow#758. +func ResolveBuildVersion(declared string) string { + switch declared { + case "", "dev", "(devel)": + return buildInfoVersion() + } + return declared +} + +func buildInfoVersion() string { + info, ok := debug.ReadBuildInfo() + if !ok { + return "(devel)" + } + var sha, modified string + for _, s := range info.Settings { + switch s.Key { + case "vcs.revision": + if len(s.Value) >= 7 { + sha = s.Value[:7] + } else { + sha = s.Value + } + case "vcs.modified": + if s.Value == "true" { + modified = ".dirty" + } + } + } + if sha == "" { + return "(devel)" + } + return fmt.Sprintf("(devel) [@ %s%s]", sha, modified) +} diff --git a/plugin/external/sdk/buildversion_test.go b/plugin/external/sdk/buildversion_test.go new file mode 100644 index 00000000..a55da743 --- /dev/null +++ b/plugin/external/sdk/buildversion_test.go @@ -0,0 +1,41 @@ +package sdk + +import ( + "strings" + "testing" +) + +func TestResolveBuildVersion_ReleaseDeclaredPassThrough(t *testing.T) { + got := ResolveBuildVersion("v1.2.3") + if got != "v1.2.3" { + t.Errorf("got %q, want v1.2.3", got) + } +} + +func TestResolveBuildVersion_EmptyFallsToBuildInfo(t *testing.T) { + got := ResolveBuildVersion("") + if !strings.HasPrefix(got, "(devel)") { + t.Errorf("got %q, want prefix (devel) for empty declared", got) + } +} + +func TestResolveBuildVersion_DevSentinelFallsToBuildInfo(t *testing.T) { + got := ResolveBuildVersion("dev") + if !strings.HasPrefix(got, "(devel)") { + t.Errorf("got %q, want prefix (devel) for 'dev' sentinel", got) + } +} + +func TestResolveBuildVersion_DevelSentinelFallsToBuildInfo(t *testing.T) { + got := ResolveBuildVersion("(devel)") + if !strings.HasPrefix(got, "(devel)") { + t.Errorf("got %q, want prefix (devel) for '(devel)' sentinel", got) + } +} + +func TestResolveBuildVersion_NonStandardDeclaredPassThrough(t *testing.T) { + got := ResolveBuildVersion("v0.0.0-rc.1+build.42") + if got != "v0.0.0-rc.1+build.42" { + t.Errorf("got %q, want pass-through of non-sentinel declared", got) + } +} diff --git a/plugin/external/sdk/grpc_server.go b/plugin/external/sdk/grpc_server.go index 58d30c40..300b7117 100644 --- a/plugin/external/sdk/grpc_server.go +++ b/plugin/external/sdk/grpc_server.go @@ -37,6 +37,12 @@ type grpcServer struct { // compile-time embed plugin.json without re-declaring fields in the // PluginProvider implementation. Per workflow ADR-0031. diskManifest *pluginpkg.PluginManifest + + // buildVersion, when non-empty, overrides Version in the GetManifest RPC + // response (regardless of whether the underlying source is diskManifest + // or provider.Manifest()). Set via sdk.WithBuildVersion — single-channel + // precedence: BuildVersion always wins when set. Closes workflow#758. + buildVersion string } // newGRPCServer creates a gRPC server implementation wrapping the given provider. @@ -140,25 +146,31 @@ func (s *grpcSubscriber) Unsubscribe(topic string) error { // --- Metadata RPCs --- func (s *grpcServer) GetManifest(_ context.Context, _ *emptypb.Empty) (*pb.Manifest, error) { + var out *pb.Manifest if s.diskManifest != nil { - return &pb.Manifest{ + out = &pb.Manifest{ Name: s.diskManifest.Name, Version: s.diskManifest.Version, Author: s.diskManifest.Author, Description: s.diskManifest.Description, ConfigMutable: s.diskManifest.ConfigMutable, SampleCategory: s.diskManifest.SampleCategory, - }, nil + } + } else { + m := s.provider.Manifest() + out = &pb.Manifest{ + Name: m.Name, + Version: m.Version, + Author: m.Author, + Description: m.Description, + ConfigMutable: m.ConfigMutable, + SampleCategory: m.SampleCategory, + } } - m := s.provider.Manifest() - return &pb.Manifest{ - Name: m.Name, - Version: m.Version, - Author: m.Author, - Description: m.Description, - ConfigMutable: m.ConfigMutable, - SampleCategory: m.SampleCategory, - }, nil + if s.buildVersion != "" { + out.Version = s.buildVersion + } + return out, nil } func (s *grpcServer) GetAsset(_ context.Context, req *pb.GetAssetRequest) (*pb.GetAssetResponse, error) { diff --git a/plugin/external/sdk/grpc_server_test.go b/plugin/external/sdk/grpc_server_test.go index 0e5ec8ae..2e9468c2 100644 --- a/plugin/external/sdk/grpc_server_test.go +++ b/plugin/external/sdk/grpc_server_test.go @@ -949,6 +949,52 @@ func TestGetManifestFallsBackToProviderWhenNoDisk(t *testing.T) { } } +// TestGetManifest_BuildVersionOverridesDiskVersion locks the single-channel +// precedence rule for workflow#758: when s.buildVersion is non-empty, it +// overrides diskManifest.Version in the GetManifest response. +func TestGetManifest_BuildVersionOverridesDiskVersion(t *testing.T) { + disk := &pluginpkg.PluginManifest{ + Name: "p", Version: "1.0.0", Author: "a", Description: "d", + } + s := newGRPCServer(&stubProvider{manifest: PluginManifest{Name: "p", Version: "0.0.0"}}) + s.diskManifest = disk + s.buildVersion = "v1.2.3" + got, err := s.GetManifest(context.Background(), &emptypb.Empty{}) + if err != nil { + t.Fatalf("GetManifest: %v", err) + } + if got.Version != "v1.2.3" { + t.Errorf("Version = %q, want BuildVersion-augmented v1.2.3", got.Version) + } + if got.Name != disk.Name { + t.Errorf("Name = %q, want %q (other fields unchanged)", got.Name, disk.Name) + } +} + +// TestGetManifest_BuildVersionOverridesProviderVersion: when there's no +// diskManifest, BuildVersion still overrides provider.Manifest().Version. +func TestGetManifest_BuildVersionOverridesProviderVersion(t *testing.T) { + s := newGRPCServer(&stubProvider{manifest: PluginManifest{Name: "p", Version: "0.0.1"}}) + s.buildVersion = "v2.0.0" + got, err := s.GetManifest(context.Background(), &emptypb.Empty{}) + if err != nil { + t.Fatalf("GetManifest: %v", err) + } + if got.Version != "v2.0.0" { + t.Errorf("Version = %q, want v2.0.0", got.Version) + } +} + +// TestWithBuildVersion_OptionSetsField verifies the WithBuildVersion +// ServeOption properly sets grpcServer.buildVersion. +func TestWithBuildVersion_OptionSetsField(t *testing.T) { + s := newGRPCServer(&stubProvider{manifest: PluginManifest{Name: "p", Version: "0.0.1"}}) + WithBuildVersion("v3.1.4")(s) + if s.buildVersion != "v3.1.4" { + t.Errorf("buildVersion = %q, want v3.1.4", s.buildVersion) + } +} + // detectContentType maps common extensions to MIME types. func detectContentType(path string) string { switch { diff --git a/plugin/external/sdk/iacserver.go b/plugin/external/sdk/iacserver.go index 1f9c567d..0c69cc1b 100644 --- a/plugin/external/sdk/iacserver.go +++ b/plugin/external/sdk/iacserver.go @@ -87,6 +87,7 @@ func registerAllIaCProviderServicesWithOpts(s *grpc.Server, provider any, opts I bridge := &iacPluginServiceBridge{ grpcSrv: s, diskManifest: opts.ManifestProvider, + buildVersion: opts.BuildVersion, } // Wire the optional grpc_server.go delegate when the caller supplied // any (legacy or typed) module/step providers. Zero-value across all @@ -205,6 +206,12 @@ type iacPluginServiceBridge struct { grpcSrv *grpc.Server diskManifest *pluginpkg.PluginManifest + // buildVersion, when non-empty, overrides diskManifest.Version in the + // GetManifest RPC response. Populated from IaCServeOptions.BuildVersion + // in registerAllIaCProviderServicesWithOpts. Single-channel precedence: + // BuildVersion always wins when set. Closes workflow#758. + buildVersion string + // delegate, when non-nil, handles GetModuleTypes / CreateModule / // InitModule / StartModule / StopModule / DestroyModule / GetStepTypes / // CreateStep / ExecuteStep / DestroyStep by forwarding to grpc_server.go's @@ -302,17 +309,22 @@ func (b *iacPluginServiceBridge) GetContractRegistry(_ context.Context, _ *empty // haven't adopted sdk.EmbedManifest still get clean registration via the // engine's manager.go-loaded plugin.json. func (b *iacPluginServiceBridge) GetManifest(_ context.Context, _ *emptypb.Empty) (*pb.Manifest, error) { - if b.diskManifest == nil { + if b.diskManifest == nil && b.buildVersion == "" { return nil, status.Error(codes.Unimplemented, "manifest not embedded; engine falls back to disk plugin.json") } - return &pb.Manifest{ - Name: b.diskManifest.Name, - Version: b.diskManifest.Version, - Author: b.diskManifest.Author, - Description: b.diskManifest.Description, - ConfigMutable: b.diskManifest.ConfigMutable, - SampleCategory: b.diskManifest.SampleCategory, - }, nil + out := &pb.Manifest{} + if b.diskManifest != nil { + out.Name = b.diskManifest.Name + out.Version = b.diskManifest.Version + out.Author = b.diskManifest.Author + out.Description = b.diskManifest.Description + out.ConfigMutable = b.diskManifest.ConfigMutable + out.SampleCategory = b.diskManifest.SampleCategory + } + if b.buildVersion != "" { + out.Version = b.buildVersion + } + return out, nil } // IaCServeOptions configures the IaC plugin gRPC server entrypoint. @@ -334,6 +346,14 @@ type IaCServeOptions struct { // Task 1). ManifestProvider *pluginpkg.PluginManifest + // BuildVersion, when non-empty, overrides any ManifestProvider.Version + // in the GetManifest RPC response. Typically populated via + // sdk.ResolveBuildVersion() so + // operator + engine observe the release tag at runtime even when the + // committed plugin.json carries a dev sentinel ("0.0.0"). Closes + // workflow#758. + BuildVersion string + // Modules supplies plugin-native module providers. When non-nil, the // bridge wires GetModuleTypes / CreateModule / InitModule / StartModule / // StopModule / DestroyModule to delegate to grpc_server.go's existing diff --git a/plugin/external/sdk/iacserver_internal_test.go b/plugin/external/sdk/iacserver_internal_test.go index ab8d4e3b..4cacbc16 100644 --- a/plugin/external/sdk/iacserver_internal_test.go +++ b/plugin/external/sdk/iacserver_internal_test.go @@ -70,5 +70,47 @@ func TestIaCBridgeGetManifestUnimplementedWhenNoProvider(t *testing.T) { } } +// TestIaCBridgeGetManifest_BuildVersionOverridesDiskVersion locks the +// single-channel precedence rule for workflow#758: when bridge.buildVersion +// is non-empty, it overrides diskManifest.Version in the response. +func TestIaCBridgeGetManifest_BuildVersionOverridesDiskVersion(t *testing.T) { + disk := &pluginpkg.PluginManifest{ + Name: "do", Version: "1.0.0", Author: "GoCodeAlone", Description: "test", + } + bridge := &iacPluginServiceBridge{ + grpcSrv: grpc.NewServer(), + diskManifest: disk, + buildVersion: "v1.2.3", + } + got, err := bridge.GetManifest(context.Background(), &emptypb.Empty{}) + if err != nil { + t.Fatalf("GetManifest: %v", err) + } + if got.GetVersion() != "v1.2.3" { + t.Errorf("Version = %q, want BuildVersion-augmented v1.2.3", got.GetVersion()) + } + // Other fields still come from diskManifest. + if got.GetName() != disk.Name { + t.Errorf("Name = %q, want %q (other fields unchanged)", got.GetName(), disk.Name) + } +} + +// TestIaCBridgeGetManifest_BuildVersionOnlyNoDisk: when there's no +// diskManifest but BuildVersion is set, the bridge still returns a Manifest +// (not Unimplemented) carrying only the runtime version. +func TestIaCBridgeGetManifest_BuildVersionOnlyNoDisk(t *testing.T) { + bridge := &iacPluginServiceBridge{ + grpcSrv: grpc.NewServer(), + buildVersion: "v2.0.0", + } + got, err := bridge.GetManifest(context.Background(), &emptypb.Empty{}) + if err != nil { + t.Fatalf("GetManifest: %v", err) + } + if got.GetVersion() != "v2.0.0" { + t.Errorf("Version = %q, want v2.0.0", got.GetVersion()) + } +} + // Compile-time guard: bridge must satisfy pb.PluginServiceServer. var _ pb.PluginServiceServer = (*iacPluginServiceBridge)(nil) diff --git a/plugin/external/sdk/serve.go b/plugin/external/sdk/serve.go index cfd6a091..99c50c12 100644 --- a/plugin/external/sdk/serve.go +++ b/plugin/external/sdk/serve.go @@ -35,6 +35,37 @@ func WithManifestProvider(m *pluginpkg.PluginManifest) ServeOption { } } +// WithBuildVersion sets the runtime build-version surfaced via GetManifest. +// Single-channel precedence: takes precedence over any ManifestProvider.Version +// or provider.Manifest().Version. Typically populated via +// sdk.ResolveBuildVersion(). +// +// Recommended pattern: +// +// import ( +// "github.com/<...>/internal" // ldflag-injected Version var +// sdk "github.com/GoCodeAlone/workflow/plugin/external/sdk" +// ) +// +// func main() { +// sdk.Serve(&myPlugin{}, +// sdk.WithManifestProvider(manifest), +// sdk.WithBuildVersion(sdk.ResolveBuildVersion(internal.Version)), +// ) +// } +// +// Goreleaser config injects the tag at build time: +// +// ldflags: +// - -X github.com/<...>/internal.Version={{.Version}} +// +// Closes workflow#758. +func WithBuildVersion(v string) ServeOption { + return func(s *grpcServer) { + s.buildVersion = v + } +} + // Serve is the entry point for plugin authors. It starts the gRPC plugin server // and blocks until the host process terminates the connection. // From 21de74b40fa004b9a63dcfa6c0920a3cdc7ff1d2 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sat, 23 May 2026 16:19:42 -0400 Subject: [PATCH 15/16] feat(wfctl): plugin validate-contract subcommand + PLUGIN_RELEASE_GATES.md (#758) New 'wfctl plugin validate-contract' subcommand: - Always: plugin.json valid + capabilities + minEngineVersion + main.go wires sdk.ResolveBuildVersion via IaCServeOptions.BuildVersion or sdk.WithBuildVersion + goreleaser ldflag -X *.Version= - --for-publish: strict-semver tag (^v\d+\.\d+\.\d+$); resolves from --tag flag > $GITHUB_REF_NAME > git describe - --release-dir: assert tarball plugin.json.version equals --tag (post-build) Eliminates curl|bash supply-chain risk by collapsing the contract-check script into the binary plugin authors already install via setup-wfctl. Test coverage: good fixture passes; bad-missing-caps + bad-missing-ldflag + prerelease tag + non-semver tag + release-dir-stale all fail; GITHUB_REF_NAME fallback verified. PLUGIN_RELEASE_GATES.md documents the full contract + migration checklist. --- cmd/wfctl/plugin.go | 3 + cmd/wfctl/plugin_validate_contract.go | 237 ++++++++++++++++++ cmd/wfctl/plugin_validate_contract_test.go | 103 ++++++++ .../bad-missing-caps/.goreleaser.yaml | 5 + .../bad-missing-caps/cmd/plugin/main.go | 7 + .../bad-missing-caps/plugin.json | 6 + .../bad-missing-ldflag/.goreleaser.yaml | 5 + .../bad-missing-ldflag/cmd/plugin/main.go | 6 + .../bad-missing-ldflag/plugin.json | 10 + .../good/.goreleaser.yaml | 5 + .../good/cmd/plugin/main.go | 14 ++ .../plugin_validate_contract/good/plugin.json | 10 + .../release-dir-good/.goreleaser.yaml | 5 + .../release-dir-good/.release/plugin.json | 10 + .../release-dir-good/cmd/plugin/main.go | 14 ++ .../release-dir-good/plugin.json | 10 + .../release-dir-stale/.goreleaser.yaml | 5 + .../release-dir-stale/.release/plugin.json | 10 + .../release-dir-stale/cmd/plugin/main.go | 14 ++ .../release-dir-stale/plugin.json | 10 + docs/PLUGIN_RELEASE_GATES.md | 150 +++++++++++ 21 files changed, 639 insertions(+) create mode 100644 cmd/wfctl/plugin_validate_contract.go create mode 100644 cmd/wfctl/plugin_validate_contract_test.go create mode 100644 cmd/wfctl/testdata/plugin_validate_contract/bad-missing-caps/.goreleaser.yaml create mode 100644 cmd/wfctl/testdata/plugin_validate_contract/bad-missing-caps/cmd/plugin/main.go create mode 100644 cmd/wfctl/testdata/plugin_validate_contract/bad-missing-caps/plugin.json create mode 100644 cmd/wfctl/testdata/plugin_validate_contract/bad-missing-ldflag/.goreleaser.yaml create mode 100644 cmd/wfctl/testdata/plugin_validate_contract/bad-missing-ldflag/cmd/plugin/main.go create mode 100644 cmd/wfctl/testdata/plugin_validate_contract/bad-missing-ldflag/plugin.json create mode 100644 cmd/wfctl/testdata/plugin_validate_contract/good/.goreleaser.yaml create mode 100644 cmd/wfctl/testdata/plugin_validate_contract/good/cmd/plugin/main.go create mode 100644 cmd/wfctl/testdata/plugin_validate_contract/good/plugin.json create mode 100644 cmd/wfctl/testdata/plugin_validate_contract/release-dir-good/.goreleaser.yaml create mode 100644 cmd/wfctl/testdata/plugin_validate_contract/release-dir-good/.release/plugin.json create mode 100644 cmd/wfctl/testdata/plugin_validate_contract/release-dir-good/cmd/plugin/main.go create mode 100644 cmd/wfctl/testdata/plugin_validate_contract/release-dir-good/plugin.json create mode 100644 cmd/wfctl/testdata/plugin_validate_contract/release-dir-stale/.goreleaser.yaml create mode 100644 cmd/wfctl/testdata/plugin_validate_contract/release-dir-stale/.release/plugin.json create mode 100644 cmd/wfctl/testdata/plugin_validate_contract/release-dir-stale/cmd/plugin/main.go create mode 100644 cmd/wfctl/testdata/plugin_validate_contract/release-dir-stale/plugin.json create mode 100644 docs/PLUGIN_RELEASE_GATES.md diff --git a/cmd/wfctl/plugin.go b/cmd/wfctl/plugin.go index 41751486..91620f06 100644 --- a/cmd/wfctl/plugin.go +++ b/cmd/wfctl/plugin.go @@ -35,6 +35,8 @@ func runPlugin(args []string) error { return runPluginRemove(args[1:]) case "validate": return runPluginValidate(args[1:]) + case "validate-contract": + return runPluginValidateContract(args[1:]) case "conformance": return runPluginConformance(args[1:]) case "info": @@ -63,6 +65,7 @@ Subcommands: update Update an installed plugin to its latest version remove Uninstall a plugin (also removes from manifest + lockfile) validate Validate a plugin manifest from the registry or a local file + validate-contract Validate a plugin source directory against the release contract (workflow#758) conformance Run executable plugin/host conformance checks info Show details about an installed plugin deps List dependencies for a plugin diff --git a/cmd/wfctl/plugin_validate_contract.go b/cmd/wfctl/plugin_validate_contract.go new file mode 100644 index 00000000..db9b293f --- /dev/null +++ b/cmd/wfctl/plugin_validate_contract.go @@ -0,0 +1,237 @@ +package main + +import ( + "encoding/json" + "flag" + "fmt" + "os" + "os/exec" + "path/filepath" + "regexp" + "strings" + + "github.com/GoCodeAlone/workflow/plugin" +) + +// runPluginValidateContract implements `wfctl plugin validate-contract` +// (workflow#758). Verifies that a plugin source directory satisfies the +// release contract: parseable plugin.json + populated capabilities + +// minEngineVersion + sdk.ResolveBuildVersion call site + goreleaser ldflag. +// +// With --for-publish, additionally enforces the strict-semver tag whitelist +// (^v\d+\.\d+\.\d+$). With --release-dir, asserts the shipped plugin.json's +// .version equals --tag (post-goreleaser-build verification). +func runPluginValidateContract(args []string) error { + fs := flag.NewFlagSet("plugin validate-contract", flag.ContinueOnError) + forPublish := fs.Bool("for-publish", false, "Apply publish-grade checks (strict-semver tag, etc.)") + tag := fs.String("tag", "", "Release tag (e.g. v1.2.3); falls back to $GITHUB_REF_NAME then `git describe --tags --exact-match HEAD`") + releaseDir := fs.String("release-dir", "", "Post-build verification: assert this dir's plugin.json carries --tag") + fs.Usage = func() { + fmt.Fprintf(fs.Output(), `Usage: wfctl plugin validate-contract [options] + +Validate a plugin source directory against the workflow release contract. + +Checks (always): + 1. plugin.json exists, parses, passes PluginManifest.Validate() + 2. capabilities populated (non-empty) + 3. minEngineVersion populated + 4. main.go calls sdk.ResolveBuildVersion(...) and wires it via + IaCServeOptions.BuildVersion or sdk.WithBuildVersion(...) + 5. .goreleaser.{yaml,yml} carries -X *.Version= ldflag injection + +Additional with --for-publish: + 6. Resolved tag matches ^v\d+\.\d+\.\d+$ + 7. With --release-dir: /plugin.json .version equals tag (minus leading v) + +Examples: + wfctl plugin validate-contract . + wfctl plugin validate-contract --for-publish --tag v1.2.3 . + wfctl plugin validate-contract --for-publish --tag v1.2.3 --release-dir .release . + +See docs/PLUGIN_RELEASE_GATES.md for the full contract spec. + +Options: +`) + fs.PrintDefaults() + } + if err := fs.Parse(args); err != nil { + return err + } + if fs.NArg() != 1 { + fs.Usage() + return fmt.Errorf("exactly one argument required") + } + pluginDir := fs.Arg(0) + abs, err := filepath.Abs(pluginDir) + if err != nil { + return fmt.Errorf("resolve %q: %w", pluginDir, err) + } + + var failures []string + addFail := func(msg string) { failures = append(failures, msg) } + + // Check 1: plugin.json parses + Validate() OK + manifestPath := filepath.Join(abs, "plugin.json") + manifestBytes, err := os.ReadFile(manifestPath) // #nosec G304 -- operator-supplied path + if err != nil { + addFail(fmt.Sprintf("plugin.json: %v", err)) + } + var manifest plugin.PluginManifest + if err == nil { + if jerr := json.Unmarshal(manifestBytes, &manifest); jerr != nil { + addFail(fmt.Sprintf("plugin.json: parse: %v", jerr)) + } else if verr := manifest.Validate(); verr != nil { + addFail(fmt.Sprintf("plugin.json: validate: %v", verr)) + } else if manifest.Version == "0.0.0" { + fmt.Fprintln(os.Stderr, " INFO plugin.json.version is dev sentinel \"0.0.0\" — release builds inject the tag via goreleaser ldflag") + } + } + + // Check 2 + 3: capabilities + minEngineVersion populated + if err == nil { + var raw map[string]any + if jerr := json.Unmarshal(manifestBytes, &raw); jerr == nil { + caps, ok := raw["capabilities"].(map[string]any) + if !ok || len(caps) == 0 { + addFail("plugin.json.capabilities: missing or empty") + } + mev, _ := raw["minEngineVersion"].(string) + if strings.TrimSpace(mev) == "" { + addFail("plugin.json.minEngineVersion: missing or empty") + } + } + } + + // Check 4: any cmd/**/main.go contains ResolveBuildVersion AND BuildVersion wiring + mainFound, mainHasContract := scanMainGoFilesForContract(abs) + if !mainFound { + addFail("no cmd/**/main.go (or .go file under repo root) found to scan for contract") + } else if !mainHasContract { + addFail("no main.go contains both sdk.ResolveBuildVersion(...) AND (IaCServeOptions.BuildVersion: ... OR sdk.WithBuildVersion(...))") + } + + // Check 5: goreleaser config carries -X *.Version= ldflag + if !goreleaserHasVersionLdflag(abs) { + addFail(".goreleaser.{yaml,yml}: missing `-X *.Version=` ldflag (mandatory mechanism to deliver release tag into binary)") + } + + // --for-publish: check 6 (tag format) + check 7 (release-dir match) + if *forPublish { + resolved := resolveTag(*tag) + if resolved == "" { + addFail("--for-publish: no tag supplied via --tag, $GITHUB_REF_NAME, or `git describe --tags --exact-match HEAD`") + } else if !publishGradeSemverRe.MatchString(resolved) { + addFail(fmt.Sprintf("--for-publish: tag %q is not release-grade semver (allowed: vN.N.N)", resolved)) + } + if *releaseDir != "" && resolved != "" { + rdManifest := filepath.Join(*releaseDir, "plugin.json") + rdBytes, rerr := os.ReadFile(rdManifest) // #nosec G304 -- operator-supplied path + if rerr != nil { + addFail(fmt.Sprintf("--release-dir %q: %v", *releaseDir, rerr)) + } else { + var rdRaw map[string]any + _ = json.Unmarshal(rdBytes, &rdRaw) + rdVer, _ := rdRaw["version"].(string) + want := strings.TrimPrefix(resolved, "v") + if rdVer != want { + addFail(fmt.Sprintf("--release-dir %q: plugin.json.version=%q does not match --tag %q (want %q)", *releaseDir, rdVer, resolved, want)) + } + } + } + } + + if len(failures) > 0 { + fmt.Fprintln(os.Stderr, "wfctl plugin validate-contract: FAIL") + for _, f := range failures { + fmt.Fprintf(os.Stderr, " - %s\n", f) + } + fmt.Fprintln(os.Stderr, "See docs/PLUGIN_RELEASE_GATES.md for contract details.") + return fmt.Errorf("%d contract check(s) failed", len(failures)) + } + fmt.Println("wfctl plugin validate-contract: PASS") + return nil +} + +var ( + publishGradeSemverRe = regexp.MustCompile(`^v[0-9]+\.[0-9]+\.[0-9]+$`) + resolveBuildVersionRe = regexp.MustCompile(`sdk\.ResolveBuildVersion\s*\(`) + buildVersionFieldRe = regexp.MustCompile(`BuildVersion\s*:`) + withBuildVersionRe = regexp.MustCompile(`sdk\.WithBuildVersion\s*\(`) + goreleaserLdflagRe = regexp.MustCompile(`-X\s+\S*\.Version=`) +) + +// scanMainGoFilesForContract walks dir/cmd/**/*.go and dir/*.go looking for +// the contract pattern. Returns (anyMainFound, anySatisfiesContract). The +// contract pattern is "file contains sdk.ResolveBuildVersion( AND (BuildVersion: +// OR sdk.WithBuildVersion()" — whole-file scoped (gofmt formats across lines). +func scanMainGoFilesForContract(dir string) (mainFound, satisfies bool) { + candidates := []string{} + // Walk cmd/**/main.go + cmdDir := filepath.Join(dir, "cmd") + if info, err := os.Stat(cmdDir); err == nil && info.IsDir() { + _ = filepath.Walk(cmdDir, func(path string, fi os.FileInfo, werr error) error { + if werr != nil { + return nil + } + if fi.IsDir() { + return nil + } + if filepath.Base(path) == "main.go" { + candidates = append(candidates, path) + } + return nil + }) + } + // Also include *.go at repo root (some single-file plugins put main package there) + if entries, err := os.ReadDir(dir); err == nil { + for _, e := range entries { + if !e.IsDir() && strings.HasSuffix(e.Name(), ".go") { + candidates = append(candidates, filepath.Join(dir, e.Name())) + } + } + } + for _, c := range candidates { + mainFound = true + body, err := os.ReadFile(c) // #nosec G304 -- bounded set, operator-supplied root + if err != nil { + continue + } + hasResolve := resolveBuildVersionRe.Match(body) + hasField := buildVersionFieldRe.Match(body) + hasOpt := withBuildVersionRe.Match(body) + if hasResolve && (hasField || hasOpt) { + satisfies = true + return + } + } + return +} + +func goreleaserHasVersionLdflag(dir string) bool { + for _, name := range []string{".goreleaser.yaml", ".goreleaser.yml"} { + body, err := os.ReadFile(filepath.Join(dir, name)) // #nosec G304 -- bounded set + if err != nil { + continue + } + if goreleaserLdflagRe.Match(body) { + return true + } + } + return false +} + +// resolveTag returns explicit --tag > GITHUB_REF_NAME env > git describe. +func resolveTag(explicit string) string { + if t := strings.TrimSpace(explicit); t != "" { + return t + } + if t := strings.TrimSpace(os.Getenv("GITHUB_REF_NAME")); t != "" { + return t + } + cmd := exec.Command("git", "describe", "--tags", "--exact-match", "HEAD") + out, err := cmd.Output() + if err != nil { + return "" + } + return strings.TrimSpace(string(out)) +} diff --git a/cmd/wfctl/plugin_validate_contract_test.go b/cmd/wfctl/plugin_validate_contract_test.go new file mode 100644 index 00000000..e54a3463 --- /dev/null +++ b/cmd/wfctl/plugin_validate_contract_test.go @@ -0,0 +1,103 @@ +package main + +import ( + "strings" + "testing" +) + +func TestRunPluginValidateContract_GoodPasses(t *testing.T) { + err := runPluginValidateContract([]string{"testdata/plugin_validate_contract/good"}) + if err != nil { + t.Fatalf("expected PASS for good fixture, got %v", err) + } +} + +func TestRunPluginValidateContract_BadMissingCapsFails(t *testing.T) { + err := runPluginValidateContract([]string{"testdata/plugin_validate_contract/bad-missing-caps"}) + if err == nil { + t.Fatal("expected FAIL for bad-missing-caps fixture, got nil") + } + if !strings.Contains(err.Error(), "contract check") { + t.Errorf("error should mention contract check, got %v", err) + } +} + +func TestRunPluginValidateContract_BadMissingLdflagFails(t *testing.T) { + err := runPluginValidateContract([]string{"testdata/plugin_validate_contract/bad-missing-ldflag"}) + if err == nil { + t.Fatal("expected FAIL for bad-missing-ldflag fixture, got nil") + } +} + +func TestRunPluginValidateContract_ForPublishGoodTag(t *testing.T) { + err := runPluginValidateContract([]string{ + "--for-publish", "--tag", "v1.2.3", + "testdata/plugin_validate_contract/good", + }) + if err != nil { + t.Fatalf("expected PASS for good fixture + good tag, got %v", err) + } +} + +func TestRunPluginValidateContract_ForPublishBadTag(t *testing.T) { + err := runPluginValidateContract([]string{ + "--for-publish", "--tag", "v1.2.3-rc.1", + "testdata/plugin_validate_contract/good", + }) + if err == nil { + t.Fatal("expected FAIL for prerelease tag, got nil") + } + if !strings.Contains(err.Error(), "contract check") { + t.Errorf("error should mention contract check, got %v", err) + } +} + +func TestRunPluginValidateContract_ForPublishBadTagShape(t *testing.T) { + err := runPluginValidateContract([]string{ + "--for-publish", "--tag", "release-2026", + "testdata/plugin_validate_contract/good", + }) + if err == nil { + t.Fatal("expected FAIL for non-semver tag, got nil") + } +} + +func TestRunPluginValidateContract_ReleaseDirGoodMatches(t *testing.T) { + err := runPluginValidateContract([]string{ + "--for-publish", "--tag", "v1.2.3", + "--release-dir", "testdata/plugin_validate_contract/release-dir-good/.release", + "testdata/plugin_validate_contract/release-dir-good", + }) + if err != nil { + t.Fatalf("expected PASS for release-dir-good, got %v", err) + } +} + +func TestRunPluginValidateContract_ReleaseDirStaleFails(t *testing.T) { + err := runPluginValidateContract([]string{ + "--for-publish", "--tag", "v1.2.3", + "--release-dir", "testdata/plugin_validate_contract/release-dir-stale/.release", + "testdata/plugin_validate_contract/release-dir-stale", + }) + if err == nil { + t.Fatal("expected FAIL for release-dir-stale (.release plugin.json has 1.0.0 not 1.2.3)") + } +} + +func TestRunPluginValidateContract_GithubRefNameFallback(t *testing.T) { + t.Setenv("GITHUB_REF_NAME", "v1.2.3") + err := runPluginValidateContract([]string{ + "--for-publish", + "testdata/plugin_validate_contract/good", + }) + if err != nil { + t.Fatalf("expected PASS via GITHUB_REF_NAME fallback, got %v", err) + } +} + +func TestRunPluginValidateContract_MissingArg(t *testing.T) { + err := runPluginValidateContract([]string{}) + if err == nil { + t.Fatal("expected error for missing plugin-dir arg") + } +} diff --git a/cmd/wfctl/testdata/plugin_validate_contract/bad-missing-caps/.goreleaser.yaml b/cmd/wfctl/testdata/plugin_validate_contract/bad-missing-caps/.goreleaser.yaml new file mode 100644 index 00000000..8d2d9935 --- /dev/null +++ b/cmd/wfctl/testdata/plugin_validate_contract/bad-missing-caps/.goreleaser.yaml @@ -0,0 +1,5 @@ +version: 2 +builds: + - id: test + ldflags: + - -X github.com/example/internal.Version={{.Version}} diff --git a/cmd/wfctl/testdata/plugin_validate_contract/bad-missing-caps/cmd/plugin/main.go b/cmd/wfctl/testdata/plugin_validate_contract/bad-missing-caps/cmd/plugin/main.go new file mode 100644 index 00000000..22b8ea9c --- /dev/null +++ b/cmd/wfctl/testdata/plugin_validate_contract/bad-missing-caps/cmd/plugin/main.go @@ -0,0 +1,7 @@ +// Fixture (not compiled). +package main + +func main() { + _ = "sdk.ResolveBuildVersion(internal.Version)" // contract token present in string + _ = "BuildVersion:" +} diff --git a/cmd/wfctl/testdata/plugin_validate_contract/bad-missing-caps/plugin.json b/cmd/wfctl/testdata/plugin_validate_contract/bad-missing-caps/plugin.json new file mode 100644 index 00000000..77381559 --- /dev/null +++ b/cmd/wfctl/testdata/plugin_validate_contract/bad-missing-caps/plugin.json @@ -0,0 +1,6 @@ +{ + "name": "workflow-plugin-test-bad", + "version": "0.0.0", + "author": "test", + "description": "fixture missing capabilities" +} diff --git a/cmd/wfctl/testdata/plugin_validate_contract/bad-missing-ldflag/.goreleaser.yaml b/cmd/wfctl/testdata/plugin_validate_contract/bad-missing-ldflag/.goreleaser.yaml new file mode 100644 index 00000000..1b3e8e8d --- /dev/null +++ b/cmd/wfctl/testdata/plugin_validate_contract/bad-missing-ldflag/.goreleaser.yaml @@ -0,0 +1,5 @@ +version: 2 +builds: + - id: test + ldflags: + - -s -w diff --git a/cmd/wfctl/testdata/plugin_validate_contract/bad-missing-ldflag/cmd/plugin/main.go b/cmd/wfctl/testdata/plugin_validate_contract/bad-missing-ldflag/cmd/plugin/main.go new file mode 100644 index 00000000..501a83da --- /dev/null +++ b/cmd/wfctl/testdata/plugin_validate_contract/bad-missing-ldflag/cmd/plugin/main.go @@ -0,0 +1,6 @@ +// Fixture (not compiled). +package main + +func main() { + _ = "sdk.ResolveBuildVersion(x) BuildVersion:" +} diff --git a/cmd/wfctl/testdata/plugin_validate_contract/bad-missing-ldflag/plugin.json b/cmd/wfctl/testdata/plugin_validate_contract/bad-missing-ldflag/plugin.json new file mode 100644 index 00000000..eebde1a4 --- /dev/null +++ b/cmd/wfctl/testdata/plugin_validate_contract/bad-missing-ldflag/plugin.json @@ -0,0 +1,10 @@ +{ + "name": "workflow-plugin-test-noflag", + "version": "0.0.0", + "author": "test", + "description": "fixture missing ldflag", + "minEngineVersion": "0.61.0", + "capabilities": { + "moduleTypes": ["test.module"] + } +} diff --git a/cmd/wfctl/testdata/plugin_validate_contract/good/.goreleaser.yaml b/cmd/wfctl/testdata/plugin_validate_contract/good/.goreleaser.yaml new file mode 100644 index 00000000..f7e44bd9 --- /dev/null +++ b/cmd/wfctl/testdata/plugin_validate_contract/good/.goreleaser.yaml @@ -0,0 +1,5 @@ +version: 2 +builds: + - id: test + ldflags: + - -s -w -X github.com/example/internal.Version={{.Version}} diff --git a/cmd/wfctl/testdata/plugin_validate_contract/good/cmd/plugin/main.go b/cmd/wfctl/testdata/plugin_validate_contract/good/cmd/plugin/main.go new file mode 100644 index 00000000..d320d2a5 --- /dev/null +++ b/cmd/wfctl/testdata/plugin_validate_contract/good/cmd/plugin/main.go @@ -0,0 +1,14 @@ +// Fixture for wfctl plugin validate-contract good case (workflow#758). +// NOT compiled — fixture only; lives under testdata/. +package main + +import ( + sdk "github.com/GoCodeAlone/workflow/plugin/external/sdk" + "github.com/example/internal" +) + +func main() { + sdk.ServeIaCPlugin(internal.NewIaCServer(), sdk.IaCServeOptions{ + BuildVersion: sdk.ResolveBuildVersion(internal.Version), + }) +} diff --git a/cmd/wfctl/testdata/plugin_validate_contract/good/plugin.json b/cmd/wfctl/testdata/plugin_validate_contract/good/plugin.json new file mode 100644 index 00000000..cefaf216 --- /dev/null +++ b/cmd/wfctl/testdata/plugin_validate_contract/good/plugin.json @@ -0,0 +1,10 @@ +{ + "name": "workflow-plugin-test-good", + "version": "0.0.0", + "author": "test", + "description": "fixture", + "minEngineVersion": "0.61.0", + "capabilities": { + "moduleTypes": ["test.module"] + } +} diff --git a/cmd/wfctl/testdata/plugin_validate_contract/release-dir-good/.goreleaser.yaml b/cmd/wfctl/testdata/plugin_validate_contract/release-dir-good/.goreleaser.yaml new file mode 100644 index 00000000..f7e44bd9 --- /dev/null +++ b/cmd/wfctl/testdata/plugin_validate_contract/release-dir-good/.goreleaser.yaml @@ -0,0 +1,5 @@ +version: 2 +builds: + - id: test + ldflags: + - -s -w -X github.com/example/internal.Version={{.Version}} diff --git a/cmd/wfctl/testdata/plugin_validate_contract/release-dir-good/.release/plugin.json b/cmd/wfctl/testdata/plugin_validate_contract/release-dir-good/.release/plugin.json new file mode 100644 index 00000000..ba98ae98 --- /dev/null +++ b/cmd/wfctl/testdata/plugin_validate_contract/release-dir-good/.release/plugin.json @@ -0,0 +1,10 @@ +{ + "name": "workflow-plugin-test-rd-good", + "version": "1.2.3", + "author": "test", + "description": "fixture release dir good", + "minEngineVersion": "0.61.0", + "capabilities": { + "moduleTypes": ["test.module"] + } +} diff --git a/cmd/wfctl/testdata/plugin_validate_contract/release-dir-good/cmd/plugin/main.go b/cmd/wfctl/testdata/plugin_validate_contract/release-dir-good/cmd/plugin/main.go new file mode 100644 index 00000000..d320d2a5 --- /dev/null +++ b/cmd/wfctl/testdata/plugin_validate_contract/release-dir-good/cmd/plugin/main.go @@ -0,0 +1,14 @@ +// Fixture for wfctl plugin validate-contract good case (workflow#758). +// NOT compiled — fixture only; lives under testdata/. +package main + +import ( + sdk "github.com/GoCodeAlone/workflow/plugin/external/sdk" + "github.com/example/internal" +) + +func main() { + sdk.ServeIaCPlugin(internal.NewIaCServer(), sdk.IaCServeOptions{ + BuildVersion: sdk.ResolveBuildVersion(internal.Version), + }) +} diff --git a/cmd/wfctl/testdata/plugin_validate_contract/release-dir-good/plugin.json b/cmd/wfctl/testdata/plugin_validate_contract/release-dir-good/plugin.json new file mode 100644 index 00000000..cefaf216 --- /dev/null +++ b/cmd/wfctl/testdata/plugin_validate_contract/release-dir-good/plugin.json @@ -0,0 +1,10 @@ +{ + "name": "workflow-plugin-test-good", + "version": "0.0.0", + "author": "test", + "description": "fixture", + "minEngineVersion": "0.61.0", + "capabilities": { + "moduleTypes": ["test.module"] + } +} diff --git a/cmd/wfctl/testdata/plugin_validate_contract/release-dir-stale/.goreleaser.yaml b/cmd/wfctl/testdata/plugin_validate_contract/release-dir-stale/.goreleaser.yaml new file mode 100644 index 00000000..f7e44bd9 --- /dev/null +++ b/cmd/wfctl/testdata/plugin_validate_contract/release-dir-stale/.goreleaser.yaml @@ -0,0 +1,5 @@ +version: 2 +builds: + - id: test + ldflags: + - -s -w -X github.com/example/internal.Version={{.Version}} diff --git a/cmd/wfctl/testdata/plugin_validate_contract/release-dir-stale/.release/plugin.json b/cmd/wfctl/testdata/plugin_validate_contract/release-dir-stale/.release/plugin.json new file mode 100644 index 00000000..3484930e --- /dev/null +++ b/cmd/wfctl/testdata/plugin_validate_contract/release-dir-stale/.release/plugin.json @@ -0,0 +1,10 @@ +{ + "name": "workflow-plugin-test-rd-good", + "version": "1.0.0", + "author": "test", + "description": "fixture release dir good", + "minEngineVersion": "0.61.0", + "capabilities": { + "moduleTypes": ["test.module"] + } +} diff --git a/cmd/wfctl/testdata/plugin_validate_contract/release-dir-stale/cmd/plugin/main.go b/cmd/wfctl/testdata/plugin_validate_contract/release-dir-stale/cmd/plugin/main.go new file mode 100644 index 00000000..d320d2a5 --- /dev/null +++ b/cmd/wfctl/testdata/plugin_validate_contract/release-dir-stale/cmd/plugin/main.go @@ -0,0 +1,14 @@ +// Fixture for wfctl plugin validate-contract good case (workflow#758). +// NOT compiled — fixture only; lives under testdata/. +package main + +import ( + sdk "github.com/GoCodeAlone/workflow/plugin/external/sdk" + "github.com/example/internal" +) + +func main() { + sdk.ServeIaCPlugin(internal.NewIaCServer(), sdk.IaCServeOptions{ + BuildVersion: sdk.ResolveBuildVersion(internal.Version), + }) +} diff --git a/cmd/wfctl/testdata/plugin_validate_contract/release-dir-stale/plugin.json b/cmd/wfctl/testdata/plugin_validate_contract/release-dir-stale/plugin.json new file mode 100644 index 00000000..cefaf216 --- /dev/null +++ b/cmd/wfctl/testdata/plugin_validate_contract/release-dir-stale/plugin.json @@ -0,0 +1,10 @@ +{ + "name": "workflow-plugin-test-good", + "version": "0.0.0", + "author": "test", + "description": "fixture", + "minEngineVersion": "0.61.0", + "capabilities": { + "moduleTypes": ["test.module"] + } +} diff --git a/docs/PLUGIN_RELEASE_GATES.md b/docs/PLUGIN_RELEASE_GATES.md new file mode 100644 index 00000000..03d59d7c --- /dev/null +++ b/docs/PLUGIN_RELEASE_GATES.md @@ -0,0 +1,150 @@ +# Plugin Release Gates (workflow#758) + +Plugin authors must satisfy a small contract so that: + +- Plugin binaries surface their build-injected version through the gRPC `GetManifest` RPC (operator + engine observability). +- The committed `plugin.json.version` field stops drifting between releases (no more `sync-plugin-version.yml` PR pileup). +- Non-semver tags cannot enter the public registry. + +This page documents the contract, the migration steps, and the `wfctl plugin validate-contract` gate that enforces it at release time. + +## Contract + +Every plugin's source repository MUST: + +1. **`plugin.json` carries a sentinel version**: committed `.version` field is `"0.0.0"`. The release tag is the truth; the committed file is a structural placeholder. `PluginManifest.Validate()` accepts `0.0.0` (parses through `ParseSemver`). +2. **`capabilities` and `minEngineVersion` populated**: these are read by `workflow-registry/scripts/sync-versions.sh` at tag time (`fetch_plugin_json` path). Stale capabilities cause registry to publish wrong type info; freshness is the maintainer's responsibility pre-tag. +3. **Goreleaser injects the tag via ldflag**: `.goreleaser.yaml` (or `.goreleaser.yml`) carries an `ldflags` line matching the regex `-X .*\.Version=`. Standard pattern: + + ```yaml + builds: + - id: workflow-plugin-foo + ldflags: + - -s -w -X github.com/GoCodeAlone/workflow-plugin-foo/internal.Version={{.Version}} + ``` + + The injected package-level Go var's name is flexible — DO plugin uses `internal.Version`, AWS uses `provider.ProviderVersion`. wfctl validates the ldflag's PRESENCE, not the symbol path. +4. **`cmd/**/main.go` (or any `.go` at repo root) wires `sdk.ResolveBuildVersion`**: the plugin's serve binary calls `sdk.ResolveBuildVersion()` and passes the result to either `IaCServeOptions.BuildVersion` (for IaC plugins via `sdk.ServeIaCPlugin`) or `sdk.WithBuildVersion(...)` (for non-IaC plugins via `sdk.Serve`). + + Example (IaC): + + ```go + import ( + "github.com/GoCodeAlone/workflow-plugin-foo/internal" + sdk "github.com/GoCodeAlone/workflow/plugin/external/sdk" + ) + func main() { + sdk.ServeIaCPlugin(internal.NewIaCServer(), sdk.IaCServeOptions{ + BuildVersion: sdk.ResolveBuildVersion(internal.Version), + }) + } + ``` + + Example (non-IaC): + + ```go + sdk.Serve(internal.NewFooPlugin(), + sdk.WithManifestProvider(manifest), + sdk.WithBuildVersion(sdk.ResolveBuildVersion(internal.Version)), + ) + ``` +5. **Release tag is publish-grade semver**: `^v[0-9]+\.[0-9]+\.[0-9]+$`. Pre-release strings (`-rc1`, `-alpha.1`, `-feat-foo`, `+meta`) are rejected by both `wfctl plugin validate-contract --for-publish` and `workflow-registry`. Pre-release publishing is deferred to a separate design that updates `ParseSemver` end-to-end. + +## release.yml two-step gate + +Each plugin's `.github/workflows/release.yml` runs the gate twice — once before the build (static contract + tag format) and once after the build (tarball-version-equals-tag): + +```yaml +on: + push: + tags: ['v*'] + +jobs: + release: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: { fetch-depth: 0 } + - uses: actions/setup-go@v5 + with: { go-version-file: go.mod } + - uses: GoCodeAlone/setup-wfctl@v1 + with: { version: v0.61.0 } + + # 1. Pre-build gate: static contract + tag format + - name: Validate plugin contract for publish (pre-build) + run: wfctl plugin validate-contract --for-publish --tag "${{ github.ref_name }}" . + + # 2. Build (goreleaser mutates plugin.json or writes .release/plugin.json) + - uses: goreleaser/goreleaser-action@v7 + with: + distribution: goreleaser + version: '~> v2' + args: release --clean --release-notes=/dev/null + + # 3. Post-build gate: tarball carries the tag (run BEFORE Publish-release) + - name: Verify shipped plugin.json carries tag (post-build) + run: | + if [ -f .release/plugin.json ]; then + wfctl plugin validate-contract --for-publish --tag "${{ github.ref_name }}" --release-dir .release . + else + wfctl plugin validate-contract --for-publish --tag "${{ github.ref_name }}" --release-dir . . + fi + + # 4. Promote the GitHub Release out of draft + - name: Publish release (was draft during asset upload) + env: { GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} } + run: gh release edit ${{ github.ref_name }} --draft=false --repo ${{ github.repository }} +``` + +Malformed tag → step 1 fails. Stale capabilities or missing ldflag → step 1 fails. Goreleaser mis-writing the tarball plugin.json → step 3 fails. None of these promote the release to public. + +## What got deleted + +- `.github/workflows/sync-plugin-version.yml`: gone. The committed `plugin.json.version` no longer syncs with the tag. Goreleaser's `before:` hook continues to write the tag into the shipped tarball. +- `chore: sync plugin.json version to vX.Y.Z` bot PRs: no longer fire. +- The MISMATCH warning in `workflow-registry/scripts/sync-versions.sh` is unaffected — it compares registry's local manifest copy against the upstream tag, not against the plugin repo's committed plugin.json. + +## Operator-visible build version + +For release builds, plugins surface their tag via `GetManifest`: + +``` +$ wfctl plugin info workflow-plugin-foo +Name: workflow-plugin-foo +Version: v1.2.3 # from binary's runtime, not disk plugin.json +... +``` + +For local `go build` / dev installs (no ldflag injection), the binary reports `(devel) [@ abc1234.dirty]` so operators see the test-build nature in the version string. `wfctl plugin install --local ` reads the committed `plugin.json.version` (the sentinel `0.0.0`) but the binary's runtime GetManifest is authoritative. + +## Migration checklist (per plugin repo) + +1. `git rm .github/workflows/sync-plugin-version.yml` +2. Edit `cmd/**/main.go` to call `sdk.ResolveBuildVersion()` and wire via `IaCServeOptions.BuildVersion` or `sdk.WithBuildVersion`. If no Version var exists in the package the goreleaser ldflag targets, add `var Version = "dev"`. +3. Set `plugin.json.version` to `"0.0.0"`. +4. Verify `.goreleaser.{yaml,yml}` has `-X .*\.Version=` ldflag. +5. Edit `.github/workflows/release.yml` to add the `setup-wfctl` step + pre-build + post-build `wfctl plugin validate-contract` invocations (snippet above). +6. Locally: `wfctl plugin validate-contract .` must PASS. +7. Open PR, CI green, admin-merge. +8. Tag next release. release.yml's gates fire on tag push. + +## Registry-side gate (defense in depth) + +`workflow-registry/scripts/sync-versions.sh` rejects ingest of any plugin whose upstream release tag is not strict-semver: + +```bash +if [[ ! "$latest_tag" =~ ^v[0-9]+\.[0-9]+\.[0-9]+$ ]]; then + echo " REJECT $plugin_name — upstream release tag $latest_tag is not release-grade semver" + continue +fi +``` + +Same regex as `wfctl plugin validate-contract --for-publish`. Catches plugins that bypass release.yml (self-hosted runner, manual upload, force-push). + +## References + +- Tracking issue: [workflow#758](https://github.com/GoCodeAlone/workflow/issues/758) +- Design + plan: `docs/plans/2026-05-23-plugin-version-discipline-design.md` + `docs/plans/2026-05-23-plugin-version-discipline.md` +- SDK: `plugin/external/sdk/buildversion.go`, `plugin/external/sdk/iacserver.go` (IaCServeOptions.BuildVersion), `plugin/external/sdk/serve.go` (WithBuildVersion) +- wfctl: `cmd/wfctl/plugin_validate_contract.go` +- Registry: `workflow-registry/scripts/sync-versions.sh` From 73540bbe1a929e5249431f03d90a35b3670cd8b7 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sat, 23 May 2026 16:32:41 -0400 Subject: [PATCH 16/16] fix(wfctl): propagate filepath.Walk error in scanMainGoFilesForContract (#758) Lint nilerr: returned nil when werr was non-nil. Propagate werr instead so permission errors / broken symlinks surface as walk errors. --- cmd/wfctl/plugin_validate_contract.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/wfctl/plugin_validate_contract.go b/cmd/wfctl/plugin_validate_contract.go index db9b293f..5d3fc4bf 100644 --- a/cmd/wfctl/plugin_validate_contract.go +++ b/cmd/wfctl/plugin_validate_contract.go @@ -171,7 +171,7 @@ func scanMainGoFilesForContract(dir string) (mainFound, satisfies bool) { if info, err := os.Stat(cmdDir); err == nil && info.IsDir() { _ = filepath.Walk(cmdDir, func(path string, fi os.FileInfo, werr error) error { if werr != nil { - return nil + return werr } if fi.IsDir() { return nil