Migrate workflow-plugin-cicd to strict gRPC proto contracts#2
Conversation
…contracts.json, and tests Agent-Logs-Url: https://github.com/GoCodeAlone/workflow-plugin-cicd/sessions/c48a5e3f-ead1-40f0-8d02-aab4c138c991 Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Migrates workflow-plugin-cicd to “strict” contract declarations so host-side tooling (e.g., wfctl audits and schema-driven validation) can reason about module/step configuration and outputs instead of relying on untyped definitions.
Changes:
- Adds
stepSchemasentries toplugin.jsoncovering all advertised step types. - Introduces
plugin.contracts.jsonas a strict contract audit artifact covering all module + step types. - Implements
sdk.SchemaProvider(ModuleSchemas) foraws.codebuildand adds tests enforcing contract completeness/consistency.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| plugin.json | Adds stepSchemas for all declared step types with typed config fields + outputs. |
| plugin.contracts.json | Adds strict audit contract descriptors for the module and all steps. |
| internal/plugin.go | Adds ModuleSchemas() (SchemaProvider) for aws.codebuild and updates package documentation. |
| internal/plugin_test.go | Adds tests to enforce schema/contract coverage and consistency between JSON artifacts and code. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if output != "hello" { | ||
| t.Errorf("output = %q, want %q", output, "hello") | ||
| } | ||
| exitCode, _ := result.Output["exit_code"].(int) |
There was a problem hiding this comment.
Fixed in d187e06. The shell_exec tests now assert the output and exit_code type assertions before checking values, so wrong output types fail the test instead of using zero values.
| "description": "Checks out a Git branch, tag, or commit reference.", | ||
| "configFields": [ | ||
| {"key": "ref", "label": "Ref", "type": "string", "description": "Git ref (branch, tag, or commit SHA) to check out", "required": true}, | ||
| {"key": "branch", "label": "Branch", "type": "string", "description": "Alias for ref; used when ref is absent"}, |
There was a problem hiding this comment.
Fixed in d187e06. The manifest no longer marks ref as required for step.git_checkout and now documents that either ref or branch is required, matching runtime behavior.
| { | ||
| "type": "step.git_checkout", | ||
| "configFields": [ | ||
| {"key": "ref", "type": "string", "required": true}, |
There was a problem hiding this comment.
Fixed in d187e06. The contract descriptor for step.git_checkout now treats ref as optional so branch-only configs are not rejected by strict contract validation.
|
Validation after d187e06:
|
|
@copilot Add this as part of the PR scope: include or update a GitHub Actions PR check that validates strict Workflow plugin contracts. The check should fail when name: wfctl strict contracts
on:
pull_request:
push:
branches: [main, master]
jobs:
wfctl-strict-contracts:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: actions/setup-go@v5
with:
go-version-file: go.mod
- name: Validate strict plugin contracts
run: go run github.com/GoCodeAlone/workflow/cmd/wfctl@v0.20.1 plugin validate --file plugin.json --strict-contractsAdjust branch filters and Go setup to match this repository. If the repo uses an existing CI workflow, add this as a job there instead of creating duplicate workflow clutter. Include the check result in the PR validation notes. |
|
@copilot please try again using Claude Sonnet 4.6. Continue the original task for this PR. Make sure the PR is not left empty, add or update strict gRPC contract descriptors as applicable, add a PR check that runs |
…ct validation Agent-Logs-Url: https://github.com/GoCodeAlone/workflow-plugin-cicd/sessions/65798021-41c6-4ec7-b00d-b73825037aae Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
Continuing from where the PR left off. The strict contract descriptors from the previous commits are intact. Added
Both jobs use |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "outputs": [ | ||
| { | ||
| "key": "status", | ||
| "type": "string" | ||
| }, | ||
| { | ||
| "key": "image", | ||
| "type": "string" | ||
| }, | ||
| { | ||
| "key": "message", | ||
| "type": "string" | ||
| } | ||
| ] |
| var contracts struct { | ||
| Version string `json:"version"` | ||
| Plugin string `json:"plugin"` | ||
| Strict bool `json:"strict"` | ||
| Modules []struct { | ||
| Type string `json:"type"` | ||
| } `json:"modules"` | ||
| Steps []struct { | ||
| Type string `json:"type"` | ||
| } `json:"steps"` | ||
| } |
| result, err := inst.Execute(context.Background(), nil, nil, nil, nil, map[string]any{ | ||
| "command": "echo hello", | ||
| }) |
| sort.Strings(manifest.Capabilities.StepTypes) | ||
| sort.Strings(codeTypes) | ||
|
|
| { | ||
| "key": "message", | ||
| "type": "string", | ||
| "description": "Human-readable result message" |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var contracts struct { | ||
| Version string `json:"version"` | ||
| Plugin string `json:"plugin"` | ||
| Strict bool `json:"strict"` | ||
| Modules []struct { | ||
| Type string `json:"type"` | ||
| } `json:"modules"` | ||
| Steps []struct { | ||
| Type string `json:"type"` | ||
| } `json:"steps"` | ||
| } | ||
| if err := json.Unmarshal(data, &contracts); err != nil { | ||
| t.Fatalf("parse plugin.contracts.json: %v", err) | ||
| } | ||
|
|
||
| if contracts.Version == "" { | ||
| t.Error("contracts.version is empty") | ||
| } | ||
| if !contracts.Strict { | ||
| t.Error("contracts.strict must be true") | ||
| } | ||
|
|
||
| // Verify modules coverage. | ||
| p := internal.NewCICDPlugin() | ||
| mp := p.(sdk.ModuleProvider) | ||
| moduleSet := make(map[string]bool, len(contracts.Modules)) | ||
| for _, m := range contracts.Modules { | ||
| moduleSet[m.Type] = true | ||
| } | ||
| for _, modType := range mp.ModuleTypes() { | ||
| if !moduleSet[modType] { | ||
| t.Errorf("plugin.contracts.json missing module descriptor for %q", modType) | ||
| } | ||
| } | ||
|
|
||
| // Verify steps coverage. | ||
| sp := p.(sdk.StepProvider) | ||
| stepSet := make(map[string]bool, len(contracts.Steps)) | ||
| for _, s := range contracts.Steps { | ||
| stepSet[s.Type] = true | ||
| } | ||
| for _, stepType := range sp.StepTypes() { | ||
| if !stepSet[stepType] { | ||
| t.Errorf("plugin.contracts.json missing step descriptor for %q", stepType) | ||
| } | ||
| } | ||
| } |
| sort.Strings(manifest.Capabilities.StepTypes) | ||
| sort.Strings(codeTypes) | ||
|
|
|
@copilot please continue this PR using Claude Sonnet 4.6 and address the remaining merge gates. Current gate state I see:
Please inspect and fix any failing checks, address all unresolved review comments with code changes where appropriate, add or repair the PR check that runs |
… contracts validation, test cleanup Agent-Logs-Url: https://github.com/GoCodeAlone/workflow-plugin-cicd/sessions/7396e2ba-5940-43be-ac0b-b02213cffee3 Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
Addressed all 8 unresolved review threads in 18eeb31:
All tests pass; CI workflow with |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 10 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| // Every module type returned by ModuleTypes must have a matching schema. | ||
| mp := p.(sdk.ModuleProvider) |
| // returns combined output and exit code. | ||
| func TestShellExecStep(t *testing.T) { | ||
| p := internal.NewCICDPlugin() | ||
| sp := p.(sdk.StepProvider) |
| // returns combined output and exit code. | ||
| func TestShellExecStep(t *testing.T) { | ||
| p := internal.NewCICDPlugin() | ||
| sp := p.(sdk.StepProvider) |
| exitCode, ok := result.Output["exit_code"].(int) | ||
| if !ok { | ||
| t.Fatalf("exit_code type = %T, want int", result.Output["exit_code"]) | ||
| } |
| // TestDeploySteps verifies that all deploy step types execute without error and | ||
| // return the expected output keys. |
| if _, ok := result.Output["status"]; !ok { | ||
| t.Error("output missing 'status' key") | ||
| } |
| "description": "Git ref (branch, tag, or commit SHA) to check out; either ref or branch is required" | ||
| }, | ||
| { | ||
| "key": "branch", | ||
| "label": "Branch", | ||
| "type": "string", | ||
| "description": "Alias for ref; used when ref is absent" |
| // Each advertised module and step declares a typed schema contract that is | ||
| // exposed via the SchemaProvider interface (gRPC) and as stepSchemas entries | ||
| // in plugin.json, so that host-side tooling can validate payloads at | ||
| // code-generation, compilation, or startup time rather than at runtime. |
| - name: Configure Go private modules | ||
| run: git config --global url."https://${{ github.actor }}:${{ secrets.GITHUB_TOKEN }}@github.com/".insteadOf "https://github.com/" |
| - name: Configure Go private modules | ||
| run: git config --global url."https://${{ github.actor }}:${{ secrets.GITHUB_TOKEN }}@github.com/".insteadOf "https://github.com/" |
|
@copilot apply changes based on the comments in this thread |
…ypes, deploy outputs, CI scope Agent-Logs-Url: https://github.com/GoCodeAlone/workflow-plugin-cicd/sessions/48561ecb-b02b-4383-8ef0-f3b879b4e8f9 Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
Applied all changes from that review thread in 7cfed68:
|
The plugin advertised 31 step types and 1 module type with no typed schema declarations, causing
wfctlstrict contract audit to report 0/1 strict modules and 0/31 strict steps.Changes
plugin.json—stepSchemasfor all 31 step typesAdded a
stepSchemasarray covering every advertised step. Each entry declares typedconfigFields(key, type, required, default) andoutputs(key, type, description). Output declarations forstep.container_buildand all deployment step types include the full set of runtime output keys (status,service,image,strategy,message) to match whatdeployStep.Execute()actually emits.internal/plugin.go—SchemaProviderforaws.codebuildImplements
sdk.SchemaProviderso the host engine receives a typedModuleSchemaDatadescriptor foraws.codebuildvia gRPC at startup, rather than having no descriptor at all:plugin.contracts.json(new)Standalone audit artifact with
"strict": trueand explicit contract descriptors for all 32 types (1 module + 31 steps), consumable bywfctl audit plugins --strict-contractstooling. Output declarations are accurate for all step types includingstep.container_build.internal/plugin_test.go(new)Tests enforcing contract completeness as code:
TestSchemaProvider— every module type has a schemaTestPluginJSONStepSchemas—stepSchemascovers every declared step typeTestPluginContractsJSON—plugin.contracts.jsoncovers all types withstrict: trueand validates thecontractssection, asserting every module/step type has amode: "strict"entryTestStepTypesConsistency—plugin.jsoncapabilities and plugin binary agree exactlyTestShellExecStepand related — assert type assertions on output values succeed before comparing;Execute()calls usenilfor the unused config parameter (config is set viaCreateStep).github/workflows/ci.yml(new)GitHub Actions CI workflow that runs on every PR and push to
main/masterwith two jobs:test— runsgo test ./...to validate the full test suitewfctl-strict-contracts— explicitly fails ifplugin.jsonis missing, then runsgo run github.com/GoCodeAlone/workflow/cmd/wfctl@v0.20.1 plugin validate --file plugin.json --strict-contractsto enforce strict contract compliance in CIBoth jobs use
go-version-file: go.mod,GOWORK: "off", and private module URL rewrite consistent with the existingrelease.yml. A top-levelpermissions: contents: readblock limits GITHUB_TOKEN scope.