feat: plugin ecosystem + step.workflow_call#331
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds support for declaring external plugins in workflow config, auto-fetching them via wfctl, and verifying installed plugin binaries against pinned SHA-256 checksums in .wfctl.yaml before loading. This extends the external plugin lifecycle (install → pin → verify → load) and tightens runtime behavior around plugin supply-chain integrity.
Changes:
- Introduces
.wfctl.yaml-based external plugin binary integrity verification and wires it into external plugin loading. - Adds config support for declared external plugins (with optional auto-fetch) and integrates auto-fetch into server startup.
- Improves plugin scaffolding/tests and adds an HTTP timeout for static registry fetches; enhances
workflow_callstep behavior.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| plugin/sdk/generator_test.go | Adds a project-structure test for plugin scaffolding output. |
| plugin/integrity.go | New integrity verifier for plugin binaries using .wfctl.yaml pinned SHA-256. |
| plugin/integrity_test.go | Unit tests for lockfile discovery and checksum verification behaviors. |
| plugin/external/manager.go | Runs integrity verification before launching external plugin binaries. |
| plugin/autofetch.go | Adds wfctl-shell-out auto-fetch for declared external plugins. |
| plugin/autofetch_test.go | Unit tests for auto-fetch helpers and version constraint handling. |
| module/pipeline_step_workflow_call.go | Adds stop_pipeline support and resolves workflow name via templates. |
| engine.go | Minor formatting-only adjustments. |
| docs/PLUGIN_AUTHORING.md | Updates authoring guidance around plugin naming in plugin.json. |
| config/config.go | Adds plugins.external config shape and merges it during imports. |
| config/config_test.go | Tests parsing for plugins.external and absent plugins section. |
| cmd/wfctl/registry_source.go | Uses a timeout-configured HTTP client for static registry fetches. |
| cmd/wfctl/plugin_init_test.go | Adds comprehensive unit tests for wfctl plugin init scaffold output. |
| cmd/server/main.go | Auto-fetches declared external plugins before discovery/loading. |
Comment on lines
+12
to
+23
| // AutoFetchPlugin downloads a plugin from the registry if it's not already installed. | ||
| // It shells out to wfctl for the actual download/install logic. | ||
| // version is an optional semver constraint (e.g., ">=0.1.0" or "0.2.0"). | ||
| func AutoFetchPlugin(pluginName, version, pluginDir string) error { | ||
| // Check both pluginName and workflow-plugin-<pluginName> (or the short form | ||
| // if pluginName already has the "workflow-plugin-" prefix). | ||
| if isPluginInstalled(pluginName, pluginDir) { | ||
| return nil | ||
| } | ||
|
|
||
| fmt.Fprintf(os.Stderr, "[auto-fetch] Plugin %q not found locally, fetching from registry...\n", pluginName) | ||
|
|
Comment on lines
+51
to
+55
| binaryPath := filepath.Join(pluginDir, pluginName, pluginName) | ||
| binaryData, err := os.ReadFile(binaryPath) | ||
| if err != nil { | ||
| return fmt.Errorf("read plugin binary %s: %w", binaryPath, err) | ||
| } |
| p := filepath.Join(dir, ".wfctl.yaml") | ||
| if err := os.WriteFile(p, []byte("plugins:\n my-plugin:\n sha256: abc\n"), 0000); err != nil { | ||
| t.Fatalf("write lockfile: %v", err) | ||
| } |
Comment on lines
+109
to
+113
| workflowName, resolveErr := s.tmpl.Resolve(s.workflow, pc) | ||
| if resolveErr != nil { | ||
| return nil, fmt.Errorf("workflow_call step %q: failed to resolve workflow name %q: %w", s.name, s.workflow, resolveErr) | ||
| } | ||
| target, ok := s.lookup(workflowName) |
Comment on lines
123
to
133
| // WorkflowConfig represents the overall configuration for the workflow engine | ||
| type WorkflowConfig struct { | ||
| Imports []string `json:"imports,omitempty" yaml:"imports,omitempty"` | ||
| Modules []ModuleConfig `json:"modules" yaml:"modules"` | ||
| Workflows map[string]any `json:"workflows" yaml:"workflows"` | ||
| Triggers map[string]any `json:"triggers" yaml:"triggers"` | ||
| Pipelines map[string]any `json:"pipelines,omitempty" yaml:"pipelines,omitempty"` | ||
| Platform map[string]any `json:"platform,omitempty" yaml:"platform,omitempty"` | ||
| Requires *RequiresConfig `json:"requires,omitempty" yaml:"requires,omitempty"` | ||
| Plugins *PluginsConfig `json:"plugins,omitempty" yaml:"plugins,omitempty"` | ||
| Sidecars []SidecarConfig `json:"sidecars,omitempty" yaml:"sidecars,omitempty"` |
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
intel352
added a commit
that referenced
this pull request
Mar 15, 2026
- registry_source.go: switch GitHubRegistrySource to use registryHTTPClient (timeout-configured) for both ListPlugins and FetchManifest; update comment - autofetch.go: scan for AutoFetch=true entries before exec.LookPath to avoid misleading startup warning when no plugins need auto-fetch - autofetch.go: add internal autoFetchPlugin helper that accepts *slog.Logger and emits structured log entries when a logger is available; public AutoFetchPlugin delegates to it; AutoFetchDeclaredPlugins passes its logger - autofetch_test.go: rename TestAutoFetchPlugin_CorrectArgs to TestAutoFetchPlugin_SkipsWhenExists to match what the test actually asserts - integrity.go: replace os.ReadFile + sha256.Sum256 with streaming os.Open + io.Copy into sha256.New() to keep memory bounded for large binaries - integrity_test.go: add t.Skip guard in UnreadableLockfile test when the file is actually readable (Windows / root environments) - pipeline_step_workflow_call.go: use resolved workflowName (not s.workflow) in async return payload and sync error message for consistency - docs/PLUGIN_AUTHORING.md: clarify the distinction between plugin.json name (short, used by engine) and the registry/provider manifest name (workflow-plugin- prefixed), and which is used for dependency resolution - config/config.go: MergeApplicationConfig now merges Plugins.External from each referenced workflow file into the combined config, deduplicated by name Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds --url flag to wfctl plugin install that downloads a tar.gz archive from a direct URL, extracts plugin.json to identify the plugin name, installs to the plugin directory, and records the SHA-256 checksum in the lockfile. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Extends wfctl plugin init to generate cmd/workflow-plugin-<name>/main.go, internal/provider.go, internal/steps.go, go.mod, .goreleaser.yml, CI/release GitHub Actions workflows, Makefile, and README.md. Adds --module flag for custom Go module paths. Preserves existing plugin.json and .go skeleton for backward compatibility. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add AutoFetchPlugin and AutoFetchDeclaredPlugins to plugin/autofetch.go, which shell out to wfctl to download plugins not found locally. Extend WorkflowConfig with a new PluginsConfig / ExternalPluginDecl type so configs can declare plugins with autoFetch: true and an optional version constraint. StdEngine gains SetExternalPluginDir and calls AutoFetchDeclaredPlugins in BuildFromConfig before module loading. The server's buildEngine registers the plugin dir so auto-fetch is active at runtime. If wfctl is absent, a warning is logged and startup continues. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Use struct conversion for staticIndexEntry → PluginSummary (staticcheck S1016) - Remove unused updateLockfile and writePluginJSON functions - Add nilerr annotations for intentional nil returns in integrity.go - Add gosec annotation for exec.Command in autofetch.go - Fix TestLoadRegistryConfigDefault to use DefaultRegistryConfig() directly Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- integrity.go: fail closed when lockfile exists but is unreadable or unparseable, preventing integrity enforcement bypass - autofetch.go: extract stripVersionConstraint helper; detect compound version constraints and fall back to latest; check both pluginName and workflow-plugin-<name> alternate form for installed-check; log restart warning when new plugins are downloaded (they require a server restart) - autofetch_test.go: test stripVersionConstraint directly instead of duplicating the logic inline; add compound-constraint cases - engine.go: clarify comment that auto-fetched plugins need a restart Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- generator.go: use plugin/external/sdk imports and types (PluginProvider, StepInstance, StepResult, StepTypes/CreateStep) instead of plugin/sdk - PLUGIN_AUTHORING.md: update examples to match external SDK interfaces - plugin_install.go: hash installed binary (not archive) for lockfile, add hashFileSHA256 helper, add install mode mutual exclusivity check, update installFromLocal to write lockfile, normalize plugin names - plugin_lockfile.go: add registry param to updateLockfileWithChecksum, pass version/registry in installFromLockfile, remove dir on mismatch - registry_source.go: validate URL in NewStaticRegistrySource - config.go: clarify Version field forwarding semantics Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- registry_source.go: use explicit field assignment for PluginSummary instead of struct type conversion (clearer, avoids tag confusion) - plugin_lockfile.go: don't pass @Version in installFromLockfile to prevent lockfile overwrite before checksum verification - plugin_install.go: add verifyInstalledPlugin() call in installFromURL for parity with registry installs - engine.go: add TODO to move auto-fetch before plugin discovery so newly fetched plugins are available without restart - integrity_test.go: add tests for unreadable and malformed lockfile to verify fail-closed behavior Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- registry_source.go: add nolint:gosimple for S1016 — explicit field assignment preferred for clarity across different struct tags - generator_test.go: add TestGenerateProjectStructure verifying all generated files exist and use correct external SDK imports/types Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…immediately Auto-fetch was running inside BuildFromConfig, which executes after external plugins are already discovered and loaded. Plugins downloaded by auto-fetch required a server restart to take effect. Move auto-fetch to buildEngine in cmd/server/main.go, before DiscoverPlugins/LoadPlugin. Remove the now-unused externalPluginDir field and SetExternalPluginDir from the engine. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Enables pipelines to call other pipelines by name with full context forwarding. Supports template-resolved workflow names and stop_pipeline option. Required for WebSocket message routing to game-specific pipelines. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- registry_source.go: switch GitHubRegistrySource to use registryHTTPClient (timeout-configured) for both ListPlugins and FetchManifest; update comment - autofetch.go: scan for AutoFetch=true entries before exec.LookPath to avoid misleading startup warning when no plugins need auto-fetch - autofetch.go: add internal autoFetchPlugin helper that accepts *slog.Logger and emits structured log entries when a logger is available; public AutoFetchPlugin delegates to it; AutoFetchDeclaredPlugins passes its logger - autofetch_test.go: rename TestAutoFetchPlugin_CorrectArgs to TestAutoFetchPlugin_SkipsWhenExists to match what the test actually asserts - integrity.go: replace os.ReadFile + sha256.Sum256 with streaming os.Open + io.Copy into sha256.New() to keep memory bounded for large binaries - integrity_test.go: add t.Skip guard in UnreadableLockfile test when the file is actually readable (Windows / root environments) - pipeline_step_workflow_call.go: use resolved workflowName (not s.workflow) in async return payload and sync error message for consistency - docs/PLUGIN_AUTHORING.md: clarify the distinction between plugin.json name (short, used by engine) and the registry/provider manifest name (workflow-plugin- prefixed), and which is used for dependency resolution - config/config.go: MergeApplicationConfig now merges Plugins.External from each referenced workflow file into the combined config, deduplicated by name Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
intel352
force-pushed
the
feat/plugin-ecosystem-engine
branch
from
9dccf19 to
56469b1
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves plugin management (integrity verification, registry fetching, install/lockfile behavior, and auto-fetch logging), extends pipeline behavior for workflow_call, and updates authoring documentation to clarify plugin naming.
Changes:
- Stream SHA-256 hashing for plugin integrity checks; improve tests around lockfile unreadability.
- Add structured logging option to plugin auto-fetch and reduce unnecessary
wfctlchecks; centralize registry HTTP client usage with timeouts. - Enhance
workflow_callpipeline step (templated workflow name + optionalstop_pipeline), and mergeplugins.externalacross application workflow files.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| plugin/integrity.go | Switches integrity hashing to streaming (io.Copy) to avoid loading binaries into memory. |
| plugin/integrity_test.go | Makes unreadable-lockfile test resilient across platforms/privileged execution. |
| plugin/autofetch.go | Adds internal logger-aware implementation and avoids wfctl lookup when not needed. |
| plugin/autofetch_test.go | Renames/adjusts an auto-fetch test (now overlaps with an existing one). |
| module/pipeline_step_workflow_call.go | Resolves workflow name via templates and supports stopping parent pipeline after call. |
| docs/PLUGIN_AUTHORING.md | Clarifies plugin.json naming expectations and short vs prefixed names. |
| config/config.go | Merges/deduplicates plugins.external when merging application workflow files. |
| cmd/wfctl/registry_source.go | Uses a shared HTTP client with timeout for registry operations; avoids default client. |
| cmd/wfctl/plugin_lockfile.go | Improves behavior on checksum mismatch and reports removal failures. |
| cmd/wfctl/plugin_install.go | Tweaks mode validation messaging, lockfile checksum handling, and adds helper hashing function. |
| cmd/wfctl/multi_registry.go | Minor rename/cleanup while building registry sources. |
Comments suppressed due to low confidence (1)
plugin/autofetch_test.go:103
- This test now duplicates TestAutoFetchPlugin_AlreadyInstalled (same behavior: returns nil when plugin.json exists). Keeping both adds maintenance cost without increasing coverage. Consider removing one of them or rewriting this one to validate a distinct behavior (e.g., the logger-aware autoFetchPlugin path / version constraint handling).
// TestAutoFetchPlugin_SkipsWhenExists verifies that AutoFetchPlugin returns nil
// immediately when the plugin is already installed, without invoking wfctl.
func TestAutoFetchPlugin_SkipsWhenExists(t *testing.T) {
dir := t.TempDir()
pluginDir := filepath.Join(dir, "plugins")
pluginName := "test-plugin"
destDir := filepath.Join(pluginDir, pluginName)
if err := os.MkdirAll(destDir, 0755); err != nil {
t.Fatalf("failed to create plugin dir: %v", err)
}
manifestPath := filepath.Join(destDir, "plugin.json")
if err := os.WriteFile(manifestPath, []byte(`{"name":"test-plugin","version":"0.1.0"}`), 0644); err != nil {
t.Fatalf("failed to write plugin.json: %v", err)
}
// With plugin.json present, AutoFetchPlugin must return nil (no wfctl invoked).
if err := AutoFetchPlugin(pluginName, ">=0.1.0", pluginDir); err != nil {
t.Errorf("expected nil for already-installed plugin, got: %v", err)
}
Comment on lines
76
to
86
| // CreateStep implements sdk.StepProvider. | ||
| func (p *Provider) CreateStep(typeName, name string, config map[string]any) (sdk.StepInstance, error) { | ||
| switch typeName { | ||
| case "step.my_action": | ||
| return &MyStep{config: config}, nil | ||
| } | ||
| return nil, fmt.Errorf("unknown step type: %s", typeName) | ||
| } | ||
| ``` | ||
|
|
||
| ## Implementing Modules | ||
|
|
Comment on lines
+423
to
+439
| // Merge external plugin declarations — deduplicate by name (first definition wins). | ||
| if wfCfg.Plugins != nil && len(wfCfg.Plugins.External) > 0 { | ||
| if combined.Plugins == nil { | ||
| combined.Plugins = &PluginsConfig{} | ||
| } | ||
| existingPlugins := make(map[string]struct{}, len(combined.Plugins.External)) | ||
| for _, ep := range combined.Plugins.External { | ||
| existingPlugins[ep.Name] = struct{}{} | ||
| } | ||
| for _, ep := range wfCfg.Plugins.External { | ||
| if _, exists := existingPlugins[ep.Name]; exists { | ||
| continue | ||
| } | ||
| combined.Plugins.External = append(combined.Plugins.External, ep) | ||
| existingPlugins[ep.Name] = struct{}{} | ||
| } | ||
| } |
Comment on lines
167
to
175
| if _, ver := parseNameVersion(nameArg); ver != "" { | ||
| // Hash the installed binary (not the archive) so verifyInstalledChecksum matches. | ||
| pluginName = normalizePluginName(pluginName) | ||
| binaryChecksum := "" | ||
| binaryPath := filepath.Join(pluginDirVal, pluginName, pluginName) | ||
| sha, hashErr := hashFileSHA256(binaryPath) | ||
| if hashErr != nil { | ||
| fmt.Fprintf(os.Stderr, "warning: could not hash installed binary: %v\n", hashErr) | ||
| if cs, hashErr := hashFileSHA256(binaryPath); hashErr == nil { | ||
| binaryChecksum = cs | ||
| } | ||
| updateLockfileWithChecksum(pluginName, manifest.Version, manifest.Repository, sourceName, sha) | ||
| updateLockfileWithChecksum(pluginName, manifest.Version, manifest.Repository, sourceName, binaryChecksum) | ||
| } |
cmd/wfctl/plugin_install.go
Outdated
Comment on lines
+539
to
+547
| // hashFileSHA256 computes the SHA-256 hex digest of the file at path. | ||
| func hashFileSHA256(path string) (string, error) { | ||
| data, err := os.ReadFile(path) | ||
| if err != nil { | ||
| return "", fmt.Errorf("hash file %s: %w", path, err) | ||
| } | ||
| h := sha256.Sum256(data) | ||
| return hex.EncodeToString(h[:]), nil | ||
| } |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- docs/PLUGIN_AUTHORING.md: close unclosed code fence after StepProvider example - cmd/wfctl/plugin_install.go: streaming hashFileSHA256 via io.Copy + sha256.New() - cmd/wfctl/plugin_install.go: warn on hash failure instead of silent empty checksum - config/merge_test.go: add TestMergeApplicationConfig_PluginDedup covering first-definition-wins dedup Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR tightens and standardizes plugin-related behavior across the engine and wfctl by improving checksum handling, registry HTTP behavior, and application config merging, while adding a new pipeline control option for workflow_call steps.
Changes:
- Switch plugin/binary SHA-256 computation to streaming I/O and harden related tests for cross-platform behavior.
- Add structured logging support to plugin auto-fetch and avoid unnecessary
wfctlchecks when auto-fetch isn’t requested. - Merge/deduplicate external plugin declarations across application workflow files and extend
workflow_callwith templated workflow names +stop_pipeline.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| plugin/integrity.go | Stream SHA-256 hashing for plugin binary integrity verification. |
| plugin/integrity_test.go | Make unreadable-lockfile test robust on platforms where chmod isn’t enforced. |
| plugin/autofetch.go | Add internal logger-capable implementation; avoid wfctl lookup when no auto-fetch is requested. |
| plugin/autofetch_test.go | Adjust/rename test around auto-fetch short-circuit behavior. |
| module/pipeline_step_workflow_call.go | Add stop_pipeline and template-based workflow name resolution for workflow_call. |
| docs/PLUGIN_AUTHORING.md | Clarify plugin naming expectations between plugin.json and registry naming. |
| config/config.go | Deduplicate merged external plugin declarations by name in MergeApplicationConfig. |
| config/merge_test.go | Add coverage for plugin deduplication in application config merging. |
| cmd/wfctl/registry_source.go | Route registry requests through a shared HTTP client with timeout; minor search result construction tweak. |
| cmd/wfctl/plugin_lockfile.go | Improve checksum-mismatch handling by warning on removal failures. |
| cmd/wfctl/plugin_install.go | Improve mode validation messaging; compute lockfile checksums via streaming hash helper. |
| cmd/wfctl/multi_registry.go | Minor variable naming cleanup in static registry creation. |
Comments suppressed due to low confidence (1)
plugin/autofetch.go:155
- When
wfctlis missing andloggeris nil, this returns silently even if there are AutoFetch=true declarations. That contradicts the function comment (“a warning is logged”) and can make auto-fetch failures hard to diagnose in non-structured-logging callers. Consider also printing a stderr warning when logger is nil (or return an error/bool so the caller can surface it).
if _, err := exec.LookPath("wfctl"); err != nil {
if logger != nil {
logger.Warn("wfctl not found on PATH; skipping auto-fetch for declared plugins",
"plugin_dir", pluginDir)
}
| _, _ = target.Execute(asyncCtx, data) //nolint:errcheck | ||
| }(ctx, triggerData) | ||
| return &StepResult{Output: map[string]any{"workflow": s.workflow, "mode": "async", "dispatched": true}}, nil | ||
| return &StepResult{Output: map[string]any{"workflow": workflowName, "mode": "async", "dispatched": true}}, nil |
plugin/integrity.go
Outdated
| binaryData, err := os.ReadFile(binaryPath) | ||
| f, err := os.Open(binaryPath) | ||
| if err != nil { | ||
| return fmt.Errorf("read plugin binary %s: %w", binaryPath, err) |
cmd/wfctl/plugin_install.go
Outdated
| if cs, hashErr := hashFileSHA256(binaryPath); hashErr == nil { | ||
| binaryChecksum = cs | ||
| } else { | ||
| fmt.Fprintf(os.Stderr, "Warning: could not hash binary %s: %v (lockfile will have no checksum)\n", binaryPath, hashErr) |
config/merge_test.go
Outdated
Comment on lines
+541
to
+543
| wfA := dir + "/a.yaml" | ||
| writeFileContent(wfA, ` | ||
| plugins: |
- pipeline_step_workflow_call.go: propagate stop_pipeline in async mode - integrity.go: "open plugin binary" instead of "read" for os.Open error - plugin_install.go: lowercase "warning:" for consistency - merge_test.go: use filepath.Join and check writeFileContent errors Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
intel352
added a commit
that referenced
this pull request
Mar 16, 2026
* feat(wfctl): add plugin install --url for direct URL installs Adds --url flag to wfctl plugin install that downloads a tar.gz archive from a direct URL, extracts plugin.json to identify the plugin name, installs to the plugin directory, and records the SHA-256 checksum in the lockfile. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(wfctl): enhanced plugin init scaffold with full project structure Extends wfctl plugin init to generate cmd/workflow-plugin-<name>/main.go, internal/provider.go, internal/steps.go, go.mod, .goreleaser.yml, CI/release GitHub Actions workflows, Makefile, and README.md. Adds --module flag for custom Go module paths. Preserves existing plugin.json and .go skeleton for backward compatibility. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat: engine auto-fetch for declared external plugins on startup Add AutoFetchPlugin and AutoFetchDeclaredPlugins to plugin/autofetch.go, which shell out to wfctl to download plugins not found locally. Extend WorkflowConfig with a new PluginsConfig / ExternalPluginDecl type so configs can declare plugins with autoFetch: true and an optional version constraint. StdEngine gains SetExternalPluginDir and calls AutoFetchDeclaredPlugins in BuildFromConfig before module loading. The server's buildEngine registers the plugin dir so auto-fetch is active at runtime. If wfctl is absent, a warning is logged and startup continues. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: resolve all CI lint failures - Use struct conversion for staticIndexEntry → PluginSummary (staticcheck S1016) - Remove unused updateLockfile and writePluginJSON functions - Add nilerr annotations for intentional nil returns in integrity.go - Add gosec annotation for exec.Command in autofetch.go - Fix TestLoadRegistryConfigDefault to use DefaultRegistryConfig() directly Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address Copilot review feedback on engine PR - integrity.go: fail closed when lockfile exists but is unreadable or unparseable, preventing integrity enforcement bypass - autofetch.go: extract stripVersionConstraint helper; detect compound version constraints and fall back to latest; check both pluginName and workflow-plugin-<name> alternate form for installed-check; log restart warning when new plugins are downloaded (they require a server restart) - autofetch_test.go: test stripVersionConstraint directly instead of duplicating the logic inline; add compound-constraint cases - engine.go: clarify comment that auto-fetched plugins need a restart Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address all Copilot review comments on engine PR (#330) - generator.go: use plugin/external/sdk imports and types (PluginProvider, StepInstance, StepResult, StepTypes/CreateStep) instead of plugin/sdk - PLUGIN_AUTHORING.md: update examples to match external SDK interfaces - plugin_install.go: hash installed binary (not archive) for lockfile, add hashFileSHA256 helper, add install mode mutual exclusivity check, update installFromLocal to write lockfile, normalize plugin names - plugin_lockfile.go: add registry param to updateLockfileWithChecksum, pass version/registry in installFromLockfile, remove dir on mismatch - registry_source.go: validate URL in NewStaticRegistrySource - config.go: clarify Version field forwarding semantics Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address remaining Copilot review comments on engine PR (#330) - registry_source.go: use explicit field assignment for PluginSummary instead of struct type conversion (clearer, avoids tag confusion) - plugin_lockfile.go: don't pass @Version in installFromLockfile to prevent lockfile overwrite before checksum verification - plugin_install.go: add verifyInstalledPlugin() call in installFromURL for parity with registry installs - engine.go: add TODO to move auto-fetch before plugin discovery so newly fetched plugins are available without restart - integrity_test.go: add tests for unreadable and malformed lockfile to verify fail-closed behavior Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: suppress S1016 lint, add generator project structure tests - registry_source.go: add nolint:gosimple for S1016 — explicit field assignment preferred for clarity across different struct tags - generator_test.go: add TestGenerateProjectStructure verifying all generated files exist and use correct external SDK imports/types Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: move auto-fetch before plugin discovery so fetched plugins load immediately Auto-fetch was running inside BuildFromConfig, which executes after external plugins are already discovered and loaded. Plugins downloaded by auto-fetch required a server restart to take effect. Move auto-fetch to buildEngine in cmd/server/main.go, before DiscoverPlugins/LoadPlugin. Remove the now-unused externalPluginDir field and SetExternalPluginDir from the engine. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat: add step.workflow_call for cross-pipeline dispatch Enables pipelines to call other pipelines by name with full context forwarding. Supports template-resolved workflow names and stop_pipeline option. Required for WebSocket message routing to game-specific pipelines. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address all 9 Copilot review comments on PR #331 - registry_source.go: switch GitHubRegistrySource to use registryHTTPClient (timeout-configured) for both ListPlugins and FetchManifest; update comment - autofetch.go: scan for AutoFetch=true entries before exec.LookPath to avoid misleading startup warning when no plugins need auto-fetch - autofetch.go: add internal autoFetchPlugin helper that accepts *slog.Logger and emits structured log entries when a logger is available; public AutoFetchPlugin delegates to it; AutoFetchDeclaredPlugins passes its logger - autofetch_test.go: rename TestAutoFetchPlugin_CorrectArgs to TestAutoFetchPlugin_SkipsWhenExists to match what the test actually asserts - integrity.go: replace os.ReadFile + sha256.Sum256 with streaming os.Open + io.Copy into sha256.New() to keep memory bounded for large binaries - integrity_test.go: add t.Skip guard in UnreadableLockfile test when the file is actually readable (Windows / root environments) - pipeline_step_workflow_call.go: use resolved workflowName (not s.workflow) in async return payload and sync error message for consistency - docs/PLUGIN_AUTHORING.md: clarify the distinction between plugin.json name (short, used by engine) and the registry/provider manifest name (workflow-plugin- prefixed), and which is used for dependency resolution - config/config.go: MergeApplicationConfig now merges Plugins.External from each referenced workflow file into the combined config, deduplicated by name Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: remove duplicate hashFileSHA256 from merge conflict resolution Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address new PR #331 review comments - docs/PLUGIN_AUTHORING.md: close unclosed code fence after StepProvider example - cmd/wfctl/plugin_install.go: streaming hashFileSHA256 via io.Copy + sha256.New() - cmd/wfctl/plugin_install.go: warn on hash failure instead of silent empty checksum - config/merge_test.go: add TestMergeApplicationConfig_PluginDedup covering first-definition-wins dedup Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address 4 new PR #331 review comments - pipeline_step_workflow_call.go: propagate stop_pipeline in async mode - integrity.go: "open plugin binary" instead of "read" for os.Open error - plugin_install.go: lowercase "warning:" for consistency - merge_test.go: use filepath.Join and check writeFileContent errors Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: add skip_if and if fields to pipeline step execution Steps now support optional `skip_if` and `if` Go template guards. When `skip_if` evaluates to a truthy value the step is skipped and the pipeline continues; `if` is the logical inverse. Skipped steps produce `{"skipped": true, "reason": "..."}` output so downstream steps can inspect the result. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
intel352
added a commit
that referenced
this pull request
Mar 16, 2026
…adata keys for skipped output (#333) * feat(wfctl): add plugin install --url for direct URL installs Adds --url flag to wfctl plugin install that downloads a tar.gz archive from a direct URL, extracts plugin.json to identify the plugin name, installs to the plugin directory, and records the SHA-256 checksum in the lockfile. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(wfctl): enhanced plugin init scaffold with full project structure Extends wfctl plugin init to generate cmd/workflow-plugin-<name>/main.go, internal/provider.go, internal/steps.go, go.mod, .goreleaser.yml, CI/release GitHub Actions workflows, Makefile, and README.md. Adds --module flag for custom Go module paths. Preserves existing plugin.json and .go skeleton for backward compatibility. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat: engine auto-fetch for declared external plugins on startup Add AutoFetchPlugin and AutoFetchDeclaredPlugins to plugin/autofetch.go, which shell out to wfctl to download plugins not found locally. Extend WorkflowConfig with a new PluginsConfig / ExternalPluginDecl type so configs can declare plugins with autoFetch: true and an optional version constraint. StdEngine gains SetExternalPluginDir and calls AutoFetchDeclaredPlugins in BuildFromConfig before module loading. The server's buildEngine registers the plugin dir so auto-fetch is active at runtime. If wfctl is absent, a warning is logged and startup continues. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: resolve all CI lint failures - Use struct conversion for staticIndexEntry → PluginSummary (staticcheck S1016) - Remove unused updateLockfile and writePluginJSON functions - Add nilerr annotations for intentional nil returns in integrity.go - Add gosec annotation for exec.Command in autofetch.go - Fix TestLoadRegistryConfigDefault to use DefaultRegistryConfig() directly Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address Copilot review feedback on engine PR - integrity.go: fail closed when lockfile exists but is unreadable or unparseable, preventing integrity enforcement bypass - autofetch.go: extract stripVersionConstraint helper; detect compound version constraints and fall back to latest; check both pluginName and workflow-plugin-<name> alternate form for installed-check; log restart warning when new plugins are downloaded (they require a server restart) - autofetch_test.go: test stripVersionConstraint directly instead of duplicating the logic inline; add compound-constraint cases - engine.go: clarify comment that auto-fetched plugins need a restart Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address all Copilot review comments on engine PR (#330) - generator.go: use plugin/external/sdk imports and types (PluginProvider, StepInstance, StepResult, StepTypes/CreateStep) instead of plugin/sdk - PLUGIN_AUTHORING.md: update examples to match external SDK interfaces - plugin_install.go: hash installed binary (not archive) for lockfile, add hashFileSHA256 helper, add install mode mutual exclusivity check, update installFromLocal to write lockfile, normalize plugin names - plugin_lockfile.go: add registry param to updateLockfileWithChecksum, pass version/registry in installFromLockfile, remove dir on mismatch - registry_source.go: validate URL in NewStaticRegistrySource - config.go: clarify Version field forwarding semantics Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address remaining Copilot review comments on engine PR (#330) - registry_source.go: use explicit field assignment for PluginSummary instead of struct type conversion (clearer, avoids tag confusion) - plugin_lockfile.go: don't pass @Version in installFromLockfile to prevent lockfile overwrite before checksum verification - plugin_install.go: add verifyInstalledPlugin() call in installFromURL for parity with registry installs - engine.go: add TODO to move auto-fetch before plugin discovery so newly fetched plugins are available without restart - integrity_test.go: add tests for unreadable and malformed lockfile to verify fail-closed behavior Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: suppress S1016 lint, add generator project structure tests - registry_source.go: add nolint:gosimple for S1016 — explicit field assignment preferred for clarity across different struct tags - generator_test.go: add TestGenerateProjectStructure verifying all generated files exist and use correct external SDK imports/types Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: move auto-fetch before plugin discovery so fetched plugins load immediately Auto-fetch was running inside BuildFromConfig, which executes after external plugins are already discovered and loaded. Plugins downloaded by auto-fetch required a server restart to take effect. Move auto-fetch to buildEngine in cmd/server/main.go, before DiscoverPlugins/LoadPlugin. Remove the now-unused externalPluginDir field and SetExternalPluginDir from the engine. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat: add step.workflow_call for cross-pipeline dispatch Enables pipelines to call other pipelines by name with full context forwarding. Supports template-resolved workflow names and stop_pipeline option. Required for WebSocket message routing to game-specific pipelines. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address all 9 Copilot review comments on PR #331 - registry_source.go: switch GitHubRegistrySource to use registryHTTPClient (timeout-configured) for both ListPlugins and FetchManifest; update comment - autofetch.go: scan for AutoFetch=true entries before exec.LookPath to avoid misleading startup warning when no plugins need auto-fetch - autofetch.go: add internal autoFetchPlugin helper that accepts *slog.Logger and emits structured log entries when a logger is available; public AutoFetchPlugin delegates to it; AutoFetchDeclaredPlugins passes its logger - autofetch_test.go: rename TestAutoFetchPlugin_CorrectArgs to TestAutoFetchPlugin_SkipsWhenExists to match what the test actually asserts - integrity.go: replace os.ReadFile + sha256.Sum256 with streaming os.Open + io.Copy into sha256.New() to keep memory bounded for large binaries - integrity_test.go: add t.Skip guard in UnreadableLockfile test when the file is actually readable (Windows / root environments) - pipeline_step_workflow_call.go: use resolved workflowName (not s.workflow) in async return payload and sync error message for consistency - docs/PLUGIN_AUTHORING.md: clarify the distinction between plugin.json name (short, used by engine) and the registry/provider manifest name (workflow-plugin- prefixed), and which is used for dependency resolution - config/config.go: MergeApplicationConfig now merges Plugins.External from each referenced workflow file into the combined config, deduplicated by name Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: remove duplicate hashFileSHA256 from merge conflict resolution Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address new PR #331 review comments - docs/PLUGIN_AUTHORING.md: close unclosed code fence after StepProvider example - cmd/wfctl/plugin_install.go: streaming hashFileSHA256 via io.Copy + sha256.New() - cmd/wfctl/plugin_install.go: warn on hash failure instead of silent empty checksum - config/merge_test.go: add TestMergeApplicationConfig_PluginDedup covering first-definition-wins dedup Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address 4 new PR #331 review comments - pipeline_step_workflow_call.go: propagate stop_pipeline in async mode - integrity.go: "open plugin binary" instead of "read" for os.Open error - plugin_install.go: lowercase "warning:" for consistency - merge_test.go: use filepath.Join and check writeFileContent errors Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: add skip_if and if fields to pipeline step execution Steps now support optional `skip_if` and `if` Go template guards. When `skip_if` evaluates to a truthy value the step is skipped and the pipeline continues; `if` is the logical inverse. Skipped steps produce `{"skipped": true, "reason": "..."}` output so downstream steps can inspect the result. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Initial plan * fix: address review feedback on SkippableStep guard template handling - Return errors (fail closed) instead of swallowing skip_if/if template failures - Use _skipped/_error keys in skipped step output (align with ErrorStrategySkip convention) - Fix misleading comment for If field in config/pipeline.go - Update tests to reflect new key names; add 2 tests for template error behavior Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> --------- Co-authored-by: Jon Langevin <codingsloth@pm.me> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
wfctl plugin install(URL, local, static registry sources), comprehensive plugin scaffold generatorstep.workflow_call— new pipeline step for cross-pipeline dispatch. Enables a pipeline to invoke another pipeline by name with full context forwarding. Template-resolved workflow names supported. Required for WebSocket message routing in multi-pipeline applications (e.g., card game platform).Key Changes
step.workflow_call
Calls another pipeline by name, forwarding trigger data and current context:
Plugin auto-fetch
Declared external plugins in config are automatically downloaded on engine startup if not present locally.
Binary integrity
Plugin binaries verified against SHA-256 checksums in lockfile on load.
wfctl enhancements
plugin install --urlfor direct URL installsplugin install --localfor local directory installsTest plan
go test ./...passes🤖 Generated with Claude Code