From 81a64542fbe6817bf6de4813861673d01442e33d Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Tue, 19 May 2026 14:47:04 -0400 Subject: [PATCH 01/15] docs(plan): design for live-deployment example validation CI matrix Files a design doc for the live-deploy CI matrix deferred from the 2026-05-19 multi-repo QoL sweep. Schema-level validation is insufficient to promote a plugin to 'verified'; this design adds a weekly OIDC-driven GitHub Actions matrix that exercises each IaC plugin's examples/minimal/config.yaml against staging cloud accounts, auto-promotes on green, demotes on 2 consecutive REDs. Execution is gated on operator provisioning staging accounts + GitHub OIDC trust per provider. Document this as the next concrete step. Companion to workflow#725 (marketplace-verify subcommand). Closes #723. Co-Authored-By: Claude Opus 4.7 (1M context) --- ...026-05-19-live-deploy-validation-design.md | 127 ++++++++++++++++++ 1 file changed, 127 insertions(+) create mode 100644 _worktrees/live-deploy-design-1779216240/docs/plans/2026-05-19-live-deploy-validation-design.md diff --git a/_worktrees/live-deploy-design-1779216240/docs/plans/2026-05-19-live-deploy-validation-design.md b/_worktrees/live-deploy-design-1779216240/docs/plans/2026-05-19-live-deploy-validation-design.md new file mode 100644 index 00000000..4a281901 --- /dev/null +++ b/_worktrees/live-deploy-design-1779216240/docs/plans/2026-05-19-live-deploy-validation-design.md @@ -0,0 +1,127 @@ +# Live-Deployment Example Validation — Design + +**Date:** 2026-05-19 +**Trigger:** The 2026-05-19 multi-repo QoL sweep validated plugin examples at SCHEMA level (`wfctl validate --skip-unknown-types`) but never ran them end-to-end against real cloud accounts. Promotion from `experimental` to `verified` remains a manual decision tied to GoCodeAlone-internal usage. +**Mode:** Design only (operator must provision CI secrets before execution). + +## Problem + +`wfctl validate --skip-unknown-types` confirms a YAML config parses and references known module types, but does not exercise the providers. A plugin can ship with valid YAML that fails at runtime — wrong field types, deprecated APIs, broken auth flow, infra-side rate limits. Today nothing catches this until an operator pins the plugin in a real project. + +Symptoms today: +- `aws#23`, `gcp#16`, `azure#20`, `tofu#11`, `ci-generator#9` shipped READMEs + examples that pass schema validation but have never been live-tested. +- `digitalocean` is the only IaC plugin with merged production usage (BMW + core-dump + workflow-compute). +- Promotion from `experimental` → `verified` requires a human to pin the plugin in a real wfctl.yaml. Slow + manual. + +## Goal + +Add a CI matrix that runs each P0/P1 plugin's `examples/minimal/config.yaml` against staging cloud accounts via OIDC. On green, the plugin auto-promotes to `verified` via a registry-manifest PR. On red, surfaces a failure annotation on the plugin's repo + opens a tracking issue. + +## Non-Goals + +- Replace the existing `wfctl validate --skip-unknown-types` schema check (still useful as a fast gate). +- Run examples for non-IaC, non-cloud plugins (eventbus, payments, twilio etc. need different validation surfaces — payments needs a Stripe test API; twilio needs a sandbox account; those are out of scope here). +- Validate against PRODUCTION accounts. Staging only. + +## Approach + +### Phase 1: per-provider staging accounts + OIDC + +For each IaC provider (AWS, GCP, Azure, DigitalOcean, OpenTofu-via-any-provider): + +1. Operator creates a dedicated staging account/project/subscription. +2. Configure GitHub OIDC trust: + - AWS: IAM role + `aws-actions/configure-aws-credentials@v4`. + - GCP: Workload Identity Federation + `google-github-actions/auth@v2`. + - Azure: federated credential + `azure/login@v2`. + - DigitalOcean: short-lived API token rotated via OIDC + Vault (or accept long-lived staging token). +3. Repo secret matrix populated: + ``` + STAGING_AWS_ROLE_ARN + STAGING_GCP_WORKLOAD_IDENTITY_PROVIDER + STAGING_GCP_SERVICE_ACCOUNT + STAGING_AZURE_TENANT_ID + AZURE_CLIENT_ID + AZURE_SUBSCRIPTION_ID + STAGING_DIGITALOCEAN_TOKEN + ``` + +### Phase 2: workflow main repo — `live-deploy.yml` workflow + +New workflow file `.github/workflows/live-deploy.yml`: + +```yaml +name: live-deploy +on: + workflow_dispatch: + schedule: + - cron: '0 6 * * 1' # weekly Monday 06:00 UTC +permissions: + id-token: write + contents: read + pull-requests: write +jobs: + live-deploy: + strategy: + fail-fast: false + matrix: + plugin: [aws, gcp, azure, digitalocean, tofu] + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + repository: GoCodeAlone/workflow-plugin-${{ matrix.plugin }} + ref: main + - uses: GoCodeAlone/setup-wfctl@v1 + - name: Configure cloud auth (${{ matrix.plugin }}) + run: ./.github/scripts/cloud-auth.sh ${{ matrix.plugin }} + - name: wfctl deploy --dry-run + run: wfctl deploy --dry-run examples/minimal/config.yaml + timeout-minutes: 10 + - name: Report status + if: always() + run: ./.github/scripts/report-validation.sh ${{ matrix.plugin }} ${{ job.status }} +``` + +### Phase 3: registry promotion / demotion + +`report-validation.sh` consumes the job status and: + +- **GREEN:** if the plugin's registry manifest is currently `experimental`, opens a PR against `workflow-registry` flipping it to `verified` with a citation to the workflow run. +- **RED:** if the plugin's manifest is currently `verified`, opens a PR demoting it to `experimental` + opens a tracking issue on the plugin repo. +- **NO CHANGE:** no PR; record the run in a structured artifact for audit. + +A `--explain` flag on `wfctl plugin marketplace-verify` (already shipped in `workflow#725`) can read the latest validation-run artifact to display the live-deploy history alongside the org-usage signal. + +## Assumptions + +- Operator can provision dedicated staging accounts. **Load-bearing.** Without this, the workflow is inert. +- `wfctl deploy --dry-run` exists for all 5 IaC providers. **Verify before execution** — `digitalocean` has it (used in BMW); `aws`/`gcp`/`azure`/`tofu` need verification. +- OIDC trust for all 4 cloud providers is achievable from GitHub Actions. True today — all 4 publish official auth actions. +- The cost of running the matrix weekly is bounded (no idle infra; each example deploys + tears down). Estimated <$5/week if examples are correctly written. +- Promotion/demotion PRs are admin-mergeable autonomously (per `feedback_admin_override_pr_merge`). + +## Top doubts + +1. **Cost runaway.** Examples that fail to tear down can leave cloud infra running. Mitigation: each example must include a teardown step + the workflow runs `wfctl destroy` after `deploy --dry-run` even on failure. Verify in alignment-check. +2. **Flaky staging.** Cloud-provider transient errors will cause false demotions. Mitigation: require 2 consecutive RED runs before opening a demotion PR. The first RED opens an investigation issue. +3. **wfctl deploy --dry-run semantics differ across providers.** If `--dry-run` is too permissive, the signal is meaningless. Verify each provider's dry-run actually validates IAM/API access, not just YAML. + +## Rollback + +The workflow is fire-and-forget reporting; rollback = disable the workflow file. No data is persisted in workflow main beyond the PR/issue creations, which are independently revertible. + +## Dependencies + +- `workflow#725` (`marketplace-verify` subcommand) is the human-readable counterpart to this automated promotion path. +- ADR-0041 (experimental-status marker) defines the manifest schema this PR exercises. + +## Success criteria + +- 5 IaC plugins (aws, gcp, azure, digitalocean, tofu) have weekly green live-deploy runs. +- Promotion PR opened automatically when a plugin earns its first GREEN run. +- Demotion PR + issue opened automatically when a `verified` plugin earns 2 consecutive REDs. +- Operator inspects monthly cost report and signs off on continued scheduled runs. + +## Out of scope + +- Live-deployment validation for non-IaC plugins (eventbus / payments / twilio / etc.) — file separate designs per provider category. +- Replacing the existing `wfctl validate --skip-unknown-types` schema check. +- Integration with workflow-cloud SaaS — this is workflow-engine OSS scope. From 961c4c80f5986d540614fa999a653841debf3fe8 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Tue, 19 May 2026 14:47:48 -0400 Subject: [PATCH 02/15] Revert "docs(plan): design for live-deployment example validation CI matrix" This reverts commit e4be5734dfcf93a34acb10c622bd148161f42d11. --- ...026-05-19-live-deploy-validation-design.md | 127 ------------------ 1 file changed, 127 deletions(-) delete mode 100644 _worktrees/live-deploy-design-1779216240/docs/plans/2026-05-19-live-deploy-validation-design.md diff --git a/_worktrees/live-deploy-design-1779216240/docs/plans/2026-05-19-live-deploy-validation-design.md b/_worktrees/live-deploy-design-1779216240/docs/plans/2026-05-19-live-deploy-validation-design.md deleted file mode 100644 index 4a281901..00000000 --- a/_worktrees/live-deploy-design-1779216240/docs/plans/2026-05-19-live-deploy-validation-design.md +++ /dev/null @@ -1,127 +0,0 @@ -# Live-Deployment Example Validation — Design - -**Date:** 2026-05-19 -**Trigger:** The 2026-05-19 multi-repo QoL sweep validated plugin examples at SCHEMA level (`wfctl validate --skip-unknown-types`) but never ran them end-to-end against real cloud accounts. Promotion from `experimental` to `verified` remains a manual decision tied to GoCodeAlone-internal usage. -**Mode:** Design only (operator must provision CI secrets before execution). - -## Problem - -`wfctl validate --skip-unknown-types` confirms a YAML config parses and references known module types, but does not exercise the providers. A plugin can ship with valid YAML that fails at runtime — wrong field types, deprecated APIs, broken auth flow, infra-side rate limits. Today nothing catches this until an operator pins the plugin in a real project. - -Symptoms today: -- `aws#23`, `gcp#16`, `azure#20`, `tofu#11`, `ci-generator#9` shipped READMEs + examples that pass schema validation but have never been live-tested. -- `digitalocean` is the only IaC plugin with merged production usage (BMW + core-dump + workflow-compute). -- Promotion from `experimental` → `verified` requires a human to pin the plugin in a real wfctl.yaml. Slow + manual. - -## Goal - -Add a CI matrix that runs each P0/P1 plugin's `examples/minimal/config.yaml` against staging cloud accounts via OIDC. On green, the plugin auto-promotes to `verified` via a registry-manifest PR. On red, surfaces a failure annotation on the plugin's repo + opens a tracking issue. - -## Non-Goals - -- Replace the existing `wfctl validate --skip-unknown-types` schema check (still useful as a fast gate). -- Run examples for non-IaC, non-cloud plugins (eventbus, payments, twilio etc. need different validation surfaces — payments needs a Stripe test API; twilio needs a sandbox account; those are out of scope here). -- Validate against PRODUCTION accounts. Staging only. - -## Approach - -### Phase 1: per-provider staging accounts + OIDC - -For each IaC provider (AWS, GCP, Azure, DigitalOcean, OpenTofu-via-any-provider): - -1. Operator creates a dedicated staging account/project/subscription. -2. Configure GitHub OIDC trust: - - AWS: IAM role + `aws-actions/configure-aws-credentials@v4`. - - GCP: Workload Identity Federation + `google-github-actions/auth@v2`. - - Azure: federated credential + `azure/login@v2`. - - DigitalOcean: short-lived API token rotated via OIDC + Vault (or accept long-lived staging token). -3. Repo secret matrix populated: - ``` - STAGING_AWS_ROLE_ARN - STAGING_GCP_WORKLOAD_IDENTITY_PROVIDER + STAGING_GCP_SERVICE_ACCOUNT - STAGING_AZURE_TENANT_ID + AZURE_CLIENT_ID + AZURE_SUBSCRIPTION_ID - STAGING_DIGITALOCEAN_TOKEN - ``` - -### Phase 2: workflow main repo — `live-deploy.yml` workflow - -New workflow file `.github/workflows/live-deploy.yml`: - -```yaml -name: live-deploy -on: - workflow_dispatch: - schedule: - - cron: '0 6 * * 1' # weekly Monday 06:00 UTC -permissions: - id-token: write - contents: read - pull-requests: write -jobs: - live-deploy: - strategy: - fail-fast: false - matrix: - plugin: [aws, gcp, azure, digitalocean, tofu] - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - with: - repository: GoCodeAlone/workflow-plugin-${{ matrix.plugin }} - ref: main - - uses: GoCodeAlone/setup-wfctl@v1 - - name: Configure cloud auth (${{ matrix.plugin }}) - run: ./.github/scripts/cloud-auth.sh ${{ matrix.plugin }} - - name: wfctl deploy --dry-run - run: wfctl deploy --dry-run examples/minimal/config.yaml - timeout-minutes: 10 - - name: Report status - if: always() - run: ./.github/scripts/report-validation.sh ${{ matrix.plugin }} ${{ job.status }} -``` - -### Phase 3: registry promotion / demotion - -`report-validation.sh` consumes the job status and: - -- **GREEN:** if the plugin's registry manifest is currently `experimental`, opens a PR against `workflow-registry` flipping it to `verified` with a citation to the workflow run. -- **RED:** if the plugin's manifest is currently `verified`, opens a PR demoting it to `experimental` + opens a tracking issue on the plugin repo. -- **NO CHANGE:** no PR; record the run in a structured artifact for audit. - -A `--explain` flag on `wfctl plugin marketplace-verify` (already shipped in `workflow#725`) can read the latest validation-run artifact to display the live-deploy history alongside the org-usage signal. - -## Assumptions - -- Operator can provision dedicated staging accounts. **Load-bearing.** Without this, the workflow is inert. -- `wfctl deploy --dry-run` exists for all 5 IaC providers. **Verify before execution** — `digitalocean` has it (used in BMW); `aws`/`gcp`/`azure`/`tofu` need verification. -- OIDC trust for all 4 cloud providers is achievable from GitHub Actions. True today — all 4 publish official auth actions. -- The cost of running the matrix weekly is bounded (no idle infra; each example deploys + tears down). Estimated <$5/week if examples are correctly written. -- Promotion/demotion PRs are admin-mergeable autonomously (per `feedback_admin_override_pr_merge`). - -## Top doubts - -1. **Cost runaway.** Examples that fail to tear down can leave cloud infra running. Mitigation: each example must include a teardown step + the workflow runs `wfctl destroy` after `deploy --dry-run` even on failure. Verify in alignment-check. -2. **Flaky staging.** Cloud-provider transient errors will cause false demotions. Mitigation: require 2 consecutive RED runs before opening a demotion PR. The first RED opens an investigation issue. -3. **wfctl deploy --dry-run semantics differ across providers.** If `--dry-run` is too permissive, the signal is meaningless. Verify each provider's dry-run actually validates IAM/API access, not just YAML. - -## Rollback - -The workflow is fire-and-forget reporting; rollback = disable the workflow file. No data is persisted in workflow main beyond the PR/issue creations, which are independently revertible. - -## Dependencies - -- `workflow#725` (`marketplace-verify` subcommand) is the human-readable counterpart to this automated promotion path. -- ADR-0041 (experimental-status marker) defines the manifest schema this PR exercises. - -## Success criteria - -- 5 IaC plugins (aws, gcp, azure, digitalocean, tofu) have weekly green live-deploy runs. -- Promotion PR opened automatically when a plugin earns its first GREEN run. -- Demotion PR + issue opened automatically when a `verified` plugin earns 2 consecutive REDs. -- Operator inspects monthly cost report and signs off on continued scheduled runs. - -## Out of scope - -- Live-deployment validation for non-IaC plugins (eventbus / payments / twilio / etc.) — file separate designs per provider category. -- Replacing the existing `wfctl validate --skip-unknown-types` schema check. -- Integration with workflow-cloud SaaS — this is workflow-engine OSS scope. From 6743bc566d4b3d29544c950e906c003e96927318 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sat, 23 May 2026 23:53:13 -0400 Subject: [PATCH 03/15] docs(plan): wfctl plugin verify-capabilities design (workflow#765) Closes the runtime-truth gap left by #762/#764 static check (validate-contract). Spawns plugin binary, calls PluginService.GetManifest, diffs against plugin.json. New subcommand sibling to validate-contract. --- .../2026-05-24-verify-capabilities-design.md | 130 ++++++++++++++++++ 1 file changed, 130 insertions(+) create mode 100644 docs/plans/2026-05-24-verify-capabilities-design.md diff --git a/docs/plans/2026-05-24-verify-capabilities-design.md b/docs/plans/2026-05-24-verify-capabilities-design.md new file mode 100644 index 00000000..ee1155ad --- /dev/null +++ b/docs/plans/2026-05-24-verify-capabilities-design.md @@ -0,0 +1,130 @@ +# `wfctl plugin verify-capabilities` Design + +**Issue:** [workflow#765](https://github.com/GoCodeAlone/workflow/issues/765) +**Status:** Approved 2026-05-24 — awaiting adversarial design review +**Author:** Jon Langevin +**Parent contract:** workflow#762 (plugin-version contract); workflow#764 (Layer 3b 71-PR sweep that wired all 64 plugin repos) + +## Problem + +`wfctl plugin validate-contract` is a SOURCE-tree static analysis pass: it verifies plugin.json parses, capabilities are populated, main.go calls `sdk.ResolveBuildVersion`, goreleaser carries `-X *.Version=` ldflag. It does NOT verify that the resulting binary, when spawned, actually advertises those capabilities at runtime. + +A plugin can pass `validate-contract` and still ship a binary whose `PluginService.GetManifest` reports different capabilities (or an empty BuildVersion) than declared. This is the gap workflow#764's audit identified: 4 of 66 repos READY by static check, but no test confirms runtime equivalence. + +Now that all 64 plugin repos are wired with `sdk.WithBuildVersion` and `sdk.ResolveBuildVersion`, the runtime manifest carries the build-time tag and the declared capabilities. The truth-check is finally implementable. + +## Solution + +New subcommand `wfctl plugin verify-capabilities` that spawns the plugin binary as a go-plugin subprocess, gRPC-dials, calls `PluginService.GetManifest`, and diffs the returned `Manifest` against the source-tree `plugin.json`. + +### Synopsis + +``` +wfctl plugin verify-capabilities [--binary ] [--build-tag ] +``` + +### Behavior + +1. Read `/plugin.json` → declared `PluginManifest` (parse via `workflow/plugin.PluginManifest`). +2. Resolve binary: + - `--binary ` supplied: use as-is. Skip build. + - Else: discover plugin's main package by convention (`/cmd//main.go` where `` matches `plugin.json.name` OR sole `cmd/*/main.go` if only one). Build with: + ``` + go build -ldflags="-X /internal.Version=" -o ./cmd/ + ``` + `` = `--build-tag` if set, else `v0.0.0-verify`. +3. Spawn binary via `goplugin.NewClient` with workflow's canonical ext handshake. +4. gRPC-dial; call `pb.PluginServiceClient.GetManifest(Empty)`. +5. Diff strict: + +| Field | Comparison | Failure | +|---|---|---| +| `name` | exact string equal | plugin.json.name ≠ manifest.name | +| `version` | non-empty | manifest.version is "" (BuildVersion never wired) | +| `minEngineVersion` | exact string equal | drift | +| `moduleTypes` | set-equal (sort then compare) | missing/extra entry | +| `stepTypes` | set-equal | missing/extra entry | +| `triggerTypes` | set-equal | missing/extra entry | +| `resourceTypes` | set-equal | missing/extra entry | +| `iacTypes` | set-equal | missing/extra entry | + +6. Exit 0 on clean. Exit 1 with field-by-field report: + ``` + FAIL workflow-plugin-foo v0.0.0-verify (plugin.json) + error: 2 mismatch(es) + - moduleTypes: declared [a, b]; binary advertises [a, c] + missing-from-binary: [b]; extra-in-binary: [c] + - version: binary advertises "" (sdk.WithBuildVersion not wired) + ``` +7. Cleanup: kill spawned process; rm tmpfile if built. + +### CI integration + +Append step to scaffold-template `release.yml` post-goreleaser, pre-publish: + +```yaml +- name: Verify capabilities (post-build runtime check) + run: wfctl plugin verify-capabilities --binary dist/_linux_amd64/ . +``` + +This closes the truth loop: the binary that ships IS the one whose manifest gets verified. Drift between declared and runtime is caught BEFORE the release publishes. + +The scaffold-side wiring is a follow-up commit on `scaffold-workflow-plugin` after this workflow PR lands, not part of this design's scope. + +## Files (workflow repo) + +- `cmd/wfctl/plugin_verify_capabilities.go` — subcommand entry + impl +- `cmd/wfctl/plugin_verify_capabilities_test.go` — table-driven tests (sample plugin compiled at test time) +- `cmd/wfctl/plugin.go` — register `case "verify-capabilities"` in dispatcher +- `docs/PLUGIN_RELEASE_GATES.md` — append `Verify-capabilities` section + +## Architecture choices + +| Choice | Picked | Rejected (reason) | +|---|---|---| +| Surface | new subcommand | flag on validate-contract (mixes static + runtime; harder to skip in CI when binary unavailable) | +| Binary source | build by default + `--binary` override | require `--binary` always (more friction for dev); `go run` (debug.ReadBuildInfo can't surface ldflag tag) | +| Diff strictness | strict (exact / set-equal) | permissive warn-only (weak CI gate); --strict/--permissive flag (more surface; defer YAGNI) | +| CI integration | release.yml post-build | ci.yml on PR (binary not built yet); both (two integration points) | +| Capability list semantics | set (sort both before compare) | sequence (brittle on canonical order) | + +## Assumptions + +1. `sdk.ServePluginFull` / `sdk.Serve` / `sdk.ServeIaCPlugin` all wire `PluginService.GetManifest` via the SDK-internal `grpcServer.GetManifest` impl. Verified: workflow v0.62.0 SDK source (`plugin/external/sdk/grpc_server.go:148` and `plugin/external/sdk/iacserver.go:301` for the iacPluginServiceBridge). No plugin type opts out. +2. The go-plugin handshake config used by SDK is importable from `workflow/plugin/external/sdk` (or exposed via a public helper in `workflow/plugin/external`). If only internal: wfctl already imports the SDK; same import path works. +3. Plugin's main-package directory is discoverable from `` by convention: + - Single `cmd/*/main.go` → use it. + - Multiple → require `cmd//main.go` to exist where `` = plugin.json's `name`. + - Neither matches → exit 1 with "unable to locate main package; pass --binary explicitly". +4. `--build-tag` ldflag path matches the canonical `-X /internal.Version=` pattern (workflow#762 contract). For non-internal Version locations (gcp: `provider/...`; security: root pkg; edge-compute: `internal/plugin/...`), the design must derive the path from plugin.json's main module + `internal.Version` convention. **Mitigation**: if --binary is supplied, no build → no ldflag path issue. Build-from-source mode is best-effort dev convenience; production verification uses --binary. +5. CI runner has `go` available when build-from-source path is used. Release pipelines that use `--binary dist/...` skip this. +6. Plugins that don't satisfy IaCProviderRequiredServer / ModuleProvider interfaces are still spawnable; SDK handles "no provider" gracefully via the bridge. Verified per SDK source. + +## Failure modes addressed + +- **Spawn fails (binary doesn't run)**: hard exit 1 with goplugin's error message. +- **gRPC-dial fails (handshake mismatch / process exits)**: hard exit 1. +- **GetManifest RPC returns Unimplemented**: hard exit 1 with "plugin SDK appears stale; expected GetManifest available since workflow v0.40". +- **Plugin process leaks (verify exits between spawn + RPC)**: explicit `client.Kill()` in defer + tmpfile cleanup via `t.Cleanup` in tests / `defer os.Remove` in CLI. +- **Malformed plugin.json**: existing validate-contract logic; reuse PluginManifest.Validate() before spawning. + +## Rollback + +Runtime-affecting change class: this PR adds a CLI subcommand + (follow-up) a CI step. Rollback path: + +- **Subcommand**: revert workflow PR; subcommand stops being registered; `wfctl plugin verify-capabilities` → "unknown subcommand" error. Existing pipelines unaffected — nothing depends on it yet. +- **CI step** (scaffold follow-up): revert the scaffold-template PR; release.yml stops invoking the subcommand; existing release pipelines still pass. + +Backwards-compat: subcommand is purely additive. wfctl v0.62.0 callers without verify-capabilities continue to work; downstream consumers only see the new subcommand when they upgrade their CI's wfctl pin to ≥ the release containing this PR. + +## Decisions to record + +This design makes one non-trivial trade-off that warrants an ADR per `recording-decisions`: + +- **Build-from-source default vs --binary-only**: chose build-from-source for dev convenience despite goreleaser-logic duplication risk (mitigated by recommending --binary in CI). This is divergent from `validate-contract` which is pure-static. ADR target: `decisions/NNNN-verify-capabilities-build-strategy.md`. + +## Open questions for adversarial review + +- Should verify-capabilities ALSO be invoked from `ci.yml` (PR gate) on every push, building from source? Today's design defers (release.yml only). Trade-off: PR-time verification catches drift earlier but doubles build cost per PR. +- Should the diff report machine-readable JSON via `--json` flag for downstream tooling? Defer per YAGNI; workflow#762 follow-up if needed. +- For multi-binary repos (migrations: 2 binaries), should verify-capabilities run against EACH? Today's design verifies the plugin matching plugin.json.name only. Multi-binary follow-up if needed. From a14035feb2300dcacea2e789b1653bab7da0d44a Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sat, 23 May 2026 23:58:30 -0400 Subject: [PATCH 04/15] =?UTF-8?q?docs(plan):=20verify-capabilities=20desig?= =?UTF-8?q?n=20cycle=202=20(adversarial=20review=20FAIL=20=E2=86=92=20revi?= =?UTF-8?q?sed)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cycle-1 had 3 Critical findings: - F1: diff table fields not on pb.Manifest (only 6 scalars; type lists on separate RPCs) - F2: IaC bridge returns Unimplemented for type-list RPCs - F4: handshake export path wrong (external.Handshake, not sdk) Plus 4 Important + 1 Minor. Cycle-2 pivots: - Adopt reviewer Option 3: Manifest scalars + ContractRegistry only (drops per-type RPC walk; collapses F1+F2+F7) - Adopt reviewer Option 2: drop build-from-source default; require --binary (addresses F5+F8 + cycle-1 self-challenge) - Adopt reviewer Option 1 partial: extract spawnAndDial helper from plugin_conformance.go (kills F3 duplication) - Fix F4 import; F5 version-diff matrix; F7 contract-diff for declared-contract half --- .../2026-05-24-verify-capabilities-design.md | 159 ++++++++++-------- 1 file changed, 93 insertions(+), 66 deletions(-) diff --git a/docs/plans/2026-05-24-verify-capabilities-design.md b/docs/plans/2026-05-24-verify-capabilities-design.md index ee1155ad..6c202759 100644 --- a/docs/plans/2026-05-24-verify-capabilities-design.md +++ b/docs/plans/2026-05-24-verify-capabilities-design.md @@ -1,62 +1,78 @@ # `wfctl plugin verify-capabilities` Design **Issue:** [workflow#765](https://github.com/GoCodeAlone/workflow/issues/765) -**Status:** Approved 2026-05-24 — awaiting adversarial design review +**Status:** Revised 2026-05-24 (cycle 2 — adversarial review found 3 Critical) — awaiting re-review **Author:** Jon Langevin **Parent contract:** workflow#762 (plugin-version contract); workflow#764 (Layer 3b 71-PR sweep that wired all 64 plugin repos) +## Revision history + +- **Cycle 1** (2026-05-24): initial design. FAILED adversarial review (3 Critical). +- **Cycle 2** (2026-05-24, this version): pivoted to Manifest-scalars + ContractRegistry-only diff (reviewer Option 3); dropped build-from-source (reviewer Option 2); reuses spawn-and-dial pattern from `plugin_conformance.go:462-515` (reviewer Option 1 partial); fixed handshake import; fixed version-diff rule; added contract-diff for user-intent's "+ declared contract" half. + ## Problem `wfctl plugin validate-contract` is a SOURCE-tree static analysis pass: it verifies plugin.json parses, capabilities are populated, main.go calls `sdk.ResolveBuildVersion`, goreleaser carries `-X *.Version=` ldflag. It does NOT verify that the resulting binary, when spawned, actually advertises those capabilities at runtime. -A plugin can pass `validate-contract` and still ship a binary whose `PluginService.GetManifest` reports different capabilities (or an empty BuildVersion) than declared. This is the gap workflow#764's audit identified: 4 of 66 repos READY by static check, but no test confirms runtime equivalence. +A plugin can pass `validate-contract` and still ship a binary whose `PluginService.GetManifest` returns an empty BuildVersion, or whose typed-IaC service registration drifts from `plugin.json` declarations. Workflow#764's audit identified this gap: 4 of 66 repos READY by static check, but no test confirms runtime equivalence. -Now that all 64 plugin repos are wired with `sdk.WithBuildVersion` and `sdk.ResolveBuildVersion`, the runtime manifest carries the build-time tag and the declared capabilities. The truth-check is finally implementable. +Now that all 64 plugin repos are wired with `sdk.WithBuildVersion` and `sdk.ResolveBuildVersion`, the runtime manifest carries the build-time tag and the SDK exposes contract discovery. The truth-check is finally implementable. ## Solution -New subcommand `wfctl plugin verify-capabilities` that spawns the plugin binary as a go-plugin subprocess, gRPC-dials, calls `PluginService.GetManifest`, and diffs the returned `Manifest` against the source-tree `plugin.json`. +New subcommand `wfctl plugin verify-capabilities` that spawns the plugin binary as a go-plugin subprocess, gRPC-dials, calls `PluginService.GetManifest` AND `PluginService.GetContractRegistry`, and diffs results against the source-tree `plugin.json`. ### Synopsis ``` -wfctl plugin verify-capabilities [--binary ] [--build-tag ] +wfctl plugin verify-capabilities --binary +``` + +`--binary` is REQUIRED. (Cycle-1 build-from-source convenience dropped; see reviewer Option 2 — caused false-PASS in dev when ldflag paths varied per repo. Documented invocation pattern: + +```bash +# Local development: +go build -o /tmp/p ./cmd/ +wfctl plugin verify-capabilities --binary /tmp/p . + +# CI (post-goreleaser): +wfctl plugin verify-capabilities --binary dist/_linux_amd64/ . ``` ### Behavior 1. Read `/plugin.json` → declared `PluginManifest` (parse via `workflow/plugin.PluginManifest`). -2. Resolve binary: - - `--binary ` supplied: use as-is. Skip build. - - Else: discover plugin's main package by convention (`/cmd//main.go` where `` matches `plugin.json.name` OR sole `cmd/*/main.go` if only one). Build with: - ``` - go build -ldflags="-X /internal.Version=" -o ./cmd/ - ``` - `` = `--build-tag` if set, else `v0.0.0-verify`. -3. Spawn binary via `goplugin.NewClient` with workflow's canonical ext handshake. -4. gRPC-dial; call `pb.PluginServiceClient.GetManifest(Empty)`. -5. Diff strict: - -| Field | Comparison | Failure | +2. Spawn `` via shared `spawnAndDial(ctx, binaryPath) (*external.PluginAdapter, func() cleanup)` helper extracted from `cmd/wfctl/plugin_conformance.go:462-515`. Uses `external.Handshake` from `workflow/plugin/external/handshake.go:23`. Returns the typed adapter + a deferred cleanup that kills the process. +3. Two RPC calls, in order: + - **`PluginService.GetManifest(Empty) → pb.Manifest`** — covers Name + Version + Author + Description + ConfigMutable + SampleCategory (the only 6 scalar fields on `pb.Manifest`). + - **`PluginService.GetContractRegistry(Empty) → pb.ContractRegistry`** — enumerates the typed gRPC services the plugin registered (e.g. `pb.IaCProviderRequiredServer`, `pb.IaCProviderFinalizerServer`). +4. Diff strict: + +| Field | Source A | Source B | Rule | +|---|---|---|---| +| `Name` | binary `Manifest.Name` | plugin.json `name` | exact string equal | +| `Version` | binary `Manifest.Version` | plugin.json `version` + git tag context | rule below | +| `MinEngineVersion` | n/a (not on wire) | plugin.json `minEngineVersion` | static check only — not verified at runtime; flagged in design notes per F1 | +| `ModuleTypes`/`StepTypes`/`TriggerTypes` lists | n/a (not on Manifest; per-type RPCs Unimplemented in IaC bridge per F2) | plugin.json `capabilities.*` | static-check via `validate-contract` only; NOT in scope here | +| Contract services | binary `ContractRegistry.contracts[*].service_name` | plugin.json `capabilities.iacResources` etc. (if declared) | set-equal (sort then compare) — only if plugin.json declares iac capabilities | + +**Version rule** (resolves cycle-1 inconsistency F5): + +| plugin.json.version | binary Manifest.Version | Rule | |---|---|---| -| `name` | exact string equal | plugin.json.name ≠ manifest.name | -| `version` | non-empty | manifest.version is "" (BuildVersion never wired) | -| `minEngineVersion` | exact string equal | drift | -| `moduleTypes` | set-equal (sort then compare) | missing/extra entry | -| `stepTypes` | set-equal | missing/extra entry | -| `triggerTypes` | set-equal | missing/extra entry | -| `resourceTypes` | set-equal | missing/extra entry | -| `iacTypes` | set-equal | missing/extra entry | - -6. Exit 0 on clean. Exit 1 with field-by-field report: +| `"0.0.0"` (dev sentinel) | non-empty AND not `"0.0.0"` | PASS (binary was ldflag-built; verify-capabilities running on a CI artifact) | +| `"0.0.0"` (dev sentinel) | `"0.0.0"` (sentinel; ldflag never fired) | FAIL — ldflag injection missing; the truth-loop bug verify-capabilities exists to catch | +| `"X.Y.Z"` (release manifest) | `"vX.Y.Z"` or `"X.Y.Z"` | PASS — normalized comparison; strip leading `v` from binary value before compare | +| `"X.Y.Z"` (release manifest) | empty / `"0.0.0"` / anything else | FAIL — drift between declared release version and shipped binary | + +5. Exit 0 on clean. Exit 1 with field-by-field report: ``` - FAIL workflow-plugin-foo v0.0.0-verify (plugin.json) + FAIL workflow-plugin-foo (plugin.json) error: 2 mismatch(es) - - moduleTypes: declared [a, b]; binary advertises [a, c] - missing-from-binary: [b]; extra-in-binary: [c] - - version: binary advertises "" (sdk.WithBuildVersion not wired) + - version: plugin.json="1.2.3"; binary Manifest.Version="" (sdk.WithBuildVersion not wired or ldflag missing) + - contracts: plugin.json declares [iac.foo]; binary advertises [iac.foo, iac.bar] + extra-in-binary: [iac.bar] ``` -7. Cleanup: kill spawned process; rm tmpfile if built. ### CI integration @@ -64,67 +80,78 @@ Append step to scaffold-template `release.yml` post-goreleaser, pre-publish: ```yaml - name: Verify capabilities (post-build runtime check) - run: wfctl plugin verify-capabilities --binary dist/_linux_amd64/ . + run: wfctl plugin verify-capabilities --binary dist/_linux_amd64// . ``` -This closes the truth loop: the binary that ships IS the one whose manifest gets verified. Drift between declared and runtime is caught BEFORE the release publishes. - The scaffold-side wiring is a follow-up commit on `scaffold-workflow-plugin` after this workflow PR lands, not part of this design's scope. ## Files (workflow repo) -- `cmd/wfctl/plugin_verify_capabilities.go` — subcommand entry + impl -- `cmd/wfctl/plugin_verify_capabilities_test.go` — table-driven tests (sample plugin compiled at test time) -- `cmd/wfctl/plugin.go` — register `case "verify-capabilities"` in dispatcher -- `docs/PLUGIN_RELEASE_GATES.md` — append `Verify-capabilities` section +- `cmd/wfctl/plugin_spawn.go` — NEW; extracts `spawnAndDial(ctx, binaryPath) (*external.PluginAdapter, func())` from `plugin_conformance.go:462-515`. Both `plugin conformance` AND new `plugin verify-capabilities` call it. Refactor cleanup belongs in conformance file. +- `cmd/wfctl/plugin_conformance.go` — refactored to call new shared helper; behavior unchanged. +- `cmd/wfctl/plugin_verify_capabilities.go` — NEW; subcommand entry + diff impl. +- `cmd/wfctl/plugin_verify_capabilities_test.go` — table-driven tests using a stub plugin binary built at test time via `go build` invocation within the test (no external fixtures). +- `cmd/wfctl/plugin.go` — register `case "verify-capabilities"`. +- `docs/PLUGIN_RELEASE_GATES.md` — append `Verify-Capabilities` section. -## Architecture choices +## Architecture choices (cycle-2 revised) | Choice | Picked | Rejected (reason) | |---|---|---| -| Surface | new subcommand | flag on validate-contract (mixes static + runtime; harder to skip in CI when binary unavailable) | -| Binary source | build by default + `--binary` override | require `--binary` always (more friction for dev); `go run` (debug.ReadBuildInfo can't surface ldflag tag) | -| Diff strictness | strict (exact / set-equal) | permissive warn-only (weak CI gate); --strict/--permissive flag (more surface; defer YAGNI) | -| CI integration | release.yml post-build | ci.yml on PR (binary not built yet); both (two integration points) | -| Capability list semantics | set (sort both before compare) | sequence (brittle on canonical order) | +| Surface | new subcommand | flag on validate-contract (mixes static + runtime; harder to skip in CI when binary unavailable); flag on `plugin conformance` (conformance is IaC-typed-only today; verify targets all 64 repos) | +| Binary source | REQUIRE `--binary` | build-from-source default — REJECTED cycle 2 per reviewer Option 2; admitted false-PASS in dev when ldflag paths vary (gcp `provider/`, security root, edge-compute `internal/plugin/`); recommended invocation is documented in §Synopsis | +| Diff scope | Manifest scalars + ContractRegistry | per-type RPCs (GetModuleTypes/GetStepTypes/GetTriggerTypes) — REJECTED cycle 2 per reviewer Option 3 + F2; IaC bridge returns Unimplemented for these; verify-capabilities collapses to Manifest + ContractRegistry (the only runtime surface that works uniformly) | +| Version diff rule | matrix in §Behavior step 4 (dev-sentinel-vs-real, normalized v-prefix) | cycle-1 "non-empty" — REJECTED cycle 2 per F5; broke truth-loop premise (the field user said was load-bearing) | +| Spawn-and-dial | extract shared helper, refactor conformance | re-implement from scratch (REJECTED per F3); add unconditional verify to conformance (REJECTED — different scope, see Surface row) | +| Capability list semantics | set (sort then compare) | sequence (brittle on canonical order) | ## Assumptions -1. `sdk.ServePluginFull` / `sdk.Serve` / `sdk.ServeIaCPlugin` all wire `PluginService.GetManifest` via the SDK-internal `grpcServer.GetManifest` impl. Verified: workflow v0.62.0 SDK source (`plugin/external/sdk/grpc_server.go:148` and `plugin/external/sdk/iacserver.go:301` for the iacPluginServiceBridge). No plugin type opts out. -2. The go-plugin handshake config used by SDK is importable from `workflow/plugin/external/sdk` (or exposed via a public helper in `workflow/plugin/external`). If only internal: wfctl already imports the SDK; same import path works. -3. Plugin's main-package directory is discoverable from `` by convention: - - Single `cmd/*/main.go` → use it. - - Multiple → require `cmd//main.go` to exist where `` = plugin.json's `name`. - - Neither matches → exit 1 with "unable to locate main package; pass --binary explicitly". -4. `--build-tag` ldflag path matches the canonical `-X /internal.Version=` pattern (workflow#762 contract). For non-internal Version locations (gcp: `provider/...`; security: root pkg; edge-compute: `internal/plugin/...`), the design must derive the path from plugin.json's main module + `internal.Version` convention. **Mitigation**: if --binary is supplied, no build → no ldflag path issue. Build-from-source mode is best-effort dev convenience; production verification uses --binary. -5. CI runner has `go` available when build-from-source path is used. Release pipelines that use `--binary dist/...` skip this. -6. Plugins that don't satisfy IaCProviderRequiredServer / ModuleProvider interfaces are still spawnable; SDK handles "no provider" gracefully via the bridge. Verified per SDK source. +1. **`PluginService.GetManifest` exists and returns 6 scalars uniformly across all plugin types.** Verified per `/tmp/wfprobe/plugin/external/proto/plugin.proto:96-104` (Manifest message def) + `/tmp/wfprobe/plugin/external/sdk/grpc_server.go:148-174` (non-IaC impl) + `/tmp/wfprobe/plugin/external/sdk/iacserver.go:301` (iacPluginServiceBridge.GetManifest). Cycle 1 assumption #1 was too broad ("manifest carries the type lists") — corrected to the actual wire-shape this cycle. + +2. **`external.Handshake` is exported at `workflow/plugin/external/handshake.go:23`.** Verified. wfctl already imports this package in `plugin_conformance.go`. (Cycle-1 assumption #2 had wrong import path; corrected this cycle per F4.) + +3. **`PluginService.GetContractRegistry` returns the registered typed gRPC services with `service_name`, `kind`, `mode` per `pb.ContractDescriptor`.** Verified per `/tmp/wfprobe/plugin/external/sdk/iacserver.go:297-301` + `/tmp/wfprobe/plugin/external/sdk/grpc_server.go` (non-IaC bridge). All plugins built with workflow v0.62.0+ register at least the PluginService bridge → ContractRegistry call succeeds. + +4. **`plugin.json.capabilities` field shape is the authoritative on-disk schema for the diff.** Verified per `/tmp/wfprobe/plugin/manifest.go:43-47` — fields are `ModuleTypes`, `StepTypes`, `TriggerTypes`, `WorkflowTypes`, `WiringHooks`. For IaC, the relevant declarations live under different keys (TBD during implementation — read `PluginManifest` struct precisely; per F1, cycle-1 assumed wrong field names). + +5. **CI runner already has wfctl pinned to the release containing this PR.** Already handled by `GoCodeAlone/setup-wfctl@v1` step in scaffold release.yml; CI step is additive. + +6. **--binary path is the exact post-goreleaser binary that will publish.** Documented in §Synopsis CI invocation. If operator passes a stale binary, verify-capabilities runs against stale binary — that's the operator's bug, not the design's. ## Failure modes addressed -- **Spawn fails (binary doesn't run)**: hard exit 1 with goplugin's error message. -- **gRPC-dial fails (handshake mismatch / process exits)**: hard exit 1. -- **GetManifest RPC returns Unimplemented**: hard exit 1 with "plugin SDK appears stale; expected GetManifest available since workflow v0.40". -- **Plugin process leaks (verify exits between spawn + RPC)**: explicit `client.Kill()` in defer + tmpfile cleanup via `t.Cleanup` in tests / `defer os.Remove` in CLI. -- **Malformed plugin.json**: existing validate-contract logic; reuse PluginManifest.Validate() before spawning. +- **Spawn fails (binary doesn't run)**: hard exit 1 with goplugin's error (handled by shared spawnAndDial helper). +- **gRPC-dial fails (handshake mismatch / process exits)**: hard exit 1 (same path). +- **GetManifest returns `Unimplemented`**: hard exit 1 with "plugin SDK appears stale; expected GetManifest available since workflow v0.20" (cite when SDK started serving it). +- **GetContractRegistry returns `Unimplemented`**: WARN-only — older SDK versions; skip the contract-diff and report partial verification. +- **Plugin process leaks (verify exits between spawn + RPC)**: explicit `client.Kill()` in defer + tmpfile cleanup in spawnAndDial helper. +- **Malformed plugin.json**: existing `PluginManifest.Validate()` before spawning; reuse from validate-contract path. +- **Mid-RPC plugin crash**: gRPC error surfaces as failed call; exit 1 with the RPC error message. ## Rollback Runtime-affecting change class: this PR adds a CLI subcommand + (follow-up) a CI step. Rollback path: - **Subcommand**: revert workflow PR; subcommand stops being registered; `wfctl plugin verify-capabilities` → "unknown subcommand" error. Existing pipelines unaffected — nothing depends on it yet. +- **Shared spawnAndDial helper refactor**: revert is part of the same PR; conformance returns to inline pattern; no behavior change in conformance. - **CI step** (scaffold follow-up): revert the scaffold-template PR; release.yml stops invoking the subcommand; existing release pipelines still pass. -Backwards-compat: subcommand is purely additive. wfctl v0.62.0 callers without verify-capabilities continue to work; downstream consumers only see the new subcommand when they upgrade their CI's wfctl pin to ≥ the release containing this PR. +Backwards-compat: subcommand is purely additive. wfctl callers without verify-capabilities continue to work; downstream consumers only see the new subcommand when they upgrade their CI's wfctl pin. ## Decisions to record -This design makes one non-trivial trade-off that warrants an ADR per `recording-decisions`: +Two non-trivial trade-offs that warrant ADRs per `recording-decisions`: + +1. **--binary required (no build-from-source)** — chose explicit-binary requirement over dev-mode convenience to avoid the per-repo ldflag-path divergence that produces false-PASS results. ADR target: `decisions/NNNN-verify-capabilities-binary-required.md`. -- **Build-from-source default vs --binary-only**: chose build-from-source for dev convenience despite goreleaser-logic duplication risk (mitigated by recommending --binary in CI). This is divergent from `validate-contract` which is pure-static. ADR target: `decisions/NNNN-verify-capabilities-build-strategy.md`. +2. **Scope limited to Manifest + ContractRegistry** — chose NOT to walk per-type RPCs (GetModuleTypes/GetStepTypes/GetTriggerTypes) because the IaC SDK bridge returns `Unimplemented` for them; full per-type verification stays in `validate-contract` (static, source-of-truth from plugin.json). ADR target: `decisions/NNNN-verify-capabilities-scope.md`. -## Open questions for adversarial review +## What this design does NOT do (explicit non-goals) -- Should verify-capabilities ALSO be invoked from `ci.yml` (PR gate) on every push, building from source? Today's design defers (release.yml only). Trade-off: PR-time verification catches drift earlier but doubles build cost per PR. -- Should the diff report machine-readable JSON via `--json` flag for downstream tooling? Defer per YAGNI; workflow#762 follow-up if needed. -- For multi-binary repos (migrations: 2 binaries), should verify-capabilities run against EACH? Today's design verifies the plugin matching plugin.json.name only. Multi-binary follow-up if needed. +- **Does NOT verify ModuleTypes/StepTypes/TriggerTypes** at runtime. Those lists are declared in plugin.json and statically lintable; per F1 they are not on the wire and per F2 the per-type RPCs are Unimplemented for IaC plugins. Static check (`validate-contract`) remains the truth source for those. +- **Does NOT build the binary**. Operator must produce one (local: `go build`; CI: goreleaser). Documented in §Synopsis. +- **Does NOT verify `minEngineVersion`** at runtime — not on `pb.Manifest`. Static-check responsibility (already in `validate-contract`). +- **Does NOT run inside `plugin conformance`** (subcommand stays separate per architecture choice). Shared spawn-and-dial helper is the only refactor. +- **Does NOT use `--json` output mode** (defer per YAGNI; follow-up if needed). +- **Does NOT support multi-binary repos** (verify-capabilities runs against the binary passed via `--binary`; multi-binary needs multiple invocations). From 6421c69849ff2d70f24db2365d9ddb509b6dcea8 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sun, 24 May 2026 00:14:10 -0400 Subject: [PATCH 05/15] docs(plan): verify-capabilities design cycle 3 (scope-down per cycle-2 review) Cycle-2 FAILed with 2 Critical + 4 Important. Cycle-3 pivots: - Drop contract-diff entirely (F-NEW-1: plugin.json has no iacResources key; F-NEW-2: BuildContractRegistry emits infra-internal noise). Defer to follow-up issue #766 (introduces capabilities.iacServices schema + server-side BuildContractRegistryForPlugin filter). - Fix version-diff matrix (F-NEW-3): sentinel set is {"", "(devel)...", "0.0.0"}, cite buildversion.go:36-42. Matrix's isSentinel() predicate covers all SDK-emitted forms. - Specify test fixtures concretely (F-NEW-5): testdata/verify_capabilities/{good,release-good,missing-ldflag,version-drift,name-drift}/ mirroring plugin_validate_contract precedent. - CI binary path via jq dist/artifacts.json lookup (F-NEW-6). Scope reduces to: Manifest.Name exact + Manifest.Version sentinel-pattern matrix. Catches the truth-loop bug (ldflag-missing) the user actually asked about. Contract-diff deferred to follow-up after plugin.json schema gains LHS field. --- .../2026-05-24-verify-capabilities-design.md | 174 ++++++++++-------- 1 file changed, 97 insertions(+), 77 deletions(-) diff --git a/docs/plans/2026-05-24-verify-capabilities-design.md b/docs/plans/2026-05-24-verify-capabilities-design.md index 6c202759..941ccd81 100644 --- a/docs/plans/2026-05-24-verify-capabilities-design.md +++ b/docs/plans/2026-05-24-verify-capabilities-design.md @@ -1,26 +1,25 @@ # `wfctl plugin verify-capabilities` Design **Issue:** [workflow#765](https://github.com/GoCodeAlone/workflow/issues/765) -**Status:** Revised 2026-05-24 (cycle 2 — adversarial review found 3 Critical) — awaiting re-review +**Status:** Revised 2026-05-24 (cycle 3 — scope-down per cycle-2 review) — awaiting re-review **Author:** Jon Langevin -**Parent contract:** workflow#762 (plugin-version contract); workflow#764 (Layer 3b 71-PR sweep that wired all 64 plugin repos) +**Parent contract:** workflow#762 (plugin-version contract); workflow#764 (Layer 3b 71-PR sweep) ## Revision history -- **Cycle 1** (2026-05-24): initial design. FAILED adversarial review (3 Critical). -- **Cycle 2** (2026-05-24, this version): pivoted to Manifest-scalars + ContractRegistry-only diff (reviewer Option 3); dropped build-from-source (reviewer Option 2); reuses spawn-and-dial pattern from `plugin_conformance.go:462-515` (reviewer Option 1 partial); fixed handshake import; fixed version-diff rule; added contract-diff for user-intent's "+ declared contract" half. +- **Cycle 1**: initial design. FAILED — 3 Critical (diff fields not on wire; IaC bridge Unimplemented; handshake path wrong). +- **Cycle 2**: pivot to Manifest scalars + ContractRegistry. FAILED — 2 Critical (plugin.json has no iacResources key; BuildContractRegistry returns ALL services including infra-internal noise). +- **Cycle 3** (this version): scope-down per reviewer Option 2. **Drop contract-diff entirely**; defer to follow-up issue (#766 to be filed). Verify Manifest scalars + correct sentinel-pattern Version. Test fixtures specified per `plugin_validate_contract` precedent. ## Problem -`wfctl plugin validate-contract` is a SOURCE-tree static analysis pass: it verifies plugin.json parses, capabilities are populated, main.go calls `sdk.ResolveBuildVersion`, goreleaser carries `-X *.Version=` ldflag. It does NOT verify that the resulting binary, when spawned, actually advertises those capabilities at runtime. +`wfctl plugin validate-contract` is a SOURCE-tree static analysis pass. It verifies the source tree but cannot verify the BINARY actually surfaces what plugin.json declares at runtime. Workflow#764's Layer 3b sweep wired all 64 plugin repos with `sdk.WithBuildVersion + sdk.ResolveBuildVersion + ldflag`. But nothing yet verifies that the SHIPPED binary's runtime `Manifest.Version` matches the build tag. -A plugin can pass `validate-contract` and still ship a binary whose `PluginService.GetManifest` returns an empty BuildVersion, or whose typed-IaC service registration drifts from `plugin.json` declarations. Workflow#764's audit identified this gap: 4 of 66 repos READY by static check, but no test confirms runtime equivalence. - -Now that all 64 plugin repos are wired with `sdk.WithBuildVersion` and `sdk.ResolveBuildVersion`, the runtime manifest carries the build-time tag and the SDK exposes contract discovery. The truth-check is finally implementable. +The single load-bearing truth-loop the user named: **"binary's BuildVersion is populated and matches the release tag — proving ldflag fired during goreleaser build."** This is the bug class verify-capabilities exists to catch. ## Solution -New subcommand `wfctl plugin verify-capabilities` that spawns the plugin binary as a go-plugin subprocess, gRPC-dials, calls `PluginService.GetManifest` AND `PluginService.GetContractRegistry`, and diffs results against the source-tree `plugin.json`. +New subcommand `wfctl plugin verify-capabilities` that spawns the plugin binary as a go-plugin subprocess, calls `PluginService.GetManifest`, and verifies the returned `Manifest.Version` is non-sentinel + matches plugin.json + git tag context. Scope strictly limited to fields the SDK reliably surfaces; broader contract-diff deferred to follow-up. ### Synopsis @@ -28,130 +27,151 @@ New subcommand `wfctl plugin verify-capabilities` that spawns the plugin binary wfctl plugin verify-capabilities --binary ``` -`--binary` is REQUIRED. (Cycle-1 build-from-source convenience dropped; see reviewer Option 2 — caused false-PASS in dev when ldflag paths varied per repo. Documented invocation pattern: +`--binary` REQUIRED (cycle-1 build-from-source dropped per reviewer Option 2; produced false-PASS in dev when ldflag paths varied per repo). Documented invocation: ```bash # Local development: -go build -o /tmp/p ./cmd/ +go build -ldflags="-X github.com/GoCodeAlone/workflow-plugin-/internal.Version=v1.2.3" \ + -o /tmp/p ./cmd/ wfctl plugin verify-capabilities --binary /tmp/p . -# CI (post-goreleaser): -wfctl plugin verify-capabilities --binary dist/_linux_amd64/ . +# CI (post-goreleaser, in release.yml): +wfctl plugin verify-capabilities --binary "$(jq -r '.[0].path' dist/artifacts.json)" . +# (jq picks the right architecture's binary from goreleaser's artifacts.json +# manifest — avoids hard-coding goreleaser layout per F-NEW-6 cycle 2) ``` ### Behavior -1. Read `/plugin.json` → declared `PluginManifest` (parse via `workflow/plugin.PluginManifest`). -2. Spawn `` via shared `spawnAndDial(ctx, binaryPath) (*external.PluginAdapter, func() cleanup)` helper extracted from `cmd/wfctl/plugin_conformance.go:462-515`. Uses `external.Handshake` from `workflow/plugin/external/handshake.go:23`. Returns the typed adapter + a deferred cleanup that kills the process. -3. Two RPC calls, in order: - - **`PluginService.GetManifest(Empty) → pb.Manifest`** — covers Name + Version + Author + Description + ConfigMutable + SampleCategory (the only 6 scalar fields on `pb.Manifest`). - - **`PluginService.GetContractRegistry(Empty) → pb.ContractRegistry`** — enumerates the typed gRPC services the plugin registered (e.g. `pb.IaCProviderRequiredServer`, `pb.IaCProviderFinalizerServer`). +1. Load `/plugin.json`. Parse + run `PluginManifest.Validate()` (reuse from validate-contract). +2. Spawn `` via shared `spawnAndDial(ctx, binaryPath) (*external.PluginAdapter, func())` helper extracted from `cmd/wfctl/plugin_conformance.go:462-515`. Uses `external.Handshake` from `workflow/plugin/external/handshake.go:23`. +3. Call `PluginService.GetManifest(Empty) → pb.Manifest` (6 scalar fields per `plugin/external/proto/plugin.proto:96-104`). 4. Diff strict: -| Field | Source A | Source B | Rule | +| Field | Source A (plugin.json) | Source B (binary `Manifest`) | Rule | |---|---|---|---| -| `Name` | binary `Manifest.Name` | plugin.json `name` | exact string equal | -| `Version` | binary `Manifest.Version` | plugin.json `version` + git tag context | rule below | -| `MinEngineVersion` | n/a (not on wire) | plugin.json `minEngineVersion` | static check only — not verified at runtime; flagged in design notes per F1 | -| `ModuleTypes`/`StepTypes`/`TriggerTypes` lists | n/a (not on Manifest; per-type RPCs Unimplemented in IaC bridge per F2) | plugin.json `capabilities.*` | static-check via `validate-contract` only; NOT in scope here | -| Contract services | binary `ContractRegistry.contracts[*].service_name` | plugin.json `capabilities.iacResources` etc. (if declared) | set-equal (sort then compare) — only if plugin.json declares iac capabilities | +| `Name` | `name` | `Name` | exact string equal; FAIL on drift | +| `Version` | `version` | `Version` | matrix below | -**Version rule** (resolves cycle-1 inconsistency F5): +**Version rule** (cycle-3, addresses F-NEW-3 with correct sentinel pattern): -| plugin.json.version | binary Manifest.Version | Rule | -|---|---|---| -| `"0.0.0"` (dev sentinel) | non-empty AND not `"0.0.0"` | PASS (binary was ldflag-built; verify-capabilities running on a CI artifact) | -| `"0.0.0"` (dev sentinel) | `"0.0.0"` (sentinel; ldflag never fired) | FAIL — ldflag injection missing; the truth-loop bug verify-capabilities exists to catch | -| `"X.Y.Z"` (release manifest) | `"vX.Y.Z"` or `"X.Y.Z"` | PASS — normalized comparison; strip leading `v` from binary value before compare | -| `"X.Y.Z"` (release manifest) | empty / `"0.0.0"` / anything else | FAIL — drift between declared release version and shipped binary | +The dev-sentinel set is `{"", "(devel)", "0.0.0"}` plus any string starting with `"(devel)"` (since `buildInfoVersion()` returns `"(devel) [@ [.dirty]]"`). Source: `/tmp/wfprobe/plugin/external/sdk/buildversion.go:36-42`. + +``` +isSentinel(v) := v == "" || v == "0.0.0" || strings.HasPrefix(v, "(devel)") +``` -5. Exit 0 on clean. Exit 1 with field-by-field report: +| plugin.json `version` | binary `Manifest.Version` | Outcome | Rationale | +|---|---|---|---| +| `"0.0.0"` (dev sentinel) | non-sentinel (e.g. `"v1.2.3"`) | **PASS** | binary built via ldflag-injecting CI; verify-capabilities running on a real artifact | +| `"0.0.0"` | sentinel (`""` / `"(devel)..."` / `"0.0.0"`) | **FAIL** | ldflag injection missing — the truth-loop bug verify-capabilities exists to catch | +| `"X.Y.Z"` (release) | `"vX.Y.Z"` or `"X.Y.Z"` | **PASS** | normalize: strip leading `v` from binary, then exact compare to plugin.json | +| `"X.Y.Z"` | sentinel | **FAIL** | plugin.json declares release tag but binary lacks ldflag injection | +| `"X.Y.Z"` | non-sentinel and not `X.Y.Z` | **FAIL** | drift between declared release version and shipped binary | + +5. Exit 0 on clean. Exit 1 with report: ``` FAIL workflow-plugin-foo (plugin.json) - error: 2 mismatch(es) - - version: plugin.json="1.2.3"; binary Manifest.Version="" (sdk.WithBuildVersion not wired or ldflag missing) - - contracts: plugin.json declares [iac.foo]; binary advertises [iac.foo, iac.bar] - extra-in-binary: [iac.bar] + error: 1 mismatch + - version: plugin.json="1.2.3"; binary Manifest.Version="(devel) [@ a1b2c3d]" + (sdk.ResolveBuildVersion returned the build-info fallback; ldflag injection missing) ``` ### CI integration -Append step to scaffold-template `release.yml` post-goreleaser, pre-publish: +Append to scaffold-template `release.yml` post-goreleaser, pre-publish: ```yaml - name: Verify capabilities (post-build runtime check) - run: wfctl plugin verify-capabilities --binary dist/_linux_amd64// . + run: | + BIN=$(jq -r '[.[] | select(.type=="Binary") | .path] | .[0]' dist/artifacts.json) + "${RUNNER_TEMP}/wfctl-bin/wfctl" plugin verify-capabilities --binary "$BIN" . ``` -The scaffold-side wiring is a follow-up commit on `scaffold-workflow-plugin` after this workflow PR lands, not part of this design's scope. +`dist/artifacts.json` is goreleaser's manifest of all built artifacts; jq picks the first binary (any arch — verify-capabilities only needs ONE binary to confirm ldflag fired). Avoids hard-coding goreleaser's directory layout per cycle-2 F-NEW-6. + +Scaffold-side wiring is a follow-up commit on `scaffold-workflow-plugin` after this workflow PR lands — not part of this design's scope. ## Files (workflow repo) -- `cmd/wfctl/plugin_spawn.go` — NEW; extracts `spawnAndDial(ctx, binaryPath) (*external.PluginAdapter, func())` from `plugin_conformance.go:462-515`. Both `plugin conformance` AND new `plugin verify-capabilities` call it. Refactor cleanup belongs in conformance file. +- `cmd/wfctl/plugin_spawn.go` — NEW; extracts `spawnAndDial(ctx, binaryPath) (*external.PluginAdapter, func())` from `plugin_conformance.go:462-515`. Both `plugin conformance` AND new `plugin verify-capabilities` call it. - `cmd/wfctl/plugin_conformance.go` — refactored to call new shared helper; behavior unchanged. - `cmd/wfctl/plugin_verify_capabilities.go` — NEW; subcommand entry + diff impl. -- `cmd/wfctl/plugin_verify_capabilities_test.go` — table-driven tests using a stub plugin binary built at test time via `go build` invocation within the test (no external fixtures). +- `cmd/wfctl/plugin_verify_capabilities_test.go` — table-driven tests against `testdata/verify_capabilities//`. +- `cmd/wfctl/testdata/verify_capabilities/` — NEW fixture tree, mirrors `cmd/wfctl/testdata/plugin_validate_contract/` precedent: + - `good/` — plugin.json `version="0.0.0"`, ldflag-injected binary tag `v0.1.0`. Expect PASS. + - `release-good/` — plugin.json `version="1.2.3"`, ldflag tag `v1.2.3`. Expect PASS. + - `missing-ldflag/` — plugin.json `version="0.0.0"`, no ldflag (binary surfaces sentinel `(devel)`). Expect FAIL. + - `version-drift/` — plugin.json `version="1.2.3"`, ldflag tag `v0.9.0`. Expect FAIL. + - `name-drift/` — plugin.json `name="foo"`, binary advertises `Name="bar"`. Expect FAIL. + + Each scenario contains `plugin.json` + `cmd/plugin/main.go` (minimal `sdk.Serve` stub). Tests compile the fixture via `go build` invocation at test-time (one fixture per scenario), then run verify-capabilities against the compiled binary in `t.TempDir()`. Pattern mirrors existing `validate-contract` test approach where source fixtures + plugin.json live in testdata. - `cmd/wfctl/plugin.go` — register `case "verify-capabilities"`. - `docs/PLUGIN_RELEASE_GATES.md` — append `Verify-Capabilities` section. -## Architecture choices (cycle-2 revised) +## Architecture choices (cycle-3) | Choice | Picked | Rejected (reason) | |---|---|---| -| Surface | new subcommand | flag on validate-contract (mixes static + runtime; harder to skip in CI when binary unavailable); flag on `plugin conformance` (conformance is IaC-typed-only today; verify targets all 64 repos) | -| Binary source | REQUIRE `--binary` | build-from-source default — REJECTED cycle 2 per reviewer Option 2; admitted false-PASS in dev when ldflag paths vary (gcp `provider/`, security root, edge-compute `internal/plugin/`); recommended invocation is documented in §Synopsis | -| Diff scope | Manifest scalars + ContractRegistry | per-type RPCs (GetModuleTypes/GetStepTypes/GetTriggerTypes) — REJECTED cycle 2 per reviewer Option 3 + F2; IaC bridge returns Unimplemented for these; verify-capabilities collapses to Manifest + ContractRegistry (the only runtime surface that works uniformly) | -| Version diff rule | matrix in §Behavior step 4 (dev-sentinel-vs-real, normalized v-prefix) | cycle-1 "non-empty" — REJECTED cycle 2 per F5; broke truth-loop premise (the field user said was load-bearing) | -| Spawn-and-dial | extract shared helper, refactor conformance | re-implement from scratch (REJECTED per F3); add unconditional verify to conformance (REJECTED — different scope, see Surface row) | -| Capability list semantics | set (sort then compare) | sequence (brittle on canonical order) | +| Surface | new subcommand | flag on validate-contract (cycle 2 considered + rejected: mixes static + runtime); flag on `plugin conformance` (conformance IaC-typed-only today) | +| Binary source | REQUIRE `--binary` | build-from-source default — rejected cycle 2: false-PASS in dev with per-repo ldflag-path variance | +| Diff scope | Manifest.Name + Manifest.Version ONLY | + per-type RPCs (rejected cycle 2: Unimplemented in IaC bridge); + ContractRegistry (rejected cycle 3: plugin.json has no iacResources LHS + BuildContractRegistry returns infra-internal noise; defer to follow-up #766) | +| Version diff rule | sentinel-pattern matrix (`{"", "(devel)...", "0.0.0"}`) | cycle-1 "non-empty" (broke truth-loop); cycle-2 literal "0.0.0" (didn't match SDK's `(devel)` output) | +| Spawn-and-dial | extract shared helper, refactor conformance | re-implement from scratch (cycle 1 F3); leave conformance unchanged (duplicates ~200 LOC) | +| CI binary path | `jq -r '...' dist/artifacts.json` lookup | hard-coded `dist/_linux_amd64/` (cycle 2 F-NEW-6: goreleaser layout varies by arch + goamd64 level) | ## Assumptions -1. **`PluginService.GetManifest` exists and returns 6 scalars uniformly across all plugin types.** Verified per `/tmp/wfprobe/plugin/external/proto/plugin.proto:96-104` (Manifest message def) + `/tmp/wfprobe/plugin/external/sdk/grpc_server.go:148-174` (non-IaC impl) + `/tmp/wfprobe/plugin/external/sdk/iacserver.go:301` (iacPluginServiceBridge.GetManifest). Cycle 1 assumption #1 was too broad ("manifest carries the type lists") — corrected to the actual wire-shape this cycle. +1. **`PluginService.GetManifest` exists + uniformly returns 6 scalars across all plugin types.** Verified: `/tmp/wfprobe/plugin/external/proto/plugin.proto:96-104` defines `Manifest{name, version, author, description, config_mutable, sample_category}`. Non-IaC impl at `plugin/external/sdk/grpc_server.go:148-174`. IaC bridge impl at `plugin/external/sdk/iacserver.go:301`. All plugins built with workflow v0.62.0+ serve this RPC. (Pre-v0.20 plugins predate the RPC; not in our 64-repo target set per #764 audit — all pinned to v0.62.0.) -2. **`external.Handshake` is exported at `workflow/plugin/external/handshake.go:23`.** Verified. wfctl already imports this package in `plugin_conformance.go`. (Cycle-1 assumption #2 had wrong import path; corrected this cycle per F4.) +2. **`external.Handshake` is exported at `workflow/plugin/external/handshake.go:23`.** Verified. wfctl imports it in `plugin_conformance.go`. -3. **`PluginService.GetContractRegistry` returns the registered typed gRPC services with `service_name`, `kind`, `mode` per `pb.ContractDescriptor`.** Verified per `/tmp/wfprobe/plugin/external/sdk/iacserver.go:297-301` + `/tmp/wfprobe/plugin/external/sdk/grpc_server.go` (non-IaC bridge). All plugins built with workflow v0.62.0+ register at least the PluginService bridge → ContractRegistry call succeeds. +3. **`sdk.ResolveBuildVersion` sentinel set is `{"", "dev", "(devel)"}` plus the function returns `"(devel) [@ sha[.dirty]]"` from build-info fallback.** Verified at `/tmp/wfprobe/plugin/external/sdk/buildversion.go:36-42`. Diff matrix's `isSentinel()` predicate covers all SDK-emitted sentinel forms. -4. **`plugin.json.capabilities` field shape is the authoritative on-disk schema for the diff.** Verified per `/tmp/wfprobe/plugin/manifest.go:43-47` — fields are `ModuleTypes`, `StepTypes`, `TriggerTypes`, `WorkflowTypes`, `WiringHooks`. For IaC, the relevant declarations live under different keys (TBD during implementation — read `PluginManifest` struct precisely; per F1, cycle-1 assumed wrong field names). +4. **plugin.json `version` field is canonical authority for declared version.** Verified at `plugin/manifest.go`. Set by goreleaser before-hook at release time per workflow#762 contract. -5. **CI runner already has wfctl pinned to the release containing this PR.** Already handled by `GoCodeAlone/setup-wfctl@v1` step in scaffold release.yml; CI step is additive. +5. **CI runner has `jq` available.** `jq` is preinstalled on `ubuntu-latest` GitHub runners (verified standard image). Custom runners must install it. -6. **--binary path is the exact post-goreleaser binary that will publish.** Documented in §Synopsis CI invocation. If operator passes a stale binary, verify-capabilities runs against stale binary — that's the operator's bug, not the design's. +6. **`--binary` path points to the exact post-goreleaser binary that will publish.** Operator responsibility. Documented in §Synopsis with `jq dist/artifacts.json` pattern for CI. ## Failure modes addressed -- **Spawn fails (binary doesn't run)**: hard exit 1 with goplugin's error (handled by shared spawnAndDial helper). -- **gRPC-dial fails (handshake mismatch / process exits)**: hard exit 1 (same path). -- **GetManifest returns `Unimplemented`**: hard exit 1 with "plugin SDK appears stale; expected GetManifest available since workflow v0.20" (cite when SDK started serving it). -- **GetContractRegistry returns `Unimplemented`**: WARN-only — older SDK versions; skip the contract-diff and report partial verification. -- **Plugin process leaks (verify exits between spawn + RPC)**: explicit `client.Kill()` in defer + tmpfile cleanup in spawnAndDial helper. -- **Malformed plugin.json**: existing `PluginManifest.Validate()` before spawning; reuse from validate-contract path. -- **Mid-RPC plugin crash**: gRPC error surfaces as failed call; exit 1 with the RPC error message. +- **Spawn fails**: hard exit 1 with goplugin error (handled by shared spawnAndDial). +- **gRPC-dial fails**: hard exit 1. +- **GetManifest returns Unimplemented**: hard exit 1 with "plugin SDK appears stale; expected GetManifest available since workflow v0.20". +- **Plugin process leaks**: explicit `client.Kill()` in defer + cleanup via spawnAndDial helper. +- **Malformed plugin.json**: reuse `PluginManifest.Validate()`. +- **Mid-RPC plugin crash**: gRPC error surfaces; exit 1 with the error message. +- **plugin.json declares version "1.2.3" but binary ldflag never fired**: matrix row "X.Y.Z + sentinel → FAIL" catches this — the primary truth-loop bug class. +- **plugin.json declares "0.0.0" sentinel but binary somehow has non-sentinel Version**: matrix row "0.0.0 + non-sentinel → PASS" — acceptable, indicates CI artifact under verification. ## Rollback -Runtime-affecting change class: this PR adds a CLI subcommand + (follow-up) a CI step. Rollback path: - -- **Subcommand**: revert workflow PR; subcommand stops being registered; `wfctl plugin verify-capabilities` → "unknown subcommand" error. Existing pipelines unaffected — nothing depends on it yet. -- **Shared spawnAndDial helper refactor**: revert is part of the same PR; conformance returns to inline pattern; no behavior change in conformance. -- **CI step** (scaffold follow-up): revert the scaffold-template PR; release.yml stops invoking the subcommand; existing release pipelines still pass. +Runtime-affecting change class (CLI subcommand + CI step). Rollback path: -Backwards-compat: subcommand is purely additive. wfctl callers without verify-capabilities continue to work; downstream consumers only see the new subcommand when they upgrade their CI's wfctl pin. +- **Subcommand**: revert workflow PR; subcommand stops being registered. Existing pipelines unaffected — nothing depends on it yet. +- **Shared spawnAndDial helper refactor**: revert is part of same PR; conformance returns to inline pattern; no behavior change in conformance. +- **CI step** (scaffold follow-up): revert scaffold-template PR; release.yml stops invoking the subcommand; existing release pipelines still pass. -## Decisions to record +Backwards-compat: subcommand is purely additive. Older wfctl callers continue to work. -Two non-trivial trade-offs that warrant ADRs per `recording-decisions`: +## Decisions to record (ADRs) -1. **--binary required (no build-from-source)** — chose explicit-binary requirement over dev-mode convenience to avoid the per-repo ldflag-path divergence that produces false-PASS results. ADR target: `decisions/NNNN-verify-capabilities-binary-required.md`. +1. **--binary required (no build-from-source)** — chose explicit-binary requirement over dev-mode convenience to avoid per-repo ldflag-path divergence. ADR target: `decisions/NNNN-verify-capabilities-binary-required.md`. -2. **Scope limited to Manifest + ContractRegistry** — chose NOT to walk per-type RPCs (GetModuleTypes/GetStepTypes/GetTriggerTypes) because the IaC SDK bridge returns `Unimplemented` for them; full per-type verification stays in `validate-contract` (static, source-of-truth from plugin.json). ADR target: `decisions/NNNN-verify-capabilities-scope.md`. +2. **Scope limited to Name + Version** — chose NOT to verify ContractRegistry (no plugin.json LHS exists today + BuildContractRegistry returns infra-internal services). Follow-up issue (to be filed: workflow#766) introduces `capabilities.iacServices` schema on PluginManifest + a server-side `BuildContractRegistryForPlugin()` filter; cycle-4 of verify-capabilities can then add the contract-diff against a clean wire surface. ADR target: `decisions/NNNN-verify-capabilities-scope-name-version-only.md`. ## What this design does NOT do (explicit non-goals) -- **Does NOT verify ModuleTypes/StepTypes/TriggerTypes** at runtime. Those lists are declared in plugin.json and statically lintable; per F1 they are not on the wire and per F2 the per-type RPCs are Unimplemented for IaC plugins. Static check (`validate-contract`) remains the truth source for those. -- **Does NOT build the binary**. Operator must produce one (local: `go build`; CI: goreleaser). Documented in §Synopsis. -- **Does NOT verify `minEngineVersion`** at runtime — not on `pb.Manifest`. Static-check responsibility (already in `validate-contract`). -- **Does NOT run inside `plugin conformance`** (subcommand stays separate per architecture choice). Shared spawn-and-dial helper is the only refactor. -- **Does NOT use `--json` output mode** (defer per YAGNI; follow-up if needed). -- **Does NOT support multi-binary repos** (verify-capabilities runs against the binary passed via `--binary`; multi-binary needs multiple invocations). +- **Does NOT verify ModuleTypes/StepTypes/TriggerTypes** at runtime (per-type RPCs Unimplemented in IaC bridge; per-cycle-1 F2). Static check via `validate-contract` is authoritative. +- **Does NOT verify typed-contract surface** via `GetContractRegistry` (no plugin.json LHS + binary side emits infra-internal services as noise; cycle-2 F-NEW-1 + F-NEW-2). Deferred to follow-up issue. +- **Does NOT build the binary** — operator must produce one (local: `go build` with explicit ldflag; CI: goreleaser). +- **Does NOT verify `minEngineVersion`** at runtime (not on `pb.Manifest`). Static-check responsibility. +- **Does NOT run inside `plugin conformance`** (separate subcommand; shared helper is the only overlap). +- **Does NOT support `--json` output mode** (defer YAGNI; follow-up). +- **Does NOT support multi-binary repos** (runs against the binary passed via `--binary`; multi-binary repos invoke multiple times). +- **Does NOT verify Author/Description/ConfigMutable/SampleCategory** Manifest fields (Author/Description are display strings, drift not load-bearing; ConfigMutable/SampleCategory are runtime configuration not contract surface). Scope limited to fields that catch real bugs. + +## Open questions + +None blocking. Implementation can proceed. From 20a14014817447ee968e7525afd6eb9850d30a69 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sun, 24 May 2026 00:19:28 -0400 Subject: [PATCH 06/15] docs(plan): verify-capabilities design cycle 4 (fix cycle-3 sentinel + fixture precedent) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cycle-3 FAILed: 2 Critical (sentinel-set drift; fixture precedent miscited). Cycle-4 fixes: - F-CYCLE3-1: isSentinel() superset includes 'dev' to match SDK sentinel set {"", "dev", "(devel)"} from buildversion.go:36-42 + plugin.json '0.0.0' + HasPrefix '(devel)'. - F-CYCLE3-2: Cite correct precedent — plugin_conformance_test.go:buildFixtureBinary with per-fixture go.mod + replace directive + GOWORK=off. Validate-contract is pure-static; never builds. Per-fixture layout spec includes go.mod template. - F-CYCLE3-3: Add preflight binary-path validation + security note explicit; jq fallback ('// ""') guards against null returns. - F-CYCLE3-4: Rewrite Surface row honestly — conformance-flag (--mode manifest-verify) is technically viable since checkTypedIaCPlugin handles non-IaC too; pick new-subcommand on separation-of-concerns basis (truth-check vs typed-IaC scan). - F-CYCLE3-5: Explicit fixture PluginManifest.Validate() prereqs documented (name + version + author + description all required). --- .../2026-05-24-verify-capabilities-design.md | 60 +++++++++++++++---- 1 file changed, 48 insertions(+), 12 deletions(-) diff --git a/docs/plans/2026-05-24-verify-capabilities-design.md b/docs/plans/2026-05-24-verify-capabilities-design.md index 941ccd81..4008b33b 100644 --- a/docs/plans/2026-05-24-verify-capabilities-design.md +++ b/docs/plans/2026-05-24-verify-capabilities-design.md @@ -9,7 +9,8 @@ - **Cycle 1**: initial design. FAILED — 3 Critical (diff fields not on wire; IaC bridge Unimplemented; handshake path wrong). - **Cycle 2**: pivot to Manifest scalars + ContractRegistry. FAILED — 2 Critical (plugin.json has no iacResources key; BuildContractRegistry returns ALL services including infra-internal noise). -- **Cycle 3** (this version): scope-down per reviewer Option 2. **Drop contract-diff entirely**; defer to follow-up issue (#766 to be filed). Verify Manifest scalars + correct sentinel-pattern Version. Test fixtures specified per `plugin_validate_contract` precedent. +- **Cycle 3**: scope-down per reviewer Option 2. Drop contract-diff entirely. FAILED — 2 Critical (isSentinel() missed `"dev"` form per SDK sentinel set; cited wrong fixture precedent — `validate-contract` is pure-static, never compiles fixtures). +- **Cycle 4** (this version): fix isSentinel() superset; cite correct precedent (`plugin_conformance_test.go:buildFixtureBinary` + per-fixture `go.mod` with `replace` directive + `GOWORK=off`); add preflight binary-path validation + security note; rewrite "Surface" rationale honestly (conformance-flag is technically viable; pick new-subcommand on separation-of-concerns); explicit fixture `PluginManifest.Validate()` prereqs. ## Problem @@ -27,7 +28,16 @@ New subcommand `wfctl plugin verify-capabilities` that spawns the plugin binary wfctl plugin verify-capabilities --binary ``` -`--binary` REQUIRED (cycle-1 build-from-source dropped per reviewer Option 2; produced false-PASS in dev when ldflag paths varied per repo). Documented invocation: +`--binary` REQUIRED (cycle-1 build-from-source dropped per reviewer Option 2; produced false-PASS in dev when ldflag paths varied per repo). + +**⚠ Security note**: verify-capabilities EXECUTES `--binary ` as a subprocess. The plugin's `main()` runs (including any code before `sdk.ServeIaCPlugin`). Only run against build artifacts you trust. Matches `plugin conformance`'s posture. + +Preflight checks performed by the subcommand before exec: +- `os.Stat(path)` — file exists, is regular (not directory/symlink to one) +- `mode & 0o111 != 0` — file is executable +- `path != "" && path != "null"` — guards against CI lookups (jq) returning empty / null + +Documented invocation: ```bash # Local development: @@ -36,9 +46,10 @@ go build -ldflags="-X github.com/GoCodeAlone/workflow-plugin-/internal.Ver wfctl plugin verify-capabilities --binary /tmp/p . # CI (post-goreleaser, in release.yml): -wfctl plugin verify-capabilities --binary "$(jq -r '.[0].path' dist/artifacts.json)" . -# (jq picks the right architecture's binary from goreleaser's artifacts.json -# manifest — avoids hard-coding goreleaser layout per F-NEW-6 cycle 2) +wfctl plugin verify-capabilities --binary "$(jq -r '[.[] | select(.type=="Binary") | .path] | .[0] // ""' dist/artifacts.json)" . +# jq filter: +# - select(.type=="Binary") — goreleaser v2 schema; v1 also uses "Binary" for the binary artifact type +# - // "" fallback to empty string on null/missing → caught by preflight ``` ### Behavior @@ -53,14 +64,16 @@ wfctl plugin verify-capabilities --binary "$(jq -r '.[0].path' dist/artifacts.js | `Name` | `name` | `Name` | exact string equal; FAIL on drift | | `Version` | `version` | `Version` | matrix below | -**Version rule** (cycle-3, addresses F-NEW-3 with correct sentinel pattern): +**Version rule** (cycle-4, addresses F-CYCLE3-1 sentinel-set drift): -The dev-sentinel set is `{"", "(devel)", "0.0.0"}` plus any string starting with `"(devel)"` (since `buildInfoVersion()` returns `"(devel) [@ [.dirty]]"`). Source: `/tmp/wfprobe/plugin/external/sdk/buildversion.go:36-42`. +The dev-sentinel set MUST be a SUPERSET of SDK's `ResolveBuildVersion` sentinel set `{"", "dev", "(devel)"}` (source: `plugin/external/sdk/buildversion.go:36-42`) PLUS the on-disk plugin.json sentinel `"0.0.0"` (workflow#762 convention) PLUS any string starting with `"(devel)"` (since `buildInfoVersion()` returns `"(devel) [@ [.dirty]]"`). ``` -isSentinel(v) := v == "" || v == "0.0.0" || strings.HasPrefix(v, "(devel)") +isSentinel(v) := v == "" || v == "dev" || v == "0.0.0" || strings.HasPrefix(v, "(devel)") ``` +Note: `"dev"` is in the predicate because a plugin author may set `-X ...Version=dev` (or pipeline accident) and the binary's `Manifest.Version` then surfaces literal `"dev"` (since SDK only consults build-info fallback when the *declared* string matches; the ldflag-set value flows through unchanged). Without this entry, the matrix's row "0.0.0 + non-sentinel → PASS" would green-light a broken build. + | plugin.json `version` | binary `Manifest.Version` | Outcome | Rationale | |---|---|---|---| | `"0.0.0"` (dev sentinel) | non-sentinel (e.g. `"v1.2.3"`) | **PASS** | binary built via ldflag-injecting CI; verify-capabilities running on a real artifact | @@ -98,14 +111,37 @@ Scaffold-side wiring is a follow-up commit on `scaffold-workflow-plugin` after t - `cmd/wfctl/plugin_conformance.go` — refactored to call new shared helper; behavior unchanged. - `cmd/wfctl/plugin_verify_capabilities.go` — NEW; subcommand entry + diff impl. - `cmd/wfctl/plugin_verify_capabilities_test.go` — table-driven tests against `testdata/verify_capabilities//`. -- `cmd/wfctl/testdata/verify_capabilities/` — NEW fixture tree, mirrors `cmd/wfctl/testdata/plugin_validate_contract/` precedent: +- `cmd/wfctl/testdata/verify_capabilities/` — NEW fixture tree. **Precedent: `plugin_conformance_test.go:buildFixtureBinary` + `testdata/conformance/iac-pass/` layout** (NOT `validate-contract`'s pattern — that one is pure-static and never compiles fixtures). Each fixture is a self-contained compilable Go module. + + Scenarios: - `good/` — plugin.json `version="0.0.0"`, ldflag-injected binary tag `v0.1.0`. Expect PASS. - `release-good/` — plugin.json `version="1.2.3"`, ldflag tag `v1.2.3`. Expect PASS. - - `missing-ldflag/` — plugin.json `version="0.0.0"`, no ldflag (binary surfaces sentinel `(devel)`). Expect FAIL. + - `missing-ldflag/` — plugin.json `version="0.0.0"`, no ldflag (binary surfaces sentinel `(devel) [@ sha]`). Expect FAIL. - `version-drift/` — plugin.json `version="1.2.3"`, ldflag tag `v0.9.0`. Expect FAIL. - `name-drift/` — plugin.json `name="foo"`, binary advertises `Name="bar"`. Expect FAIL. - Each scenario contains `plugin.json` + `cmd/plugin/main.go` (minimal `sdk.Serve` stub). Tests compile the fixture via `go build` invocation at test-time (one fixture per scenario), then run verify-capabilities against the compiled binary in `t.TempDir()`. Pattern mirrors existing `validate-contract` test approach where source fixtures + plugin.json live in testdata. + Per-fixture layout (each scenario directory): + ``` + testdata/verify_capabilities// + plugin.json # MUST satisfy PluginManifest.Validate(): + # name, version, author, description ALL required + # (per plugin/manifest.go:194-225) + cmd/plugin/main.go # minimal `sdk.Serve(stub{}, sdk.WithBuildVersion(...))` + go.mod # module github.com/test/ + # replace github.com/GoCodeAlone/workflow => ../../../../.. + ``` + + Test invocation pattern (mirrors `plugin_conformance_test.go:buildFixtureBinary`): + ```go + cmd := exec.Command("go", "build", "-mod=mod", + "-ldflags=-X test//internal.Version=", + "-o", filepath.Join(t.TempDir(), "p"), "./cmd/plugin") + cmd.Dir = "testdata/verify_capabilities/" + cmd.Env = append(os.Environ(), "GOWORK=off") + ``` + `GOWORK=off` is mandatory — without it, the workspace go.work in the workflow repo overrides the per-fixture `replace` directive and the build resolves the wrong workflow version. + + Optional: factor `buildFixtureBinary` from `plugin_conformance_test.go` into a shared `cmd/wfctl/fixture_build_test.go` helper if both test files use it. Defer if duplication is minimal. - `cmd/wfctl/plugin.go` — register `case "verify-capabilities"`. - `docs/PLUGIN_RELEASE_GATES.md` — append `Verify-Capabilities` section. @@ -113,7 +149,7 @@ Scaffold-side wiring is a follow-up commit on `scaffold-workflow-plugin` after t | Choice | Picked | Rejected (reason) | |---|---|---| -| Surface | new subcommand | flag on validate-contract (cycle 2 considered + rejected: mixes static + runtime); flag on `plugin conformance` (conformance IaC-typed-only today) | +| Surface | new subcommand | flag on validate-contract (mixes static + runtime); flag on `plugin conformance --mode manifest-verify` (TECHNICALLY VIABLE — conformance's `checkTypedIaCPlugin` uses `external.NewExternalPluginAdapter` which handles ANY plugin type, not just IaC; chose new subcommand on separation-of-concerns basis: verify-capabilities is contract truth-check, conformance is typed-IaC interface scan — distinct mental models. F-CYCLE3-4 rejection rewritten per cycle-3 review) | | Binary source | REQUIRE `--binary` | build-from-source default — rejected cycle 2: false-PASS in dev with per-repo ldflag-path variance | | Diff scope | Manifest.Name + Manifest.Version ONLY | + per-type RPCs (rejected cycle 2: Unimplemented in IaC bridge); + ContractRegistry (rejected cycle 3: plugin.json has no iacResources LHS + BuildContractRegistry returns infra-internal noise; defer to follow-up #766) | | Version diff rule | sentinel-pattern matrix (`{"", "(devel)...", "0.0.0"}`) | cycle-1 "non-empty" (broke truth-loop); cycle-2 literal "0.0.0" (didn't match SDK's `(devel)` output) | From e846ec7841277cdd50a0437defb77fc56563ccb4 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sun, 24 May 2026 00:23:20 -0400 Subject: [PATCH 07/15] docs(plan): verify-capabilities design cycle 5 (fix cycle-4 helper cut + fixture path + jq filter) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cycle-4 FAIL: 3 Important. Cycle-5 fixes: - spawn-helper cut-point explicit (line 504 only; IaC-validation lines 505-513 stay in conformance caller) - fixture build is in-place (NOT copy-to-TempDir) + main.go at fixture root matching precedent - jq filter pins goos==linux + goarch matching runner (uname -m sed map x86_64→amd64, aarch64→arm64) --- .../2026-05-24-verify-capabilities-design.md | 45 ++++++++++++------- 1 file changed, 29 insertions(+), 16 deletions(-) diff --git a/docs/plans/2026-05-24-verify-capabilities-design.md b/docs/plans/2026-05-24-verify-capabilities-design.md index 4008b33b..13bd5692 100644 --- a/docs/plans/2026-05-24-verify-capabilities-design.md +++ b/docs/plans/2026-05-24-verify-capabilities-design.md @@ -10,7 +10,8 @@ - **Cycle 1**: initial design. FAILED — 3 Critical (diff fields not on wire; IaC bridge Unimplemented; handshake path wrong). - **Cycle 2**: pivot to Manifest scalars + ContractRegistry. FAILED — 2 Critical (plugin.json has no iacResources key; BuildContractRegistry returns ALL services including infra-internal noise). - **Cycle 3**: scope-down per reviewer Option 2. Drop contract-diff entirely. FAILED — 2 Critical (isSentinel() missed `"dev"` form per SDK sentinel set; cited wrong fixture precedent — `validate-contract` is pure-static, never compiles fixtures). -- **Cycle 4** (this version): fix isSentinel() superset; cite correct precedent (`plugin_conformance_test.go:buildFixtureBinary` + per-fixture `go.mod` with `replace` directive + `GOWORK=off`); add preflight binary-path validation + security note; rewrite "Surface" rationale honestly (conformance-flag is technically viable; pick new-subcommand on separation-of-concerns); explicit fixture `PluginManifest.Validate()` prereqs. +- **Cycle 4**: fix isSentinel() superset; cite correct precedent; preflight + security note; honest Surface rationale; fixture Validate() prereqs. FAILED — 3 Important (spawn-helper cut-point ambiguous; fixture go.mod relative-replace conflicts with copy-to-TempDir precedent; jq picks first binary non-deterministic on multiarch). +- **Cycle 5** (this version): explicit cut-point for `spawnAndDial` helper (line 504, IaC-validation stays in conformance); fixture build is in-place not copy-to-TempDir + main.go at fixture root not cmd/plugin/; jq filter pins goos+goarch matching runner. ## Problem @@ -46,10 +47,18 @@ go build -ldflags="-X github.com/GoCodeAlone/workflow-plugin-/internal.Ver wfctl plugin verify-capabilities --binary /tmp/p . # CI (post-goreleaser, in release.yml): -wfctl plugin verify-capabilities --binary "$(jq -r '[.[] | select(.type=="Binary") | .path] | .[0] // ""' dist/artifacts.json)" . +RUNNER_ARCH=$(uname -m | sed 's/x86_64/amd64/;s/aarch64/arm64/') +BIN=$(jq -r --arg arch "$RUNNER_ARCH" \ + '[.[] | select(.type=="Binary" and .goos=="linux" and .goarch==$arch)] | .[0].path // ""' \ + dist/artifacts.json) +wfctl plugin verify-capabilities --binary "$BIN" . # jq filter: -# - select(.type=="Binary") — goreleaser v2 schema; v1 also uses "Binary" for the binary artifact type -# - // "" fallback to empty string on null/missing → caught by preflight +# - select(.type=="Binary" and .goos=="linux" and .goarch==$arch) +# — pick THIS RUNNER's binary; multiarch builds (amd64+arm64+darwin+...) emit +# multiple Binary artifacts; without the goos/goarch filter, `.[0]` selects +# in arbitrary goreleaser-version-dependent order — may pick darwin-arm64 +# and produce "exec format error" on a linux-amd64 runner. +# - // "" fallback to empty string on null/missing → caught by --binary preflight ``` ### Behavior @@ -107,7 +116,7 @@ Scaffold-side wiring is a follow-up commit on `scaffold-workflow-plugin` after t ## Files (workflow repo) -- `cmd/wfctl/plugin_spawn.go` — NEW; extracts `spawnAndDial(ctx, binaryPath) (*external.PluginAdapter, func())` from `plugin_conformance.go:462-515`. Both `plugin conformance` AND new `plugin verify-capabilities` call it. +- `cmd/wfctl/plugin_spawn.go` — NEW; extracts `spawnAndDial(ctx, binaryPath) (*external.PluginAdapter, func())` from `plugin_conformance.go:462-504` ONLY (spawn + dial + Dispense + `NewExternalPluginAdapter`). **Cut-point: line 504 (right after adapter construction succeeds).** Lines 505-513 of conformance contain IaC-specific post-dial validation (`ContractRegistryError`, `AssertIaCPluginAdvertisesRequiredService`, typed-IaC adapter setup) — these REMAIN inline in `checkTypedIaCPlugin` after calling the helper. The helper is plugin-type-agnostic; IaC-specific assertions stay where they belong. - `cmd/wfctl/plugin_conformance.go` — refactored to call new shared helper; behavior unchanged. - `cmd/wfctl/plugin_verify_capabilities.go` — NEW; subcommand entry + diff impl. - `cmd/wfctl/plugin_verify_capabilities_test.go` — table-driven tests against `testdata/verify_capabilities//`. @@ -123,25 +132,29 @@ Scaffold-side wiring is a follow-up commit on `scaffold-workflow-plugin` after t Per-fixture layout (each scenario directory): ``` testdata/verify_capabilities// - plugin.json # MUST satisfy PluginManifest.Validate(): - # name, version, author, description ALL required - # (per plugin/manifest.go:194-225) - cmd/plugin/main.go # minimal `sdk.Serve(stub{}, sdk.WithBuildVersion(...))` - go.mod # module github.com/test/ - # replace github.com/GoCodeAlone/workflow => ../../../../.. + plugin.json # MUST satisfy PluginManifest.Validate(): + # name, version, author, description ALL required + # (per plugin/manifest.go:194-225) + main.go # minimal `sdk.Serve(stub{}, sdk.WithBuildVersion(...))` at fixture root + # (matches plugin_conformance_test.go fixture layout; NOT cmd/plugin/main.go) + go.mod # module github.com/test/ + # replace github.com/GoCodeAlone/workflow => ../../../../.. + # (5 ups: / → verify_capabilities/ → testdata/ → wfctl/ → cmd/ → REPO_ROOT) ``` - Test invocation pattern (mirrors `plugin_conformance_test.go:buildFixtureBinary`): + **Build pattern: in-place (NOT copy-to-TempDir)**. The fixture's `go.mod` carries a checked-in relative `replace` directive that resolves only when built at the fixture's own checked-in path. The precedent's `prepareConformanceFixture` (`plugin_conformance_test.go:401-419`) copies-to-TempDir AND rewrites go.mod with an absolute path — that pattern's overhead is unnecessary here because verify-capabilities fixtures don't need scenario mutation; they're read-only. Build in-place; emit the binary to `t.TempDir()`: + ```go cmd := exec.Command("go", "build", "-mod=mod", - "-ldflags=-X test//internal.Version=", - "-o", filepath.Join(t.TempDir(), "p"), "./cmd/plugin") + "-ldflags=-X github.com/test/.Version=", + "-o", filepath.Join(t.TempDir(), "p"), ".") cmd.Dir = "testdata/verify_capabilities/" cmd.Env = append(os.Environ(), "GOWORK=off") ``` - `GOWORK=off` is mandatory — without it, the workspace go.work in the workflow repo overrides the per-fixture `replace` directive and the build resolves the wrong workflow version. - Optional: factor `buildFixtureBinary` from `plugin_conformance_test.go` into a shared `cmd/wfctl/fixture_build_test.go` helper if both test files use it. Defer if duplication is minimal. + `GOWORK=off` is mandatory — without it, the workflow repo's workspace go.work overrides the per-fixture `replace` directive and the build resolves the wrong workflow version. The `-ldflags -X` package path is `github.com/test/` (the fixture's module path; Version var at fixture root), NOT a `/internal.Version` subpath (different from production plugins; simpler test fixture). + + Optional: factor `buildFixtureBinary` from `plugin_conformance_test.go:509-519` into a shared `cmd/wfctl/fixture_build_test.go` helper if both test files use it. Defer if duplication is minimal. - `cmd/wfctl/plugin.go` — register `case "verify-capabilities"`. - `docs/PLUGIN_RELEASE_GATES.md` — append `Verify-Capabilities` section. From 560ec668d7028a28449782de7ce3e9000bfe9e66 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sun, 24 May 2026 00:27:15 -0400 Subject: [PATCH 08/15] docs(plan): verify-capabilities design cycle 6 (propagate cycle-5 fixes fully) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cycle-5 FAIL: 3 Important regressions on cycle-4 fixes. Cycle-6 fixes: - F5-1: jq filter (goos+goarch pin) now propagated to BOTH Synopsis AND CI integration blocks (was only in Synopsis); strike 'any arch — only needs ONE binary' rationale (false: wrong-arch binary won't exec). - F5-2: pin -mod=readonly + check in fixture go.sum; sidesteps both source-tree pollution (mod=mod writes go.sum) and copy-to-TempDir overhead; maintenance note for regen on SDK dep bumps. - F5-3: explicit defer-transform note — line 484's defer client.Kill() returned as func() cleanup, caller defers; literal extraction would either kill plugin too early OR leak processes. - Reconcile line-range citations: 462-504 throughout (was 462-515 in Behavior step 2). --- .../2026-05-24-verify-capabilities-design.md | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/docs/plans/2026-05-24-verify-capabilities-design.md b/docs/plans/2026-05-24-verify-capabilities-design.md index 13bd5692..42da43a8 100644 --- a/docs/plans/2026-05-24-verify-capabilities-design.md +++ b/docs/plans/2026-05-24-verify-capabilities-design.md @@ -11,7 +11,8 @@ - **Cycle 2**: pivot to Manifest scalars + ContractRegistry. FAILED — 2 Critical (plugin.json has no iacResources key; BuildContractRegistry returns ALL services including infra-internal noise). - **Cycle 3**: scope-down per reviewer Option 2. Drop contract-diff entirely. FAILED — 2 Critical (isSentinel() missed `"dev"` form per SDK sentinel set; cited wrong fixture precedent — `validate-contract` is pure-static, never compiles fixtures). - **Cycle 4**: fix isSentinel() superset; cite correct precedent; preflight + security note; honest Surface rationale; fixture Validate() prereqs. FAILED — 3 Important (spawn-helper cut-point ambiguous; fixture go.mod relative-replace conflicts with copy-to-TempDir precedent; jq picks first binary non-deterministic on multiarch). -- **Cycle 5** (this version): explicit cut-point for `spawnAndDial` helper (line 504, IaC-validation stays in conformance); fixture build is in-place not copy-to-TempDir + main.go at fixture root not cmd/plugin/; jq filter pins goos+goarch matching runner. +- **Cycle 5**: cut-point at line 504; in-place fixture build; jq filter pins goos+goarch. FAILED — 3 Important (jq filter fix not propagated to §CI integration block; in-place build omits go.sum handling; cut-point missed `defer client.Kill()` transform). +- **Cycle 6** (this version): propagate jq filter to §CI integration; pin `-mod=readonly` + checked-in go.sum (sidesteps both pollution and copy-to-TempDir overhead); explicit defer-transform note in cut-point; reconcile line-range citations (462-504 throughout). ## Problem @@ -64,7 +65,7 @@ wfctl plugin verify-capabilities --binary "$BIN" . ### Behavior 1. Load `/plugin.json`. Parse + run `PluginManifest.Validate()` (reuse from validate-contract). -2. Spawn `` via shared `spawnAndDial(ctx, binaryPath) (*external.PluginAdapter, func())` helper extracted from `cmd/wfctl/plugin_conformance.go:462-515`. Uses `external.Handshake` from `workflow/plugin/external/handshake.go:23`. +2. Spawn `` via shared `spawnAndDial(ctx, binaryPath) (*external.PluginAdapter, func())` helper extracted from `cmd/wfctl/plugin_conformance.go:462-504` (cut-point details in §Files). Uses `external.Handshake` from `workflow/plugin/external/handshake.go:23`. 3. Call `PluginService.GetManifest(Empty) → pb.Manifest` (6 scalar fields per `plugin/external/proto/plugin.proto:96-104`). 4. Diff strict: @@ -106,17 +107,20 @@ Append to scaffold-template `release.yml` post-goreleaser, pre-publish: ```yaml - name: Verify capabilities (post-build runtime check) run: | - BIN=$(jq -r '[.[] | select(.type=="Binary") | .path] | .[0]' dist/artifacts.json) + RUNNER_ARCH=$(uname -m | sed 's/x86_64/amd64/;s/aarch64/arm64/') + BIN=$(jq -r --arg arch "$RUNNER_ARCH" \ + '[.[] | select(.type=="Binary" and .goos=="linux" and .goarch==$arch)] | .[0].path // ""' \ + dist/artifacts.json) "${RUNNER_TEMP}/wfctl-bin/wfctl" plugin verify-capabilities --binary "$BIN" . ``` -`dist/artifacts.json` is goreleaser's manifest of all built artifacts; jq picks the first binary (any arch — verify-capabilities only needs ONE binary to confirm ldflag fired). Avoids hard-coding goreleaser's directory layout per cycle-2 F-NEW-6. +`dist/artifacts.json` is goreleaser's manifest of all built artifacts. The jq filter pins to the CURRENT runner's goos/goarch — multiarch builds emit multiple Binary artifacts (linux+darwin × amd64+arm64); picking arbitrarily by `.[0]` would yield e.g. darwin-arm64 on a linux-amd64 runner → `exec format error`. The `uname -m` sed map converts kernel arch names to Go arch names (`x86_64`→`amd64`, `aarch64`→`arm64`). Scaffold-side wiring is a follow-up commit on `scaffold-workflow-plugin` after this workflow PR lands — not part of this design's scope. ## Files (workflow repo) -- `cmd/wfctl/plugin_spawn.go` — NEW; extracts `spawnAndDial(ctx, binaryPath) (*external.PluginAdapter, func())` from `plugin_conformance.go:462-504` ONLY (spawn + dial + Dispense + `NewExternalPluginAdapter`). **Cut-point: line 504 (right after adapter construction succeeds).** Lines 505-513 of conformance contain IaC-specific post-dial validation (`ContractRegistryError`, `AssertIaCPluginAdvertisesRequiredService`, typed-IaC adapter setup) — these REMAIN inline in `checkTypedIaCPlugin` after calling the helper. The helper is plugin-type-agnostic; IaC-specific assertions stay where they belong. +- `cmd/wfctl/plugin_spawn.go` — NEW; extracts `spawnAndDial(ctx, binaryPath) (*external.PluginAdapter, func())` from `plugin_conformance.go:462-504` ONLY (spawn + dial + Dispense + `NewExternalPluginAdapter`). **Cut-point: line 504 (right after adapter construction succeeds).** Lines 505-513 of conformance contain IaC-specific post-dial validation (`ContractRegistryError`, `AssertIaCPluginAdvertisesRequiredService`, typed-IaC adapter setup) — these REMAIN inline in `checkTypedIaCPlugin` after calling the helper. The helper is plugin-type-agnostic; IaC-specific assertions stay where they belong. **Defer transform**: existing line 484's `defer client.Kill()` does NOT move into the helper body — instead, the helper returns the `client.Kill` (or a closure wrapping it) as the `func()` cleanup; the CALLER defers the returned cleanup. Without this transformation, a literal extraction either (a) kills the plugin when the helper returns, before the caller can RPC against it, or (b) drops the defer and leaks processes. - `cmd/wfctl/plugin_conformance.go` — refactored to call new shared helper; behavior unchanged. - `cmd/wfctl/plugin_verify_capabilities.go` — NEW; subcommand entry + diff impl. - `cmd/wfctl/plugin_verify_capabilities_test.go` — table-driven tests against `testdata/verify_capabilities//`. @@ -142,10 +146,10 @@ Scaffold-side wiring is a follow-up commit on `scaffold-workflow-plugin` after t # (5 ups: / → verify_capabilities/ → testdata/ → wfctl/ → cmd/ → REPO_ROOT) ``` - **Build pattern: in-place (NOT copy-to-TempDir)**. The fixture's `go.mod` carries a checked-in relative `replace` directive that resolves only when built at the fixture's own checked-in path. The precedent's `prepareConformanceFixture` (`plugin_conformance_test.go:401-419`) copies-to-TempDir AND rewrites go.mod with an absolute path — that pattern's overhead is unnecessary here because verify-capabilities fixtures don't need scenario mutation; they're read-only. Build in-place; emit the binary to `t.TempDir()`: + **Build pattern: in-place + `-mod=readonly` + CHECKED-IN go.sum**. Each fixture ships a complete checked-in `go.mod` AND `go.sum` so `go build -mod=readonly` succeeds without writing into the source tree. (Cycle-5 F5-2: `go build -mod=mod` writes `go.sum` to the fixture source tree on first build — pollutes the repo and dirties `vcs.modified=true`. The conformance precedent dodges this via copy-to-TempDir; we sidestep both via readonly + checked-in sums.) ```go - cmd := exec.Command("go", "build", "-mod=mod", + cmd := exec.Command("go", "build", "-mod=readonly", "-ldflags=-X github.com/test/.Version=", "-o", filepath.Join(t.TempDir(), "p"), ".") cmd.Dir = "testdata/verify_capabilities/" @@ -154,6 +158,8 @@ Scaffold-side wiring is a follow-up commit on `scaffold-workflow-plugin` after t `GOWORK=off` is mandatory — without it, the workflow repo's workspace go.work overrides the per-fixture `replace` directive and the build resolves the wrong workflow version. The `-ldflags -X` package path is `github.com/test/` (the fixture's module path; Version var at fixture root), NOT a `/internal.Version` subpath (different from production plugins; simpler test fixture). + **Fixture-maintenance note**: when workflow SDK adds a new transitive dep that the fixtures pick up, regenerate fixture `go.sum` files via a one-shot `for d in testdata/verify_capabilities/*/; do (cd "$d" && GOWORK=off go mod tidy); done` and commit. Documented in `cmd/wfctl/testdata/verify_capabilities/README.md` (NEW; one-page maintenance note). + Optional: factor `buildFixtureBinary` from `plugin_conformance_test.go:509-519` into a shared `cmd/wfctl/fixture_build_test.go` helper if both test files use it. Defer if duplication is minimal. - `cmd/wfctl/plugin.go` — register `case "verify-capabilities"`. - `docs/PLUGIN_RELEASE_GATES.md` — append `Verify-Capabilities` section. From ace34abc65335e6e5d566cf5aceb9fb669077f36 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sun, 24 May 2026 00:32:11 -0400 Subject: [PATCH 09/15] docs(plan): verify-capabilities implementation plan (workflow#765) 9 tasks, 1 PR. Pairs with design doc cycle-6 PASS adversarial. --- docs/plans/2026-05-24-verify-capabilities.md | 1218 ++++++++++++++++++ 1 file changed, 1218 insertions(+) create mode 100644 docs/plans/2026-05-24-verify-capabilities.md diff --git a/docs/plans/2026-05-24-verify-capabilities.md b/docs/plans/2026-05-24-verify-capabilities.md new file mode 100644 index 00000000..a318651c --- /dev/null +++ b/docs/plans/2026-05-24-verify-capabilities.md @@ -0,0 +1,1218 @@ +# wfctl plugin verify-capabilities Implementation Plan + +> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. + +**Goal:** Implement `wfctl plugin verify-capabilities --binary ` subcommand that spawns a plugin binary, calls `PluginService.GetManifest`, and diffs `Name` + `Version` against plugin.json with sentinel-pattern matching to catch the ldflag-missing truth-loop bug from workflow#762/#764. + +**Architecture:** New subcommand registered in `cmd/wfctl/plugin.go`. Extracts existing spawn-and-dial pattern from `plugin_conformance.go:462-504` into a shared helper (`spawnAndDial`) so verify-capabilities and conformance both use it. Diff logic is set-equal-on-Name + sentinel-aware-on-Version (see design doc §Version rule matrix). Fixtures use in-place build with `-mod=readonly` + checked-in `go.sum` + `GOWORK=off`. + +**Tech Stack:** Go (workflow CLI), `goplugin` (go-plugin v1.7), `pb.PluginService` (gRPC), `external.Handshake`, `external.NewExternalPluginAdapter`. + +**Base branch:** `main` + +**Design doc:** `docs/plans/2026-05-24-verify-capabilities-design.md` (cycle-6 PASS adversarial). + +**Issue:** workflow#765 + +--- + +## Scope Manifest + +**PR Count:** 1 +**Tasks:** 9 +**Estimated Lines of Change:** ~600 (helper + subcommand + tests + 5 fixtures + docs) + +**Out of scope:** +- Contract-diff (`GetContractRegistry` walk) — deferred to follow-up issue #766; needs `capabilities.iacServices` schema on `PluginManifest` first +- Per-type RPC walk (`GetModuleTypes`/`GetStepTypes`/`GetTriggerTypes`) — IaC bridge returns Unimplemented for these +- Build-from-source mode — `--binary` REQUIRED; dev convenience builds documented in §Synopsis +- `--json` output mode — defer YAGNI +- Multi-binary repos — runs against the binary passed; multi-binary plugins invoke multiple times +- Author/Description/ConfigMutable/SampleCategory diff — display fields, not contract surface +- `minEngineVersion` runtime check — not on `pb.Manifest`; static-check responsibility +- Scaffold-template release.yml wiring — separate follow-up PR on scaffold-workflow-plugin after this lands + +**PR Grouping:** + +| PR # | Title | Tasks | Branch | +|------|-------|-------|--------| +| 1 | feat(wfctl): plugin verify-capabilities subcommand (workflow#765) | Task 1, Task 2, Task 3, Task 4, Task 5, Task 6, Task 7, Task 8, Task 9 | feat/765-verify-capabilities | + +**Status:** Draft + +--- + +## Task 1: Extract shared `spawnAndDial` helper from conformance + +**Change class:** Internal logic refactor (existing functionality moves; behavior unchanged). + +**Files:** +- Create: `cmd/wfctl/plugin_spawn.go` +- Modify: `cmd/wfctl/plugin_conformance.go:462-504` (replace inline spawn with `spawnAndDial` call; preserve lines 505-513 IaC-validation inline) +- Test: existing `cmd/wfctl/plugin_conformance_test.go` covers the refactored path; no new test needed for the move itself + +**Step 1: Confirm conformance test baseline passes** + +Run: `cd cmd/wfctl && go test -run TestConformance -count=1 ./...` +Expected: PASS (establishes baseline before refactor). + +**Step 2: Create the shared helper** + +Create `cmd/wfctl/plugin_spawn.go`: + +```go +// Package main — shared spawn-and-dial helper for wfctl plugin subcommands +// that need to exec a plugin binary and obtain a *external.PluginAdapter. +// +// Extracted from plugin_conformance.go:462-504 per workflow#765 design doc +// §Files. Used by `plugin conformance` and `plugin verify-capabilities`. +// Plugin-type-agnostic; IaC-specific post-dial assertions stay in callers. +package main + +import ( + "context" + "fmt" + "os/exec" + + external "github.com/GoCodeAlone/workflow/plugin/external" + goplugin "github.com/GoCodeAlone/go-plugin" + hclog "github.com/hashicorp/go-hclog" +) + +// spawnedPlugin is the result of spawning a plugin binary and dialing its +// gRPC server. Cleanup MUST be called by the caller (typically via defer) +// to kill the spawned subprocess. stdoutTail and stderrTail are the captured +// tail buffers (useful for error reporting when spawn or dial fails). +type spawnedPlugin struct { + Adapter *external.PluginAdapter + Cleanup func() + StdoutTail *tailBuffer + StderrTail *tailBuffer +} + +// spawnAndDial executes binaryPath as a go-plugin subprocess, dials its +// gRPC server with the canonical workflow ext handshake, dispenses the +// "plugin" interface, and wraps it in a *external.PluginAdapter. +// +// The returned Cleanup func wraps client.Kill — caller must `defer cleanup()`. +// If spawn or dial fails, Cleanup is non-nil but a no-op (still safe to defer). +// +// pluginName is informational (used for error messages + adapter naming). +// pluginDir is the working directory for the subprocess (typically the +// plugin's install directory; can be empty to inherit wfctl's cwd). +// env is the subprocess environment (typically conformancePluginEnv() — +// reuse from plugin_conformance.go for consistency). +func spawnAndDial(ctx context.Context, binaryPath, pluginName, pluginDir string, env []string) (*spawnedPlugin, error) { + var stdout, stderr tailBuffer + cmd := exec.CommandContext(ctx, binaryPath) //nolint:gosec // operator-supplied binary path. + if pluginDir != "" { + cmd.Dir = pluginDir + } + if env != nil { + cmd.Env = env + } + client := goplugin.NewClient(&goplugin.ClientConfig{ + HandshakeConfig: external.Handshake, + Plugins: goplugin.PluginSet{"plugin": &external.GRPCPlugin{}}, + Cmd: cmd, + AllowedProtocols: []goplugin.Protocol{goplugin.ProtocolGRPC}, + Stderr: &stderr, + SyncStdout: &stdout, + SyncStderr: &stderr, + Logger: hclog.NewNullLogger(), + }) + + rpcClient, err := client.Client() + if err != nil { + client.Kill() + if ctx.Err() != nil { + return nil, fmt.Errorf("timeout waiting for plugin handshake (stderr: %s)", stderr.String()) + } + return nil, fmt.Errorf("plugin dial: %w (stderr: %s)", err, stderr.String()) + } + raw, err := rpcClient.Dispense("plugin") + if err != nil { + client.Kill() + return nil, fmt.Errorf("dispense plugin: %w (stderr: %s)", err, stderr.String()) + } + pluginClient, ok := raw.(*external.PluginClient) + if !ok { + client.Kill() + return nil, fmt.Errorf("dispensed object is %T, want *external.PluginClient", raw) + } + adapter, err := external.NewExternalPluginAdapter(pluginName, pluginClient, nil) + if err != nil { + client.Kill() + return nil, fmt.Errorf("new adapter: %w", err) + } + return &spawnedPlugin{ + Adapter: adapter, + Cleanup: client.Kill, + StdoutTail: &stdout, + StderrTail: &stderr, + }, nil +} +``` + +**Step 3: Refactor `checkTypedIaCPlugin` to call the helper** + +Edit `cmd/wfctl/plugin_conformance.go` — replace lines 462-504 (spawn+dial+dispense+adapter) with a single `spawnAndDial` call. KEEP lines 505-513 (IaC-validation: `ContractRegistryError`, `AssertIaCPluginAdvertisesRequiredService`, `registeredIaCServices`, `newTypedIaCAdapter`) inline after the call. Pattern: + +```go +func checkTypedIaCPlugin(timeout time.Duration, pluginsDir, name string) (string, string, error) { + ctx, cancel := context.WithTimeout(context.Background(), timeout) + defer cancel() + + pluginDir := filepath.Join(pluginsDir, name) + binaryPath, err := filepath.Abs(filepath.Join(pluginDir, name)) + if err != nil { + return "", "", err + } + sp, err := spawnAndDial(ctx, binaryPath, name, pluginDir, conformancePluginEnv()) + if err != nil { + stdout := "" + stderr := "" + if sp != nil { + if sp.StdoutTail != nil { stdout = sp.StdoutTail.String() } + if sp.StderrTail != nil { stderr = sp.StderrTail.String() } + } + return stdout, stderr, err + } + defer sp.Cleanup() + + // IaC-specific post-dial validation (NOT in shared helper). + if regErr := sp.Adapter.ContractRegistryError(); regErr != nil { + return sp.StdoutTail.String(), sp.StderrTail.String(), regErr + } + if err := AssertIaCPluginAdvertisesRequiredService(name, "", sp.Adapter.ContractRegistry()); err != nil { + return sp.StdoutTail.String(), sp.StderrTail.String(), err + } + registered := registeredIaCServices(sp.Adapter.ContractRegistry()) + typed := newTypedIaCAdapter(sp.Adapter.Conn(), registered) + _ = typed.SupportedCanonicalKeys() + return sp.StdoutTail.String(), sp.StderrTail.String(), nil +} +``` + +**Step 4: Build + run conformance tests** + +Run: `cd cmd/wfctl && go build ./... && go test -run TestConformance -count=1 ./...` +Expected: build exit 0; tests PASS (behavior unchanged). + +**Step 5: Commit** + +```bash +git add cmd/wfctl/plugin_spawn.go cmd/wfctl/plugin_conformance.go +git commit -m "refactor(wfctl): extract spawnAndDial helper from conformance (workflow#765 prep)" +``` + +--- + +## Task 2: Subcommand registration + flag parsing skeleton + +**Change class:** CLI command. + +**Files:** +- Create: `cmd/wfctl/plugin_verify_capabilities.go` +- Modify: `cmd/wfctl/plugin.go` (add `case "verify-capabilities"` + help line) + +**Step 1: Write the failing test** + +Create `cmd/wfctl/plugin_verify_capabilities_test.go`: + +```go +package main + +import ( + "strings" + "testing" +) + +// TestVerifyCapabilitiesUsage asserts the subcommand prints usage when invoked +// with no args. +func TestVerifyCapabilitiesUsage(t *testing.T) { + err := runPluginVerifyCapabilities([]string{}) + if err == nil { + t.Fatal("want error for missing args") + } + if !strings.Contains(err.Error(), "--binary") { + t.Errorf("error %q should mention --binary", err.Error()) + } +} + +// TestVerifyCapabilitiesRequiresBinary asserts --binary is required. +func TestVerifyCapabilitiesRequiresBinary(t *testing.T) { + err := runPluginVerifyCapabilities([]string{"."}) + if err == nil { + t.Fatal("want error when --binary missing") + } + if !strings.Contains(err.Error(), "--binary") { + t.Errorf("error %q should mention --binary", err.Error()) + } +} +``` + +**Step 2: Run test to verify it fails** + +Run: `cd cmd/wfctl && go test -run TestVerifyCapabilitiesUsage -count=1 ./...` +Expected: FAIL with `undefined: runPluginVerifyCapabilities`. + +**Step 3: Write minimal implementation** + +Create `cmd/wfctl/plugin_verify_capabilities.go`: + +```go +// Package main — `wfctl plugin verify-capabilities` subcommand. +// Spawns a plugin binary, calls PluginService.GetManifest, diffs returned +// Manifest against plugin.json. Catches ldflag-missing truth-loop bug from +// workflow#762/#764. +// +// Design: docs/plans/2026-05-24-verify-capabilities-design.md +// Issue: https://github.com/GoCodeAlone/workflow/issues/765 +package main + +import ( + "flag" + "fmt" + "os" +) + +func runPluginVerifyCapabilities(args []string) error { + fs := flag.NewFlagSet("plugin verify-capabilities", flag.ContinueOnError) + binary := fs.String("binary", "", "Path to plugin binary (REQUIRED; see help for goreleaser dist/artifacts.json pattern)") + fs.Usage = func() { + fmt.Fprintf(fs.Output(), `Usage: wfctl plugin verify-capabilities --binary + +Spawn the plugin binary and verify its runtime PluginService.GetManifest +matches the declared plugin.json. Catches ldflag-missing / version-drift +bugs at release time (workflow#762 truth-loop closure). + +REQUIRED: --binary (no build-from-source; operator builds the binary) + +WARNING: this command EXECUTES as a subprocess. Only run against +build artifacts you trust. + +Examples: + # Local dev: + go build -ldflags="-X github.com/.../internal.Version=v1.2.3" -o /tmp/p ./cmd/ + wfctl plugin verify-capabilities --binary /tmp/p . + + # CI (post-goreleaser, in release.yml): + RUNNER_ARCH=$(uname -m | sed 's/x86_64/amd64/;s/aarch64/arm64/') + BIN=$(jq -r --arg arch "$RUNNER_ARCH" \ + '[.[] | select(.type=="Binary" and .goos=="linux" and .goarch==$arch)] | .[0].path // ""' \ + dist/artifacts.json) + wfctl plugin verify-capabilities --binary "$BIN" . + +Options: +`) + fs.PrintDefaults() + } + if err := fs.Parse(args); err != nil { + return err + } + if *binary == "" { + fs.Usage() + return fmt.Errorf("--binary is required") + } + if fs.NArg() != 1 { + fs.Usage() + return fmt.Errorf("exactly one argument required") + } + pluginDir := fs.Arg(0) + + // TODO: preflight + spawn + diff (Tasks 3-5) + _ = pluginDir + _ = os.Stat + return fmt.Errorf("not yet implemented") +} +``` + +Edit `cmd/wfctl/plugin.go` — find the `case "validate-contract":` dispatcher block (line ~38) and add right after: + +```go + case "verify-capabilities": + return runPluginVerifyCapabilities(args[1:]) +``` + +Also find the help-text block in `plugin.go`'s usage() function and add (alphabetical order with other verbs): + +```go + fmt.Fprintln(out, " verify-capabilities Spawn plugin binary, verify runtime GetManifest matches plugin.json") +``` + +**Step 4: Run test to verify it passes** + +Run: `cd cmd/wfctl && go build ./... && go test -run TestVerifyCapabilities -count=1 ./...` +Expected: build exit 0; both tests PASS. + +**Step 5: Commit** + +```bash +git add cmd/wfctl/plugin_verify_capabilities.go cmd/wfctl/plugin_verify_capabilities_test.go cmd/wfctl/plugin.go +git commit -m "feat(wfctl): plugin verify-capabilities subcommand skeleton (workflow#765)" +``` + +--- + +## Task 3: Preflight binary path validation + +**Change class:** Internal logic refactor (input validation). + +**Files:** +- Modify: `cmd/wfctl/plugin_verify_capabilities.go` +- Modify: `cmd/wfctl/plugin_verify_capabilities_test.go` + +**Step 1: Write the failing tests** + +Append to `cmd/wfctl/plugin_verify_capabilities_test.go`: + +```go +func TestPreflightBinaryEmpty(t *testing.T) { + err := preflightBinary("") + if err == nil || !strings.Contains(err.Error(), "binary path") { + t.Errorf("want empty-path error, got %v", err) + } +} + +func TestPreflightBinaryNull(t *testing.T) { + err := preflightBinary("null") + if err == nil || !strings.Contains(err.Error(), "binary path") { + t.Errorf("want null-path error (jq fallback), got %v", err) + } +} + +func TestPreflightBinaryMissing(t *testing.T) { + err := preflightBinary("/nonexistent/missing-binary-xyz") + if err == nil || !strings.Contains(err.Error(), "stat") { + t.Errorf("want stat error, got %v", err) + } +} + +func TestPreflightBinaryDirectory(t *testing.T) { + d := t.TempDir() + err := preflightBinary(d) + if err == nil || !strings.Contains(err.Error(), "directory") { + t.Errorf("want directory error, got %v", err) + } +} + +func TestPreflightBinaryNonExecutable(t *testing.T) { + d := t.TempDir() + f := filepath.Join(d, "p") + if err := os.WriteFile(f, []byte("not-exec"), 0o644); err != nil { + t.Fatal(err) + } + err := preflightBinary(f) + if err == nil || !strings.Contains(err.Error(), "executable") { + t.Errorf("want non-executable error, got %v", err) + } +} + +func TestPreflightBinaryOK(t *testing.T) { + d := t.TempDir() + f := filepath.Join(d, "p") + if err := os.WriteFile(f, []byte("#!/bin/sh\necho ok"), 0o755); err != nil { + t.Fatal(err) + } + if err := preflightBinary(f); err != nil { + t.Errorf("want PASS, got %v", err) + } +} +``` + +Add to imports: `"os"`, `"path/filepath"`. + +**Step 2: Run test to verify it fails** + +Run: `cd cmd/wfctl && go test -run TestPreflightBinary -count=1 ./...` +Expected: FAIL with `undefined: preflightBinary`. + +**Step 3: Implement** + +Append to `cmd/wfctl/plugin_verify_capabilities.go`: + +```go +// preflightBinary validates the --binary path before exec: +// - non-empty + not literal "null" (guards against jq fallback returning empty) +// - file exists and is a regular file (not directory or symlink-to-dir) +// - has at least one executable bit set +// +// Design: §Synopsis preflight checks. +func preflightBinary(path string) error { + if path == "" || path == "null" { + return fmt.Errorf("--binary path empty (jq filter may have returned no match)") + } + fi, err := os.Stat(path) + if err != nil { + return fmt.Errorf("stat %q: %w", path, err) + } + if fi.IsDir() { + return fmt.Errorf("--binary %q is a directory", path) + } + if !fi.Mode().IsRegular() { + return fmt.Errorf("--binary %q is not a regular file (mode=%s)", path, fi.Mode()) + } + if fi.Mode()&0o111 == 0 { + return fmt.Errorf("--binary %q is not executable (mode=%s)", path, fi.Mode()) + } + return nil +} +``` + +Update `runPluginVerifyCapabilities` to call it before the `return "not yet implemented"` line: + +```go + if err := preflightBinary(*binary); err != nil { + return err + } +``` + +**Step 4: Run test to verify it passes** + +Run: `cd cmd/wfctl && go test -run TestPreflightBinary -count=1 ./...` +Expected: all 6 preflight tests PASS. + +**Step 5: Commit** + +```bash +git add cmd/wfctl/plugin_verify_capabilities.go cmd/wfctl/plugin_verify_capabilities_test.go +git commit -m "feat(wfctl): verify-capabilities preflight binary-path validation (workflow#765)" +``` + +--- + +## Task 4: Sentinel-pattern Version diff matrix + +**Change class:** Internal logic refactor (pure-logic diff function). + +**Files:** +- Modify: `cmd/wfctl/plugin_verify_capabilities.go` +- Modify: `cmd/wfctl/plugin_verify_capabilities_test.go` + +**Step 1: Write the failing tests** (table-driven) + +Append to `cmd/wfctl/plugin_verify_capabilities_test.go`: + +```go +func TestIsSentinel(t *testing.T) { + cases := map[string]bool{ + "": true, + "dev": true, + "0.0.0": true, + "(devel)": true, + "(devel) [@ a1b2c3d]": true, + "(devel) [@ a1b2c3d.dirty]": true, + "v1.2.3": false, + "1.2.3": false, + "v0.0.1": false, + } + for v, want := range cases { + if got := isSentinel(v); got != want { + t.Errorf("isSentinel(%q) = %v, want %v", v, got, want) + } + } +} + +func TestDiffVersion(t *testing.T) { + // (declared, runtime, wantPass, wantReason-substring) + cases := []struct { + declared, runtime string + wantPass bool + wantReason string + }{ + // Dev-sentinel row 1: 0.0.0 + non-sentinel -> PASS (CI artifact) + {"0.0.0", "v1.2.3", true, ""}, + {"0.0.0", "0.1.0", true, ""}, + // Dev-sentinel row 2: 0.0.0 + sentinel -> FAIL (ldflag missing) + {"0.0.0", "", false, "ldflag"}, + {"0.0.0", "(devel)", false, "ldflag"}, + {"0.0.0", "(devel) [@ abc1234]", false, "ldflag"}, + {"0.0.0", "dev", false, "ldflag"}, + {"0.0.0", "0.0.0", false, "ldflag"}, + // Release row 3: X.Y.Z + vX.Y.Z or X.Y.Z -> PASS (normalize leading v) + {"1.2.3", "v1.2.3", true, ""}, + {"1.2.3", "1.2.3", true, ""}, + // Release row 4: X.Y.Z + sentinel -> FAIL + {"1.2.3", "", false, "ldflag"}, + {"1.2.3", "(devel)", false, "ldflag"}, + {"1.2.3", "(devel) [@ deadbee]", false, "ldflag"}, + // Release row 5: X.Y.Z + drift -> FAIL + {"1.2.3", "v0.9.0", false, "drift"}, + {"1.2.3", "v2.0.0", false, "drift"}, + } + for _, c := range cases { + pass, reason := diffVersion(c.declared, c.runtime) + if pass != c.wantPass { + t.Errorf("diffVersion(%q, %q) pass = %v, want %v (reason: %q)", + c.declared, c.runtime, pass, c.wantPass, reason) + continue + } + if !pass && !strings.Contains(reason, c.wantReason) { + t.Errorf("diffVersion(%q, %q) reason = %q, want substring %q", + c.declared, c.runtime, reason, c.wantReason) + } + } +} +``` + +**Step 2: Run tests to verify failure** + +Run: `cd cmd/wfctl && go test -run "TestIsSentinel|TestDiffVersion" -count=1 ./...` +Expected: FAIL with `undefined: isSentinel`, `undefined: diffVersion`. + +**Step 3: Implement** + +Append to `cmd/wfctl/plugin_verify_capabilities.go`: + +```go +import "strings" // add to existing import block if absent + +// isSentinel returns true when v is one of the SDK's dev-sentinel forms +// OR the on-disk plugin.json sentinel "0.0.0". +// +// SDK sentinel set (per plugin/external/sdk/buildversion.go:36-42): +// "", "dev", "(devel)" — ResolveBuildVersion replaces these with build-info +// Plus build-info fallback produces "(devel) [@ [.dirty]]" — HasPrefix catches all forms. +// Plus on-disk plugin.json "0.0.0" sentinel (workflow#762 convention). +// +// The predicate MUST be a SUPERSET of the SDK's set; "dev" is defensive even +// though canonical wiring (sdk.ResolveBuildVersion) prevents literal "dev" +// from reaching the wire — included to catch non-canonical wiring accidents. +func isSentinel(v string) bool { + switch v { + case "", "dev", "0.0.0", "(devel)": + return true + } + return strings.HasPrefix(v, "(devel)") +} + +// diffVersion implements the Version-rule matrix from the design doc: +// +// plugin.json binary Manifest.Version outcome +// ------------ ------------------------ ------- +// "0.0.0" non-sentinel PASS (CI artifact under verification) +// "0.0.0" sentinel FAIL (ldflag injection missing) +// "X.Y.Z" "vX.Y.Z" or "X.Y.Z" PASS (normalize leading v) +// "X.Y.Z" sentinel FAIL (ldflag missing) +// "X.Y.Z" anything else FAIL (version drift) +// +// Returns (pass bool, reason string). reason is non-empty only when pass=false. +func diffVersion(declared, runtime string) (bool, string) { + runtimeSentinel := isSentinel(runtime) + if declared == "0.0.0" { + if runtimeSentinel { + return false, fmt.Sprintf("ldflag injection missing: plugin.json=%q; binary Manifest.Version=%q (sentinel)", declared, runtime) + } + return true, "" + } + if runtimeSentinel { + return false, fmt.Sprintf("ldflag injection missing: plugin.json=%q (release); binary Manifest.Version=%q (sentinel)", declared, runtime) + } + // Normalize leading v on runtime side + rNorm := strings.TrimPrefix(runtime, "v") + if rNorm == declared { + return true, "" + } + return false, fmt.Sprintf("version drift: plugin.json=%q; binary Manifest.Version=%q", declared, runtime) +} +``` + +**Step 4: Run tests to verify pass** + +Run: `cd cmd/wfctl && go test -run "TestIsSentinel|TestDiffVersion" -count=1 ./...` +Expected: all matrix cases PASS. + +**Step 5: Commit** + +```bash +git add cmd/wfctl/plugin_verify_capabilities.go cmd/wfctl/plugin_verify_capabilities_test.go +git commit -m "feat(wfctl): verify-capabilities sentinel-pattern Version diff matrix (workflow#765)" +``` + +--- + +## Task 5: Wire spawn + GetManifest + diff into runPluginVerifyCapabilities + +**Change class:** Plugin / extension (CLI subcommand that calls a plugin via gRPC). + +**Files:** +- Modify: `cmd/wfctl/plugin_verify_capabilities.go` + +**Step 1: Write the failing integration test** (deferred to Task 7 fixture work — this task is the implementation wiring; test in Task 8) + +For this task, the verification is `go build ./... && wfctl plugin verify-capabilities --help` showing the help text. + +**Step 2: Read plugin.json + load PluginManifest** + +Modify `runPluginVerifyCapabilities` — replace the `return fmt.Errorf("not yet implemented")` line with: + +```go + abs, err := filepath.Abs(pluginDir) + if err != nil { + return fmt.Errorf("resolve %q: %w", pluginDir, err) + } + manifestPath := filepath.Join(abs, "plugin.json") + manifestBytes, err := os.ReadFile(manifestPath) //nolint:gosec // operator-supplied path. + if err != nil { + return fmt.Errorf("plugin.json: %w", err) + } + var declared plugin.PluginManifest + if err := json.Unmarshal(manifestBytes, &declared); err != nil { + return fmt.Errorf("plugin.json parse: %w", err) + } + if err := declared.Validate(); err != nil { + return fmt.Errorf("plugin.json validate: %w", err) + } +``` + +Add imports: `"encoding/json"`, `"path/filepath"`, `"github.com/GoCodeAlone/workflow/plugin"`. + +**Step 3: Spawn + dial via shared helper + call GetManifest** + +Continue in `runPluginVerifyCapabilities`: + +```go + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + binAbs, err := filepath.Abs(*binary) + if err != nil { + return fmt.Errorf("resolve --binary %q: %w", *binary, err) + } + sp, err := spawnAndDial(ctx, binAbs, declared.Name, "", nil) + if err != nil { + return fmt.Errorf("spawn %s: %w", declared.Name, err) + } + defer sp.Cleanup() + + runtime, err := sp.Adapter.EngineManifest() + if err != nil { + return fmt.Errorf("GetManifest RPC: %w (stderr: %s)", err, sp.StderrTail.String()) + } +``` + +Note: `external.PluginAdapter.EngineManifest()` returns `(*pb.Manifest, error)` per `/tmp/wfprobe/plugin/external/adapter.go:367`. Verify with `grep -n "func.*EngineManifest" /tmp/wfprobe/plugin/external/adapter.go` if the method name has changed; substitute the exact accessor. + +Add imports: `"context"`, `"time"`. + +**Step 4: Diff Name + Version and report** + +```go + var failures []string + if runtime.GetName() != declared.Name { + failures = append(failures, fmt.Sprintf("name: plugin.json=%q; binary Manifest.Name=%q", declared.Name, runtime.GetName())) + } + if pass, reason := diffVersion(declared.Version, runtime.GetVersion()); !pass { + failures = append(failures, "version: "+reason) + } + if len(failures) > 0 { + fmt.Fprintf(os.Stderr, "FAIL %s (plugin.json)\nerror: %d mismatch(es)\n", declared.Name, len(failures)) + for _, f := range failures { + fmt.Fprintf(os.Stderr, " - %s\n", f) + } + return fmt.Errorf("verify-capabilities: %d mismatch(es)", len(failures)) + } + fmt.Printf("OK %s %s (plugin.json: %s)\n", declared.Name, runtime.GetVersion(), declared.Version) + return nil +``` + +**Step 5: Build + help-output sanity check** + +Run: +```bash +cd cmd/wfctl && go build -o /tmp/wfctl ./... && /tmp/wfctl plugin verify-capabilities --help +``` +Expected: help text printed; exit 0. The help text contains "REQUIRED: --binary", "WARNING: this command EXECUTES", and the `jq` CI example. + +**Step 6: Commit** + +```bash +git add cmd/wfctl/plugin_verify_capabilities.go +git commit -m "feat(wfctl): wire spawn + GetManifest + Name/Version diff (workflow#765)" +``` + +--- + +## Task 6: Create 5 fixture scenarios under testdata/verify_capabilities/ + +**Change class:** Documentation / fixture data (no runtime impact). + +**Files:** +- Create: `cmd/wfctl/testdata/verify_capabilities/good/{plugin.json,main.go,go.mod,go.sum}` +- Create: `cmd/wfctl/testdata/verify_capabilities/release-good/{plugin.json,main.go,go.mod,go.sum}` +- Create: `cmd/wfctl/testdata/verify_capabilities/missing-ldflag/{plugin.json,main.go,go.mod,go.sum}` +- Create: `cmd/wfctl/testdata/verify_capabilities/version-drift/{plugin.json,main.go,go.mod,go.sum}` +- Create: `cmd/wfctl/testdata/verify_capabilities/name-drift/{plugin.json,main.go,go.mod,go.sum}` +- Create: `cmd/wfctl/testdata/verify_capabilities/README.md` (maintenance note) + +**Step 1: Write a generator script** (one-off; not committed) + +Create `/tmp/gen-fixtures.sh` (not committed; just for generating): + +```bash +#!/bin/bash +set -euo pipefail +BASE=cmd/wfctl/testdata/verify_capabilities +REPO_ROOT=$(git rev-parse --show-toplevel) +declare -A NAMES=( + [good]=verify-good + [release-good]=verify-release-good + [missing-ldflag]=verify-missing-ldflag + [version-drift]=verify-version-drift + [name-drift]=verify-name-drift +) +declare -A VERS=( + [good]=0.0.0 + [release-good]=1.2.3 + [missing-ldflag]=0.0.0 + [version-drift]=1.2.3 + [name-drift]=0.0.0 +) +for s in good release-good missing-ldflag version-drift name-drift; do + d="$BASE/$s" + mkdir -p "$d" + name="${NAMES[$s]}" + version="${VERS[$s]}" + cat > "$d/plugin.json" < "$d/main.go" < "$d/go.mod" < $REPO_ROOT +MOD +done +``` + +Note: the `replace` directive uses an absolute path during generation, but we'll convert to relative `../../../../..` before committing for portability across machines. + +**Step 2: Generate fixtures + verify go.sum + adjust replace to relative** + +```bash +bash /tmp/gen-fixtures.sh +for d in cmd/wfctl/testdata/verify_capabilities/*/; do + (cd "$d" && GOWORK=off go mod tidy) + # rewrite replace from absolute to relative + sed -i.bak "s|replace github.com/GoCodeAlone/workflow => .*|replace github.com/GoCodeAlone/workflow => ../../../../..|" "$d/go.mod" + rm -f "$d/go.mod.bak" +done +``` + +Verify each fixture builds standalone: +```bash +for d in cmd/wfctl/testdata/verify_capabilities/*/; do + (cd "$d" && GOWORK=off go build -mod=readonly -o /tmp/p .) && echo "$d: ok" || echo "$d: FAIL" +done +``` +Expected: all 5 print `ok`. (This catches go.sum drift before the integration tests run.) + +**Step 3: Create the maintenance README** + +```bash +cat > cmd/wfctl/testdata/verify_capabilities/README.md <<'MD' +# verify_capabilities test fixtures + +Fixtures for `plugin_verify_capabilities_test.go` (workflow#765). + +Each scenario directory contains a self-contained Go module that builds into +a minimal plugin binary. Tests call `go build -mod=readonly` in-place; the +binary is emitted to `t.TempDir()`. + +## Maintenance + +When the workflow SDK adds a new transitive dep that the fixtures' build +graph picks up, regenerate each fixture's `go.sum`: + +```bash +for d in cmd/wfctl/testdata/verify_capabilities/*/; do + (cd "$d" && GOWORK=off go mod tidy) +done +git add cmd/wfctl/testdata/verify_capabilities/*/go.sum +``` + +The `replace github.com/GoCodeAlone/workflow => ../../../../..` directive +resolves 5-ups from each scenario directory to the workflow repo root. +DO NOT use an absolute path — it will diverge across developer machines. + +## Scenarios + +- `good/` — plugin.json version=0.0.0, ldflag injects v0.1.0 → PASS +- `release-good/` — plugin.json version=1.2.3, ldflag injects v1.2.3 → PASS +- `missing-ldflag/` — plugin.json version=0.0.0, no ldflag → FAIL +- `version-drift/` — plugin.json version=1.2.3, ldflag injects v0.9.0 → FAIL +- `name-drift/` — plugin.json name="foo", binary advertises Name="bar" → FAIL +MD +``` + +**Step 4: Verify all fixtures build (rerun)** + +Run: +```bash +for d in cmd/wfctl/testdata/verify_capabilities/*/; do + (cd "$d" && GOWORK=off go build -mod=readonly -o /tmp/p .) || exit 1 +done +echo "all fixtures build" +``` +Expected: `all fixtures build`. + +**Step 5: Commit** + +```bash +git add cmd/wfctl/testdata/verify_capabilities/ +git commit -m "test(wfctl): verify-capabilities testdata fixtures (workflow#765)" +``` + +--- + +## Task 7: Special-case name-drift fixture (binary advertises different Name) + +**Change class:** Internal logic refactor (fixture variant). + +**Files:** +- Modify: `cmd/wfctl/testdata/verify_capabilities/name-drift/main.go` + +**Step 1: Adjust the name-drift fixture's main.go** + +Replace `cmd/wfctl/testdata/verify_capabilities/name-drift/main.go` so the binary advertises `Name="verify-name-drift-binary"` while plugin.json declares `name="verify-name-drift"`: + +```go +package main + +import ( + sdk "github.com/GoCodeAlone/workflow/plugin/external/sdk" + "github.com/GoCodeAlone/workflow/plugin" +) + +var Version = "0.0.0" + +type stubProvider struct{} + +func (stubProvider) Manifest() (*plugin.PluginManifest, error) { + return &plugin.PluginManifest{ + Name: "verify-name-drift-binary", // intentional drift vs plugin.json "verify-name-drift" + Version: sdk.ResolveBuildVersion(Version), + }, nil +} + +func main() { + sdk.Serve(stubProvider{}, sdk.WithBuildVersion(sdk.ResolveBuildVersion(Version))) +} +``` + +**Step 2: Verify fixture still builds** + +Run: `(cd cmd/wfctl/testdata/verify_capabilities/name-drift && GOWORK=off go build -mod=readonly -o /tmp/p .)` +Expected: exit 0. + +**Step 3: Commit** + +```bash +git add cmd/wfctl/testdata/verify_capabilities/name-drift/main.go +git commit -m "test(wfctl): name-drift fixture (binary advertises mismatched Name) (workflow#765)" +``` + +--- + +## Task 8: Integration tests — 5 scenarios end-to-end + +**Change class:** Plugin / extension (exercise spawn + RPC + diff against real fixture binaries). + +**Files:** +- Modify: `cmd/wfctl/plugin_verify_capabilities_test.go` + +**Step 1: Write the integration test scaffolding** + +Append to `cmd/wfctl/plugin_verify_capabilities_test.go`: + +```go +import ( + "os/exec" + // ...existing imports +) + +// buildFixtureBinaryForVerify builds the fixture scenario in-place and emits +// the binary to t.TempDir(). ldflag is the -X ...Version= value (may be ""). +// Returns the absolute path to the built binary. +// +// Pattern mirrors plugin_conformance_test.go:buildFixtureBinary but adapted +// for in-place build per verify-capabilities design (no copy-to-TempDir). +func buildFixtureBinaryForVerify(t *testing.T, scenario, ldflagTag string) string { + t.Helper() + binPath := filepath.Join(t.TempDir(), "p") + args := []string{"build", "-mod=readonly", "-o", binPath, "."} + if ldflagTag != "" { + args = append([]string{"build", "-mod=readonly", + "-ldflags", fmt.Sprintf("-X github.com/test/%s.Version=%s", scenario, ldflagTag), + "-o", binPath, "."}, args[4:]...) + // Re-stitch to keep only the ldflags variant. + args = []string{"build", "-mod=readonly", + "-ldflags", fmt.Sprintf("-X github.com/test/%s.Version=%s", scenario, ldflagTag), + "-o", binPath, "."} + } + cmd := exec.Command("go", args...) + cmd.Dir = filepath.Join("testdata", "verify_capabilities", scenario) + cmd.Env = append(os.Environ(), "GOWORK=off") + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("build %s: %v\n%s", scenario, err, out) + } + return binPath +} + +func TestVerifyCapabilities_Good(t *testing.T) { + bin := buildFixtureBinaryForVerify(t, "good", "v0.1.0") + err := runPluginVerifyCapabilities([]string{"--binary", bin, "testdata/verify_capabilities/good"}) + if err != nil { + t.Fatalf("want PASS, got: %v", err) + } +} + +func TestVerifyCapabilities_ReleaseGood(t *testing.T) { + bin := buildFixtureBinaryForVerify(t, "release-good", "v1.2.3") + err := runPluginVerifyCapabilities([]string{"--binary", bin, "testdata/verify_capabilities/release-good"}) + if err != nil { + t.Fatalf("want PASS, got: %v", err) + } +} + +func TestVerifyCapabilities_MissingLdflag(t *testing.T) { + bin := buildFixtureBinaryForVerify(t, "missing-ldflag", "") // no ldflag → sentinel reaches wire + err := runPluginVerifyCapabilities([]string{"--binary", bin, "testdata/verify_capabilities/missing-ldflag"}) + if err == nil { + t.Fatal("want FAIL, got nil") + } + if !strings.Contains(err.Error(), "mismatch") { + t.Errorf("want mismatch error, got: %v", err) + } +} + +func TestVerifyCapabilities_VersionDrift(t *testing.T) { + bin := buildFixtureBinaryForVerify(t, "version-drift", "v0.9.0") // declared 1.2.3, binary 0.9.0 + err := runPluginVerifyCapabilities([]string{"--binary", bin, "testdata/verify_capabilities/version-drift"}) + if err == nil { + t.Fatal("want FAIL, got nil") + } + if !strings.Contains(err.Error(), "mismatch") { + t.Errorf("want mismatch error, got: %v", err) + } +} + +func TestVerifyCapabilities_NameDrift(t *testing.T) { + bin := buildFixtureBinaryForVerify(t, "name-drift", "v0.1.0") + err := runPluginVerifyCapabilities([]string{"--binary", bin, "testdata/verify_capabilities/name-drift"}) + if err == nil { + t.Fatal("want FAIL, got nil") + } + if !strings.Contains(err.Error(), "mismatch") { + t.Errorf("want mismatch error, got: %v", err) + } +} +``` + +**Step 2: Simplify buildFixtureBinaryForVerify** (the stitched logic above is muddled — rewrite cleanly): + +Replace the helper with: + +```go +func buildFixtureBinaryForVerify(t *testing.T, scenario, ldflagTag string) string { + t.Helper() + binPath := filepath.Join(t.TempDir(), "p") + args := []string{"build", "-mod=readonly"} + if ldflagTag != "" { + args = append(args, "-ldflags", + fmt.Sprintf("-X github.com/test/%s.Version=%s", scenario, ldflagTag)) + } + args = append(args, "-o", binPath, ".") + cmd := exec.Command("go", args...) + cmd.Dir = filepath.Join("testdata", "verify_capabilities", scenario) + cmd.Env = append(os.Environ(), "GOWORK=off") + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("build %s: %v\n%s", scenario, err, out) + } + return binPath +} +``` + +**Step 3: Run all integration tests** + +Run: `cd cmd/wfctl && go test -run TestVerifyCapabilities -count=1 -timeout 120s ./...` +Expected: all 5 scenario tests PASS (3 expect verify-success, 2 expect verify-failure). + +**Step 4: Commit** + +```bash +git add cmd/wfctl/plugin_verify_capabilities_test.go +git commit -m "test(wfctl): verify-capabilities integration tests (5 scenarios end-to-end) (workflow#765)" +``` + +--- + +## Task 9: Documentation update — PLUGIN_RELEASE_GATES.md + +**Change class:** Documentation. + +**Files:** +- Modify: `docs/PLUGIN_RELEASE_GATES.md` (append Verify-Capabilities section) + +**Step 1: Read current doc** + +```bash +wc -l docs/PLUGIN_RELEASE_GATES.md # know the size before append +``` + +**Step 2: Append Verify-Capabilities section** + +Append to `docs/PLUGIN_RELEASE_GATES.md`: + +```markdown + +## Verify-Capabilities (workflow#765 — runtime truth-check) + +`wfctl plugin verify-capabilities` is the runtime sibling of `validate-contract`: +it spawns the plugin binary, calls `PluginService.GetManifest`, and verifies +the returned `Name` + `Version` match `plugin.json`. Catches the +**ldflag-missing truth-loop bug**: a plugin can pass `validate-contract` +(static check) and still ship a binary whose `Manifest.Version` is the +SDK's `(devel) [@ sha]` sentinel because the goreleaser ldflag never fired. + +### Synopsis + +``` +wfctl plugin verify-capabilities --binary +``` + +`--binary` REQUIRED (no build-from-source — operator builds via goreleaser +or `go build`). + +⚠ **Executes the binary** as a subprocess. Only run against artifacts you trust. + +### Local development + +```bash +go build -ldflags="-X github.com/GoCodeAlone/workflow-plugin-/internal.Version=v1.2.3" \ + -o /tmp/p ./cmd/ +wfctl plugin verify-capabilities --binary /tmp/p . +``` + +### CI integration (release.yml post-goreleaser, pre-publish) + +```yaml +- name: Verify capabilities (post-build runtime check) + run: | + RUNNER_ARCH=$(uname -m | sed 's/x86_64/amd64/;s/aarch64/arm64/') + BIN=$(jq -r --arg arch "$RUNNER_ARCH" \ + '[.[] | select(.type=="Binary" and .goos=="linux" and .goarch==$arch)] | .[0].path // ""' \ + dist/artifacts.json) + "${RUNNER_TEMP}/wfctl-bin/wfctl" plugin verify-capabilities --binary "$BIN" . +``` + +### Version diff matrix + +| plugin.json `version` | binary `Manifest.Version` | Outcome | +|---|---|---| +| `"0.0.0"` (sentinel) | non-sentinel (`"v1.2.3"`) | PASS — CI artifact under verification | +| `"0.0.0"` | sentinel (`""`, `"dev"`, `"0.0.0"`, `"(devel)..."`) | FAIL — ldflag missing | +| `"X.Y.Z"` (release) | `"vX.Y.Z"` or `"X.Y.Z"` | PASS — normalize leading v | +| `"X.Y.Z"` | sentinel | FAIL — ldflag missing | +| `"X.Y.Z"` | anything else | FAIL — version drift | + +### Non-goals + +- Does NOT walk per-type RPCs (`GetModuleTypes`/`GetStepTypes`/`GetTriggerTypes`) — IaC bridge returns Unimplemented. +- Does NOT diff `GetContractRegistry` — deferred to workflow#766 (requires `capabilities.iacServices` schema on PluginManifest first). +- Does NOT build the binary — operator's responsibility. +- Does NOT verify `minEngineVersion` at runtime (not on `pb.Manifest`). + +See `docs/plans/2026-05-24-verify-capabilities-design.md` for full design. +``` + +**Step 3: Verify markdown renders without broken anchors** + +Run: `markdown-link-check docs/PLUGIN_RELEASE_GATES.md 2>&1 | head` (skip if tool absent — eyeball check works for an append). + +Expected: no broken links. + +**Step 4: Commit** + +```bash +git add docs/PLUGIN_RELEASE_GATES.md +git commit -m "docs: add Verify-Capabilities section to PLUGIN_RELEASE_GATES (workflow#765)" +``` + +--- + +## Final verification (post-Task-9) + +Before opening the PR: + +```bash +# 1. All tests pass +cd cmd/wfctl && go test -count=1 -timeout 120s ./... + +# 2. Lint clean +cd cmd/wfctl && go vet ./... +golangci-lint run ./cmd/wfctl/... + +# 3. Help text correct +go build -o /tmp/wfctl ./cmd/wfctl && /tmp/wfctl plugin verify-capabilities --help + +# 4. Conformance still works (smoke — full conformance suite runs in CI) +go test -run TestConformance -count=1 -timeout 300s ./cmd/wfctl/... + +# 5. End-to-end smoke against a real plugin (out-of-tree) +# cd /tmp && git clone --depth=1 git@github.com:GoCodeAlone/workflow-plugin-discord.git +# cd workflow-plugin-discord +# go build -ldflags="-X .../internal.Version=v0.1.1" -o /tmp/p ./cmd/workflow-plugin-discord +# /tmp/wfctl plugin verify-capabilities --binary /tmp/p . +# Expected: "OK workflow-plugin-discord v0.1.1 (plugin.json: 0.1.1)" +``` + +## Rollback + +This PR adds a CLI subcommand + a shared helper refactor. Rollback path: +- `git revert ` removes the new subcommand and re-inlines the spawn logic in `plugin_conformance.go`. +- No data migration, no schema change, no upstream consumer change (scaffold-template wiring is a separate PR not included here). +- Backwards-compat: subcommand is purely additive; pre-PR wfctl callers continue to work. From 8202b904bf3c45349cf26c5791b4f6d5f486dce8 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sun, 24 May 2026 00:39:29 -0400 Subject: [PATCH 10/15] docs(plan): verify-capabilities plan cycle 2 (fix 4 Critical from cycle-1 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cycle 1 FAIL: 4 Critical (fictional EngineManifest signature; fixture wrong PluginManifest type; missing-ldflag mechanics wrong; fixture json shape diverges). Cycle 2: - Drop Task 1 (spawnAndDial extraction) per reviewer Option 3; inline ~40 LOC. Eliminates I2 verification-class mismatch on refactor-without-test. - C1: GetManifest direct via pb.NewPluginServiceClient(pluginClient.Conn()) — bypasses adapter precedence. - C2: fixture uses sdk.PluginManifest (sdk-package value), Manifest() returns value (no error). - C3: initial Version='dev' so ResolveBuildVersion falls back to '(devel)' for missing-ldflag. - C4: minimal plugin.json (only PluginManifest fields). 8 tasks now (was 9). Single PR. --- docs/plans/2026-05-24-verify-capabilities.md | 792 ++++++++----------- 1 file changed, 313 insertions(+), 479 deletions(-) diff --git a/docs/plans/2026-05-24-verify-capabilities.md b/docs/plans/2026-05-24-verify-capabilities.md index a318651c..faa07995 100644 --- a/docs/plans/2026-05-24-verify-capabilities.md +++ b/docs/plans/2026-05-24-verify-capabilities.md @@ -2,11 +2,11 @@ > **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. -**Goal:** Implement `wfctl plugin verify-capabilities --binary ` subcommand that spawns a plugin binary, calls `PluginService.GetManifest`, and diffs `Name` + `Version` against plugin.json with sentinel-pattern matching to catch the ldflag-missing truth-loop bug from workflow#762/#764. +**Goal:** Implement `wfctl plugin verify-capabilities --binary ` subcommand that spawns a plugin binary, calls `PluginService.GetManifest` directly via gRPC, and diffs `Name` + `Version` against plugin.json with sentinel-pattern matching to catch the ldflag-missing truth-loop bug from workflow#762/#764. -**Architecture:** New subcommand registered in `cmd/wfctl/plugin.go`. Extracts existing spawn-and-dial pattern from `plugin_conformance.go:462-504` into a shared helper (`spawnAndDial`) so verify-capabilities and conformance both use it. Diff logic is set-equal-on-Name + sentinel-aware-on-Version (see design doc §Version rule matrix). Fixtures use in-place build with `-mod=readonly` + checked-in `go.sum` + `GOWORK=off`. +**Architecture:** New subcommand registered in `cmd/wfctl/plugin.go`. Spawn-and-dial wired INLINE in the subcommand (~40 LOC) — `plugin_conformance.go`'s spawn pattern is the reference; no shared-helper extraction in this PR (defer until a 3rd caller appears). GetManifest called DIRECTLY via `pb.NewPluginServiceClient(pluginClient.Conn())` to bypass `ExternalPluginAdapter.EngineManifest()`'s precedence rules (which would defeat the truth-loop check). Diff logic: exact Name + sentinel-aware Version matrix. -**Tech Stack:** Go (workflow CLI), `goplugin` (go-plugin v1.7), `pb.PluginService` (gRPC), `external.Handshake`, `external.NewExternalPluginAdapter`. +**Tech Stack:** Go (workflow CLI), `goplugin` (go-plugin v1.7), `pb.PluginService` (gRPC, raw client), `external.Handshake`, `external.PluginClient.Conn()`. **Base branch:** `main` @@ -14,13 +14,17 @@ **Issue:** workflow#765 +**Revision history:** +- Cycle 1: 9-task plan with shared `spawnAndDial` helper extraction. FAILED — 4 Critical (fictional `EngineManifest()` signature; fixture template wrong PluginManifest type; missing-ldflag mechanics misstated; fixture plugin.json shape diverges from PluginManifest). +- Cycle 2 (this version): drop helper extraction (Task 1 → inline ~40 LOC; eliminates I2 — verification-class mismatch on refactor-without-test). Replace `EngineManifest()` with direct `pb.NewPluginServiceClient(conn).GetManifest(ctx, ...)`. Fix fixture template (`sdk.PluginManifest` value, no error). Set fixture initial `Version = "dev"` so `ResolveBuildVersion` falls back to `(devel)` for the missing-ldflag scenario. Minimal plugin.json fixture (drop nonexistent fields). + --- ## Scope Manifest **PR Count:** 1 -**Tasks:** 9 -**Estimated Lines of Change:** ~600 (helper + subcommand + tests + 5 fixtures + docs) +**Tasks:** 8 +**Estimated Lines of Change:** ~500 (subcommand + tests + 5 fixtures + docs) **Out of scope:** - Contract-diff (`GetContractRegistry` walk) — deferred to follow-up issue #766; needs `capabilities.iacServices` schema on `PluginManifest` first @@ -31,192 +35,28 @@ - Author/Description/ConfigMutable/SampleCategory diff — display fields, not contract surface - `minEngineVersion` runtime check — not on `pb.Manifest`; static-check responsibility - Scaffold-template release.yml wiring — separate follow-up PR on scaffold-workflow-plugin after this lands +- Shared `spawnAndDial` helper extraction — defer until a 3rd caller exists (cycle-2 reviewer Option 3) **PR Grouping:** | PR # | Title | Tasks | Branch | |------|-------|-------|--------| -| 1 | feat(wfctl): plugin verify-capabilities subcommand (workflow#765) | Task 1, Task 2, Task 3, Task 4, Task 5, Task 6, Task 7, Task 8, Task 9 | feat/765-verify-capabilities | +| 1 | feat(wfctl): plugin verify-capabilities subcommand (workflow#765) | Task 1, Task 2, Task 3, Task 4, Task 5, Task 6, Task 7, Task 8 | feat/765-verify-capabilities | **Status:** Draft --- -## Task 1: Extract shared `spawnAndDial` helper from conformance - -**Change class:** Internal logic refactor (existing functionality moves; behavior unchanged). - -**Files:** -- Create: `cmd/wfctl/plugin_spawn.go` -- Modify: `cmd/wfctl/plugin_conformance.go:462-504` (replace inline spawn with `spawnAndDial` call; preserve lines 505-513 IaC-validation inline) -- Test: existing `cmd/wfctl/plugin_conformance_test.go` covers the refactored path; no new test needed for the move itself - -**Step 1: Confirm conformance test baseline passes** - -Run: `cd cmd/wfctl && go test -run TestConformance -count=1 ./...` -Expected: PASS (establishes baseline before refactor). - -**Step 2: Create the shared helper** - -Create `cmd/wfctl/plugin_spawn.go`: - -```go -// Package main — shared spawn-and-dial helper for wfctl plugin subcommands -// that need to exec a plugin binary and obtain a *external.PluginAdapter. -// -// Extracted from plugin_conformance.go:462-504 per workflow#765 design doc -// §Files. Used by `plugin conformance` and `plugin verify-capabilities`. -// Plugin-type-agnostic; IaC-specific post-dial assertions stay in callers. -package main - -import ( - "context" - "fmt" - "os/exec" - - external "github.com/GoCodeAlone/workflow/plugin/external" - goplugin "github.com/GoCodeAlone/go-plugin" - hclog "github.com/hashicorp/go-hclog" -) - -// spawnedPlugin is the result of spawning a plugin binary and dialing its -// gRPC server. Cleanup MUST be called by the caller (typically via defer) -// to kill the spawned subprocess. stdoutTail and stderrTail are the captured -// tail buffers (useful for error reporting when spawn or dial fails). -type spawnedPlugin struct { - Adapter *external.PluginAdapter - Cleanup func() - StdoutTail *tailBuffer - StderrTail *tailBuffer -} - -// spawnAndDial executes binaryPath as a go-plugin subprocess, dials its -// gRPC server with the canonical workflow ext handshake, dispenses the -// "plugin" interface, and wraps it in a *external.PluginAdapter. -// -// The returned Cleanup func wraps client.Kill — caller must `defer cleanup()`. -// If spawn or dial fails, Cleanup is non-nil but a no-op (still safe to defer). -// -// pluginName is informational (used for error messages + adapter naming). -// pluginDir is the working directory for the subprocess (typically the -// plugin's install directory; can be empty to inherit wfctl's cwd). -// env is the subprocess environment (typically conformancePluginEnv() — -// reuse from plugin_conformance.go for consistency). -func spawnAndDial(ctx context.Context, binaryPath, pluginName, pluginDir string, env []string) (*spawnedPlugin, error) { - var stdout, stderr tailBuffer - cmd := exec.CommandContext(ctx, binaryPath) //nolint:gosec // operator-supplied binary path. - if pluginDir != "" { - cmd.Dir = pluginDir - } - if env != nil { - cmd.Env = env - } - client := goplugin.NewClient(&goplugin.ClientConfig{ - HandshakeConfig: external.Handshake, - Plugins: goplugin.PluginSet{"plugin": &external.GRPCPlugin{}}, - Cmd: cmd, - AllowedProtocols: []goplugin.Protocol{goplugin.ProtocolGRPC}, - Stderr: &stderr, - SyncStdout: &stdout, - SyncStderr: &stderr, - Logger: hclog.NewNullLogger(), - }) - - rpcClient, err := client.Client() - if err != nil { - client.Kill() - if ctx.Err() != nil { - return nil, fmt.Errorf("timeout waiting for plugin handshake (stderr: %s)", stderr.String()) - } - return nil, fmt.Errorf("plugin dial: %w (stderr: %s)", err, stderr.String()) - } - raw, err := rpcClient.Dispense("plugin") - if err != nil { - client.Kill() - return nil, fmt.Errorf("dispense plugin: %w (stderr: %s)", err, stderr.String()) - } - pluginClient, ok := raw.(*external.PluginClient) - if !ok { - client.Kill() - return nil, fmt.Errorf("dispensed object is %T, want *external.PluginClient", raw) - } - adapter, err := external.NewExternalPluginAdapter(pluginName, pluginClient, nil) - if err != nil { - client.Kill() - return nil, fmt.Errorf("new adapter: %w", err) - } - return &spawnedPlugin{ - Adapter: adapter, - Cleanup: client.Kill, - StdoutTail: &stdout, - StderrTail: &stderr, - }, nil -} -``` - -**Step 3: Refactor `checkTypedIaCPlugin` to call the helper** - -Edit `cmd/wfctl/plugin_conformance.go` — replace lines 462-504 (spawn+dial+dispense+adapter) with a single `spawnAndDial` call. KEEP lines 505-513 (IaC-validation: `ContractRegistryError`, `AssertIaCPluginAdvertisesRequiredService`, `registeredIaCServices`, `newTypedIaCAdapter`) inline after the call. Pattern: - -```go -func checkTypedIaCPlugin(timeout time.Duration, pluginsDir, name string) (string, string, error) { - ctx, cancel := context.WithTimeout(context.Background(), timeout) - defer cancel() - - pluginDir := filepath.Join(pluginsDir, name) - binaryPath, err := filepath.Abs(filepath.Join(pluginDir, name)) - if err != nil { - return "", "", err - } - sp, err := spawnAndDial(ctx, binaryPath, name, pluginDir, conformancePluginEnv()) - if err != nil { - stdout := "" - stderr := "" - if sp != nil { - if sp.StdoutTail != nil { stdout = sp.StdoutTail.String() } - if sp.StderrTail != nil { stderr = sp.StderrTail.String() } - } - return stdout, stderr, err - } - defer sp.Cleanup() - - // IaC-specific post-dial validation (NOT in shared helper). - if regErr := sp.Adapter.ContractRegistryError(); regErr != nil { - return sp.StdoutTail.String(), sp.StderrTail.String(), regErr - } - if err := AssertIaCPluginAdvertisesRequiredService(name, "", sp.Adapter.ContractRegistry()); err != nil { - return sp.StdoutTail.String(), sp.StderrTail.String(), err - } - registered := registeredIaCServices(sp.Adapter.ContractRegistry()) - typed := newTypedIaCAdapter(sp.Adapter.Conn(), registered) - _ = typed.SupportedCanonicalKeys() - return sp.StdoutTail.String(), sp.StderrTail.String(), nil -} -``` - -**Step 4: Build + run conformance tests** - -Run: `cd cmd/wfctl && go build ./... && go test -run TestConformance -count=1 ./...` -Expected: build exit 0; tests PASS (behavior unchanged). - -**Step 5: Commit** - -```bash -git add cmd/wfctl/plugin_spawn.go cmd/wfctl/plugin_conformance.go -git commit -m "refactor(wfctl): extract spawnAndDial helper from conformance (workflow#765 prep)" -``` - ---- - -## Task 2: Subcommand registration + flag parsing skeleton +## Task 1: Subcommand registration + flag parsing skeleton **Change class:** CLI command. **Files:** - Create: `cmd/wfctl/plugin_verify_capabilities.go` - Modify: `cmd/wfctl/plugin.go` (add `case "verify-capabilities"` + help line) +- Test: `cmd/wfctl/plugin_verify_capabilities_test.go` -**Step 1: Write the failing test** +**Step 1: Write the failing tests** Create `cmd/wfctl/plugin_verify_capabilities_test.go`: @@ -228,8 +68,6 @@ import ( "testing" ) -// TestVerifyCapabilitiesUsage asserts the subcommand prints usage when invoked -// with no args. func TestVerifyCapabilitiesUsage(t *testing.T) { err := runPluginVerifyCapabilities([]string{}) if err == nil { @@ -240,7 +78,6 @@ func TestVerifyCapabilitiesUsage(t *testing.T) { } } -// TestVerifyCapabilitiesRequiresBinary asserts --binary is required. func TestVerifyCapabilitiesRequiresBinary(t *testing.T) { err := runPluginVerifyCapabilities([]string{"."}) if err == nil { @@ -252,20 +89,20 @@ func TestVerifyCapabilitiesRequiresBinary(t *testing.T) { } ``` -**Step 2: Run test to verify it fails** +**Step 2: Run test — verify FAIL** -Run: `cd cmd/wfctl && go test -run TestVerifyCapabilitiesUsage -count=1 ./...` +Run: `cd cmd/wfctl && go test -run TestVerifyCapabilities -count=1 ./...` Expected: FAIL with `undefined: runPluginVerifyCapabilities`. -**Step 3: Write minimal implementation** +**Step 3: Create the subcommand skeleton** Create `cmd/wfctl/plugin_verify_capabilities.go`: ```go // Package main — `wfctl plugin verify-capabilities` subcommand. -// Spawns a plugin binary, calls PluginService.GetManifest, diffs returned -// Manifest against plugin.json. Catches ldflag-missing truth-loop bug from -// workflow#762/#764. +// Spawns a plugin binary, calls PluginService.GetManifest directly via gRPC, +// diffs returned Manifest against plugin.json. Catches ldflag-missing +// truth-loop bug from workflow#762/#764. // // Design: docs/plans/2026-05-24-verify-capabilities-design.md // Issue: https://github.com/GoCodeAlone/workflow/issues/765 @@ -274,12 +111,11 @@ package main import ( "flag" "fmt" - "os" ) func runPluginVerifyCapabilities(args []string) error { fs := flag.NewFlagSet("plugin verify-capabilities", flag.ContinueOnError) - binary := fs.String("binary", "", "Path to plugin binary (REQUIRED; see help for goreleaser dist/artifacts.json pattern)") + binary := fs.String("binary", "", "Path to plugin binary (REQUIRED)") fs.Usage = func() { fmt.Fprintf(fs.Output(), `Usage: wfctl plugin verify-capabilities --binary @@ -320,28 +156,23 @@ Options: return fmt.Errorf("exactly one argument required") } pluginDir := fs.Arg(0) - - // TODO: preflight + spawn + diff (Tasks 3-5) _ = pluginDir - _ = os.Stat return fmt.Errorf("not yet implemented") } ``` -Edit `cmd/wfctl/plugin.go` — find the `case "validate-contract":` dispatcher block (line ~38) and add right after: - -```go - case "verify-capabilities": - return runPluginVerifyCapabilities(args[1:]) -``` - -Also find the help-text block in `plugin.go`'s usage() function and add (alphabetical order with other verbs): +Edit `cmd/wfctl/plugin.go`: +- Find the `case "validate-contract":` dispatcher block (~line 38) and add right after: + ```go + case "verify-capabilities": + return runPluginVerifyCapabilities(args[1:]) + ``` +- In the usage() help-text block, add (alphabetical): + ```go + fmt.Fprintln(out, " verify-capabilities Spawn plugin binary, verify runtime GetManifest matches plugin.json") + ``` -```go - fmt.Fprintln(out, " verify-capabilities Spawn plugin binary, verify runtime GetManifest matches plugin.json") -``` - -**Step 4: Run test to verify it passes** +**Step 4: Run test — verify PASS** Run: `cd cmd/wfctl && go build ./... && go test -run TestVerifyCapabilities -count=1 ./...` Expected: build exit 0; both tests PASS. @@ -355,7 +186,7 @@ git commit -m "feat(wfctl): plugin verify-capabilities subcommand skeleton (work --- -## Task 3: Preflight binary path validation +## Task 2: Preflight binary path validation **Change class:** Internal logic refactor (input validation). @@ -368,31 +199,33 @@ git commit -m "feat(wfctl): plugin verify-capabilities subcommand skeleton (work Append to `cmd/wfctl/plugin_verify_capabilities_test.go`: ```go +import ( + "os" + "path/filepath" + "strings" + "testing" +) + func TestPreflightBinaryEmpty(t *testing.T) { - err := preflightBinary("") - if err == nil || !strings.Contains(err.Error(), "binary path") { + if err := preflightBinary(""); err == nil || !strings.Contains(err.Error(), "binary path") { t.Errorf("want empty-path error, got %v", err) } } func TestPreflightBinaryNull(t *testing.T) { - err := preflightBinary("null") - if err == nil || !strings.Contains(err.Error(), "binary path") { + if err := preflightBinary("null"); err == nil || !strings.Contains(err.Error(), "binary path") { t.Errorf("want null-path error (jq fallback), got %v", err) } } func TestPreflightBinaryMissing(t *testing.T) { - err := preflightBinary("/nonexistent/missing-binary-xyz") - if err == nil || !strings.Contains(err.Error(), "stat") { + if err := preflightBinary("/nonexistent/missing-xyz"); err == nil || !strings.Contains(err.Error(), "stat") { t.Errorf("want stat error, got %v", err) } } func TestPreflightBinaryDirectory(t *testing.T) { - d := t.TempDir() - err := preflightBinary(d) - if err == nil || !strings.Contains(err.Error(), "directory") { + if err := preflightBinary(t.TempDir()); err == nil || !strings.Contains(err.Error(), "directory") { t.Errorf("want directory error, got %v", err) } } @@ -403,8 +236,7 @@ func TestPreflightBinaryNonExecutable(t *testing.T) { if err := os.WriteFile(f, []byte("not-exec"), 0o644); err != nil { t.Fatal(err) } - err := preflightBinary(f) - if err == nil || !strings.Contains(err.Error(), "executable") { + if err := preflightBinary(f); err == nil || !strings.Contains(err.Error(), "executable") { t.Errorf("want non-executable error, got %v", err) } } @@ -421,24 +253,26 @@ func TestPreflightBinaryOK(t *testing.T) { } ``` -Add to imports: `"os"`, `"path/filepath"`. - -**Step 2: Run test to verify it fails** +**Step 2: Run test — verify FAIL** Run: `cd cmd/wfctl && go test -run TestPreflightBinary -count=1 ./...` -Expected: FAIL with `undefined: preflightBinary`. +Expected: FAIL `undefined: preflightBinary`. **Step 3: Implement** Append to `cmd/wfctl/plugin_verify_capabilities.go`: ```go +import ( + "flag" + "fmt" + "os" +) + // preflightBinary validates the --binary path before exec: // - non-empty + not literal "null" (guards against jq fallback returning empty) -// - file exists and is a regular file (not directory or symlink-to-dir) +// - file exists and is a regular file (not directory) // - has at least one executable bit set -// -// Design: §Synopsis preflight checks. func preflightBinary(path string) error { if path == "" || path == "null" { return fmt.Errorf("--binary path empty (jq filter may have returned no match)") @@ -460,15 +294,15 @@ func preflightBinary(path string) error { } ``` -Update `runPluginVerifyCapabilities` to call it before the `return "not yet implemented"` line: +In `runPluginVerifyCapabilities`, before the `return fmt.Errorf("not yet implemented")` line: ```go - if err := preflightBinary(*binary); err != nil { - return err - } +if err := preflightBinary(*binary); err != nil { + return err +} ``` -**Step 4: Run test to verify it passes** +**Step 4: Run test — verify PASS** Run: `cd cmd/wfctl && go test -run TestPreflightBinary -count=1 ./...` Expected: all 6 preflight tests PASS. @@ -482,7 +316,7 @@ git commit -m "feat(wfctl): verify-capabilities preflight binary-path validation --- -## Task 4: Sentinel-pattern Version diff matrix +## Task 3: Sentinel-pattern Version diff matrix **Change class:** Internal logic refactor (pure-logic diff function). @@ -497,15 +331,15 @@ Append to `cmd/wfctl/plugin_verify_capabilities_test.go`: ```go func TestIsSentinel(t *testing.T) { cases := map[string]bool{ - "": true, - "dev": true, - "0.0.0": true, - "(devel)": true, - "(devel) [@ a1b2c3d]": true, + "": true, + "dev": true, + "0.0.0": true, + "(devel)": true, + "(devel) [@ a1b2c3d]": true, "(devel) [@ a1b2c3d.dirty]": true, - "v1.2.3": false, - "1.2.3": false, - "v0.0.1": false, + "v1.2.3": false, + "1.2.3": false, + "v0.0.1": false, } for v, want := range cases { if got := isSentinel(v); got != want { @@ -515,59 +349,56 @@ func TestIsSentinel(t *testing.T) { } func TestDiffVersion(t *testing.T) { - // (declared, runtime, wantPass, wantReason-substring) cases := []struct { declared, runtime string wantPass bool wantReason string }{ - // Dev-sentinel row 1: 0.0.0 + non-sentinel -> PASS (CI artifact) + // 0.0.0 + non-sentinel -> PASS (CI artifact) {"0.0.0", "v1.2.3", true, ""}, {"0.0.0", "0.1.0", true, ""}, - // Dev-sentinel row 2: 0.0.0 + sentinel -> FAIL (ldflag missing) + // 0.0.0 + sentinel -> FAIL (ldflag missing) {"0.0.0", "", false, "ldflag"}, {"0.0.0", "(devel)", false, "ldflag"}, {"0.0.0", "(devel) [@ abc1234]", false, "ldflag"}, {"0.0.0", "dev", false, "ldflag"}, {"0.0.0", "0.0.0", false, "ldflag"}, - // Release row 3: X.Y.Z + vX.Y.Z or X.Y.Z -> PASS (normalize leading v) + // X.Y.Z + vX.Y.Z or X.Y.Z -> PASS (normalize leading v) {"1.2.3", "v1.2.3", true, ""}, {"1.2.3", "1.2.3", true, ""}, - // Release row 4: X.Y.Z + sentinel -> FAIL + // X.Y.Z + sentinel -> FAIL {"1.2.3", "", false, "ldflag"}, {"1.2.3", "(devel)", false, "ldflag"}, {"1.2.3", "(devel) [@ deadbee]", false, "ldflag"}, - // Release row 5: X.Y.Z + drift -> FAIL + // X.Y.Z + drift -> FAIL {"1.2.3", "v0.9.0", false, "drift"}, {"1.2.3", "v2.0.0", false, "drift"}, } for _, c := range cases { pass, reason := diffVersion(c.declared, c.runtime) if pass != c.wantPass { - t.Errorf("diffVersion(%q, %q) pass = %v, want %v (reason: %q)", + t.Errorf("diffVersion(%q, %q) pass=%v want=%v reason=%q", c.declared, c.runtime, pass, c.wantPass, reason) continue } if !pass && !strings.Contains(reason, c.wantReason) { - t.Errorf("diffVersion(%q, %q) reason = %q, want substring %q", + t.Errorf("diffVersion(%q, %q) reason=%q want substring %q", c.declared, c.runtime, reason, c.wantReason) } } } ``` -**Step 2: Run tests to verify failure** +**Step 2: Run tests — verify FAIL** Run: `cd cmd/wfctl && go test -run "TestIsSentinel|TestDiffVersion" -count=1 ./...` -Expected: FAIL with `undefined: isSentinel`, `undefined: diffVersion`. +Expected: FAIL `undefined: isSentinel`, `undefined: diffVersion`. **Step 3: Implement** -Append to `cmd/wfctl/plugin_verify_capabilities.go`: +Append to `cmd/wfctl/plugin_verify_capabilities.go` (add `"strings"` to imports): ```go -import "strings" // add to existing import block if absent - // isSentinel returns true when v is one of the SDK's dev-sentinel forms // OR the on-disk plugin.json sentinel "0.0.0". // @@ -576,9 +407,9 @@ import "strings" // add to existing import block if absent // Plus build-info fallback produces "(devel) [@ [.dirty]]" — HasPrefix catches all forms. // Plus on-disk plugin.json "0.0.0" sentinel (workflow#762 convention). // -// The predicate MUST be a SUPERSET of the SDK's set; "dev" is defensive even -// though canonical wiring (sdk.ResolveBuildVersion) prevents literal "dev" -// from reaching the wire — included to catch non-canonical wiring accidents. +// The predicate MUST be a SUPERSET of the SDK's set; "dev" is defensive +// (canonical wiring through sdk.ResolveBuildVersion prevents literal "dev" +// from reaching the wire — included to catch non-canonical wiring accidents). func isSentinel(v string) bool { switch v { case "", "dev", "0.0.0", "(devel)": @@ -609,7 +440,6 @@ func diffVersion(declared, runtime string) (bool, string) { if runtimeSentinel { return false, fmt.Sprintf("ldflag injection missing: plugin.json=%q (release); binary Manifest.Version=%q (sentinel)", declared, runtime) } - // Normalize leading v on runtime side rNorm := strings.TrimPrefix(runtime, "v") if rNorm == declared { return true, "" @@ -618,7 +448,7 @@ func diffVersion(declared, runtime string) (bool, string) { } ``` -**Step 4: Run tests to verify pass** +**Step 4: Run tests — verify PASS** Run: `cd cmd/wfctl && go test -run "TestIsSentinel|TestDiffVersion" -count=1 ./...` Expected: all matrix cases PASS. @@ -632,89 +462,119 @@ git commit -m "feat(wfctl): verify-capabilities sentinel-pattern Version diff ma --- -## Task 5: Wire spawn + GetManifest + diff into runPluginVerifyCapabilities +## Task 4: Inline spawn-and-dial + direct GetManifest RPC -**Change class:** Plugin / extension (CLI subcommand that calls a plugin via gRPC). +**Change class:** Plugin / extension (CLI subcommand that calls a plugin via raw gRPC). **Files:** - Modify: `cmd/wfctl/plugin_verify_capabilities.go` -**Step 1: Write the failing integration test** (deferred to Task 7 fixture work — this task is the implementation wiring; test in Task 8) +This task wires the actual spawn-and-dial INLINE (no shared helper extraction — cycle-2 reviewer Option 3 + I2 elimination). GetManifest is called DIRECTLY via `pb.NewPluginServiceClient(pluginClient.Conn())` to bypass `ExternalPluginAdapter`'s precedence rules. -For this task, the verification is `go build ./... && wfctl plugin verify-capabilities --help` showing the help text. +**Step 1: Load + validate plugin.json** -**Step 2: Read plugin.json + load PluginManifest** - -Modify `runPluginVerifyCapabilities` — replace the `return fmt.Errorf("not yet implemented")` line with: +In `runPluginVerifyCapabilities`, replace the `return fmt.Errorf("not yet implemented")` line with: ```go - abs, err := filepath.Abs(pluginDir) - if err != nil { - return fmt.Errorf("resolve %q: %w", pluginDir, err) - } - manifestPath := filepath.Join(abs, "plugin.json") - manifestBytes, err := os.ReadFile(manifestPath) //nolint:gosec // operator-supplied path. - if err != nil { - return fmt.Errorf("plugin.json: %w", err) - } - var declared plugin.PluginManifest - if err := json.Unmarshal(manifestBytes, &declared); err != nil { - return fmt.Errorf("plugin.json parse: %w", err) - } - if err := declared.Validate(); err != nil { - return fmt.Errorf("plugin.json validate: %w", err) - } +abs, err := filepath.Abs(pluginDir) +if err != nil { + return fmt.Errorf("resolve %q: %w", pluginDir, err) +} +manifestPath := filepath.Join(abs, "plugin.json") +manifestBytes, err := os.ReadFile(manifestPath) //nolint:gosec // operator-supplied path. +if err != nil { + return fmt.Errorf("plugin.json: %w", err) +} +var declared plugin.PluginManifest +if err := json.Unmarshal(manifestBytes, &declared); err != nil { + return fmt.Errorf("plugin.json parse: %w", err) +} +if err := declared.Validate(); err != nil { + return fmt.Errorf("plugin.json validate: %w", err) +} ``` Add imports: `"encoding/json"`, `"path/filepath"`, `"github.com/GoCodeAlone/workflow/plugin"`. -**Step 3: Spawn + dial via shared helper + call GetManifest** +**Step 2: Spawn + dial INLINE (no shared helper)** Continue in `runPluginVerifyCapabilities`: ```go - ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) - defer cancel() +ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) +defer cancel() - binAbs, err := filepath.Abs(*binary) - if err != nil { - return fmt.Errorf("resolve --binary %q: %w", *binary, err) - } - sp, err := spawnAndDial(ctx, binAbs, declared.Name, "", nil) - if err != nil { - return fmt.Errorf("spawn %s: %w", declared.Name, err) - } - defer sp.Cleanup() +binAbs, err := filepath.Abs(*binary) +if err != nil { + return fmt.Errorf("resolve --binary %q: %w", *binary, err) +} - runtime, err := sp.Adapter.EngineManifest() - if err != nil { - return fmt.Errorf("GetManifest RPC: %w (stderr: %s)", err, sp.StderrTail.String()) - } +var stdout, stderr tailBuffer +cmd := exec.CommandContext(ctx, binAbs) //nolint:gosec // operator-supplied binary path. +client := goplugin.NewClient(&goplugin.ClientConfig{ + HandshakeConfig: external.Handshake, + Plugins: goplugin.PluginSet{"plugin": &external.GRPCPlugin{}}, + Cmd: cmd, + AllowedProtocols: []goplugin.Protocol{goplugin.ProtocolGRPC}, + Stderr: &stderr, + SyncStdout: &stdout, + SyncStderr: &stderr, + Logger: hclog.NewNullLogger(), +}) +defer client.Kill() + +rpcClient, err := client.Client() +if err != nil { + if ctx.Err() != nil { + return fmt.Errorf("timeout waiting for plugin handshake (stderr: %s)", stderr.String()) + } + return fmt.Errorf("plugin dial: %w (stderr: %s)", err, stderr.String()) +} +raw, err := rpcClient.Dispense("plugin") +if err != nil { + return fmt.Errorf("dispense plugin: %w (stderr: %s)", err, stderr.String()) +} +pluginClient, ok := raw.(*external.PluginClient) +if !ok { + return fmt.Errorf("dispensed object is %T, want *external.PluginClient", raw) +} ``` -Note: `external.PluginAdapter.EngineManifest()` returns `(*pb.Manifest, error)` per `/tmp/wfprobe/plugin/external/adapter.go:367`. Verify with `grep -n "func.*EngineManifest" /tmp/wfprobe/plugin/external/adapter.go` if the method name has changed; substitute the exact accessor. +Add imports: `"context"`, `"os/exec"`, `"time"`, `external "github.com/GoCodeAlone/workflow/plugin/external"`, `goplugin "github.com/GoCodeAlone/go-plugin"`, `hclog "github.com/hashicorp/go-hclog"`. + +Note: `tailBuffer` is defined in `cmd/wfctl/plugin_conformance.go` (same package). No import needed. + +**Step 3: Call GetManifest DIRECTLY via raw gRPC client** (bypasses adapter precedence) + +```go +pbClient := pb.NewPluginServiceClient(pluginClient.Conn()) +runtime, err := pbClient.GetManifest(ctx, &emptypb.Empty{}) +if err != nil { + return fmt.Errorf("GetManifest RPC: %w (stderr: %s)", err, stderr.String()) +} +``` -Add imports: `"context"`, `"time"`. +Add imports: `pb "github.com/GoCodeAlone/workflow/plugin/external/proto"`, `"google.golang.org/protobuf/types/known/emptypb"`. **Step 4: Diff Name + Version and report** ```go - var failures []string - if runtime.GetName() != declared.Name { - failures = append(failures, fmt.Sprintf("name: plugin.json=%q; binary Manifest.Name=%q", declared.Name, runtime.GetName())) - } - if pass, reason := diffVersion(declared.Version, runtime.GetVersion()); !pass { - failures = append(failures, "version: "+reason) - } - if len(failures) > 0 { - fmt.Fprintf(os.Stderr, "FAIL %s (plugin.json)\nerror: %d mismatch(es)\n", declared.Name, len(failures)) - for _, f := range failures { - fmt.Fprintf(os.Stderr, " - %s\n", f) - } - return fmt.Errorf("verify-capabilities: %d mismatch(es)", len(failures)) - } - fmt.Printf("OK %s %s (plugin.json: %s)\n", declared.Name, runtime.GetVersion(), declared.Version) - return nil +var failures []string +if runtime.GetName() != declared.Name { + failures = append(failures, fmt.Sprintf("name: plugin.json=%q; binary Manifest.Name=%q", declared.Name, runtime.GetName())) +} +if pass, reason := diffVersion(declared.Version, runtime.GetVersion()); !pass { + failures = append(failures, "version: "+reason) +} +if len(failures) > 0 { + fmt.Fprintf(os.Stderr, "FAIL %s (plugin.json)\nerror: %d mismatch(es)\n", declared.Name, len(failures)) + for _, f := range failures { + fmt.Fprintf(os.Stderr, " - %s\n", f) + } + return fmt.Errorf("verify-capabilities: %d mismatch(es)", len(failures)) +} +fmt.Printf("OK %s %s (plugin.json: %s)\n", declared.Name, runtime.GetVersion(), declared.Version) +return nil ``` **Step 5: Build + help-output sanity check** @@ -723,53 +583,42 @@ Run: ```bash cd cmd/wfctl && go build -o /tmp/wfctl ./... && /tmp/wfctl plugin verify-capabilities --help ``` -Expected: help text printed; exit 0. The help text contains "REQUIRED: --binary", "WARNING: this command EXECUTES", and the `jq` CI example. +Expected: help text printed; exit 0. Help contains "REQUIRED: --binary", "WARNING: this command EXECUTES", `jq` CI example. **Step 6: Commit** ```bash git add cmd/wfctl/plugin_verify_capabilities.go -git commit -m "feat(wfctl): wire spawn + GetManifest + Name/Version diff (workflow#765)" +git commit -m "feat(wfctl): wire inline spawn + direct GetManifest + Name/Version diff (workflow#765)" ``` --- -## Task 6: Create 5 fixture scenarios under testdata/verify_capabilities/ +## Task 5: Create 4 build-PASS fixture scenarios -**Change class:** Documentation / fixture data (no runtime impact). +**Change class:** Test fixture (no runtime impact). **Files:** - Create: `cmd/wfctl/testdata/verify_capabilities/good/{plugin.json,main.go,go.mod,go.sum}` - Create: `cmd/wfctl/testdata/verify_capabilities/release-good/{plugin.json,main.go,go.mod,go.sum}` - Create: `cmd/wfctl/testdata/verify_capabilities/missing-ldflag/{plugin.json,main.go,go.mod,go.sum}` - Create: `cmd/wfctl/testdata/verify_capabilities/version-drift/{plugin.json,main.go,go.mod,go.sum}` -- Create: `cmd/wfctl/testdata/verify_capabilities/name-drift/{plugin.json,main.go,go.mod,go.sum}` -- Create: `cmd/wfctl/testdata/verify_capabilities/README.md` (maintenance note) +- Create: `cmd/wfctl/testdata/verify_capabilities/README.md` + +Cycle-2 fix C2: fixture template uses `sdk.PluginManifest` (sdk-package value, NOT `plugin.PluginManifest`), no error return on `Manifest()`. Cycle-2 fix C3: initial `Version = "dev"` so `ResolveBuildVersion("dev")` falls back to `(devel)` when no ldflag is applied (true exercise of the missing-ldflag scenario). Cycle-2 fix C4: minimal plugin.json with only fields PluginManifest actually models. -**Step 1: Write a generator script** (one-off; not committed) +**Step 1: Write the generator script** (one-off; not committed) -Create `/tmp/gen-fixtures.sh` (not committed; just for generating): +Save as `/tmp/gen-verify-fixtures.sh`: ```bash #!/bin/bash set -euo pipefail BASE=cmd/wfctl/testdata/verify_capabilities REPO_ROOT=$(git rev-parse --show-toplevel) -declare -A NAMES=( - [good]=verify-good - [release-good]=verify-release-good - [missing-ldflag]=verify-missing-ldflag - [version-drift]=verify-version-drift - [name-drift]=verify-name-drift -) -declare -A VERS=( - [good]=0.0.0 - [release-good]=1.2.3 - [missing-ldflag]=0.0.0 - [version-drift]=1.2.3 - [name-drift]=0.0.0 -) -for s in good release-good missing-ldflag version-drift name-drift; do +declare -A NAMES=( [good]=verify-good [release-good]=verify-release-good [missing-ldflag]=verify-missing-ldflag [version-drift]=verify-version-drift ) +declare -A VERS=( [good]=0.0.0 [release-good]=1.2.3 [missing-ldflag]=0.0.0 [version-drift]=1.2.3 ) +for s in good release-good missing-ldflag version-drift; do d="$BASE/$s" mkdir -p "$d" name="${NAMES[$s]}" @@ -779,39 +628,35 @@ for s in good release-good missing-ldflag version-drift name-drift; do "name": "$name", "version": "$version", "minEngineVersion": "v0.62.0", - "type": "external", "author": "test fixture", - "description": "verify-capabilities $s scenario", - "capabilities": { - "moduleTypes": ["fake.module"], - "stepTypes": [], - "triggerTypes": [], - "workflowTypes": [], - "wiringHooks": [] - } + "description": "verify-capabilities $s scenario" } JSON cat > "$d/main.go" <]" when no +// ldflag fires (exercises the missing-ldflag scenario faithfully). +var Version = "dev" type stubProvider struct{} -func (stubProvider) Manifest() (*plugin.PluginManifest, error) { - return &plugin.PluginManifest{ - Name: "$name", - Version: sdk.ResolveBuildVersion(Version), - }, nil +func (stubProvider) Manifest() sdk.PluginManifest { + return sdk.PluginManifest{ + Name: "$name", + Version: "$version", + Author: "test fixture", + Description: "verify-capabilities $s scenario", + } } func main() { - sdk.Serve(stubProvider{}, sdk.WithBuildVersion(sdk.ResolveBuildVersion(Version))) + sdk.Serve(stubProvider{}, + sdk.WithBuildVersion(sdk.ResolveBuildVersion(Version)), + ) } GO cat > "$d/go.mod" < .*|replace github.com/GoCodeAlone/workflow => ../../../../..|" "$d/go.mod" rm -f "$d/go.mod.bak" done ``` -Verify each fixture builds standalone: +**Step 3: Verify all 4 fixtures build standalone** + ```bash for d in cmd/wfctl/testdata/verify_capabilities/*/; do - (cd "$d" && GOWORK=off go build -mod=readonly -o /tmp/p .) && echo "$d: ok" || echo "$d: FAIL" + (cd "$d" && GOWORK=off go build -mod=readonly -o /tmp/p .) && echo "$d: ok" || { echo "$d: FAIL"; exit 1; } done ``` -Expected: all 5 print `ok`. (This catches go.sum drift before the integration tests run.) +Expected: all 4 print `ok`. -**Step 3: Create the maintenance README** +**Step 4: Create maintenance README** ```bash cat > cmd/wfctl/testdata/verify_capabilities/README.md <<'MD' @@ -856,14 +699,13 @@ cat > cmd/wfctl/testdata/verify_capabilities/README.md <<'MD' Fixtures for `plugin_verify_capabilities_test.go` (workflow#765). -Each scenario directory contains a self-contained Go module that builds into -a minimal plugin binary. Tests call `go build -mod=readonly` in-place; the -binary is emitted to `t.TempDir()`. +Each scenario directory is a self-contained Go module. Tests build in-place +with `go build -mod=readonly`; binary emitted to `t.TempDir()`. ## Maintenance -When the workflow SDK adds a new transitive dep that the fixtures' build -graph picks up, regenerate each fixture's `go.sum`: +When workflow SDK adds a new transitive dep that fixtures pick up, regenerate +each fixture's `go.sum`: ```bash for d in cmd/wfctl/testdata/verify_capabilities/*/; do @@ -874,74 +716,103 @@ git add cmd/wfctl/testdata/verify_capabilities/*/go.sum The `replace github.com/GoCodeAlone/workflow => ../../../../..` directive resolves 5-ups from each scenario directory to the workflow repo root. -DO NOT use an absolute path — it will diverge across developer machines. +DO NOT use an absolute path — it diverges across developer machines. ## Scenarios -- `good/` — plugin.json version=0.0.0, ldflag injects v0.1.0 → PASS -- `release-good/` — plugin.json version=1.2.3, ldflag injects v1.2.3 → PASS -- `missing-ldflag/` — plugin.json version=0.0.0, no ldflag → FAIL +- `good/` — plugin.json version=0.0.0, ldflag injects v0.1.0 → PASS (CI artifact case) +- `release-good/` — plugin.json version=1.2.3, ldflag injects v1.2.3 → PASS (release case) +- `missing-ldflag/` — plugin.json version=0.0.0, no ldflag (Version="dev" → ResolveBuildVersion returns "(devel) [@ sha]") → FAIL - `version-drift/` — plugin.json version=1.2.3, ldflag injects v0.9.0 → FAIL -- `name-drift/` — plugin.json name="foo", binary advertises Name="bar" → FAIL -MD -``` +- `name-drift/` — plugin.json name="foo", binary advertises Name="bar" (see Task 6 — created separately) → FAIL -**Step 4: Verify all fixtures build (rerun)** +## SDK semantics -Run: -```bash -for d in cmd/wfctl/testdata/verify_capabilities/*/; do - (cd "$d" && GOWORK=off go build -mod=readonly -o /tmp/p .) || exit 1 -done -echo "all fixtures build" +`sdk.ResolveBuildVersion` returns its argument unchanged UNLESS the arg is one +of `{"", "dev", "(devel)"}`, in which case it consults `debug.ReadBuildInfo()` +and returns `"(devel) [@ [.dirty]]"`. So: + +- Initial `var Version = "dev"` + no ldflag → wire Version is `"(devel) [@ sha]"` +- Initial `var Version = "dev"` + ldflag `-X .Version=v1.2.3` → wire Version is `"v1.2.3"` +- Initial `var Version = "0.0.0"` + no ldflag → wire Version is `"0.0.0"` (NOT a build-info fallback; `"0.0.0"` is NOT in the SDK's reset set) + +The `missing-ldflag` fixture uses `Version = "dev"` deliberately so it exercises +the `(devel)` fallback path, not the `"0.0.0"` pass-through. +MD ``` -Expected: `all fixtures build`. **Step 5: Commit** ```bash git add cmd/wfctl/testdata/verify_capabilities/ -git commit -m "test(wfctl): verify-capabilities testdata fixtures (workflow#765)" +git commit -m "test(wfctl): verify-capabilities fixtures (4 build-pass scenarios) (workflow#765)" ``` --- -## Task 7: Special-case name-drift fixture (binary advertises different Name) +## Task 6: Create name-drift fixture (binary advertises different Name) -**Change class:** Internal logic refactor (fixture variant). +**Change class:** Test fixture. **Files:** -- Modify: `cmd/wfctl/testdata/verify_capabilities/name-drift/main.go` - -**Step 1: Adjust the name-drift fixture's main.go** +- Create: `cmd/wfctl/testdata/verify_capabilities/name-drift/{plugin.json,main.go,go.mod,go.sum}` -Replace `cmd/wfctl/testdata/verify_capabilities/name-drift/main.go` so the binary advertises `Name="verify-name-drift-binary"` while plugin.json declares `name="verify-name-drift"`: +**Step 1: Generate the fixture** -```go +```bash +REPO_ROOT=$(git rev-parse --show-toplevel) +d=cmd/wfctl/testdata/verify_capabilities/name-drift +mkdir -p "$d" +cat > "$d/plugin.json" <<'JSON' +{ + "name": "verify-name-drift", + "version": "0.0.0", + "minEngineVersion": "v0.62.0", + "author": "test fixture", + "description": "verify-capabilities name-drift scenario" +} +JSON +cat > "$d/main.go" <<'GO' package main -import ( - sdk "github.com/GoCodeAlone/workflow/plugin/external/sdk" - "github.com/GoCodeAlone/workflow/plugin" -) +import sdk "github.com/GoCodeAlone/workflow/plugin/external/sdk" -var Version = "0.0.0" +var Version = "dev" type stubProvider struct{} -func (stubProvider) Manifest() (*plugin.PluginManifest, error) { - return &plugin.PluginManifest{ - Name: "verify-name-drift-binary", // intentional drift vs plugin.json "verify-name-drift" - Version: sdk.ResolveBuildVersion(Version), - }, nil +// Manifest intentionally returns a DIFFERENT name than plugin.json declares. +// plugin.json says "verify-name-drift"; runtime says "verify-name-drift-binary". +func (stubProvider) Manifest() sdk.PluginManifest { + return sdk.PluginManifest{ + Name: "verify-name-drift-binary", + Version: "0.0.0", + Author: "test fixture", + Description: "verify-capabilities name-drift scenario", + } } func main() { - sdk.Serve(stubProvider{}, sdk.WithBuildVersion(sdk.ResolveBuildVersion(Version))) + sdk.Serve(stubProvider{}, + sdk.WithBuildVersion(sdk.ResolveBuildVersion(Version)), + ) } +GO +cat > "$d/go.mod" < $REPO_ROOT +MOD +(cd "$d" && GOWORK=off go mod tidy) +sed -i.bak "s|replace github.com/GoCodeAlone/workflow => .*|replace github.com/GoCodeAlone/workflow => ../../../../..|" "$d/go.mod" +rm -f "$d/go.mod.bak" ``` -**Step 2: Verify fixture still builds** +**Step 2: Verify fixture builds** Run: `(cd cmd/wfctl/testdata/verify_capabilities/name-drift && GOWORK=off go build -mod=readonly -o /tmp/p .)` Expected: exit 0. @@ -949,53 +820,46 @@ Expected: exit 0. **Step 3: Commit** ```bash -git add cmd/wfctl/testdata/verify_capabilities/name-drift/main.go +git add cmd/wfctl/testdata/verify_capabilities/name-drift/ git commit -m "test(wfctl): name-drift fixture (binary advertises mismatched Name) (workflow#765)" ``` --- -## Task 8: Integration tests — 5 scenarios end-to-end +## Task 7: Integration tests — 5 scenarios end-to-end -**Change class:** Plugin / extension (exercise spawn + RPC + diff against real fixture binaries). +**Change class:** Plugin / extension (exercises spawn + RPC + diff against real fixture binaries). **Files:** - Modify: `cmd/wfctl/plugin_verify_capabilities_test.go` -**Step 1: Write the integration test scaffolding** +**Step 1: Add the fixture-build helper + 5 test cases** Append to `cmd/wfctl/plugin_verify_capabilities_test.go`: ```go import ( "os/exec" - // ...existing imports + // keep existing imports ) // buildFixtureBinaryForVerify builds the fixture scenario in-place and emits -// the binary to t.TempDir(). ldflag is the -X ...Version= value (may be ""). -// Returns the absolute path to the built binary. -// -// Pattern mirrors plugin_conformance_test.go:buildFixtureBinary but adapted -// for in-place build per verify-capabilities design (no copy-to-TempDir). +// the binary to t.TempDir(). ldflag is the -X ...Version= value ("" = no flag, +// which makes ResolveBuildVersion fall back to "(devel) [@ sha]" for fixtures +// whose initial Version var is "dev"). func buildFixtureBinaryForVerify(t *testing.T, scenario, ldflagTag string) string { t.Helper() binPath := filepath.Join(t.TempDir(), "p") - args := []string{"build", "-mod=readonly", "-o", binPath, "."} + args := []string{"build", "-mod=readonly"} if ldflagTag != "" { - args = append([]string{"build", "-mod=readonly", - "-ldflags", fmt.Sprintf("-X github.com/test/%s.Version=%s", scenario, ldflagTag), - "-o", binPath, "."}, args[4:]...) - // Re-stitch to keep only the ldflags variant. - args = []string{"build", "-mod=readonly", - "-ldflags", fmt.Sprintf("-X github.com/test/%s.Version=%s", scenario, ldflagTag), - "-o", binPath, "."} + args = append(args, "-ldflags", + fmt.Sprintf("-X github.com/test/%s.Version=%s", scenario, ldflagTag)) } + args = append(args, "-o", binPath, ".") cmd := exec.Command("go", args...) cmd.Dir = filepath.Join("testdata", "verify_capabilities", scenario) cmd.Env = append(os.Environ(), "GOWORK=off") - out, err := cmd.CombinedOutput() - if err != nil { + if out, err := cmd.CombinedOutput(); err != nil { t.Fatalf("build %s: %v\n%s", scenario, err, out) } return binPath @@ -1003,22 +867,21 @@ func buildFixtureBinaryForVerify(t *testing.T, scenario, ldflagTag string) strin func TestVerifyCapabilities_Good(t *testing.T) { bin := buildFixtureBinaryForVerify(t, "good", "v0.1.0") - err := runPluginVerifyCapabilities([]string{"--binary", bin, "testdata/verify_capabilities/good"}) - if err != nil { + if err := runPluginVerifyCapabilities([]string{"--binary", bin, "testdata/verify_capabilities/good"}); err != nil { t.Fatalf("want PASS, got: %v", err) } } func TestVerifyCapabilities_ReleaseGood(t *testing.T) { bin := buildFixtureBinaryForVerify(t, "release-good", "v1.2.3") - err := runPluginVerifyCapabilities([]string{"--binary", bin, "testdata/verify_capabilities/release-good"}) - if err != nil { + if err := runPluginVerifyCapabilities([]string{"--binary", bin, "testdata/verify_capabilities/release-good"}); err != nil { t.Fatalf("want PASS, got: %v", err) } } func TestVerifyCapabilities_MissingLdflag(t *testing.T) { - bin := buildFixtureBinaryForVerify(t, "missing-ldflag", "") // no ldflag → sentinel reaches wire + // No ldflag → Version stays "dev" → ResolveBuildVersion("dev") → "(devel) [@ sha]" + bin := buildFixtureBinaryForVerify(t, "missing-ldflag", "") err := runPluginVerifyCapabilities([]string{"--binary", bin, "testdata/verify_capabilities/missing-ldflag"}) if err == nil { t.Fatal("want FAIL, got nil") @@ -1029,7 +892,7 @@ func TestVerifyCapabilities_MissingLdflag(t *testing.T) { } func TestVerifyCapabilities_VersionDrift(t *testing.T) { - bin := buildFixtureBinaryForVerify(t, "version-drift", "v0.9.0") // declared 1.2.3, binary 0.9.0 + bin := buildFixtureBinaryForVerify(t, "version-drift", "v0.9.0") err := runPluginVerifyCapabilities([]string{"--binary", bin, "testdata/verify_capabilities/version-drift"}) if err == nil { t.Fatal("want FAIL, got nil") @@ -1051,63 +914,31 @@ func TestVerifyCapabilities_NameDrift(t *testing.T) { } ``` -**Step 2: Simplify buildFixtureBinaryForVerify** (the stitched logic above is muddled — rewrite cleanly): - -Replace the helper with: - -```go -func buildFixtureBinaryForVerify(t *testing.T, scenario, ldflagTag string) string { - t.Helper() - binPath := filepath.Join(t.TempDir(), "p") - args := []string{"build", "-mod=readonly"} - if ldflagTag != "" { - args = append(args, "-ldflags", - fmt.Sprintf("-X github.com/test/%s.Version=%s", scenario, ldflagTag)) - } - args = append(args, "-o", binPath, ".") - cmd := exec.Command("go", args...) - cmd.Dir = filepath.Join("testdata", "verify_capabilities", scenario) - cmd.Env = append(os.Environ(), "GOWORK=off") - out, err := cmd.CombinedOutput() - if err != nil { - t.Fatalf("build %s: %v\n%s", scenario, err, out) - } - return binPath -} -``` - -**Step 3: Run all integration tests** +**Step 2: Run all integration tests** Run: `cd cmd/wfctl && go test -run TestVerifyCapabilities -count=1 -timeout 120s ./...` -Expected: all 5 scenario tests PASS (3 expect verify-success, 2 expect verify-failure). +Expected: 5 scenario tests + 8 unit tests from Tasks 1-3 = 13 PASS. -**Step 4: Commit** +**Step 3: Commit** ```bash git add cmd/wfctl/plugin_verify_capabilities_test.go -git commit -m "test(wfctl): verify-capabilities integration tests (5 scenarios end-to-end) (workflow#765)" +git commit -m "test(wfctl): verify-capabilities integration tests (5 scenarios) (workflow#765)" ``` --- -## Task 9: Documentation update — PLUGIN_RELEASE_GATES.md +## Task 8: Documentation update — PLUGIN_RELEASE_GATES.md **Change class:** Documentation. **Files:** - Modify: `docs/PLUGIN_RELEASE_GATES.md` (append Verify-Capabilities section) -**Step 1: Read current doc** +**Step 1: Append section** ```bash -wc -l docs/PLUGIN_RELEASE_GATES.md # know the size before append -``` - -**Step 2: Append Verify-Capabilities section** - -Append to `docs/PLUGIN_RELEASE_GATES.md`: - -```markdown +cat >> docs/PLUGIN_RELEASE_GATES.md <<'MD' ## Verify-Capabilities (workflow#765 — runtime truth-check) @@ -1162,20 +993,20 @@ wfctl plugin verify-capabilities --binary /tmp/p . ### Non-goals - Does NOT walk per-type RPCs (`GetModuleTypes`/`GetStepTypes`/`GetTriggerTypes`) — IaC bridge returns Unimplemented. -- Does NOT diff `GetContractRegistry` — deferred to workflow#766 (requires `capabilities.iacServices` schema on PluginManifest first). +- Does NOT diff `GetContractRegistry` — deferred to workflow#766 (requires `capabilities.iacServices` schema first). - Does NOT build the binary — operator's responsibility. - Does NOT verify `minEngineVersion` at runtime (not on `pb.Manifest`). See `docs/plans/2026-05-24-verify-capabilities-design.md` for full design. +MD ``` -**Step 3: Verify markdown renders without broken anchors** +**Step 2: Verify no broken anchors** -Run: `markdown-link-check docs/PLUGIN_RELEASE_GATES.md 2>&1 | head` (skip if tool absent — eyeball check works for an append). +Run (best-effort; skip if tool absent): `markdown-link-check docs/PLUGIN_RELEASE_GATES.md 2>&1 | head` +Expected: no broken links (or tool-missing — acceptable; visual review of the append). -Expected: no broken links. - -**Step 4: Commit** +**Step 3: Commit** ```bash git add docs/PLUGIN_RELEASE_GATES.md @@ -1184,22 +1015,23 @@ git commit -m "docs: add Verify-Capabilities section to PLUGIN_RELEASE_GATES (wo --- -## Final verification (post-Task-9) +## Final verification (post-Task-8) Before opening the PR: ```bash -# 1. All tests pass +# 1. All tests pass (unit + integration) cd cmd/wfctl && go test -count=1 -timeout 120s ./... # 2. Lint clean -cd cmd/wfctl && go vet ./... +go vet ./... golangci-lint run ./cmd/wfctl/... # 3. Help text correct go build -o /tmp/wfctl ./cmd/wfctl && /tmp/wfctl plugin verify-capabilities --help +# Expected: help text contains "REQUIRED: --binary", "WARNING: this command EXECUTES", jq example -# 4. Conformance still works (smoke — full conformance suite runs in CI) +# 4. Conformance still works (no regression from inlined spawn — we did NOT touch conformance) go test -run TestConformance -count=1 -timeout 300s ./cmd/wfctl/... # 5. End-to-end smoke against a real plugin (out-of-tree) @@ -1212,7 +1044,9 @@ go test -run TestConformance -count=1 -timeout 300s ./cmd/wfctl/... ## Rollback -This PR adds a CLI subcommand + a shared helper refactor. Rollback path: -- `git revert ` removes the new subcommand and re-inlines the spawn logic in `plugin_conformance.go`. -- No data migration, no schema change, no upstream consumer change (scaffold-template wiring is a separate PR not included here). +This PR adds a CLI subcommand only — no shared-helper refactor, no schema migrations, no upstream consumer changes. Rollback path: +- `git revert ` removes the new subcommand and its tests + fixtures + doc append. +- No data migration, no schema change, no upstream consumer change. - Backwards-compat: subcommand is purely additive; pre-PR wfctl callers continue to work. + +Scaffold-template release.yml wiring is a separate follow-up PR on scaffold-workflow-plugin (not in this PR's scope). From 0d5732ba227d280538b6ef5de9148fc6b663329b Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sun, 24 May 2026 00:46:07 -0400 Subject: [PATCH 11/15] docs(plan): verify-capabilities plan cycle 3 (fix duplicate imports + rebase + tighten test) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cycle 2 FAIL 3 Critical + 2 Important. Cycle 3: - Rebased onto current main (validate-contract + registry-sync now exist in dispatcher). - Restructured Tasks 2/3/4 'append imports' instructions: explicit 'Edit the SINGLE existing import block' with warnings; Task 4 Step 1 specifies final import-block shape end-to-end (eliminates C-2-2 + C-2-3 duplicate-import compile failures). - Fixture go-directive 1.24 → 1.26.0 to match workflow root (eliminates I-2-1). - Name-drift fixture ldflag tag changed v0.1.0 → v0.0.0 so plugin.json '0.0.0' + binary 'v0.0.0' Version matrix PASSes; Name is isolated failure under test. Test assertion tightened from 'mismatch' to 'name:' substring; verify-capabilities error now embeds joined failure list so test can assert without stderr capture (eliminates I-2-2). --- docs/plans/2026-05-24-verify-capabilities.md | 103 ++++++++++++------- 1 file changed, 67 insertions(+), 36 deletions(-) diff --git a/docs/plans/2026-05-24-verify-capabilities.md b/docs/plans/2026-05-24-verify-capabilities.md index faa07995..ed2bd63e 100644 --- a/docs/plans/2026-05-24-verify-capabilities.md +++ b/docs/plans/2026-05-24-verify-capabilities.md @@ -16,7 +16,8 @@ **Revision history:** - Cycle 1: 9-task plan with shared `spawnAndDial` helper extraction. FAILED — 4 Critical (fictional `EngineManifest()` signature; fixture template wrong PluginManifest type; missing-ldflag mechanics misstated; fixture plugin.json shape diverges from PluginManifest). -- Cycle 2 (this version): drop helper extraction (Task 1 → inline ~40 LOC; eliminates I2 — verification-class mismatch on refactor-without-test). Replace `EngineManifest()` with direct `pb.NewPluginServiceClient(conn).GetManifest(ctx, ...)`. Fix fixture template (`sdk.PluginManifest` value, no error). Set fixture initial `Version = "dev"` so `ResolveBuildVersion` falls back to `(devel)` for the missing-ldflag scenario. Minimal plugin.json fixture (drop nonexistent fields). +- Cycle 2: drop helper extraction; direct GetManifest RPC; fix fixture types; fix sentinel mechanics. FAILED — 3 Critical (anchor `case "validate-contract":` didn't exist on stale worktree base; duplicate `import (...)` blocks in test+production files would fail compile; name-drift test assertion too lenient). +- Cycle 3 (this version): rebased worktree onto current main (validate-contract + registry-sync now in dispatcher). Restructured every "append imports" instruction to "Edit the SINGLE existing import block" with explicit warnings. Task 4 Step 1 documents the final import-block shape end-to-end. Fixture go-directive bumped 1.24 → 1.26.0 (matches workflow root). Name-drift fixture ldflag changed to `v0.0.0` so Version matrix PASSes (isolated Name diff); test assertion tightened to `"name:"` substring; verify-capabilities error now embeds joined failure list so tests can assert on field-name without capturing stderr. --- @@ -60,10 +61,14 @@ Create `cmd/wfctl/plugin_verify_capabilities_test.go`: +**Note: every "append" instruction in this plan EDITS the existing file's import block (Go disallows redundant imports across multiple `import` declarations in the same file). Adding new imports = Edit the existing block; never append a second `import (...)` block.** + ```go package main import ( + "os" // added in Task 2 + "path/filepath" // added in Task 2 "strings" "testing" ) @@ -196,16 +201,9 @@ git commit -m "feat(wfctl): plugin verify-capabilities subcommand skeleton (work **Step 1: Write the failing tests** -Append to `cmd/wfctl/plugin_verify_capabilities_test.go`: +Append the test functions below to `cmd/wfctl/plugin_verify_capabilities_test.go`. **DO NOT add a new `import (...)` block** — the file's existing import block (created in Task 1 with `os`, `path/filepath`, `strings`, `testing`) already covers everything these tests need. ```go -import ( - "os" - "path/filepath" - "strings" - "testing" -) - func TestPreflightBinaryEmpty(t *testing.T) { if err := preflightBinary(""); err == nil || !strings.Contains(err.Error(), "binary path") { t.Errorf("want empty-path error, got %v", err) @@ -260,15 +258,9 @@ Expected: FAIL `undefined: preflightBinary`. **Step 3: Implement** -Append to `cmd/wfctl/plugin_verify_capabilities.go`: +In `cmd/wfctl/plugin_verify_capabilities.go`: **Edit the existing single import block** to add `"os"` (alongside existing `"flag"`, `"fmt"`). DO NOT add a second `import (...)` block. Then append the `preflightBinary` function below. ```go -import ( - "flag" - "fmt" - "os" -) - // preflightBinary validates the --binary path before exec: // - non-empty + not literal "null" (guards against jq fallback returning empty) // - file exists and is a regular file (not directory) @@ -396,7 +388,7 @@ Expected: FAIL `undefined: isSentinel`, `undefined: diffVersion`. **Step 3: Implement** -Append to `cmd/wfctl/plugin_verify_capabilities.go` (add `"strings"` to imports): +In `cmd/wfctl/plugin_verify_capabilities.go`: **Edit the existing single import block** to add `"strings"`. Then append `isSentinel` + `diffVersion` below. ```go // isSentinel returns true when v is one of the SDK's dev-sentinel forms @@ -471,7 +463,46 @@ git commit -m "feat(wfctl): verify-capabilities sentinel-pattern Version diff ma This task wires the actual spawn-and-dial INLINE (no shared helper extraction — cycle-2 reviewer Option 3 + I2 elimination). GetManifest is called DIRECTLY via `pb.NewPluginServiceClient(pluginClient.Conn())` to bypass `ExternalPluginAdapter`'s precedence rules. -**Step 1: Load + validate plugin.json** +**Step 1: Edit the existing import block** + +Add these to the SINGLE existing import block at the top of `cmd/wfctl/plugin_verify_capabilities.go` (do NOT add a second `import (...)` declaration): + +- `"context"` +- `"encoding/json"` +- `"os/exec"` +- `"path/filepath"` +- `"time"` +- `external "github.com/GoCodeAlone/workflow/plugin/external"` +- `pb "github.com/GoCodeAlone/workflow/plugin/external/proto"` +- `"github.com/GoCodeAlone/workflow/plugin"` +- `goplugin "github.com/GoCodeAlone/go-plugin"` +- `hclog "github.com/hashicorp/go-hclog"` +- `"google.golang.org/protobuf/types/known/emptypb"` + +Final import block should contain (alphabetical, stdlib then 3rd-party): + +```go +import ( + "context" + "encoding/json" + "flag" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "time" + + goplugin "github.com/GoCodeAlone/go-plugin" + "github.com/GoCodeAlone/workflow/plugin" + external "github.com/GoCodeAlone/workflow/plugin/external" + pb "github.com/GoCodeAlone/workflow/plugin/external/proto" + hclog "github.com/hashicorp/go-hclog" + "google.golang.org/protobuf/types/known/emptypb" +) +``` + +**Step 2: Load + validate plugin.json** In `runPluginVerifyCapabilities`, replace the `return fmt.Errorf("not yet implemented")` line with: @@ -494,9 +525,7 @@ if err := declared.Validate(); err != nil { } ``` -Add imports: `"encoding/json"`, `"path/filepath"`, `"github.com/GoCodeAlone/workflow/plugin"`. - -**Step 2: Spawn + dial INLINE (no shared helper)** +**Step 3: Spawn + dial INLINE (no shared helper)** Continue in `runPluginVerifyCapabilities`: @@ -540,11 +569,9 @@ if !ok { } ``` -Add imports: `"context"`, `"os/exec"`, `"time"`, `external "github.com/GoCodeAlone/workflow/plugin/external"`, `goplugin "github.com/GoCodeAlone/go-plugin"`, `hclog "github.com/hashicorp/go-hclog"`. +Note: `tailBuffer` is defined in `cmd/wfctl/plugin_conformance.go` (same package). All required imports already added in Step 1. -Note: `tailBuffer` is defined in `cmd/wfctl/plugin_conformance.go` (same package). No import needed. - -**Step 3: Call GetManifest DIRECTLY via raw gRPC client** (bypasses adapter precedence) +**Step 4: Call GetManifest DIRECTLY via raw gRPC client** (bypasses adapter precedence) ```go pbClient := pb.NewPluginServiceClient(pluginClient.Conn()) @@ -554,9 +581,7 @@ if err != nil { } ``` -Add imports: `pb "github.com/GoCodeAlone/workflow/plugin/external/proto"`, `"google.golang.org/protobuf/types/known/emptypb"`. - -**Step 4: Diff Name + Version and report** +**Step 5: Diff Name + Version and report** ```go var failures []string @@ -571,13 +596,15 @@ if len(failures) > 0 { for _, f := range failures { fmt.Fprintf(os.Stderr, " - %s\n", f) } - return fmt.Errorf("verify-capabilities: %d mismatch(es)", len(failures)) + // Embed the joined failure list in the returned error so tests can assert + // on specific field names (e.g. "name:" prefix) without capturing stderr. + return fmt.Errorf("verify-capabilities: %d mismatch(es): %s", len(failures), strings.Join(failures, "; ")) } fmt.Printf("OK %s %s (plugin.json: %s)\n", declared.Name, runtime.GetVersion(), declared.Version) return nil ``` -**Step 5: Build + help-output sanity check** +**Step 6: Build + help-output sanity check** Run: ```bash @@ -585,7 +612,7 @@ cd cmd/wfctl && go build -o /tmp/wfctl ./... && /tmp/wfctl plugin verify-capabil ``` Expected: help text printed; exit 0. Help contains "REQUIRED: --binary", "WARNING: this command EXECUTES", `jq` CI example. -**Step 6: Commit** +**Step 7: Commit** ```bash git add cmd/wfctl/plugin_verify_capabilities.go @@ -662,7 +689,7 @@ GO cat > "$d/go.mod" < "$d/go.mod" < PASS via TrimPrefix); + // Name is the ISOLATED failure under test. Without this, both name AND version mismatches fire and a regression + // that breaks Name-diff while leaving Version-diff would silently pass through the lenient "mismatch" substring check. + bin := buildFixtureBinaryForVerify(t, "name-drift", "v0.0.0") err := runPluginVerifyCapabilities([]string{"--binary", bin, "testdata/verify_capabilities/name-drift"}) if err == nil { t.Fatal("want FAIL, got nil") } - if !strings.Contains(err.Error(), "mismatch") { - t.Errorf("want mismatch error, got: %v", err) + // Tighter assertion: error must specifically mention "name:" prefix from the diff report. + if !strings.Contains(err.Error(), "name:") && !strings.Contains(fmt.Sprintf("%v", err), "name:") { + t.Errorf("want name-mismatch error, got: %v", err) } } ``` From b3ad5e54a6ead55d931960a0b448cb6481a37444 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sun, 24 May 2026 00:54:26 -0400 Subject: [PATCH 12/15] docs(plan): verify-capabilities plan cycle 4 (fix Task 7 import-block + generator hazards) Cycle 3 FAIL: 1 Critical (Task 7 reintroduced duplicate-import-block; cycle-3 fix missed Task 7). Cycle 4: - Task 7 Step 1: replace literal 'import (...)' snippet with explicit 'Edit existing single import block' instruction. - Task 7 name-drift test comment: clarified which matrix row fires (declared=='0.0.0' branch returns PASS; isSentinel('v0.0.0') false). - Task 5 generator: embed relative 'replace github.com/GoCodeAlone/workflow => ../../../../..' DIRECTLY in heredoc (no $REPO_ROOT indirection, no sed dance); PLACEHOLDER for module name substituted via fixed-text sed. Eliminates shell-quoting hazard on worktrees with spaces. - Task 6 generator: same direct-replace pattern. --- docs/plans/2026-05-24-verify-capabilities.md | 41 +++++++++----------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/docs/plans/2026-05-24-verify-capabilities.md b/docs/plans/2026-05-24-verify-capabilities.md index ed2bd63e..b0872565 100644 --- a/docs/plans/2026-05-24-verify-capabilities.md +++ b/docs/plans/2026-05-24-verify-capabilities.md @@ -17,7 +17,8 @@ **Revision history:** - Cycle 1: 9-task plan with shared `spawnAndDial` helper extraction. FAILED — 4 Critical (fictional `EngineManifest()` signature; fixture template wrong PluginManifest type; missing-ldflag mechanics misstated; fixture plugin.json shape diverges from PluginManifest). - Cycle 2: drop helper extraction; direct GetManifest RPC; fix fixture types; fix sentinel mechanics. FAILED — 3 Critical (anchor `case "validate-contract":` didn't exist on stale worktree base; duplicate `import (...)` blocks in test+production files would fail compile; name-drift test assertion too lenient). -- Cycle 3 (this version): rebased worktree onto current main (validate-contract + registry-sync now in dispatcher). Restructured every "append imports" instruction to "Edit the SINGLE existing import block" with explicit warnings. Task 4 Step 1 documents the final import-block shape end-to-end. Fixture go-directive bumped 1.24 → 1.26.0 (matches workflow root). Name-drift fixture ldflag changed to `v0.0.0` so Version matrix PASSes (isolated Name diff); test assertion tightened to `"name:"` substring; verify-capabilities error now embeds joined failure list so tests can assert on field-name without capturing stderr. +- Cycle 3: rebased onto main; tightened import-block instructions Tasks 2-4; bumped fixture go directive; isolated name-drift via ldflag. FAILED — 1 Critical (Task 7 reintroduced duplicate-import-block defect — missed by cycle-3 fix that covered only Tasks 2-4) + 2 Important (name-drift comment misstated matrix row; generator shell-quoting hazard via REPO_ROOT in worktrees with spaces). +- Cycle 4 (this version): fix Task 7 import-block instruction; clarify name-drift matrix-row comment; rewrite generators to embed relative `replace` directly (no $REPO_ROOT, no sed dance). Eliminates remaining cycle-3 findings. --- @@ -642,7 +643,6 @@ Save as `/tmp/gen-verify-fixtures.sh`: #!/bin/bash set -euo pipefail BASE=cmd/wfctl/testdata/verify_capabilities -REPO_ROOT=$(git rev-parse --show-toplevel) declare -A NAMES=( [good]=verify-good [release-good]=verify-release-good [missing-ldflag]=verify-missing-ldflag [version-drift]=verify-version-drift ) declare -A VERS=( [good]=0.0.0 [release-good]=1.2.3 [missing-ldflag]=0.0.0 [version-drift]=1.2.3 ) for s in good release-good missing-ldflag version-drift; do @@ -686,26 +686,27 @@ func main() { ) } GO - cat > "$d/go.mod" < "$d/go.mod" <<'MOD' +module github.com/test/PLACEHOLDER go 1.26.0 require github.com/GoCodeAlone/workflow v0.62.0 -replace github.com/GoCodeAlone/workflow => $REPO_ROOT +replace github.com/GoCodeAlone/workflow => ../../../../.. MOD + sed -i.bak "s|PLACEHOLDER|$s|" "$d/go.mod" && rm -f "$d/go.mod.bak" done ``` -**Step 2: Generate + tidy + rewrite to relative replace** +Note: relative `replace` is written DIRECTLY (no `$REPO_ROOT` indirection) so generators run on worktrees with spaces in their path. PLACEHOLDER substitution uses fixed-text sed (no shell-expansion of module name). + +**Step 2: Generate + tidy (writes go.sum in-place)** ```bash bash /tmp/gen-verify-fixtures.sh for d in cmd/wfctl/testdata/verify_capabilities/*/; do (cd "$d" && GOWORK=off go mod tidy) - sed -i.bak "s|replace github.com/GoCodeAlone/workflow => .*|replace github.com/GoCodeAlone/workflow => ../../../../..|" "$d/go.mod" - rm -f "$d/go.mod.bak" done ``` @@ -787,7 +788,6 @@ git commit -m "test(wfctl): verify-capabilities fixtures (4 build-pass scenarios **Step 1: Generate the fixture** ```bash -REPO_ROOT=$(git rev-parse --show-toplevel) d=cmd/wfctl/testdata/verify_capabilities/name-drift mkdir -p "$d" cat > "$d/plugin.json" <<'JSON' @@ -825,18 +825,16 @@ func main() { ) } GO -cat > "$d/go.mod" < "$d/go.mod" <<'MOD' module github.com/test/name-drift go 1.26.0 require github.com/GoCodeAlone/workflow v0.62.0 -replace github.com/GoCodeAlone/workflow => $REPO_ROOT +replace github.com/GoCodeAlone/workflow => ../../../../.. MOD (cd "$d" && GOWORK=off go mod tidy) -sed -i.bak "s|replace github.com/GoCodeAlone/workflow => .*|replace github.com/GoCodeAlone/workflow => ../../../../..|" "$d/go.mod" -rm -f "$d/go.mod.bak" ``` **Step 2: Verify fixture builds** @@ -862,14 +860,9 @@ git commit -m "test(wfctl): name-drift fixture (binary advertises mismatched Nam **Step 1: Add the fixture-build helper + 5 test cases** -Append to `cmd/wfctl/plugin_verify_capabilities_test.go`: +In `cmd/wfctl/plugin_verify_capabilities_test.go`: **Edit the existing SINGLE import block** to add `"os/exec"` (alongside existing `"os"`, `"path/filepath"`, `"strings"`, `"testing"`). DO NOT add a second `import (...)` declaration — golangci-lint will fail the build. Then append the helper + test functions below. ```go -import ( - "os/exec" - // keep existing imports -) - // buildFixtureBinaryForVerify builds the fixture scenario in-place and emits // the binary to t.TempDir(). ldflag is the -X ...Version= value ("" = no flag, // which makes ResolveBuildVersion fall back to "(devel) [@ sha]" for fixtures @@ -930,9 +923,13 @@ func TestVerifyCapabilities_VersionDrift(t *testing.T) { } func TestVerifyCapabilities_NameDrift(t *testing.T) { - // Use ldflag tag matching plugin.json sentinel so Version PASSes (matrix row "0.0.0 + v0.0.0" -> PASS via TrimPrefix); - // Name is the ISOLATED failure under test. Without this, both name AND version mismatches fire and a regression - // that breaks Name-diff while leaving Version-diff would silently pass through the lenient "mismatch" substring check. + // Build with non-sentinel ldflag tag so Version PASSes — matrix row that + // fires: plugin.json="0.0.0" + binary="v0.0.0" → PASS via the + // `declared == "0.0.0"` branch returning early (isSentinel("v0.0.0")==false + // because the SDK sentinel set is {"", "dev", "0.0.0", "(devel)..."} — NOT + // "v0.0.0"). This ISOLATES Name as the sole failure under test, so a + // regression that breaks Name-diff while leaving Version-diff intact + // doesn't silently pass through a lenient `Contains("mismatch")` check. bin := buildFixtureBinaryForVerify(t, "name-drift", "v0.0.0") err := runPluginVerifyCapabilities([]string{"--binary", bin, "testdata/verify_capabilities/name-drift"}) if err == nil { From 78cff61b3c3f81851d8b3c9b372c7af3cf2bfb3a Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sun, 24 May 2026 00:58:38 -0400 Subject: [PATCH 13/15] docs(plan): verify-capabilities plan cycle 5 (fix Task 1 unused + Task 7 missing fmt) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cycle 4 FAIL: 2 Critical (compile blockers). Cycle 5: - Task 1: ship test file with only strings + testing (was pre-staging os + path/filepath which Task 1 functions don't use → 'imported and not used'). - Task 2: explicitly says 'Edit the existing SINGLE import block to add os + path/filepath' when those imports are first used. - Task 7: now adds 'fmt' AND 'os/exec' (was only os/exec; helper uses fmt.Sprintf and test uses fmt.Sprintf('%v', err) → undefined: fmt). Each task's import additions match its actual code usage. --- docs/plans/2026-05-24-verify-capabilities.md | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/docs/plans/2026-05-24-verify-capabilities.md b/docs/plans/2026-05-24-verify-capabilities.md index b0872565..b05db7e2 100644 --- a/docs/plans/2026-05-24-verify-capabilities.md +++ b/docs/plans/2026-05-24-verify-capabilities.md @@ -18,7 +18,8 @@ - Cycle 1: 9-task plan with shared `spawnAndDial` helper extraction. FAILED — 4 Critical (fictional `EngineManifest()` signature; fixture template wrong PluginManifest type; missing-ldflag mechanics misstated; fixture plugin.json shape diverges from PluginManifest). - Cycle 2: drop helper extraction; direct GetManifest RPC; fix fixture types; fix sentinel mechanics. FAILED — 3 Critical (anchor `case "validate-contract":` didn't exist on stale worktree base; duplicate `import (...)` blocks in test+production files would fail compile; name-drift test assertion too lenient). - Cycle 3: rebased onto main; tightened import-block instructions Tasks 2-4; bumped fixture go directive; isolated name-drift via ldflag. FAILED — 1 Critical (Task 7 reintroduced duplicate-import-block defect — missed by cycle-3 fix that covered only Tasks 2-4) + 2 Important (name-drift comment misstated matrix row; generator shell-quoting hazard via REPO_ROOT in worktrees with spaces). -- Cycle 4 (this version): fix Task 7 import-block instruction; clarify name-drift matrix-row comment; rewrite generators to embed relative `replace` directly (no $REPO_ROOT, no sed dance). Eliminates remaining cycle-3 findings. +- Cycle 4: fix Task 7 import-block instruction; clarify name-drift matrix-row comment; rewrite generators. FAILED — 2 Critical (Task 1 pre-staged unused `os`/`path/filepath` imports → "imported and not used" compile error; Task 7 helper uses `fmt.Sprintf` but `"fmt"` not in import list → `undefined: fmt`). +- Cycle 5 (this version): Task 1 ships test file with only `strings`/`testing` imports; Task 2 explicitly adds `os` + `path/filepath` when first used; Task 7 explicitly adds `fmt` + `os/exec` (was only `os/exec`). Each task's imports match its actual usage. --- @@ -68,8 +69,6 @@ Create `cmd/wfctl/plugin_verify_capabilities_test.go`: package main import ( - "os" // added in Task 2 - "path/filepath" // added in Task 2 "strings" "testing" ) @@ -202,7 +201,7 @@ git commit -m "feat(wfctl): plugin verify-capabilities subcommand skeleton (work **Step 1: Write the failing tests** -Append the test functions below to `cmd/wfctl/plugin_verify_capabilities_test.go`. **DO NOT add a new `import (...)` block** — the file's existing import block (created in Task 1 with `os`, `path/filepath`, `strings`, `testing`) already covers everything these tests need. +In `cmd/wfctl/plugin_verify_capabilities_test.go`: **Edit the existing SINGLE import block** to add `"os"` and `"path/filepath"` (alongside existing `"strings"`, `"testing"`). DO NOT add a second `import (...)` declaration. Then append the test functions below. ```go func TestPreflightBinaryEmpty(t *testing.T) { @@ -860,7 +859,7 @@ git commit -m "test(wfctl): name-drift fixture (binary advertises mismatched Nam **Step 1: Add the fixture-build helper + 5 test cases** -In `cmd/wfctl/plugin_verify_capabilities_test.go`: **Edit the existing SINGLE import block** to add `"os/exec"` (alongside existing `"os"`, `"path/filepath"`, `"strings"`, `"testing"`). DO NOT add a second `import (...)` declaration — golangci-lint will fail the build. Then append the helper + test functions below. +In `cmd/wfctl/plugin_verify_capabilities_test.go`: **Edit the existing SINGLE import block** to add `"fmt"` and `"os/exec"` (alongside existing `"os"`, `"path/filepath"`, `"strings"`, `"testing"`). DO NOT add a second `import (...)` declaration — golangci-lint will fail the build. Then append the helper + test functions below. ```go // buildFixtureBinaryForVerify builds the fixture scenario in-place and emits From 09ecbb56ebd83e0b1734fe42a311edcb0e9e50f4 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sun, 24 May 2026 01:00:45 -0400 Subject: [PATCH 14/15] =?UTF-8?q?docs(plan):=20promote=20##=20Task=20?= =?UTF-8?q?=E2=86=92=20###=20Task=20headings=20(alignment-check=20scope=20?= =?UTF-8?q?manifest)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/plans/2026-05-24-verify-capabilities.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/docs/plans/2026-05-24-verify-capabilities.md b/docs/plans/2026-05-24-verify-capabilities.md index b05db7e2..90c8ab58 100644 --- a/docs/plans/2026-05-24-verify-capabilities.md +++ b/docs/plans/2026-05-24-verify-capabilities.md @@ -50,7 +50,7 @@ --- -## Task 1: Subcommand registration + flag parsing skeleton +### Task 1: Subcommand registration + flag parsing skeleton **Change class:** CLI command. @@ -191,7 +191,7 @@ git commit -m "feat(wfctl): plugin verify-capabilities subcommand skeleton (work --- -## Task 2: Preflight binary path validation +### Task 2: Preflight binary path validation **Change class:** Internal logic refactor (input validation). @@ -308,7 +308,7 @@ git commit -m "feat(wfctl): verify-capabilities preflight binary-path validation --- -## Task 3: Sentinel-pattern Version diff matrix +### Task 3: Sentinel-pattern Version diff matrix **Change class:** Internal logic refactor (pure-logic diff function). @@ -454,7 +454,7 @@ git commit -m "feat(wfctl): verify-capabilities sentinel-pattern Version diff ma --- -## Task 4: Inline spawn-and-dial + direct GetManifest RPC +### Task 4: Inline spawn-and-dial + direct GetManifest RPC **Change class:** Plugin / extension (CLI subcommand that calls a plugin via raw gRPC). @@ -621,7 +621,7 @@ git commit -m "feat(wfctl): wire inline spawn + direct GetManifest + Name/Versio --- -## Task 5: Create 4 build-PASS fixture scenarios +### Task 5: Create 4 build-PASS fixture scenarios **Change class:** Test fixture (no runtime impact). @@ -777,7 +777,7 @@ git commit -m "test(wfctl): verify-capabilities fixtures (4 build-pass scenarios --- -## Task 6: Create name-drift fixture (binary advertises different Name) +### Task 6: Create name-drift fixture (binary advertises different Name) **Change class:** Test fixture. @@ -850,7 +850,7 @@ git commit -m "test(wfctl): name-drift fixture (binary advertises mismatched Nam --- -## Task 7: Integration tests — 5 scenarios end-to-end +### Task 7: Integration tests — 5 scenarios end-to-end **Change class:** Plugin / extension (exercises spawn + RPC + diff against real fixture binaries). @@ -955,7 +955,7 @@ git commit -m "test(wfctl): verify-capabilities integration tests (5 scenarios) --- -## Task 8: Documentation update — PLUGIN_RELEASE_GATES.md +### Task 8: Documentation update — PLUGIN_RELEASE_GATES.md **Change class:** Documentation. From ff537bfc6a6fad0a45cddb534ebfd5433caed8de Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sun, 24 May 2026 01:01:57 -0400 Subject: [PATCH 15/15] chore: lock scope for verify-capabilities (alignment passed) --- docs/plans/2026-05-24-verify-capabilities.md | 2 +- docs/plans/2026-05-24-verify-capabilities.md.scope-lock | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 docs/plans/2026-05-24-verify-capabilities.md.scope-lock diff --git a/docs/plans/2026-05-24-verify-capabilities.md b/docs/plans/2026-05-24-verify-capabilities.md index 90c8ab58..f8cd632d 100644 --- a/docs/plans/2026-05-24-verify-capabilities.md +++ b/docs/plans/2026-05-24-verify-capabilities.md @@ -46,7 +46,7 @@ |------|-------|-------|--------| | 1 | feat(wfctl): plugin verify-capabilities subcommand (workflow#765) | Task 1, Task 2, Task 3, Task 4, Task 5, Task 6, Task 7, Task 8 | feat/765-verify-capabilities | -**Status:** Draft +**Status:** Locked 2026-05-24T05:01:50Z --- diff --git a/docs/plans/2026-05-24-verify-capabilities.md.scope-lock b/docs/plans/2026-05-24-verify-capabilities.md.scope-lock new file mode 100644 index 00000000..5bc518dc --- /dev/null +++ b/docs/plans/2026-05-24-verify-capabilities.md.scope-lock @@ -0,0 +1 @@ +aa85b5960321a75b42c4c0bed5343c432aabcde0c2dedac42bdf17a57dd5d5bc