feat(sdk+wfctl): ResolveBuildVersion + WithBuildVersion + plugin validate-contract (#758 Layer 1)#759
Merged
Merged
Conversation
Cycle 1 surfaced 3 Critical defects in the original ldflag-injected design. Pivot to a much smaller scope per reviewer's Options 1+4: - KEEP version field in committed plugin.json (registry depends on it via sync-versions.sh:122 and downloads[].url version-tagged paths) - Replace sync-plugin-version.yml PR-opening with direct-push-to-main (or auto-merge variant for stricter repos) - Add tag-format semver gate (whitelist alpha|beta|rc pre-releases) in each release.yml as a first step - Add same gate in workflow-registry ingest as defense in depth No engine changes. No plugin SDK changes. No cross-version compatibility cliffs. Cycle-1 C1/C2/C3 + I1-I7 collapsed by not dropping the field.
…le 2 (workflow#758) - NC1 addressed: §4 adds sdk.ResolveBuildVersion + IaCServeOptions.BuildVersion contract + check-plugin-contract.sh lint - NC2 addressed: ResolveBuildVersion uses runtime/debug.ReadBuildInfo for test-build branch-nature surfacing - NI1 addressed: regex widened to (alpha|beta|rc)\.?[0-9]+ (rc1 + rc.1) - NI2 addressed: concurrency group on workflows + git pull --ff-only before commit in direct-push - NI3 addressed: registry gate validates tag string (same source as release.yml gate) - NI4 addressed: 24-row plugin audit table in migration plan; variant choice (direct-push|auto-merge) enumerated per-repo
…orization (workflow#758) Cycle 1 found 6 Critical + 9 Important against the plan. Audit table is factually wrong on 8 of 23 listed plugin repos (missing release pipelines). sdk.Serve gap for non-IaC plugins. curl|bash supply-chain risk. branch- nature unmet on go test path. Rebuilding the audit + rescoping is a user-intent-line decision; pause until user returns to authorize rework direction.
…contract (workflow#758) Per user direction post plan-cycle-1: collapse to delete-sync-workflow + wfctl subcommand for contract gate. Eliminates curl|bash (C4), audit-mismatch (C1), sdk.Serve gap (C3) by removing the sync-mechanism entirely. Sentinel committed version (v0.0.0-dev) keeps PluginManifest Validate happy; goreleaser before-hook still writes correct version into shipped tarball; registry derives version from upstream tag (verified sync-versions.sh:122,125,169). No engine change, no manifest schema break.
…rsion() no-arg (workflow#758) C1 fix: sentinel changed v0.0.0-dev → 0.0.0 (verified parses through strict ParseSemver; v0.0.0-dev was rejected as reviewer empirically demonstrated). C2 fix: tag regex tightened to ^v\d+\.\d+\.\d+$ — no prereleases since engine ParseSemver rejects them. C3 fix: sdk.BuildVersion() no-arg helper uses runtime/debug.ReadBuildInfo directly; eliminates internal.Version symbol-path variance across repos. I1/I2/I4/I5/I6 addressed; I3 acknowledged out-of-scope.
…tch *grpcServer (workflow#758) N-C1 fix: restored sdk.ResolveBuildVersion(declared) arg-taking helper. Verified goreleaser uses go build (not go install), so Main.Version is pseudo-version, not the tag. Ldflag is the only correct mechanism. N-C2 fix: §6 swept for consistent vocabulary (sentinel 0.0.0; arg-taking helper; ldflag verify present; Version-var-name variance documented). N-I1 fix: design now references actual *grpcServer struct + buildVersion field on it (mirroring diskManifest). N-I2 fix: explicit GetManifest code blocks for both grpc_server.go and iacserver.go showing single-channel precedence. N-I3 fix: --release-dir flag added to validate-contract; Layer 3 release .yml gains post-build tarball verification step.
I1: post-build verify step placement clarified (between goreleaser + Publish) I2: hard-ordering gate documented for v0.61.0 tag → Layer 3 dispatch I3: Task 7 github clarified (current main.go has no ServeOption; migration adds first) I4: pre-flight audit grep updated to accept Serve|ServeIaCPlugin I5: AWS Task 4 spells out var-existence audit + import-add requirement I6: validate-contract rule 4 spec'd as whole-file scoped + multi-binary repos
…back (workflow#758)
…uildVersion (#758) - New sdk.ResolveBuildVersion(declared) helper: pass-through for non-sentinel declared values (typical for goreleaser-injected ldflag tag); fallback to runtime/debug.ReadBuildInfo() (devel + VCS shortsha) for empty/dev sentinels - IaCServeOptions.BuildVersion field + iacPluginServiceBridge.buildVersion + GetManifest single-channel precedence: BuildVersion wins over diskManifest - WithBuildVersion(string) ServeOption + grpcServer.buildVersion + identical precedence in grpcServer.GetManifest - Unit tests for all precedence cases (BuildVersion override, no-disk path, ServeOption wiring) + ResolveBuildVersion sentinel/pass-through coverage No existing API breaks: BuildVersion field is additive on IaCServeOptions; WithBuildVersion is a new ServeOption; plugins that don't use either keep existing GetManifest behavior. Closes part of workflow#758.
…ES.md (#758) New 'wfctl plugin validate-contract' subcommand: - Always: plugin.json valid + capabilities + minEngineVersion + main.go wires sdk.ResolveBuildVersion via IaCServeOptions.BuildVersion or sdk.WithBuildVersion + goreleaser ldflag -X *.Version= - --for-publish: strict-semver tag (^v\d+\.\d+\.\d+$); resolves from --tag flag > $GITHUB_REF_NAME > git describe - --release-dir: assert tarball plugin.json.version equals --tag (post-build) Eliminates curl|bash supply-chain risk by collapsing the contract-check script into the binary plugin authors already install via setup-wfctl. Test coverage: good fixture passes; bad-missing-caps + bad-missing-ldflag + prerelease tag + non-semver tag + release-dir-stale all fail; GITHUB_REF_NAME fallback verified. PLUGIN_RELEASE_GATES.md documents the full contract + migration checklist.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
…ct (#758) Lint nilerr: returned nil when werr was non-nil. Propagate werr instead so permission errors / broken symlinks surface as walk errors.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Layer 1 of workflow#758 — SDK + wfctl foundation that enables per-plugin migration in Layer 3 (deferred to follow-up PRs).
SDK additions
sdk.ResolveBuildVersion(declared string)— returns declared when non-empty and not a dev sentinel (typical for goreleaser-built binaries where the ldflag injects the release tag), else consultsruntime/debug.ReadBuildInfo()and returns(devel) [@ shortsha[.dirty]]for local builds. Mirrors the pattern wfctl itself uses atcmd/wfctl/main.go:37-50.IaCServeOptions.BuildVersion stringfield — populates the iacPluginServiceBridge's runtime version, overridingdiskManifest.VersioninGetManifest.sdk.WithBuildVersion(string) ServeOption— same single-channel precedence for non-IaC plugins viasdk.Serve.Both bridges now augment
GetManifest: ifbuildVersionis non-empty, it overrides whichever Version came from disk/embedded manifest. Single-channel precedence; explicit + unit-tested.wfctl:
plugin validate-contractsubcommandsdk.ResolveBuildVersionviaIaCServeOptions.BuildVersionorsdk.WithBuildVersion+.goreleaser.{yaml,yml}carries-X .*\.Version=ldflag.--for-publish: resolved tag (--tag flag >$GITHUB_REF_NAME>git describe) must match strict-semver^v\d+\.\d+\.\d+$. Prerelease tags rejected; engineParseSemverdoesn't accept them.--release-dir <path>: assert<path>/plugin.json.versionequals--tag(minus leadingv) — post-goreleaser tarball verification.Eliminates a curl|bash supply-chain risk that an earlier design proposed (separate check-plugin-contract.sh delivered via raw.githubusercontent.com); collapses into the wfctl binary plugin authors already install via
setup-wfctl@v1.Docs
docs/PLUGIN_RELEASE_GATES.md— operator + plugin-author-facing reference: contract, release.yml two-step gate pattern (pre-build static checks + post-build tarball verify), migration checklist.Out of scope (this PR)
Test plan
GOWORK=off go test ./plugin/external/sdk/... ./cmd/wfctl/ -count=1— greenTestResolveBuildVersion_*(5 tests covering pass-through + 3 sentinels + non-standard declared)TestIaCBridgeGetManifest_BuildVersionOverridesDiskVersion+_BuildVersionOnlyNoDiskTestGetManifest_BuildVersionOverridesDiskVersion+_BuildVersionOverridesProviderVersion+TestWithBuildVersion_OptionSetsFieldTestRunPluginValidateContract_*(9 tests: good + bad-missing-caps + bad-missing-ldflag + good-tag + prerelease-tag-fails + non-semver-tag-fails + release-dir-good + release-dir-stale-fails + GITHUB_REF_NAME-fallback + missing-arg)Rollback
Pure-additive: SDK fields/methods are new; existing plugins that don't use BuildVersion keep current GetManifest behavior. wfctl
plugin validate-contractis a new subcommand; existingwfctl plugin validateunchanged. Revert is singlegit revert.🤖 Generated with Claude Code