Address remaining review comments: fix type/description issues, re-add provider, fix CI informational step, add field-level contract tests

Agent-Logs-Url: https://github.com/GoCodeAlone/workflow-plugin-github/sessions/c9cbf541-38c2-4769-b1b9-80f1ca0007ac

Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>

Author: Copilot

.github/workflows/ci.yml

@@ -41,7 +41,10 @@ jobs:
         run: |
           # wfctl validates registry-format manifests; strict contract schema coverage is enforced
           # by the Go tests above. This step runs informational validation and logs the result.
-          go run github.com/GoCodeAlone/workflow/cmd/wfctl@v0.3.56 plugin validate --file plugin.json 2>&1; wfctl_exit=$?
+          set +e
+          go run github.com/GoCodeAlone/workflow/cmd/wfctl@v0.3.56 plugin validate --file plugin.json 2>&1
+          wfctl_exit=$?
+          set -e
           echo "wfctl validation exit code: ${wfctl_exit}"
         env:
           GOPRIVATE: github.com/GoCodeAlone/*

internal/schemas.go

@@ -16,6 +16,13 @@ func (p *githubPlugin) ModuleSchemas() []sdk.ModuleSchemaData {
 			Category:    "github",
 			Description: "Receives GitHub webhook events via HTTP, verifies HMAC-SHA256 signatures, and publishes normalised GitEvent messages to a configurable topic.",
 			ConfigFields: []sdk.ConfigField{
+				{
+					Name:         "provider",
+					Type:         "string",
+					Description:  "Webhook provider identifier. Accepted for backward compatibility; the module always publishes events with provider 'github'.",
+					DefaultValue: "github",
+					Required:     false,
+				},
 				{
 					Name:        "secret",
 					Type:        "string",

internal/schemas_test.go

@@ -65,7 +65,7 @@ func TestModuleSchemas(t *testing.T) {
 	for _, f := range webhook.ConfigFields {
 		webhookFieldNames[f.Name] = true
 	}
-	for _, want := range []string{"secret", "events", "topic"} {
+	for _, want := range []string{"provider", "secret", "events", "topic"} {
 		if !webhookFieldNames[want] {
 			t.Errorf("git.webhook schema: required config field %q is missing", want)
 		}
@@ -206,6 +206,122 @@ func TestPluginStepSchemasJSON(t *testing.T) {
 	}
 }
 
+// TestStepSchemaFieldContracts verifies critical field-level properties across
+// step schemas in plugin.json: token required/sensitive, template-capable fields
+// typed as string, and key required flags.
+func TestStepSchemaFieldContracts(t *testing.T) {
+	data, err := os.ReadFile("../plugin.json")
+	if err != nil {
+		t.Fatalf("plugin.json not found: %v", err)
+	}
+
+	type configField struct {
+		Key         string `json:"key"`
+		Type        string `json:"type"`
+		Required    bool   `json:"required"`
+		Sensitive   bool   `json:"sensitive"`
+		DefaultValue any   `json:"defaultValue"`
+	}
+	type stepSchema struct {
+		Type         string        `json:"type"`
+		ConfigFields []configField `json:"configFields"`
+	}
+	var manifest struct {
+		StepSchemas []stepSchema `json:"stepSchemas"`
+	}
+	if err := json.Unmarshal(data, &manifest); err != nil {
+		t.Fatalf("parse plugin.json: %v", err)
+	}
+
+	// Index schemas by type for easy lookup.
+	byType := make(map[string]stepSchema, len(manifest.StepSchemas))
+	for _, s := range manifest.StepSchemas {
+		byType[s.Type] = s
+	}
+	fieldsByKey := func(s stepSchema) map[string]configField {
+		m := make(map[string]configField, len(s.ConfigFields))
+		for _, f := range s.ConfigFields {
+			m[f.Key] = f
+		}
+		return m
+	}
+
+	// Every step schema with a token field must mark it required and sensitive.
+	for _, s := range manifest.StepSchemas {
+		fields := fieldsByKey(s)
+		if tok, ok := fields["token"]; ok {
+			if !tok.Required {
+				t.Errorf("%s: token field must be required=true (step fails at runtime without it)", s.Type)
+			}
+			if !tok.Sensitive {
+				t.Errorf("%s: token field must be sensitive=true", s.Type)
+			}
+		}
+	}
+
+	// Fields that accept both numeric literals and template expressions must be
+	// declared as string so schema-aware validators accept template syntax.
+	templateCapableFields := map[string]string{
+		"step.gh_action_status":  "run_id",
+		"step.gh_release_upload": "release_id",
+	}
+	for stepType, fieldKey := range templateCapableFields {
+		s, ok := byType[stepType]
+		if !ok {
+			t.Errorf("%s: schema not found", stepType)
+			continue
+		}
+		fields := fieldsByKey(s)
+		f, ok := fields[fieldKey]
+		if !ok {
+			t.Errorf("%s: field %q not found in schema", stepType, fieldKey)
+			continue
+		}
+		if f.Type != "string" {
+			t.Errorf("%s: field %q type should be %q (supports template expressions), got %q",
+				stepType, fieldKey, "string", f.Type)
+		}
+		if !f.Required {
+			t.Errorf("%s: field %q should be required=true", stepType, fieldKey)
+		}
+	}
+
+	// Dynamic string fields that also accept template expressions must not be
+	// declared as select (which would prevent template expressions).
+	noSelectFields := map[string]string{
+		"step.gh_pr_merge":  "method",
+		"step.gh_pr_review": "event",
+	}
+	for stepType, fieldKey := range noSelectFields {
+		s, ok := byType[stepType]
+		if !ok {
+			t.Errorf("%s: schema not found", stepType)
+			continue
+		}
+		fields := fieldsByKey(s)
+		f, ok := fields[fieldKey]
+		if !ok {
+			t.Errorf("%s: field %q not found in schema", stepType, fieldKey)
+			continue
+		}
+		if f.Type == "select" {
+			t.Errorf("%s: field %q should not be type 'select' — resolveField supports templates so the type must be 'string'", stepType, fieldKey)
+		}
+	}
+
+	// gh_secret_set: repo must be required (no org-secret path exists).
+	if s, ok := byType["step.gh_secret_set"]; ok {
+		fields := fieldsByKey(s)
+		if repo, ok := fields["repo"]; ok {
+			if !repo.Required {
+				t.Errorf("step.gh_secret_set: repo field must be required=true (only repo secrets are supported)")
+			}
+		} else {
+			t.Errorf("step.gh_secret_set: repo field missing from schema")
+		}
+	}
+}
+
 // TestPluginManifestEngineValidation verifies that plugin.json is parseable as a
 // workflow engine PluginManifest and that Validate() passes required-field checks.
 func TestPluginManifestEngineValidation(t *testing.T) {

plugin.json

@@ -58,7 +58,7 @@
             "configFields": [
                 {"key": "owner", "type": "string", "description": "GitHub repository owner", "required": true},
                 {"key": "repo", "type": "string", "description": "GitHub repository name", "required": true},
-                {"key": "run_id", "type": "number", "description": "Workflow run ID (integer or template expression e.g. {{.steps.trigger.run_id}})", "required": true},
+                {"key": "run_id", "type": "string", "description": "Workflow run ID (numeric literal or template expression e.g. {{.steps.trigger_step.run_id}})", "required": true},
                 {"key": "token", "type": "string", "description": "GitHub personal access token", "required": true, "sensitive": true},
                 {"key": "wait", "type": "boolean", "description": "Poll until the run reaches a terminal state", "defaultValue": false},
                 {"key": "poll_interval", "type": "duration", "description": "Interval between status polls when wait=true", "defaultValue": "10s"},
@@ -74,7 +74,7 @@
         {
             "type": "step.gh_create_check",
             "plugin": "workflow-plugin-github",
-            "description": "Creates or updates a GitHub Check Run (status check) on a specific commit.",
+            "description": "Creates a GitHub Check Run (status check) on a specific commit.",
             "configFields": [
                 {"key": "owner", "type": "string", "description": "GitHub repository owner", "required": true},
                 {"key": "repo", "type": "string", "description": "GitHub repository name", "required": true},
@@ -122,7 +122,7 @@
                 {"key": "repo", "type": "string", "description": "GitHub repository name", "required": true},
                 {"key": "pr_number", "type": "number", "description": "Pull request number to merge", "required": true},
                 {"key": "commit_title", "type": "string", "description": "Merge commit title"},
-                {"key": "method", "type": "select", "description": "Merge method", "options": ["merge", "squash", "rebase"], "defaultValue": "merge"},
+                {"key": "method", "type": "string", "description": "Merge method: merge, squash, or rebase (also accepts template expressions)", "defaultValue": "merge"},
                 {"key": "token", "type": "string", "description": "GitHub personal access token", "required": true, "sensitive": true}
             ],
             "outputs": [
@@ -155,7 +155,7 @@
                 {"key": "owner", "type": "string", "description": "GitHub repository owner", "required": true},
                 {"key": "repo", "type": "string", "description": "GitHub repository name", "required": true},
                 {"key": "pr_number", "type": "number", "description": "Pull request number", "required": true},
-                {"key": "event", "type": "select", "description": "Review event type", "options": ["APPROVE", "REQUEST_CHANGES", "COMMENT"], "defaultValue": "COMMENT"},
+                {"key": "event", "type": "string", "description": "Review event type: APPROVE, REQUEST_CHANGES, or COMMENT (also accepts template expressions)", "defaultValue": "COMMENT"},
                 {"key": "body", "type": "string", "description": "Review body text"},
                 {"key": "token", "type": "string", "description": "GitHub personal access token", "required": true, "sensitive": true}
             ],
@@ -249,7 +249,7 @@
             "configFields": [
                 {"key": "owner", "type": "string", "description": "GitHub repository owner", "required": true},
                 {"key": "repo", "type": "string", "description": "GitHub repository name", "required": true},
-                {"key": "release_id", "type": "number", "description": "Release ID (integer or template expression e.g. {{.steps.create_release.release_id}})", "required": true},
+                {"key": "release_id", "type": "string", "description": "Release ID (numeric literal or template expression e.g. {{.steps.create_release.release_id}})", "required": true},
                 {"key": "file", "type": "filepath", "description": "Local path to the file to upload", "required": true},
                 {"key": "name", "type": "string", "description": "Display name for the release asset"},
                 {"key": "token", "type": "string", "description": "GitHub personal access token", "required": true, "sensitive": true}