From 8922a9a2eeec866d32a10b650976f56cd548b55d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Almeida?= <57680069+zeval@users.noreply.github.com> Date: Thu, 18 Jun 2026 13:07:40 +0100 Subject: [PATCH] fix(backend): prefer local copilot CLI over npx at launch (#1384) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(backend): prefer local copilot CLI over npx at launch IsInstalled probed the copilot binary for the UI badge but launch always ran `npx -y @github/copilot`, paying an npm registry round-trip every start — minutes behind a private registry. Reuse the env-correct binary check (standalone LookPath, SSH preflight probe) so when copilot is on PATH the lifecycle runs it directly; containerized runtimes keep npx. * fix(backend): make SSH native-binary probe non-fatal on transport error The native-binary probe is an optimisation, not a requirement. A transient SSH transport error (MaxSessions, reset) made "Verifying agent binary" abort the launch, blaming copilot before npx was tried. Treat probe transport errors as a miss and fall through to the required default-command probe, which reports failures with proper context. Also call NativeBinaryName() once. * fix(backend): clear stale native-binary metadata before SSH probe A pre-existing or stale MetadataKeyNativeBinary in req.Metadata could make the command builder prefer copilot --acp on an npx-only host even when this probe misses. Clear it up front so only a positive probe re-sets it. * refactor(backend): use resolved RuntimeName for native-binary check buildExecutionFromInstance re-derived the runtime from req.ExecutorType while promoteWorkspaceExecution read execution.RuntimeName. Use the already resolved execution.RuntimeName (set from rt.Name()) in both, so the two call sites are consistent. * fix(backend): reflect local copilot CLI in profile command preview The profile setup command preview always rendered `npx -y @github/copilot` because PreviewAgentCommand built the command without the native-binary probe. The preview is not bound to an executor, so assume the default standalone host and probe PATH (mirroring the standalone launch path): show `copilot --acp` when the CLI is installed, npx otherwise. --- apps/backend/internal/agent/agents/agent.go | 16 ++++ .../internal/agent/agents/copilot_acp.go | 19 +++- .../internal/agent/agents/copilot_acp_test.go | 30 +++++++ .../agent/runtime/lifecycle/executor_ssh.go | 53 +++++++++++- .../agent/runtime/lifecycle/manager_launch.go | 24 ++++-- .../runtime/lifecycle/manager_launch_test.go | 14 +-- .../agent/runtime/lifecycle/native_binary.go | 47 ++++++++++ .../runtime/lifecycle/native_binary_test.go | 86 +++++++++++++++++++ .../agent/settings/controller/agent_config.go | 28 +++++- .../settings/controller/controller_test.go | 36 ++++++++ 10 files changed, 329 insertions(+), 24 deletions(-) create mode 100644 apps/backend/internal/agent/runtime/lifecycle/native_binary.go create mode 100644 apps/backend/internal/agent/runtime/lifecycle/native_binary_test.go diff --git a/apps/backend/internal/agent/agents/agent.go b/apps/backend/internal/agent/agents/agent.go index 04bb996caa..5accd673e7 100644 --- a/apps/backend/internal/agent/agents/agent.go +++ b/apps/backend/internal/agent/agents/agent.go @@ -78,6 +78,18 @@ type PassthroughAgent interface { BuildPassthroughCommand(opts PassthroughOptions) Command } +// NativeBinaryAgent is an optional capability for agents whose npm package also +// ships a standalone CLI binary that behaves identically to `npx -y `. +// When that binary is present in the execution environment, the lifecycle +// prefers it (BuildCommand receives CommandOptions.PreferNativeBinary=true) to +// skip the per-launch npm registry round-trip — which is slow behind a private +// registry. Containerized runtimes ship a controlled image and keep npx. +type NativeBinaryAgent interface { + // NativeBinaryName is the executable to look for on PATH (e.g. "copilot"). + // An empty string disables native-binary preference. + NativeBinaryName() string +} + // LoginCommand describes an interactive CLI command for authenticating with // an agent. The kandev backend runs this under a PTY so the UI can render a // terminal and route keystrokes to the underlying process. @@ -148,6 +160,10 @@ type CommandOptions struct { // Runtime.IsContainerized() to pick a host absolute path vs. a bare // name resolved via the container's PATH. Runtime agentruntime.Runtime + // PreferNativeBinary is set by the lifecycle when a NativeBinaryAgent's + // standalone CLI was found in the execution environment. Such agents emit + // the native binary (e.g. "copilot --acp") instead of "npx -y ". + PreferNativeBinary bool } // PassthroughOptions are passed to BuildPassthroughCommand. diff --git a/apps/backend/internal/agent/agents/copilot_acp.go b/apps/backend/internal/agent/agents/copilot_acp.go index e52eaf1b50..137b85f47f 100644 --- a/apps/backend/internal/agent/agents/copilot_acp.go +++ b/apps/backend/internal/agent/agents/copilot_acp.go @@ -17,10 +17,16 @@ var copilotACPLogoDark []byte const copilotACPPkg = "@github/copilot" +// copilotNativeBinary is the standalone CLI installed by the @github/copilot +// npm package. When present on PATH we run it directly instead of paying the +// per-launch `npx` registry round-trip. +const copilotNativeBinary = "copilot" + var ( - _ Agent = (*CopilotACP)(nil) - _ PassthroughAgent = (*CopilotACP)(nil) - _ InferenceAgent = (*CopilotACP)(nil) + _ Agent = (*CopilotACP)(nil) + _ PassthroughAgent = (*CopilotACP)(nil) + _ InferenceAgent = (*CopilotACP)(nil) + _ NativeBinaryAgent = (*CopilotACP)(nil) ) // CopilotACP implements Agent for GitHub Copilot using ACP protocol mode. @@ -79,7 +85,14 @@ func (a *CopilotACP) IsInstalled(ctx context.Context) (*DiscoveryResult, error) return result, nil } +// NativeBinaryName returns the standalone Copilot CLI name probed for in the +// execution environment. See NativeBinaryAgent. +func (a *CopilotACP) NativeBinaryName() string { return copilotNativeBinary } + func (a *CopilotACP) BuildCommand(opts CommandOptions) Command { + if opts.PreferNativeBinary { + return Cmd(copilotNativeBinary, "--acp").Build() + } return Cmd("npx", "-y", copilotACPPkg, "--acp").Build() } diff --git a/apps/backend/internal/agent/agents/copilot_acp_test.go b/apps/backend/internal/agent/agents/copilot_acp_test.go index db4a10e46e..4ef9f2dd8e 100644 --- a/apps/backend/internal/agent/agents/copilot_acp_test.go +++ b/apps/backend/internal/agent/agents/copilot_acp_test.go @@ -57,3 +57,33 @@ func TestCopilotACP_BuildCommand_NoCLIFlagSpecialCasing(t *testing.T) { } } } + +// TestCopilotACP_BuildCommand_PreferNativeBinary verifies that when the +// lifecycle has found the standalone `copilot` CLI in the execution +// environment, BuildCommand emits it directly instead of `npx -y ` — +// skipping the npm registry round-trip that makes launches slow behind a +// private registry. +func TestCopilotACP_BuildCommand_PreferNativeBinary(t *testing.T) { + ag := NewCopilotACP() + + if name := ag.NativeBinaryName(); name != "copilot" { + t.Fatalf("NativeBinaryName() = %q, want %q", name, "copilot") + } + + native := ag.BuildCommand(CommandOptions{PreferNativeBinary: true}).Args() + wantNative := []string{"copilot", "--acp"} + if len(native) != len(wantNative) { + t.Fatalf("native argv mismatch\n got: %#v\n want: %#v", native, wantNative) + } + for i := range native { + if native[i] != wantNative[i] { + t.Errorf("native argv[%d] = %q, want %q", i, native[i], wantNative[i]) + } + } + + // Default (binary absent) still uses npx. + npx := ag.BuildCommand(CommandOptions{}).Args() + if len(npx) == 0 || npx[0] != "npx" { + t.Errorf("default command should start with npx, got %#v", npx) + } +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/executor_ssh.go b/apps/backend/internal/agent/runtime/lifecycle/executor_ssh.go index b2469bd8c3..b0df91d568 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/executor_ssh.go +++ b/apps/backend/internal/agent/runtime/lifecycle/executor_ssh.go @@ -578,14 +578,24 @@ func (r *SSHExecutor) preflightAgentBinary(ctx context.Context, client *ssh.Clie if req.AgentConfig == nil { return nil } + stepName := "Verifying agent binary" + shell := sshShellFromMetadata(req.Metadata) + + // Prefer the agent's standalone CLI when present on the remote: running it + // directly skips the per-launch `npx` registry round-trip. A hit is recorded + // in metadata so the command builder emits the native binary; a miss (or a + // transport hiccup during this optional probe) falls through to verifying + // the default (npx) command below. + if r.probeNativeBinary(ctx, client, shell, req, stepName) { + return nil + } + cmd := req.AgentConfig.BuildCommand(agents.CommandOptions{Runtime: agentruntime.RuntimeSSH}) args := cmd.Args() if len(args) == 0 { return nil } binary := args[0] - stepName := "Verifying agent binary" - shell := sshShellFromMetadata(req.Metadata) out, err := ProbeRemoteBinary(ctx, client, shell, binary) if err != nil { r.report(req.OnProgress, stepName, PrepareStepFailed, err.Error()) @@ -600,6 +610,45 @@ func (r *SSHExecutor) preflightAgentBinary(ctx context.Context, client *ssh.Clie return nil } +// probeNativeBinary probes the remote for the agent's standalone CLI (if it +// declares one via NativeBinaryAgent). On a hit it records the binary name in +// req.Metadata under MetadataKeyNativeBinary and returns true. It returns false +// when the agent has no native binary, the binary is absent, or the probe hits +// a transport error — the native binary is an optimisation, so a glitch here +// must not abort the launch or surface a misleading "agent binary" failure. +// The caller falls through to the required default-command probe, which reports +// transport failures with proper context. +func (r *SSHExecutor) probeNativeBinary(ctx context.Context, client *ssh.Client, shell string, req *ExecutorCreateRequest, stepName string) bool { + nb, ok := req.AgentConfig.(agents.NativeBinaryAgent) + if !ok { + return false + } + name := nb.NativeBinaryName() + if name == "" { + return false + } + // Clear any stale/caller-provided value up front so only a positive probe + // below sets it; a miss or transport error must not leave the command + // builder preferring the native binary on an npx-only host. delete on a nil + // map is a no-op. + delete(req.Metadata, MetadataKeyNativeBinary) + out, err := ProbeRemoteBinary(ctx, client, shell, name) + if err != nil { + r.logger.Warn("native binary probe failed on remote, falling back to default command", + zap.String("binary", name), zap.Error(err)) + return false + } + if out == "" { + return false + } + if req.Metadata == nil { + req.Metadata = make(map[string]interface{}) + } + req.Metadata[MetadataKeyNativeBinary] = name + r.report(req.OnProgress, stepName, PrepareStepCompleted, out) + return true +} + // sshShellFromMetadata reads the user-selected login shell from the // per-profile metadata. Empty / unset returns "" so callers can fall // back to WrapLoginShell's defaultLoginShell — keeping the default in diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_launch.go b/apps/backend/internal/agent/runtime/lifecycle/manager_launch.go index 50df99c4bc..bfe135c165 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager_launch.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_launch.go @@ -176,7 +176,7 @@ type agentCommands struct { // buildAgentCommand builds the agent command strings for the execution. // Returns both the initial command and the continue command (for one-shot agents like Amp). -func (m *Manager) buildAgentCommand(req *LaunchRequest, profileInfo *AgentProfileInfo, agentConfig agents.Agent) agentCommands { +func (m *Manager) buildAgentCommand(req *LaunchRequest, profileInfo *AgentProfileInfo, agentConfig agents.Agent, preferNative bool) agentCommands { model := "" autoApprove := false permissionValues := make(map[string]bool) @@ -208,12 +208,13 @@ func (m *Manager) buildAgentCommand(req *LaunchRequest, profileInfo *AgentProfil sessionID = "" } cmdOpts := agents.CommandOptions{ - Model: model, - SessionID: sessionID, - AutoApprove: autoApprove, - PermissionValues: permissionValues, - CLIFlagTokens: cliFlagTokens, - Runtime: models.ExecutorType(req.ExecutorType).Runtime(), + Model: model, + SessionID: sessionID, + AutoApprove: autoApprove, + PermissionValues: permissionValues, + CLIFlagTokens: cliFlagTokens, + Runtime: models.ExecutorType(req.ExecutorType).Runtime(), + PreferNativeBinary: preferNative, } return agentCommands{ initial: m.commandBuilder.BuildCommandString(agentConfig, cmdOpts), @@ -796,7 +797,8 @@ func (m *Manager) promoteWorkspaceExecution(ctx context.Context, execution *Agen if !agentConfig.Enabled() { return nil, fmt.Errorf("agent type %q is disabled", agentTypeName) } - cmds := m.buildAgentCommand(req, profileInfo, agentConfig) + preferNative := m.preferNativeBinary(agentConfig, execution.RuntimeName, execution.Metadata) + cmds := m.buildAgentCommand(req, profileInfo, agentConfig, preferNative) execution.AgentCommand = cmds.initial execution.ContinueCommand = cmds.continue_ if req.ACPSessionID != "" && execution.ACPSessionID == "" { @@ -965,7 +967,11 @@ func (m *Manager) buildExecutionFromInstance( execution.isResumedSession = true } execution.IsPassthrough = req.IsPassthrough - cmds := m.buildAgentCommand(req, profileInfo, agentConfig) + // Use the resolved runtime (set from rt.Name() above), matching + // promoteWorkspaceExecution's call site rather than re-deriving from the + // requested ExecutorType. + preferNative := m.preferNativeBinary(agentConfig, execution.RuntimeName, execReq.Metadata) + cmds := m.buildAgentCommand(req, profileInfo, agentConfig, preferNative) execution.AgentCommand = cmds.initial execution.ContinueCommand = cmds.continue_ return execution diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_launch_test.go b/apps/backend/internal/agent/runtime/lifecycle/manager_launch_test.go index d4798f636d..25617e6c8e 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager_launch_test.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_launch_test.go @@ -56,7 +56,7 @@ func TestBuildAgentCommand_ResumeFlag(t *testing.T) { t.Run("CanRecover=true with ACPSessionID includes --resume", func(t *testing.T) { ag := &resumeTestAgent{canRecover: &canRecoverTrue} req := &LaunchRequest{ACPSessionID: "sess-123"} - cmds := mgr.buildAgentCommand(req, nil, ag) + cmds := mgr.buildAgentCommand(req, nil, ag, false) require.Contains(t, cmds.initial, "--resume") require.Contains(t, cmds.initial, "sess-123") }) @@ -64,7 +64,7 @@ func TestBuildAgentCommand_ResumeFlag(t *testing.T) { t.Run("CanRecover=false with ACPSessionID omits --resume", func(t *testing.T) { ag := &resumeTestAgent{canRecover: &canRecoverFalse} req := &LaunchRequest{ACPSessionID: "sess-123"} - cmds := mgr.buildAgentCommand(req, nil, ag) + cmds := mgr.buildAgentCommand(req, nil, ag, false) require.False(t, strings.Contains(cmds.initial, "--resume"), "expected no --resume flag, got: %s", cmds.initial) require.False(t, strings.Contains(cmds.initial, "sess-123"), @@ -74,7 +74,7 @@ func TestBuildAgentCommand_ResumeFlag(t *testing.T) { t.Run("CanRecover=true with empty ACPSessionID omits --resume", func(t *testing.T) { ag := &resumeTestAgent{canRecover: &canRecoverTrue} req := &LaunchRequest{ACPSessionID: ""} - cmds := mgr.buildAgentCommand(req, nil, ag) + cmds := mgr.buildAgentCommand(req, nil, ag, false) require.False(t, strings.Contains(cmds.initial, "--resume"), "expected no --resume flag when ACPSessionID is empty, got: %s", cmds.initial) }) @@ -82,7 +82,7 @@ func TestBuildAgentCommand_ResumeFlag(t *testing.T) { t.Run("CanRecover=nil (default true) with ACPSessionID includes --resume", func(t *testing.T) { ag := &resumeTestAgent{canRecover: nil} req := &LaunchRequest{ACPSessionID: "sess-456"} - cmds := mgr.buildAgentCommand(req, nil, ag) + cmds := mgr.buildAgentCommand(req, nil, ag, false) require.Contains(t, cmds.initial, "--resume") require.Contains(t, cmds.initial, "sess-456") }) @@ -110,7 +110,7 @@ func TestBuildAgentCommand_CLIFlagsAppended(t *testing.T) { {Flag: "--add-dir /shared", Enabled: true}, // must be split }, } - cmds := mgr.buildAgentCommand(&LaunchRequest{}, profile, ag) + cmds := mgr.buildAgentCommand(&LaunchRequest{}, profile, ag, false) require.Contains(t, cmds.initial, "--allow-all-tools") require.NotContains(t, cmds.initial, "--allow-all-paths") @@ -127,7 +127,7 @@ func TestBuildAgentCommand_CLIFlagsAppended(t *testing.T) { {Flag: `--broken "unterminated`, Enabled: true}, }, } - cmds := mgr.buildAgentCommand(&LaunchRequest{}, profile, ag) + cmds := mgr.buildAgentCommand(&LaunchRequest{}, profile, ag, false) // The bad flag is dropped entirely; the launch still produces the // agent's base command so a user with a typo still gets their task // to run, just without the flag they intended. @@ -135,7 +135,7 @@ func TestBuildAgentCommand_CLIFlagsAppended(t *testing.T) { }) t.Run("nil profile produces bare command", func(t *testing.T) { - cmds := mgr.buildAgentCommand(&LaunchRequest{}, nil, ag) + cmds := mgr.buildAgentCommand(&LaunchRequest{}, nil, ag, false) require.Equal(t, "copilot --acp", cmds.initial) }) } diff --git a/apps/backend/internal/agent/runtime/lifecycle/native_binary.go b/apps/backend/internal/agent/runtime/lifecycle/native_binary.go new file mode 100644 index 0000000000..f0a96f3376 --- /dev/null +++ b/apps/backend/internal/agent/runtime/lifecycle/native_binary.go @@ -0,0 +1,47 @@ +package lifecycle + +import ( + "os/exec" + + "github.com/kandev/kandev/internal/agent/agents" + "github.com/kandev/kandev/internal/agentruntime" +) + +// MetadataKeyNativeBinary, when present in a launch's metadata, names the +// agent's standalone CLI binary that was found in the execution environment. +// It is set per launch by the remote binary probe (SSH preflight) and read by +// preferNativeBinary so the command builder emits the native binary instead of +// `npx -y `. Not persisted across resumes — the probe re-runs each launch +// so a binary installed (or removed) between runs is always reflected. +const MetadataKeyNativeBinary = "native_binary" + +// preferNativeBinary reports whether agentConfig's standalone CLI should be +// used instead of `npx -y ` for this launch. It honours the same +// "is the binary actually present in the execution environment?" check the SSH +// preflight already performs, applied per runtime: +// +// - standalone (local_pc / worktree): the agent subprocess runs directly on +// this host, so the backend's PATH is the execution environment — probe it +// with exec.LookPath. +// - ssh: the remote preflight probed the binary and recorded a hit in meta. +// - docker / sprites / remote_docker: a controlled image that pulls npx from +// the public registry — keep npx. +func (m *Manager) preferNativeBinary(agentConfig agents.Agent, runtime agentruntime.Runtime, meta map[string]interface{}) bool { + nb, ok := agentConfig.(agents.NativeBinaryAgent) + if !ok { + return false + } + name := nb.NativeBinaryName() + if name == "" { + return false + } + switch runtime { + case agentruntime.RuntimeStandalone: + _, err := exec.LookPath(name) + return err == nil + case agentruntime.RuntimeSSH: + return getMetadataString(meta, MetadataKeyNativeBinary) == name + default: + return false + } +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/native_binary_test.go b/apps/backend/internal/agent/runtime/lifecycle/native_binary_test.go new file mode 100644 index 0000000000..22ce10d1bd --- /dev/null +++ b/apps/backend/internal/agent/runtime/lifecycle/native_binary_test.go @@ -0,0 +1,86 @@ +package lifecycle + +import ( + "os" + "path/filepath" + "testing" + + "github.com/kandev/kandev/internal/agent/agents" + "github.com/kandev/kandev/internal/agentruntime" +) + +// TestPreferNativeBinary_NonNativeAgent verifies that an agent which does not +// implement NativeBinaryAgent never prefers a native binary, regardless of +// runtime or metadata. +func TestPreferNativeBinary_NonNativeAgent(t *testing.T) { + m := &Manager{} + ag := agents.NewClaudeACP() // does not implement NativeBinaryAgent + meta := map[string]interface{}{MetadataKeyNativeBinary: "claude"} + if m.preferNativeBinary(ag, agentruntime.RuntimeStandalone, meta) { + t.Error("non-native agent should never prefer a native binary") + } +} + +// TestPreferNativeBinary_SSH verifies the SSH branch reads the preflight result +// from metadata: a recorded hit (matching the agent's binary name) prefers +// native; an absent or mismatched key falls back to npx. +func TestPreferNativeBinary_SSH(t *testing.T) { + m := &Manager{} + ag := agents.NewCopilotACP() + + hit := map[string]interface{}{MetadataKeyNativeBinary: "copilot"} + if !m.preferNativeBinary(ag, agentruntime.RuntimeSSH, hit) { + t.Error("ssh with recorded copilot hit should prefer native binary") + } + + if m.preferNativeBinary(ag, agentruntime.RuntimeSSH, nil) { + t.Error("ssh without a recorded hit should fall back to npx") + } + + mismatch := map[string]interface{}{MetadataKeyNativeBinary: "something-else"} + if m.preferNativeBinary(ag, agentruntime.RuntimeSSH, mismatch) { + t.Error("ssh with a mismatched binary name should fall back to npx") + } +} + +// TestPreferNativeBinary_ContainerizedKeepsNpx verifies containerized runtimes +// keep npx even when metadata claims a hit — their controlled image pulls npx +// from the public registry, so the private-registry slowdown does not apply. +func TestPreferNativeBinary_ContainerizedKeepsNpx(t *testing.T) { + m := &Manager{} + ag := agents.NewCopilotACP() + meta := map[string]interface{}{MetadataKeyNativeBinary: "copilot"} + for _, rt := range []agentruntime.Runtime{ + agentruntime.RuntimeDocker, + agentruntime.RuntimeSprites, + agentruntime.RuntimeRemoteDocker, + } { + if m.preferNativeBinary(ag, rt, meta) { + t.Errorf("runtime %q should keep npx", rt) + } + } +} + +// TestPreferNativeBinary_StandaloneLooksUpPath verifies the standalone branch +// probes the backend host's PATH (which is the execution environment for +// local_pc / worktree). A binary on PATH prefers native; an empty PATH does not. +func TestPreferNativeBinary_StandaloneLooksUpPath(t *testing.T) { + m := &Manager{} + ag := agents.NewCopilotACP() + + dir := t.TempDir() + bin := filepath.Join(dir, "copilot") + if err := os.WriteFile(bin, []byte("#!/bin/sh\n"), 0o755); err != nil { + t.Fatalf("write fake copilot: %v", err) + } + + t.Setenv("PATH", dir) + if !m.preferNativeBinary(ag, agentruntime.RuntimeStandalone, nil) { + t.Error("standalone with copilot on PATH should prefer native binary") + } + + t.Setenv("PATH", t.TempDir()) // empty dir, no copilot + if m.preferNativeBinary(ag, agentruntime.RuntimeStandalone, nil) { + t.Error("standalone without copilot on PATH should fall back to npx") + } +} diff --git a/apps/backend/internal/agent/settings/controller/agent_config.go b/apps/backend/internal/agent/settings/controller/agent_config.go index 9b7f9ef8d9..3cf632e8ac 100644 --- a/apps/backend/internal/agent/settings/controller/agent_config.go +++ b/apps/backend/internal/agent/settings/controller/agent_config.go @@ -5,6 +5,7 @@ import ( "database/sql" "errors" "fmt" + "os/exec" "time" "github.com/kandev/kandev/internal/agent/agents" @@ -198,9 +199,10 @@ func (c *Controller) PreviewAgentCommand(ctx context.Context, agentName string, }) } else { cmd = agentConfig.BuildCommand(agents.CommandOptions{ - Model: req.Model, - PermissionValues: req.PermissionSettings, - CLIFlagTokens: cliFlagTokens, + Model: req.Model, + PermissionValues: req.PermissionSettings, + CLIFlagTokens: cliFlagTokens, + PreferNativeBinary: previewPrefersNativeBinary(agentConfig), }) if len(cliFlagTokens) > 0 { cmd = cmd.With().Flag(cliFlagTokens...).Build() @@ -214,6 +216,26 @@ func (c *Controller) PreviewAgentCommand(ctx context.Context, agentName string, }, nil } +// previewPrefersNativeBinary reports whether the command preview should show a +// NativeBinaryAgent's standalone CLI (e.g. "copilot --acp") instead of +// `npx -y `. The preview is not bound to an executor, so it assumes the +// local (standalone) host — the default executor — and probes this machine's +// PATH, mirroring the standalone branch of lifecycle.preferNativeBinary. +// Containerized executors keep npx at launch regardless; the preview reflects +// the common local case. +func previewPrefersNativeBinary(agentConfig agents.Agent) bool { + nb, ok := agentConfig.(agents.NativeBinaryAgent) + if !ok { + return false + } + name := nb.NativeBinaryName() + if name == "" { + return false + } + _, err := exec.LookPath(name) + return err == nil +} + // FetchDynamicModels returns models and modes for an agent from the host // utility capability cache populated by the ACP probe at boot. The refresh // flag triggers a live Refresh() call against the warm host instance. diff --git a/apps/backend/internal/agent/settings/controller/controller_test.go b/apps/backend/internal/agent/settings/controller/controller_test.go index 8a0173835c..f2abf5f7ca 100644 --- a/apps/backend/internal/agent/settings/controller/controller_test.go +++ b/apps/backend/internal/agent/settings/controller/controller_test.go @@ -2,6 +2,8 @@ package controller import ( "context" + "os" + "path/filepath" "strings" "testing" @@ -608,3 +610,37 @@ func TestDetectAgents_E2EMockPropagatesSupportsMCP(t *testing.T) { t.Error("mock-no-mcp: SupportsMCP = true, want false (IsInstalled said no)") } } + +// TestController_PreviewAgentCommand_CopilotPrefersNativeBinary verifies the +// command preview reflects the local copilot CLI when it is on PATH — the +// preview assumes the default standalone host — and falls back to npx when it +// is absent. Regresses the report that the preview always showed npx. +func TestController_PreviewAgentCommand_CopilotPrefersNativeBinary(t *testing.T) { + controller := newTestController(map[string]agents.Agent{ + "copilot-acp": agents.NewCopilotACP(), + }) + + // copilot on PATH → preview shows the native binary. + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "copilot"), []byte("#!/bin/sh\n"), 0o755); err != nil { + t.Fatalf("write fake copilot: %v", err) + } + t.Setenv("PATH", dir) + res, err := controller.PreviewAgentCommand(context.Background(), "copilot-acp", CommandPreviewRequest{}) + if err != nil { + t.Fatalf("PreviewAgentCommand() error = %v", err) + } + if got := res.Command; len(got) == 0 || got[0] != "copilot" { + t.Errorf("preview with copilot on PATH should start with copilot, got %v", got) + } + + // copilot absent → preview falls back to npx. + t.Setenv("PATH", t.TempDir()) + res, err = controller.PreviewAgentCommand(context.Background(), "copilot-acp", CommandPreviewRequest{}) + if err != nil { + t.Fatalf("PreviewAgentCommand() error = %v", err) + } + if got := res.Command; len(got) == 0 || got[0] != "npx" { + t.Errorf("preview without copilot on PATH should start with npx, got %v", got) + } +}