fix(wfctl): plugin CLI registry binary path uses dir name + manifest name#595
Merged
Conversation
…name PR #591 wired BuildCLIRegistry into main.go's dispatch fallback, but the binary-path computation joined manifest.Name twice: binaryPath := filepath.Join(pluginsDir, pluginName, pluginName) where pluginName comes from the manifest map key (= manifest.Name). This works only when the install directory name matches the manifest name. In practice setup-plugins composite + `wfctl plugin install` both extract tarballs to short directory names — `data/plugins/payments`, `data/plugins/digitalocean`, etc. — while the binary inside is named after the manifest (`workflow-plugin-payments`). The path therefore resolved to: data/plugins/workflow-plugin-payments/workflow-plugin-payments which doesn't exist, so DispatchCLICommand reported a fork/exec failure. Worse, when LookupCLICommand succeeded but Dispatch failed, operators got a confusing "no such file" error rather than the plugin's own help. Refactor BuildCLIRegistry to walk the plugins directory once and capture both the dir name (for the install path) and the manifest name (for the binary file name): binaryPath := filepath.Join(pluginsDir, dirEntry, manifest.Name) LoadPluginManifests is no longer used by this path because its keyed return collapses on manifest.Name and loses the dir name. Reproducer + regression tests: TestPluginCLIRegistry_DirVsManifestNameMismatch — short dir + full manifest name (the bug from buymywishlist#260 retrigger). TestPluginCLIRegistry_EmptyManifestNameUsesDirName — covers the fallback when manifest.Name is empty. Local repro (pre-fix) reported: fork/exec /tmp/test-plugins/workflow-plugin-payments/workflow-plugin-payments: no such file or directory Same plugin tree post-fix dispatches correctly (binary lives at /tmp/test-plugins/payments/workflow-plugin-payments). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes wfctl’s dynamic plugin-CLI dispatch so the plugin binary path is resolved using the installed directory name plus the manifest-declared binary name, instead of incorrectly assuming they match. This aligns wfctl with real-world plugin installation layouts where tarballs extract into short directory names but ship binaries named after plugin.json’s name.
Changes:
- Refactors
BuildCLIRegistryto scan plugin subdirectories and computeBinaryPathas<pluginsDir>/<dirName>/<manifestName>(with a dir-name fallback whenmanifest.nameis empty). - Preserves reserved-command and conflict validation while keeping deterministic behavior via directory-name sorting.
- Adds regression tests covering the dir-vs-manifest name mismatch and the empty-manifest-name fallback.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| cmd/wfctl/plugin_cli_commands.go | Updates CLI registry construction to track both plugin install dir name and manifest name when computing plugin binary paths. |
| cmd/wfctl/plugin_cli_commands_test.go | Adds tests and helpers to validate correct binary path resolution for mismatched dir/manifest names and for empty manifest names. |
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
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
PR #591 wired `BuildCLIRegistry` into `cmd/wfctl/main.go`'s dispatch fallback, but the binary-path computation joins `manifest.Name` twice:
```go
binaryPath := filepath.Join(pluginsDir, pluginName, pluginName)
```
This works only when the install directory name matches the manifest name. In practice the `setup-plugins` composite + `wfctl plugin install` both extract tarballs to short directory names (`data/plugins/payments`, `data/plugins/digitalocean`, …) while the binary inside is named after the manifest (`workflow-plugin-payments`). Path resolves to `data/plugins/workflow-plugin-payments/workflow-plugin-payments` which doesn't exist.
Reproduction (pre-fix)
```
$ tar -xzf workflow-plugin-payments-linux-amd64.tar.gz -C /tmp/data/plugins/payments/
$ WFCTL_PLUGIN_DIR=/tmp/data/plugins ./wfctl payments
error: plugin workflow-plugin-payments command payments:
fork/exec /tmp/data/plugins/workflow-plugin-payments/workflow-plugin-payments:
no such file or directory
```
Surfaced from buymywishlist #260 retrigger where v0.27.2 wfctl + workflow-plugin-payments v0.3.1 hit this for the first real-world plugin-CLI dispatch.
Fix
Refactor `BuildCLIRegistry` to walk the plugins directory once, capturing both the dir name (for the install path) and the manifest name (for the binary file name):
```go
binaryPath := filepath.Join(pluginsDir, dirEntry, manifest.Name)
```
`LoadPluginManifests` is no longer used by this path because its keyed return collapses on `manifest.Name` and loses the dir name.
Tests
Verification
🤖 Generated with Claude Code