Merge pull request #893 from entireio/nodo/ext-agents-detection

Fix external agents detection

Author: nodo

cmd/entire/cli/setup.go

@@ -916,7 +916,8 @@ func setupAgentHooks(ctx context.Context, ag agent.Agent, localDev, forceHooks b
 // Returns the detected/selected agents and any error.
 //
 // On first run (no hooks installed):
-//   - Single detected agent: used automatically
+//   - Single detected built-in agent: used automatically
+//   - Single detected external agent: interactive multi-select prompt
 //   - Multiple/no detected agents: interactive multi-select prompt
 //
 // On re-run (hooks already installed):
@@ -937,8 +938,10 @@ func detectOrSelectAgent(ctx context.Context, w io.Writer, selectFn func(availab
 	if !hasInstalledHooks {
 		switch {
 		case len(detected) == 1:
-			fmt.Fprintf(w, "Detected agent: %s\n\n", detected[0].Type())
-			return detected, nil
+			if isBuiltInAgent(detected[0]) {
+				fmt.Fprintf(w, "Detected agent: %s\n\n", detected[0].Type())
+				return detected, nil
+			}
 
 		case len(detected) > 1:
 			agentTypes := make([]string, 0, len(detected))
@@ -977,15 +980,17 @@ func detectOrSelectAgent(ctx context.Context, w io.Writer, selectFn func(availab
 
 	// Build pre-selection set.
 	// On re-run: only pre-select agents with hooks installed (respect prior deselection).
-	// On first run: pre-select all detected agents.
+	// On first run: pre-select detected built-in agents only.
 	preSelectedSet := make(map[types.AgentName]struct{})
 	if hasInstalledHooks {
 		for _, name := range installedAgentNames {
 			preSelectedSet[name] = struct{}{}
 		}
 	} else {
 		for _, ag := range detected {
-			preSelectedSet[ag.Name()] = struct{}{}
+			if isBuiltInAgent(ag) {
+				preSelectedSet[ag.Name()] = struct{}{}
+			}
 		}
 	}
 
@@ -1070,6 +1075,10 @@ func detectOrSelectAgent(ctx context.Context, w io.Writer, selectFn func(availab
 	return selectedAgents, nil
 }
 
+func isBuiltInAgent(ag agent.Agent) bool {
+	return !external.IsExternal(ag)
+}
+
 // canPromptInteractively checks if we can show interactive prompts.
 // Returns false when running in CI, tests, or other non-interactive environments.
 func canPromptInteractively() bool {

cmd/entire/cli/setup_test.go

@@ -6,18 +6,22 @@ import (
 	"encoding/json"
 	"errors"
 	"os"
+	"os/exec"
 	"path/filepath"
+	"slices"
 	"strings"
 	"testing"
 
 	"github.com/entireio/cli/cmd/entire/cli/agent"
 	_ "github.com/entireio/cli/cmd/entire/cli/agent/claudecode"
+	"github.com/entireio/cli/cmd/entire/cli/agent/external"
 	_ "github.com/entireio/cli/cmd/entire/cli/agent/geminicli"
+	"github.com/entireio/cli/cmd/entire/cli/agent/types"
 	"github.com/entireio/cli/cmd/entire/cli/paths"
 	"github.com/entireio/cli/cmd/entire/cli/session"
 	"github.com/entireio/cli/cmd/entire/cli/settings"
 	"github.com/entireio/cli/cmd/entire/cli/strategy"
-	"github.com/go-git/go-git/v6"
+	"github.com/entireio/cli/cmd/entire/cli/testutil"
 )
 
 // Note: Tests for hook manipulation functions (addHookToMatcher, hookCommandExists, etc.)
@@ -29,6 +33,7 @@ import (
 func setupTestDir(t *testing.T) string {
 	t.Helper()
 	tmpDir := t.TempDir()
+	hideExternalAgentsFromPath(t)
 	t.Chdir(tmpDir)
 	paths.ClearWorktreeRootCache()
 	session.ClearGitCommonDirCache()
@@ -39,9 +44,7 @@ func setupTestDir(t *testing.T) string {
 func setupTestRepo(t *testing.T) {
 	t.Helper()
 	tmpDir := setupTestDir(t)
-	if _, err := git.PlainInit(tmpDir, false); err != nil {
-		t.Fatalf("Failed to init repo: %v", err)
-	}
+	testutil.InitRepo(t, tmpDir)
 }
 
 // writeSettings writes settings content to the settings file.
@@ -56,6 +59,110 @@ func writeSettings(t *testing.T, content string) {
 	}
 }
 
+func hideExternalAgentsFromPath(t *testing.T) {
+	t.Helper()
+
+	pathDir := t.TempDir()
+	for _, name := range []string{"git", "sh"} {
+		if err := preserveToolOnPath(name, pathDir); err != nil {
+			t.Fatalf("preserve %s on PATH: %v", name, err)
+		}
+	}
+
+	t.Setenv("PATH", pathDir)
+}
+
+func TestSetupTestDir_HidesExternalAgentsButKeepsGitAvailable(t *testing.T) {
+	gitPath, err := exec.LookPath("git")
+	if err != nil {
+		t.Skip("git not available")
+	}
+
+	sharedDir := t.TempDir()
+	if err := copyExecutable(gitPath, filepath.Join(sharedDir, "git")); err != nil {
+		t.Fatalf("copy git executable: %v", err)
+	}
+	writeExternalAgentBinary(t, sharedDir, "ext-shared-dir")
+	t.Setenv("PATH", sharedDir)
+
+	setupTestDir(t)
+
+	if _, err := exec.LookPath("git"); err != nil {
+		t.Fatalf("expected git to remain available after test PATH isolation: %v", err)
+	}
+	if _, err := exec.LookPath("entire-agent-ext-shared-dir"); err == nil {
+		t.Fatal("expected external agent to be hidden from PATH")
+	}
+}
+
+func preserveToolOnPath(name, dstDir string) error {
+	src, err := exec.LookPath(name)
+	if err != nil {
+		if errors.Is(err, exec.ErrNotFound) {
+			return nil
+		}
+		return err
+	}
+
+	return copyExecutable(src, filepath.Join(dstDir, filepath.Base(src)))
+}
+
+func copyExecutable(src, dst string) error {
+	info, err := os.Stat(src)
+	if err != nil {
+		return err
+	}
+
+	if err := os.Symlink(src, dst); err == nil {
+		return nil
+	}
+	if err := os.Link(src, dst); err == nil {
+		return nil
+	}
+
+	data, err := os.ReadFile(src)
+	if err != nil {
+		return err
+	}
+
+	return os.WriteFile(dst, data, info.Mode())
+}
+
+func writeExternalAgentBinary(t *testing.T, dir, name string) {
+	t.Helper()
+
+	script := `#!/bin/sh
+case "$1" in
+  info)
+    echo '{"protocol_version":1,"name":"` + name + `","type":"` + name + ` Agent","description":"External test agent","is_preview":false,"protected_dirs":[],"hook_names":["stop"],"capabilities":{"hooks":true}}'
+    ;;
+  detect)
+    if [ "$ENTIRE_TEST_EXTERNAL_PRESENT" = "1" ]; then
+      echo '{"present": true}'
+    else
+      echo '{"present": false}'
+    fi
+    ;;
+  install-hooks)
+    echo '{"hooks_installed": 1}'
+    ;;
+  uninstall-hooks)
+    exit 0
+    ;;
+  are-hooks-installed)
+    echo '{"installed": false}'
+    ;;
+  *)
+    echo '{}'
+    ;;
+esac
+`
+
+	if err := os.WriteFile(filepath.Join(dir, "entire-agent-"+name), []byte(script), 0o755); err != nil {
+		t.Fatalf("Failed to write external agent binary: %v", err)
+	}
+}
+
 func TestRunEnable(t *testing.T) {
 	setupTestDir(t)
 	writeSettings(t, testSettingsDisabled)
@@ -935,6 +1042,90 @@ func TestDetectOrSelectAgent_GeminiDetected(t *testing.T) {
 	}
 }
 
+func TestDetectOrSelectAgent_OnlyExternalDetected_WithTTY_PromptsUser(t *testing.T) {
+	// Cannot use t.Parallel() because we use t.Chdir, t.Setenv, and global agent registration
+	if _, err := exec.LookPath("sh"); err != nil {
+		t.Skip("sh not available")
+	}
+
+	setupTestRepo(t)
+	t.Setenv("ENTIRE_TEST_TTY", "1")
+
+	externalAgentName := "ext-prompt-pi"
+	externalDir := t.TempDir()
+	writeExternalAgentBinary(t, externalDir, externalAgentName)
+	t.Setenv("ENTIRE_TEST_EXTERNAL_PRESENT", "1")
+	t.Setenv("PATH", externalDir)
+
+	external.DiscoverAndRegisterAlways(context.Background())
+
+	var receivedAvailable []string
+	selectFn := func(available []string) ([]string, error) {
+		receivedAvailable = available
+		return []string{string(agent.AgentNameClaudeCode)}, nil
+	}
+
+	var buf bytes.Buffer
+	agents, err := detectOrSelectAgent(context.Background(), &buf, selectFn)
+	if err != nil {
+		t.Fatalf("detectOrSelectAgent() error = %v", err)
+	}
+
+	if len(receivedAvailable) == 0 {
+		t.Fatal("Expected interactive prompt when only an external agent is detected")
+	}
+	if !slices.Contains(receivedAvailable, externalAgentName) {
+		t.Fatalf("Expected external agent %q in options, got %v", externalAgentName, receivedAvailable)
+	}
+	if !slices.Contains(receivedAvailable, string(agent.AgentNameClaudeCode)) {
+		t.Fatalf("Expected built-in agent options alongside external agent, got %v", receivedAvailable)
+	}
+	if len(agents) != 1 || agents[0].Name() != agent.AgentNameClaudeCode {
+		t.Fatalf("Expected selected Claude Code agent, got %v", agents)
+	}
+	if strings.Contains(buf.String(), "Detected agent:") {
+		t.Errorf("Expected external-only detection to prompt instead of auto-selecting, got output: %s", buf.String())
+	}
+}
+
+func TestIsBuiltInAgent_ExternalAgent_False(t *testing.T) {
+	if _, err := exec.LookPath("sh"); err != nil {
+		t.Skip("sh not available")
+	}
+
+	setupTestRepo(t)
+
+	externalAgentName := "ext-preselect-pi"
+	externalDir := t.TempDir()
+	writeExternalAgentBinary(t, externalDir, externalAgentName)
+	t.Setenv("ENTIRE_TEST_EXTERNAL_PRESENT", "1")
+	t.Setenv("PATH", externalDir)
+
+	external.DiscoverAndRegisterAlways(context.Background())
+
+	externalAgent, err := agent.Get(types.AgentName(externalAgentName))
+	if err != nil {
+		t.Fatalf("failed to get external agent %q: %v", externalAgentName, err)
+	}
+
+	if isBuiltInAgent(externalAgent) {
+		t.Fatalf("expected external agent %q to not be treated as built-in", externalAgentName)
+	}
+}
+
+func TestIsBuiltInAgent_BuiltInAgent_True(t *testing.T) {
+	t.Parallel()
+
+	claudeAgent, err := agent.Get(agent.AgentNameClaudeCode)
+	if err != nil {
+		t.Fatalf("failed to get claude agent: %v", err)
+	}
+
+	if !isBuiltInAgent(claudeAgent) {
+		t.Fatal("expected built-in agent to be treated as built-in")
+	}
+}
+
 func TestDetectOrSelectAgent_NoDetection_NoTTY_FallsBackToDefault(t *testing.T) {
 	// Cannot use t.Parallel() because we use t.Chdir and t.Setenv
 	setupTestRepo(t)