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..5d3fc4bf --- /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 werr + } + 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` 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..7de7ee9f --- /dev/null +++ b/docs/plans/2026-05-23-plugin-version-discipline-design.md @@ -0,0 +1,288 @@ +# Plugin version discipline: delete sync mechanism + wfctl contract gate — design + +Issue: GoCodeAlone/workflow#758 +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 + +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: `"0.0.0"`. + +**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. + +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. + +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 + +Goal: plugin binary surfaces its build-injected version through `GetManifest` so engine, operator, and observability tools see runtime-truth. + +**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 +// 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 `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 +// In grpc_server.go's grpcServer struct: +type grpcServer struct { + // ... existing fields ... + diskManifest *pluginpkg.PluginManifest + buildVersion string // workflow#758: runtime version override via WithBuildVersion +} + +// In serve.go: +func WithBuildVersion(v string) ServeOption { + return func(s *grpcServer) { + s.buildVersion = v + } +} +``` + +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 +// 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 +``` + +```go +// 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 +} +return out, nil +``` + +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. + +### 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 `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` (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`): + +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` + +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 # 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, incomplete contract, or tarball-without-tag → release halts before publish. + +### 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]+$ ]]; then + echo " REJECT $plugin_name — upstream release tag $latest_tag is not release-grade semver (engine ParseSemver requires flat M.m.p)" + 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 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. + +### 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 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 `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 + +- §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). +- 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. +- 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. 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..f36e2abc --- /dev/null +++ b/docs/plans/2026-05-23-plugin-version-discipline.md @@ -0,0 +1,606 @@ +# 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 + +(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:** +- 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:** Locked 2026-05-23T20:08:47Z + +--- + +### 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** + +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 +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 .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. (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** + +```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 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) + 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`, 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 + +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 (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)), + ) + ``` + +- 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)`. + +--- + +### 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. + +**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). 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 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..8719ce8b --- /dev/null +++ b/docs/plans/2026-05-23-plugin-version-ldflag-design.md @@ -0,0 +1,229 @@ +# Plugin version sync hardening + ldflag contract + publish-tag semver gate — design + +Issue: GoCodeAlone/workflow#758 +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 + +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 — 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. + +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. + +## What the adversarial cycle taught us + +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. + +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. + +## Cycle-2 direction + +**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. + +Three composable pieces: + +### 1. Replace PR-opening sync with direct-push-to-main + +`sync-plugin-version.yml` today: tag fires → workflow checks out main → rewrites plugin.json → opens PR. The PR sits unmerged. + +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. + +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. + +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. + +### 2. Tag-level semver gate in release workflow + +`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. + +Add a first step in `release.yml`: + +```yaml +- name: Validate tag is strict semver + run: | + TAG="${{ github.ref_name }}" + # 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. 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 — gates the release tag, not the manifest field + +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 +# 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 +``` + +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. Make ldflag-injected runtime version a plugin contract + +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. + +Add to `sdk` package: + +```go +// IaCServeOptions adds: +// BuildVersion string // runtime version, typically internal.Version (ldflag-injected) + +// 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 { ... } +``` + +Plugin authors then write: + +```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 + +- `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. 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 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. **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. **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 (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. + +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 + +- 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). +- 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 + +- **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). 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..2f5d82e7 --- /dev/null +++ b/docs/plans/2026-05-23-plugin-version-ldflag.md @@ -0,0 +1,652 @@ +# 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:** 29 +**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 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. + +--- + +### 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. + +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 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 + +**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. + +## 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. 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. //