fix(backend): prefer local copilot CLI over npx at launch (#1384)

* 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.
This commit is contained in:
José Almeida
2026-06-18 13:07:40 +01:00
committed by GitHub
parent 6a7bf92893
commit 8922a9a2ee
10 changed files with 329 additions and 24 deletions
@@ -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 <pkg>`.
// 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 <pkg>".
PreferNativeBinary bool
}
// PassthroughOptions are passed to BuildPassthroughCommand.
@@ -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()
}
@@ -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 <pkg>` —
// 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)
}
}
@@ -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
@@ -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
@@ -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)
})
}
@@ -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 <pkg>`. 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 <pkg>` 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
}
}
@@ -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")
}
}
@@ -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 <pkg>`. 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.
@@ -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)
}
}