diff --git a/docs/plans/2026-02-06-external-plugin-discovery-design.md b/docs/plans/2026-02-06-external-plugin-discovery-design.md new file mode 100644 index 00000000..52bebbb9 --- /dev/null +++ b/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 diff --git a/internal/cmd/external.go b/internal/cmd/external.go new file mode 100644 index 00000000..56f966be --- /dev/null +++ b/internal/cmd/external.go @@ -0,0 +1,326 @@ +package cmd + +import ( + "bytes" + "context" + "errors" + "os" + "os/exec" + "path/filepath" + "sort" + "strings" + "syscall" + "time" + + "github.com/steipete/gogcli/internal/config" +) + +// 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, matchedArgs := findExternalCommandWithMatched(args) + if path == "" { + return ErrExternalNotFound + } + + return execExternal(path, remainingArgs, matchedArgs) +} + +// 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) { + path, remaining, _ := findExternalCommandWithMatched(args) + return path, remaining +} + +// findExternalCommandWithMatched is like findExternalCommand but also returns +// the matched subcommand args (for building GOG_PLUGIN_INVOKED_AS). +func findExternalCommandWithMatched(args []string) (binaryPath string, remainingArgs []string, matchedArgs []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 := lookPath(binaryName); err == nil { + return path, args[i:], args[:i] + } + } + return "", nil, nil +} + +// execExternal replaces the current process with the external command. +// Uses syscall.Exec for true process replacement (no child process). +// +// Environment variables passed to plugins: +// - GOG_CORE_VERSION: Version of the gog CLI +// - GOG_CORE_PATH: Path to the gog binary (for plugins to call back) +// - GOG_CONFIG_PATH: Path to config file (for shared configuration) +// - GOG_PLUGIN_NAME: Binary name of the plugin being executed +// - GOG_PLUGIN_INVOKED_AS: How user invoked the command (e.g., "gog docs headings") +// - GOG_COLOR: Color preference from parent (auto/always/never) +// - GOG_OUTPUT_FORMAT: Output format if --json or --plain was specified +// +// Why pass env vars: Allows plugins to share configuration, reuse auth, +// and maintain consistent output formatting with the core CLI. +func execExternal(binaryPath string, args []string, matchedArgs []string) error { + // Build argv: binary name followed by remaining arguments + argv := append([]string{binaryPath}, args...) + + // Build environment with plugin-specific variables + env := buildPluginEnv(binaryPath, matchedArgs) + + // Use syscall.Exec to replace current process + // This is the standard pattern for CLI plugin dispatch (git, cargo) + return syscall.Exec(binaryPath, argv, env) +} + +// buildPluginEnv creates the environment for plugin execution. +// Inherits current environment and adds GOG_* variables for plugin integration. +func buildPluginEnv(binaryPath string, matchedArgs []string) []string { + // Start with current environment + env := os.Environ() + + // Add plugin-specific variables + pluginVars := map[string]string{ + "GOG_CORE_VERSION": version, + "GOG_PLUGIN_NAME": filepath.Base(binaryPath), + } + + // GOG_PLUGIN_INVOKED_AS: the command as user typed it + if len(matchedArgs) > 0 { + pluginVars["GOG_PLUGIN_INVOKED_AS"] = "gog " + strings.Join(matchedArgs, " ") + } + + // GOG_CORE_PATH: path to gog binary for callbacks + if corePath, err := os.Executable(); err == nil { + pluginVars["GOG_CORE_PATH"] = corePath + } + + // GOG_CONFIG_PATH: shared config file + if configPath, err := config.ConfigPath(); err == nil { + pluginVars["GOG_CONFIG_PATH"] = configPath + } + + // Inherit color preference if set + if color := os.Getenv("GOG_COLOR"); color != "" { + pluginVars["GOG_COLOR"] = color + } + + // Set output format if JSON or plain mode + if os.Getenv("GOG_JSON") == "true" { + pluginVars["GOG_OUTPUT_FORMAT"] = "json" + } else if os.Getenv("GOG_PLAIN") == "true" { + pluginVars["GOG_OUTPUT_FORMAT"] = "plain" + } + + // Add/override with our plugin variables + for k, v := range pluginVars { + env = setEnvVar(env, k, v) + } + + return env +} + +// setEnvVar sets or overrides an environment variable in a []string env. +func setEnvVar(env []string, key, value string) []string { + prefix := key + "=" + for i, e := range env { + if strings.HasPrefix(e, prefix) { + env[i] = prefix + value + return env + } + } + return append(env, prefix+value) +} + +// 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 +} + +// helpOnelinerTimeout is the maximum time to wait for --help-oneliner response. +// Short timeout to keep help responsive; plugins that don't respond are skipped. +const helpOnelinerTimeout = 100 * time.Millisecond + +// ExternalPlugin represents a discovered external plugin binary. +type ExternalPlugin struct { + // Name is the plugin binary name without prefix (e.g., "docs-headings") + Name string + // Path is the full path to the binary + Path string + // Subcommands is the command hierarchy (e.g., ["docs", "headings"]) + Subcommands []string + // Oneliner is the short description from --help-oneliner (may be empty) + Oneliner string +} + +// CommandName returns the user-facing command (e.g., "docs headings") +func (p *ExternalPlugin) CommandName() string { + return strings.Join(p.Subcommands, " ") +} + +// DiscoverExternalPlugins scans PATH for all gog-* binaries. +// Returns deduplicated list sorted by command name. +// +// Discovery happens lazily (only when help is requested) to avoid +// performance impact on normal command execution. +func DiscoverExternalPlugins() []ExternalPlugin { + seen := make(map[string]bool) + var plugins []ExternalPlugin + + pathDirs := filepath.SplitList(os.Getenv("PATH")) + for _, dir := range pathDirs { + entries, err := os.ReadDir(dir) + if err != nil { + continue // Skip unreadable directories + } + + for _, entry := range entries { + name := entry.Name() + if !strings.HasPrefix(name, externalCommandPrefix) { + continue + } + if entry.IsDir() { + continue + } + + // Check if executable (skip if not) + fullPath := filepath.Join(dir, name) + info, err := entry.Info() + if err != nil { + continue + } + if info.Mode()&0111 == 0 { + continue // Not executable + } + + // Extract plugin name (remove prefix) + pluginName := strings.TrimPrefix(name, externalCommandPrefix) + if pluginName == "" { + continue + } + + // Deduplicate: first occurrence in PATH wins + // Why: Matches exec.LookPath behavior and user expectations + if seen[pluginName] { + continue + } + seen[pluginName] = true + + plugins = append(plugins, ExternalPlugin{ + Name: pluginName, + Path: fullPath, + Subcommands: strings.Split(pluginName, "-"), + }) + } + } + + // Sort by command name for consistent display + sort.Slice(plugins, func(i, j int) bool { + return plugins[i].Name < plugins[j].Name + }) + + return plugins +} + +// FetchOneliners queries each plugin for its --help-oneliner description. +// Uses short timeout to keep help responsive; unresponsive plugins get empty description. +// +// Protocol: Plugins should respond to --help-oneliner with a single line (≤80 chars) +// describing what they do. Exit code 0 indicates success. +func FetchOneliners(plugins []ExternalPlugin) []ExternalPlugin { + result := make([]ExternalPlugin, len(plugins)) + copy(result, plugins) + + for i := range result { + result[i].Oneliner = fetchOneliner(result[i].Path) + } + + return result +} + +// fetchOneliner invokes a plugin with --help-oneliner and returns the response. +// Returns empty string on timeout, error, or non-zero exit code. +func fetchOneliner(binaryPath string) string { + ctx, cancel := context.WithTimeout(context.Background(), helpOnelinerTimeout) + defer cancel() + + cmd := exec.CommandContext(ctx, binaryPath, "--help-oneliner") + var stdout bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = nil // Ignore stderr + + if err := cmd.Run(); err != nil { + return "" // Timeout, not found, or non-zero exit + } + + // Take first line only, trim whitespace + oneliner := strings.TrimSpace(stdout.String()) + if idx := strings.IndexByte(oneliner, '\n'); idx >= 0 { + oneliner = oneliner[:idx] + } + + // Truncate to 80 chars for display + if len(oneliner) > 80 { + oneliner = oneliner[:77] + "..." + } + + return oneliner +} + +// GroupPluginsByTopLevel groups plugins by their first subcommand. +// Used to display plugins under their parent command in help output. +// Example: gog-docs-headings and gog-docs-bookmarks both under "docs" +func GroupPluginsByTopLevel(plugins []ExternalPlugin) map[string][]ExternalPlugin { + groups := make(map[string][]ExternalPlugin) + for _, p := range plugins { + if len(p.Subcommands) == 0 { + continue + } + topLevel := p.Subcommands[0] + groups[topLevel] = append(groups[topLevel], p) + } + return groups +} diff --git a/internal/cmd/external_test.go b/internal/cmd/external_test.go new file mode 100644 index 00000000..6784fe41 --- /dev/null +++ b/internal/cmd/external_test.go @@ -0,0 +1,372 @@ +package cmd + +import ( + "errors" + "os/exec" + "strings" + "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 +} + +func TestExternalPlugin_CommandName(t *testing.T) { + tests := []struct { + name string + subcommands []string + want string + }{ + { + name: "single subcommand", + subcommands: []string{"docs"}, + want: "docs", + }, + { + name: "two subcommands", + subcommands: []string{"docs", "headings"}, + want: "docs headings", + }, + { + name: "three subcommands", + subcommands: []string{"docs", "headings", "list"}, + want: "docs headings list", + }, + { + name: "empty subcommands", + subcommands: []string{}, + want: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + p := ExternalPlugin{Subcommands: tt.subcommands} + if got := p.CommandName(); got != tt.want { + t.Errorf("CommandName() = %q, want %q", got, tt.want) + } + }) + } +} + +func TestGroupPluginsByTopLevel(t *testing.T) { + // Test that plugins are correctly grouped by their first subcommand. + // This is used to display plugins under their parent command in help. + + plugins := []ExternalPlugin{ + {Name: "docs-headings", Subcommands: []string{"docs", "headings"}}, + {Name: "docs-bookmarks", Subcommands: []string{"docs", "bookmarks"}}, + {Name: "sheets-formulas", Subcommands: []string{"sheets", "formulas"}}, + {Name: "hello", Subcommands: []string{"hello"}}, + } + + groups := GroupPluginsByTopLevel(plugins) + + // Check docs group + if len(groups["docs"]) != 2 { + t.Errorf("docs group has %d plugins, want 2", len(groups["docs"])) + } + + // Check sheets group + if len(groups["sheets"]) != 1 { + t.Errorf("sheets group has %d plugins, want 1", len(groups["sheets"])) + } + + // Check hello group (single-level plugin) + if len(groups["hello"]) != 1 { + t.Errorf("hello group has %d plugins, want 1", len(groups["hello"])) + } + + // Check total groups + if len(groups) != 3 { + t.Errorf("got %d groups, want 3", len(groups)) + } +} + +func TestGroupPluginsByTopLevel_EmptySubcommands(t *testing.T) { + // Plugins with empty subcommands should be skipped + plugins := []ExternalPlugin{ + {Name: "", Subcommands: []string{}}, + {Name: "docs", Subcommands: []string{"docs"}}, + } + + groups := GroupPluginsByTopLevel(plugins) + + if len(groups) != 1 { + t.Errorf("got %d groups, want 1", len(groups)) + } + if len(groups["docs"]) != 1 { + t.Errorf("docs group has %d plugins, want 1", len(groups["docs"])) + } +} + +func TestSetEnvVar(t *testing.T) { + tests := []struct { + name string + env []string + key string + value string + wantLen int + wantLast string + }{ + { + name: "add new variable to empty env", + env: []string{}, + key: "GOG_TEST", + value: "value", + wantLen: 1, + wantLast: "GOG_TEST=value", + }, + { + name: "add new variable to existing env", + env: []string{"PATH=/usr/bin", "HOME=/home/user"}, + key: "GOG_TEST", + value: "value", + wantLen: 3, + wantLast: "GOG_TEST=value", + }, + { + name: "override existing variable", + env: []string{"PATH=/usr/bin", "GOG_TEST=old"}, + key: "GOG_TEST", + value: "new", + wantLen: 2, + wantLast: "GOG_TEST=new", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := setEnvVar(tt.env, tt.key, tt.value) + if len(result) != tt.wantLen { + t.Errorf("len = %d, want %d", len(result), tt.wantLen) + } + + // Check that the variable is set correctly + found := false + for _, e := range result { + if e == tt.wantLast { + found = true + break + } + } + if !found { + t.Errorf("did not find %q in result %v", tt.wantLast, result) + } + }) + } +} + +func TestBuildPluginEnv(t *testing.T) { + // Test that buildPluginEnv adds expected variables + env := buildPluginEnv("/usr/bin/gog-docs-headings", []string{"docs", "headings"}) + + // Check for expected variables + expectations := map[string]bool{ + "GOG_CORE_VERSION": false, + "GOG_PLUGIN_NAME": false, + "GOG_PLUGIN_INVOKED_AS": false, + } + + for _, e := range env { + for key := range expectations { + if strings.HasPrefix(e, key+"=") { + expectations[key] = true + } + } + } + + for key, found := range expectations { + if !found { + t.Errorf("expected %s to be set in plugin env", key) + } + } + + // Verify specific values + for _, e := range env { + if strings.HasPrefix(e, "GOG_PLUGIN_NAME=") { + if e != "GOG_PLUGIN_NAME=gog-docs-headings" { + t.Errorf("GOG_PLUGIN_NAME = %q, want gog-docs-headings", e) + } + } + if strings.HasPrefix(e, "GOG_PLUGIN_INVOKED_AS=") { + if e != "GOG_PLUGIN_INVOKED_AS=gog docs headings" { + t.Errorf("GOG_PLUGIN_INVOKED_AS = %q, want 'gog docs headings'", e) + } + } + } +} + +func TestFindExternalCommandWithMatched(t *testing.T) { + // Test that matchedArgs is returned correctly for env var construction + origLookPath := lookPath + defer func() { lookPath = origLookPath }() + + lookPath = mockLookPath(map[string]string{ + "gog-docs-headings": "/usr/bin/gog-docs-headings", + }) + + path, remaining, matched := findExternalCommandWithMatched([]string{"docs", "headings", "--docid", "ABC"}) + + if path != "/usr/bin/gog-docs-headings" { + t.Errorf("path = %q, want /usr/bin/gog-docs-headings", path) + } + + if !slicesEqual(remaining, []string{"--docid", "ABC"}) { + t.Errorf("remaining = %v, want [--docid ABC]", remaining) + } + + if !slicesEqual(matched, []string{"docs", "headings"}) { + t.Errorf("matched = %v, want [docs headings]", matched) + } +} diff --git a/internal/cmd/help_printer.go b/internal/cmd/help_printer.go index 8003c908..02ed0ed0 100644 --- a/internal/cmd/help_printer.go +++ b/internal/cmd/help_printer.go @@ -47,6 +47,7 @@ func helpPrinter(options kong.HelpOptions, ctx *kong.Context) error { out := rewriteCommandSummaries(buf.String(), ctx.Selected()) out = injectBuildLine(out) + out = injectExternalPlugins(out, ctx.Selected()) out = colorizeHelp(out, helpProfile(origStdout, helpColorMode(ctx.Args))) _, err := io.WriteString(origStdout, out) return err @@ -138,10 +139,16 @@ func colorizeHelp(out string, profile termenv.Profile) string { } inCommands := false + inPlugins := false lines := strings.Split(out, "\n") for i, line := range lines { if line == "Commands:" { inCommands = true + inPlugins = false + } + if line == "Plugins:" { + inPlugins = true + inCommands = false } switch { case strings.HasPrefix(line, "Usage:"): @@ -150,6 +157,8 @@ func colorizeHelp(out string, profile termenv.Profile) string { lines[i] = section(line) case line == "Commands:": lines[i] = section(line) + case line == "Plugins:": + lines[i] = section(line) case line == "Arguments:": lines[i] = section(line) case strings.HasPrefix(line, "Build:") || line == "Config:": @@ -160,6 +169,8 @@ func colorizeHelp(out string, profile termenv.Profile) string { lines[i] = colorizeCommandSummaryLine(line, cmdName, dim) case inCommands && strings.HasPrefix(line, " ") && strings.TrimSpace(line) != "": lines[i] = " " + dim(strings.TrimPrefix(line, " ")) + case inPlugins && strings.HasPrefix(line, " ") && strings.TrimSpace(line) != "": + lines[i] = colorizePluginLine(line, cmdName, dim) } } return strings.Join(lines, "\n") @@ -190,6 +201,33 @@ func colorizeCommandSummaryLine(line string, cmdName func(string) string, dim fu return " " + styled + " " + tail } +// colorizePluginLine colorizes a plugin line in the Plugins: section. +// Plugin lines look like: " docs headings List document headings" +func colorizePluginLine(line string, cmdName func(string) string, dim func(string) string) string { + if !strings.HasPrefix(line, " ") { + return line + } + rest := strings.TrimPrefix(line, " ") + if rest == "" { + return line + } + + // Find where the command name ends (two or more spaces indicate description start) + parts := strings.SplitN(rest, " ", 2) + name := parts[0] + if name == "" { + return line + } + + styled := cmdName(name) + if len(parts) == 1 { + return " " + styled + } + + // Description is dimmed + return " " + styled + " " + dim(parts[1]) +} + func rewriteCommandSummaries(out string, selected *kong.Node) string { if selected == nil { return out @@ -223,3 +261,59 @@ func guessColumns(w io.Writer) int { } return 80 } + +// injectExternalPlugins adds discovered external plugins to help output. +// Plugins are displayed in a separate "Plugins:" section after "Commands:". +// +// Discovery is lazy: only scans PATH when help is requested. +// The --help-oneliner protocol is used to get plugin descriptions. +// Plugins that don't respond within 100ms get no description. +func injectExternalPlugins(out string, selected *kong.Node) string { + // Only show plugins in root help (not subcommand help) + if selected != nil && selected.Parent != nil { + return out + } + + plugins := DiscoverExternalPlugins() + if len(plugins) == 0 { + return out + } + + // Fetch oneliners (with timeout) + plugins = FetchOneliners(plugins) + + // Build plugins section + var sb strings.Builder + sb.WriteString("\nPlugins:\n") + + // Find max command name length for alignment + maxLen := 0 + for _, p := range plugins { + if len(p.CommandName()) > maxLen { + maxLen = len(p.CommandName()) + } + } + + for _, p := range plugins { + cmdName := p.CommandName() + padding := strings.Repeat(" ", maxLen-len(cmdName)+2) + if p.Oneliner != "" { + sb.WriteString(fmt.Sprintf(" %s%s%s\n", cmdName, padding, p.Oneliner)) + } else { + sb.WriteString(fmt.Sprintf(" %s\n", cmdName)) + } + } + + // Insert before "Run ... --help" line or at end + lines := strings.Split(out, "\n") + for i, line := range lines { + if strings.HasPrefix(line, "Run \"") && strings.HasSuffix(line, " --help\" for more information on a command.") { + before := strings.Join(lines[:i], "\n") + after := strings.Join(lines[i:], "\n") + return before + sb.String() + after + } + } + + // Append at end if no "Run ... --help" line found + return out + sb.String() +} diff --git a/internal/cmd/root.go b/internal/cmd/root.go index 28012deb..c89b95f4 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -85,6 +85,17 @@ func Execute(args []string) (err error) { kctx, err := parser.Parse(args) if err != nil { + // External command discovery (cargo/git-style plugins) + // Design: Post-parse fallback (Option B) + // Why: Built-in commands always take precedence over external plugins. + // This prevents accidental or malicious shadowing of core functionality + // and follows the git/cargo convention where built-ins win. + // See: https://github.com/steipete/gogcli/issues/188 + if extErr := tryExternalCommand(args); extErr == nil || !errors.Is(extErr, ErrExternalNotFound) { + // Either external command executed (replaced process) or exec failed + return extErr + } + // No external command found; return original Kong parse error parsedErr := wrapParseError(err) _, _ = fmt.Fprintln(os.Stderr, errfmt.Format(parsedErr)) return parsedErr