Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions cmd/wfctl/plugin_cli_commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,14 @@ func BuildCLIRegistry(pluginsDir string) (CLIRegistry, error) {
existing.PluginName, manifestName, name,
)
}
// Binary path uses the actual on-disk directory (which `wfctl
// plugin install` controls) plus the manifest-declared binary
// name (which the tarball ships).
binaryPath := filepath.Join(pluginsDir, p.dirName, manifestName)
// Binary path uses the on-disk directory name TWICE because
// `wfctl plugin install` calls ensurePluginBinary(destDir,
// pluginName) which RENAMES the largest executable in destDir
// to match the (short) plugin name. After install the binary
// lives at destDir/{shortName}/{shortName}. Earlier code joined
// destDir+manifestName, which only works in the rare case
// where the dir name and manifest name happen to coincide.
binaryPath := filepath.Join(pluginsDir, p.dirName, p.dirName)
registry[name] = &CLIRegistryEntry{
Command: name,
PluginName: manifestName,
Expand Down
31 changes: 21 additions & 10 deletions cmd/wfctl/plugin_cli_commands_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@ func writeCLIPlugin(t *testing.T, pluginsDir, name string, commands []string) {
// writeCLIPluginNamed builds a plugin where the on-disk directory name and
// the manifest name can differ — matches the real-world install convention
// (short dir name like "payments" + full manifest name like
// "workflow-plugin-payments"). The stub binary is named after the manifest.
// "workflow-plugin-payments").
//
// `wfctl plugin install` runs ensurePluginBinary post-extract which renames
// the executable to match the (short) install dir name, so the stub binary
// here is named after dirName rather than manifestName.
func writeCLIPluginNamed(t *testing.T, pluginsDir, dirName, manifestName string, commands []string) {
t.Helper()
dir := filepath.Join(pluginsDir, dirName)
Expand All @@ -34,7 +38,7 @@ func writeCLIPluginNamed(t *testing.T, pluginsDir, dirName, manifestName string,
if err := os.WriteFile(filepath.Join(dir, "plugin.json"), []byte(manifest), 0o644); err != nil {
t.Fatalf("write plugin.json: %v", err)
}
if err := os.WriteFile(filepath.Join(dir, manifestName), []byte("#!/bin/sh\nexit 0\n"), 0o755); err != nil {
if err := os.WriteFile(filepath.Join(dir, dirName), []byte("#!/bin/sh\nexit 0\n"), 0o755); err != nil {
t.Fatalf("write stub binary: %v", err)
}
}
Expand Down Expand Up @@ -110,13 +114,20 @@ func TestPluginCLIRegistry_AllStaticCommandsReserved(t *testing.T) {
}

// TestPluginCLIRegistry_DirVsManifestNameMismatch is the regression test for
// the binary-path bug introduced in workflow#591. setup-plugins (and `wfctl
// plugin install`) extract tarballs to short directory names like
// `data/plugins/payments` while the binary inside is named after the
// manifest (`workflow-plugin-payments`). The earlier path computation
// joined `manifest.Name` twice — the binary path resolved to
// `data/plugins/workflow-plugin-payments/workflow-plugin-payments`, which
// only existed when dirName == manifestName.
// the binary-path bug. setup-plugins (and `wfctl plugin install`) extract
// tarballs to short directory names like `data/plugins/payments`. After
// extraction `ensurePluginBinary` renames the largest executable to match
// the (short) install dir name. So the binary post-install lives at
// `<dir>/<shortName>/<shortName>` regardless of what the tarball or the
// manifest call it.
//
// The earlier path computation went through two iterations:
// 1. workflow#591 joined manifest.Name twice → broke for short dirs.
// 2. workflow#595 joined dirName + manifest.Name → broke because
// ensurePluginBinary renames the binary AWAY from manifest.Name.
//
// This test pins both sides: dir name + binary file name = the install
// dir name; manifest name flows into PluginName for log/audit purposes.
func TestPluginCLIRegistry_DirVsManifestNameMismatch(t *testing.T) {
dir := t.TempDir()
writeCLIPluginNamed(t, dir, "payments", "workflow-plugin-payments", []string{"payments"})
Expand All @@ -129,7 +140,7 @@ func TestPluginCLIRegistry_DirVsManifestNameMismatch(t *testing.T) {
if !ok {
t.Fatalf("expected `payments` command registered, got %v", reg)
}
wantBin := filepath.Join(dir, "payments", "workflow-plugin-payments")
wantBin := filepath.Join(dir, "payments", "payments")
if entry.BinaryPath != wantBin {
t.Errorf("BinaryPath = %q, want %q", entry.BinaryPath, wantBin)
}
Expand Down
Loading