feat(plugin): external command discovery (Phase 1 MVP)

docs/plans/2026-02-06-external-plugin-discovery-design.md

@@ -0,0 +1,131 @@
+# External Plugin Discovery - Phase 1 MVP Design
+
+Issue: https://github.com/steipete/gogcli/issues/188
+
+## Overview
+
+Implement cargo/git-style external command discovery allowing `gog foo bar` to execute `gog-foo-bar` binary from PATH.
+
+## Design Decisions
+
+### Decision 1: Post-parse fallback (Option B) vs Pre-parse interception (Option A)
+
+**Chosen: Post-parse fallback (Option B)**
+
+| Aspect | Option A (Pre-parse) | Option B (Post-parse) |
+|--------|---------------------|----------------------|
+| Performance | Slower on normal commands (PATH check first) | Faster for built-in commands |
+| Simplicity | Cleaner - no error parsing needed | Need to detect Kong error types |
+| Plugin priority | Plugins can shadow built-ins | Built-ins always take precedence |
+| Safety | Less safe - accidental shadowing | Safer |
+
+**Why Option B:**
+
+1. **Safety**: Built-in commands always take precedence - an external binary cannot accidentally or maliciously shadow core functionality
+2. **Convention**: Follows git/cargo pattern where built-ins win
+3. **Performance**: No PATH scanning for normal command usage (majority of invocations)
+
+### Decision 2: Longest-first (greedy) matching
+
+**Chosen: Longest-first**
+
+When user types `gog docs headings list`, search order:
+1. `gog-docs-headings-list` (most specific)
+2. `gog-docs-headings`
+3. `gog-docs` (least specific)
+
+**Why longest-first:**
+
+1. **Specificity wins**: More specific plugin takes precedence over generic one
+2. **Convention**: Matches cargo/git behavior
+3. **Composability**: `gog-docs-headings` handles headings, while `gog-docs` could handle generic docs operations - no conflict
+
+### Decision 3: Binary prefix `gog-*`
+
+**Chosen: `gog-` prefix**
+
+Example: `gog docs headings` → `gog-docs-headings`
+
+**Why `gog-` not `gogcli-`:**
+
+1. **Consistency**: Matches the CLI binary name users invoke
+2. **Brevity**: Shorter for plugin developers
+3. **Convention**: git uses `git-*`, cargo uses `cargo-*` (matches binary name)
+
+## Phase 1 MVP Scope
+
+**Included:**
+
+* PATH discovery + exec
+* Longest-first matching
+* Pass remaining args to plugin
+* Unit tests documenting behavior
+
+**Excluded (Phase 2+):**
+
+* `--help-oneliner` protocol
+* Help integration (plugins in `gog --help`)
+* Environment variable passing (GOG_AUTH_TOKEN_PATH, etc.)
+* Discovery caching
+
+## Implementation
+
+### Files
+
+| File | Purpose |
+|------|---------|
+| `internal/cmd/external.go` | Discovery + exec logic |
+| `internal/cmd/external_test.go` | Unit tests |
+| `internal/cmd/root.go` | Integrate into Execute() |
+
+### Core Algorithm
+
+```go
+func tryExternalCommand(args []string) error {
+    // Longest-first: try most specific binary first
+    // Why: More specific plugins should take precedence (cargo/git pattern)
+    for i := len(args); i > 0; i-- {
+        binaryName := "gog-" + strings.Join(args[:i], "-")
+        if path, err := exec.LookPath(binaryName); err == nil {
+            return execExternal(path, args[i:])
+        }
+    }
+    return nil // not found
+}
+```
+
+### Integration Point
+
+In `root.go` `Execute()`, after `parser.Parse(args)` fails:
+
+```go
+kctx, err := parser.Parse(args)
+if err != nil {
+    // Try external command before returning parse error
+    // Why post-parse: Built-in commands always take precedence (safer)
+    if extErr := tryExternalCommand(args); extErr != nil {
+        return extErr
+    }
+    // Fall through to original error if no external command found
+    ...
+}
+```
+
+## Commits
+
+1. `feat(plugin): add external command discovery and execution`
+2. `test(plugin): add unit tests for external command discovery`
+3. `feat(plugin): integrate external commands into Execute()`
+
+## Future Phases
+
+**Phase 2 (separate PRs):**
+
+* `--help-oneliner` protocol for help integration
+* Environment variable passing to plugins
+* Plugin listing in `gog --help`
+
+**Phase 3:**
+
+* Discovery caching for performance
+* Version compatibility checks

internal/cmd/external.go

@@ -0,0 +1,93 @@
+package cmd
+
+import (
+	"errors"
+	"os"
+	"os/exec"
+	"strings"
+	"syscall"
+)
+
+// externalCommandPrefix is the prefix for external plugin binaries.
+// Uses "gog-" to match the CLI binary name (following git/cargo convention).
+const externalCommandPrefix = "gog-"
+
+// ErrExternalNotFound indicates no external command was found for the given args.
+var ErrExternalNotFound = errors.New("external command not found")
+
+// tryExternalCommand attempts to find and execute an external plugin binary.
+//
+// Design: Post-parse fallback (Option B)
+// This function is called AFTER Kong parsing fails with "unknown command".
+// Why: Built-in commands always take precedence over external plugins.
+// This prevents accidental or malicious shadowing of core functionality
+// and matches the git/cargo convention.
+//
+// Algorithm: Longest-first (greedy) matching
+// For args ["docs", "headings", "list"], tries in order:
+//  1. gog-docs-headings-list (most specific)
+//  2. gog-docs-headings
+//  3. gog-docs (least specific)
+//
+// Why longest-first: More specific plugins should win over generic ones.
+// Example: gog-docs-headings handles "headings" specifically, while gog-docs
+// might handle generic docs operations. The specific plugin takes precedence.
+//
+// Returns:
+//   - nil if no external command found (caller should return original error)
+//   - error from exec if external command found but execution failed
+//   - does not return if exec succeeds (replaces current process)
+func tryExternalCommand(args []string) error {
+	if len(args) == 0 {
+		return ErrExternalNotFound
+	}
+
+	path, remainingArgs := findExternalCommand(args)
+	if path == "" {
+		return ErrExternalNotFound
+	}
+
+	return execExternal(path, remainingArgs)
+}
+
+// findExternalCommand searches PATH for a matching external plugin binary.
+// Uses longest-first matching: tries most specific binary name first.
+// Returns the path to the binary and remaining arguments to pass to it.
+func findExternalCommand(args []string) (binaryPath string, remainingArgs []string) {
+	// Longest-first: start with all args, progressively try fewer
+	// Why: More specific plugins take precedence (e.g., gog-docs-headings over gog-docs)
+	for i := len(args); i > 0; i-- {
+		binaryName := externalCommandPrefix + strings.Join(args[:i], "-")
+		if path, err := exec.LookPath(binaryName); err == nil {
+			return path, args[i:]
+		}
+	}
+	return "", nil
+}
+
+// execExternal replaces the current process with the external command.
+// Uses syscall.Exec for true process replacement (no child process).
+func execExternal(binaryPath string, args []string) error {
+	// Build argv: binary name followed by remaining arguments
+	argv := append([]string{binaryPath}, args...)
+
+	// Use syscall.Exec to replace current process
+	// This is the standard pattern for CLI plugin dispatch (git, cargo)
+	return syscall.Exec(binaryPath, argv, os.Environ())
+}
+
+// LookPath is a variable to allow mocking in tests.
+// In production, this is exec.LookPath.
+var lookPath = exec.LookPath
+
+// findExternalCommandWithLookPath is like findExternalCommand but uses
+// the package-level lookPath variable for testability.
+func findExternalCommandWithLookPath(args []string) (binaryPath string, remainingArgs []string) {
+	for i := len(args); i > 0; i-- {
+		binaryName := externalCommandPrefix + strings.Join(args[:i], "-")
+		if path, err := lookPath(binaryName); err == nil {
+			return path, args[i:]
+		}
+	}
+	return "", nil
+}

internal/cmd/external_test.go

@@ -0,0 +1,161 @@
+package cmd
+
+import (
+	"errors"
+	"os/exec"
+	"testing"
+)
+
+// mockLookPath creates a mock LookPath function that returns success
+// for binaries in the "exists" set.
+func mockLookPath(exists map[string]string) func(string) (string, error) {
+	return func(name string) (string, error) {
+		if path, ok := exists[name]; ok {
+			return path, nil
+		}
+		return "", exec.ErrNotFound
+	}
+}
+
+func TestFindExternalCommand_LongestFirstMatching(t *testing.T) {
+	// This test documents the longest-first (greedy) matching behavior.
+	// Why longest-first: More specific plugins should take precedence.
+	// Example: gog-docs-headings should handle "docs headings" even if
+	// gog-docs also exists.
+
+	tests := []struct {
+		name             string
+		args             []string
+		existingBinaries map[string]string
+		wantPath         string
+		wantArgs         []string
+	}{
+		{
+			name: "exact match single arg",
+			args: []string{"docs"},
+			existingBinaries: map[string]string{
+				"gog-docs": "/usr/bin/gog-docs",
+			},
+			wantPath: "/usr/bin/gog-docs",
+			wantArgs: []string{},
+		},
+		{
+			name: "exact match two args",
+			args: []string{"docs", "headings"},
+			existingBinaries: map[string]string{
+				"gog-docs-headings": "/usr/bin/gog-docs-headings",
+			},
+			wantPath: "/usr/bin/gog-docs-headings",
+			wantArgs: []string{},
+		},
+		{
+			name: "longest match wins over shorter",
+			// When both gog-docs and gog-docs-headings exist,
+			// gog-docs-headings should win for "docs headings" args
+			args: []string{"docs", "headings"},
+			existingBinaries: map[string]string{
+				"gog-docs":          "/usr/bin/gog-docs",
+				"gog-docs-headings": "/usr/bin/gog-docs-headings",
+			},
+			wantPath: "/usr/bin/gog-docs-headings",
+			wantArgs: []string{},
+		},
+		{
+			name: "falls back to shorter when longer not found",
+			// gog-docs-headings-list doesn't exist, so fall back to gog-docs-headings
+			args: []string{"docs", "headings", "list"},
+			existingBinaries: map[string]string{
+				"gog-docs-headings": "/usr/bin/gog-docs-headings",
+			},
+			wantPath: "/usr/bin/gog-docs-headings",
+			wantArgs: []string{"list"},
+		},
+		{
+			name: "passes remaining args to plugin",
+			args: []string{"docs", "headings", "--docid", "ABC123"},
+			existingBinaries: map[string]string{
+				"gog-docs-headings": "/usr/bin/gog-docs-headings",
+			},
+			wantPath: "/usr/bin/gog-docs-headings",
+			wantArgs: []string{"--docid", "ABC123"},
+		},
+		{
+			name: "three-level nesting",
+			args: []string{"docs", "headings", "list", "--limit", "10"},
+			existingBinaries: map[string]string{
+				"gog-docs":               "/usr/bin/gog-docs",
+				"gog-docs-headings":      "/usr/bin/gog-docs-headings",
+				"gog-docs-headings-list": "/usr/bin/gog-docs-headings-list",
+			},
+			wantPath: "/usr/bin/gog-docs-headings-list",
+			wantArgs: []string{"--limit", "10"},
+		},
+		{
+			name:             "no match returns empty",
+			args:             []string{"unknown", "command"},
+			existingBinaries: map[string]string{},
+			wantPath:         "",
+			wantArgs:         nil,
+		},
+		{
+			name:             "empty args returns empty",
+			args:             []string{},
+			existingBinaries: map[string]string{},
+			wantPath:         "",
+			wantArgs:         nil,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Save and restore original lookPath
+			origLookPath := lookPath
+			defer func() { lookPath = origLookPath }()
+
+			lookPath = mockLookPath(tt.existingBinaries)
+
+			gotPath, gotArgs := findExternalCommandWithLookPath(tt.args)
+
+			if gotPath != tt.wantPath {
+				t.Errorf("path = %q, want %q", gotPath, tt.wantPath)
+			}
+
+			if !slicesEqual(gotArgs, tt.wantArgs) {
+				t.Errorf("args = %v, want %v", gotArgs, tt.wantArgs)
+			}
+		})
+	}
+}
+
+func TestTryExternalCommand_NotFound(t *testing.T) {
+	// Save and restore original lookPath
+	origLookPath := lookPath
+	defer func() { lookPath = origLookPath }()
+
+	lookPath = mockLookPath(map[string]string{})
+
+	err := tryExternalCommand([]string{"nonexistent"})
+	if !errors.Is(err, ErrExternalNotFound) {
+		t.Errorf("err = %v, want ErrExternalNotFound", err)
+	}
+}
+
+func TestTryExternalCommand_EmptyArgs(t *testing.T) {
+	err := tryExternalCommand([]string{})
+	if !errors.Is(err, ErrExternalNotFound) {
+		t.Errorf("err = %v, want ErrExternalNotFound", err)
+	}
+}
+
+// slicesEqual compares two string slices for equality.
+func slicesEqual(a, b []string) bool {
+	if len(a) != len(b) {
+		return false
+	}
+	for i := range a {
+		if a[i] != b[i] {
+			return false
+		}
+	}
+	return true
+}