diff --git a/internal/install/clients.go b/internal/install/clients.go index 67b8859..b8a1be1 100644 --- a/internal/install/clients.go +++ b/internal/install/clients.go @@ -50,6 +50,16 @@ func (c Client) InstallCmdDisplay(binPath string) string { return strings.Join(out, " ") } +// CLIOnPath reports whether the client's CLI binary resolves via PATH. +// Pure-config clients (empty CLIProbe) are treated as callable. +func (c Client) CLIOnPath() bool { + if c.CLIProbe == "" { + return true + } + _, err := exec.LookPath(c.CLIProbe) + return err == nil +} + // Detected returns true if this client appears installed on the system. // Check order: CLI on PATH → config file exists. func (c Client) Detected() bool { @@ -70,6 +80,19 @@ func (c Client) Detected() bool { return false } +// partitionCallableClients splits detected clients into those whose CLI is on +// PATH (callable) and those detected only via a config-file probe (skipped). +func partitionCallableClients(clients []Client) (callable, skipped []Client) { + for _, c := range clients { + if c.CLIOnPath() { + callable = append(callable, c) + } else { + skipped = append(skipped, c) + } + } + return callable, skipped +} + // Clients is the supported-client registry. Adding a new client means // adding a struct literal here and (optionally) a unit test. var Clients = []Client{ diff --git a/internal/install/init.go b/internal/install/init.go index 88a9560..8077329 100644 --- a/internal/install/init.go +++ b/internal/install/init.go @@ -145,16 +145,17 @@ func Init(ctx context.Context, repoRoot string, opts InitOptions) (*InitResult, // Detect which MCP host(s) are present. opts.clients lets tests // inject a fake set without running real detection. - var detected []Client + var allDetected []Client if opts.clients != nil { for _, c := range opts.clients { if c.Detected() { - detected = append(detected, c) + allDetected = append(allDetected, c) } } } else { - detected = detectHosts() + allDetected = detectHosts() } + callableClients, skippedMissingCLI := partitionCallableClients(allDetected) binPath := "guild" if exe, err := resolveAbsBinPath(os.Executable); err == nil { binPath = exe @@ -178,11 +179,15 @@ func Init(ctx context.Context, repoRoot string, opts InitOptions) (*InitResult, case agentsSkip: fmt.Fprintln(opts.Out, " [✓] AGENTS.md — guild section up-to-date → skip") } - if len(detected) > 0 { - for _, c := range detected { + if len(allDetected) > 0 { + for _, c := range callableClients { fmt.Fprintf(opts.Out, " [?] register guild MCP — detected: %s\n command: %s\n", c.Name, c.InstallCmdDisplay(binPath)) } + for _, c := range skippedMissingCLI { + fmt.Fprintf(opts.Out, " [i] register guild MCP — skipping %s: %s not on PATH\n", + c.Name, c.CLIProbe) + } } else { fmt.Fprintln(opts.Out, " [?] register guild MCP — no host detected; see `guild mcp install` for options") } @@ -293,7 +298,7 @@ func Init(ctx context.Context, repoRoot string, opts InitOptions) (*InitResult, // Delegate to MCPInstall with Run: true so each detected client gets a // per-client [Y/n] confirm and the registration command actually executes. // No-client path keeps the manual-setup hint. - if len(detected) > 0 { + if len(callableClients) > 0 { fmt.Fprintln(opts.Out) execFn := opts.executableFn if execFn == nil { @@ -304,14 +309,14 @@ func Init(ctx context.Context, repoRoot string, opts InitOptions) (*InitResult, Yes: opts.Yes, Out: opts.Out, In: opts.In, - clients: detected, + clients: callableClients, executableFn: execFn, execCmdFn: opts.execCmdFn, } if _, err := MCPInstall(ctx, mcpOpts); err != nil { return nil, fmt.Errorf("install: mcp register: %w", err) } - } else { + } else if len(allDetected) == 0 { fmt.Fprintln(opts.Out) fmt.Fprintln(opts.Out, " [i] no MCP client detected — see `guild mcp install` for manual setup") } diff --git a/internal/install/init_test.go b/internal/install/init_test.go index f8fdd01..ee58238 100644 --- a/internal/install/init_test.go +++ b/internal/install/init_test.go @@ -10,6 +10,29 @@ import ( "testing" ) +// configOnlyClient builds a Client detected via its config probe but whose +// CLI binary is absent from PATH. Used to exercise issue #90. +func configOnlyClient(t *testing.T, name, cliProbe, configRelPath string) Client { + t.Helper() + home := t.TempDir() + configPath := filepath.Join(home, strings.TrimPrefix(configRelPath, "~/")) + if err := os.MkdirAll(filepath.Dir(configPath), 0o755); err != nil { + t.Fatalf("mkdir config dir: %v", err) + } + if err := os.WriteFile(configPath, []byte("{}"), 0o644); err != nil { + t.Fatalf("write config probe: %v", err) + } + t.Setenv("HOME", home) + return Client{ + Name: name, + CLIProbe: cliProbe, + ConfigProbe: configRelPath, + InstallArgv: func(binPath string) []string { + return []string{cliProbe, "mcp", "add", "guild", "--", binPath, "mcp", "serve"} + }, + } +} + // fakeClient builds a Client that always reports Detected()=true by pointing // CLIProbe at a real executable on PATH. Paired with an injected execCmdFn, // lets tests observe the MCP registration path without running real CLI. @@ -566,3 +589,40 @@ func TestInit_MCPRegistration_NoClient_PrintsHint(t *testing.T) { t.Errorf("expected manual-setup hint; got:\n%s", out.String()) } } + +// Config-detected client without CLI on PATH must be skipped in the init +// plan and must not invoke registration (issue #90). +func TestInit_MCPRegistration_SkipsConfigOnlyClientWithoutCLI(t *testing.T) { + ctx := context.Background() + dir := makeRepo(t, "skipmcp") + loreDB, questDB := testDBPaths(t) + var out bytes.Buffer + var calls [][]string + + missingCLI := "nonexistent-init-cli-xyzzy-99" + client := configOnlyClient(t, "Cursor", missingCLI, "~/.cursor/mcp.json") + + _, err := Init(ctx, dir, InitOptions{ + Yes: true, + Out: &out, + In: &bytes.Buffer{}, + LoreDBPath: loreDB, + QuestDBPath: questDB, + clients: []Client{client}, + execCmdFn: recordingExec(&calls), + executableFn: fakeExecutable(t), + }) + if err != nil { + t.Fatalf("Init: %v", err) + } + output := out.String() + if !strings.Contains(output, "skipping Cursor: "+missingCLI+" not on PATH") { + t.Errorf("expected skip notice in plan; got:\n%s", output) + } + if strings.Contains(output, "no MCP client detected") { + t.Errorf("should not print no-client hint when client was detected but skipped:\n%s", output) + } + if len(calls) != 0 { + t.Errorf("registration must not run when CLI missing, got %d calls: %+v", len(calls), calls) + } +}