mirror of
https://github.com/multica-ai/multica.git
synced 2026-08-03 19:20:07 +02:00
fix(agent): spawn Copilot's native binary on Windows so the prompt survives (MUL-5586) (#6236)
On Windows the daemon passes the full multi-line prompt as `-p <prompt>` but spawns npm's `copilot.cmd`, which we already rewrite to `powershell -File copilot.ps1`. Neither launcher can carry that argument: - `copilot.cmd` forwards with `%*`, which cmd.exe expands by re-tokenising the raw command line. - `copilot.ps1` ends in `& node.exe npm-loader.js $args`, and PowerShell re-serialises `$args` onto node's command line. Under Windows PowerShell 5.1 (and pwsh <= 7.2, which default to Legacy native argument passing) embedded double quotes are not re-escaped, so the prompt is re-tokenised. Copilot then sees several argv tokens where one was intended and refuses the run with "It looks like your prompt was not quoted, so the extra words were treated as separate arguments" — the same defect class already fixed for cursor-agent in #5649, except Copilot has no stdin prompt channel to escape through, so the prompt must stay on the command line and the launchers have to go. Copilot CLI ships a native per-platform binary and `npm-loader.js` does nothing but `spawnSync` it with argv untouched, so resolve `copilot-win32-{x64,arm64}\copilot.exe` out of the npm layout and spawn it directly. That leaves exactly one hop, Go -> native binary, and Go's syscall.EscapeArg is the exact inverse of the CRT parsing that binary uses. This mirrors resolveOpenCodeNativeFromShim / resolveDevecoNativeFromShim. Both the nested (current npm) and hoisted (older npm) platform-package locations are probed; when neither resolves, we keep falling back to the PowerShell launcher, which is still better than cmd.exe. Co-authored-by: Bohan-J <bohan@devv.ai> Co-authored-by: multica-agent <github@multica.ai>
This commit is contained in:
@@ -1,22 +1,106 @@
|
|||||||
package agent
|
package agent
|
||||||
|
|
||||||
import "log/slog"
|
import (
|
||||||
|
"log/slog"
|
||||||
|
"os"
|
||||||
|
"path/filepath"
|
||||||
|
"runtime"
|
||||||
|
"strings"
|
||||||
|
)
|
||||||
|
|
||||||
// chooseCopilotInvocation selects the actual program (argv[0]) and the full
|
// chooseCopilotInvocation selects the actual program (argv[0]) and the full
|
||||||
// argv to spawn a Copilot CLI run.
|
// argv to spawn a Copilot CLI run.
|
||||||
//
|
//
|
||||||
// On macOS/Linux the npm binstub is a shebang script that execs node directly,
|
// On macOS/Linux the npm binstub is a symlink to npm-loader.js with a
|
||||||
// so argv passes through unchanged. On Windows the npm installer ships
|
// `#!/usr/bin/env node` shebang, so execve hands argv to node unchanged and
|
||||||
// copilot.cmd, which routes through cmd.exe; cmd.exe re-tokenises the raw
|
// node's spawnSync forwards it to the bundled native binary unchanged.
|
||||||
// command line via %*, mangling arguments that contain newlines or whitespace
|
|
||||||
// (e.g. the multi-line -p prompt). To avoid that, we find the sibling
|
|
||||||
// copilot.ps1 and invoke PowerShell with -File <ps1> directly, so Go passes
|
|
||||||
// each argument as a discrete token.
|
|
||||||
//
|
//
|
||||||
// The Windows rewrite lives in copilot_invocation_windows.go.
|
// On Windows there is no shebang, so npm ships copilot.cmd/copilot.ps1
|
||||||
|
// launchers and every layer in between re-serialises argv onto a new command
|
||||||
|
// line. Both launchers are lossy for the multi-line, quote-bearing -p prompt
|
||||||
|
// built by buildCopilotArgs; the symptom is Copilot's own
|
||||||
|
// "It looks like your prompt was not quoted, so the extra words were treated
|
||||||
|
// as separate arguments" error. The Windows rewrite in
|
||||||
|
// copilot_invocation_windows.go therefore skips the launchers and spawns the
|
||||||
|
// bundled copilot.exe directly.
|
||||||
func chooseCopilotInvocation(execName, lookedUp string, args []string, logger *slog.Logger) (string, []string) {
|
func chooseCopilotInvocation(execName, lookedUp string, args []string, logger *slog.Logger) (string, []string) {
|
||||||
if argv0, full, ok := platformCopilotInvocation(lookedUp, args, logger); ok {
|
if argv0, full, ok := platformCopilotInvocation(lookedUp, args, logger); ok {
|
||||||
return argv0, full
|
return argv0, full
|
||||||
}
|
}
|
||||||
return execName, args
|
return execName, args
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// resolveCopilotNativeFromShim returns the path to the native Copilot
|
||||||
|
// executable bundled inside the npm package, given the path to the npm
|
||||||
|
// `copilot.cmd` shim that PATH lookup found on Windows. Returns "" if the
|
||||||
|
// shim doesn't end in `.cmd` or no candidate platform package ships a binary
|
||||||
|
// at either known location, in which case the caller falls back to the
|
||||||
|
// PowerShell launcher.
|
||||||
|
//
|
||||||
|
// Why bypass the launchers instead of quoting harder: nothing on the Go side
|
||||||
|
// can make them lossless. Both hops re-parse a command line with rules that
|
||||||
|
// differ from the CRT rules Go's os/exec escapes for.
|
||||||
|
//
|
||||||
|
// - copilot.cmd forwards with `%*`, which cmd.exe expands by re-tokenising
|
||||||
|
// the raw command line — newlines and quotes in the -p prompt do not
|
||||||
|
// survive.
|
||||||
|
// - copilot.ps1 ends in `& node.exe npm-loader.js $args`, and PowerShell
|
||||||
|
// re-serialises $args onto node's command line. Under Windows PowerShell
|
||||||
|
// 5.1 (and pwsh <= 7.2, which default to Legacy native argument passing)
|
||||||
|
// embedded double quotes are not re-escaped, so a prompt containing them
|
||||||
|
// is re-tokenised — the same defect already documented for cursor-agent
|
||||||
|
// in buildCursorArgs (#5649).
|
||||||
|
//
|
||||||
|
// Copilot has no stdin prompt channel (the escape hatch cursor-agent offers),
|
||||||
|
// so the prompt must stay on the command line and the launchers have to go.
|
||||||
|
// Spawning copilot.exe directly leaves exactly one hop, Go -> native binary,
|
||||||
|
// and Go's syscall.EscapeArg is the exact inverse of the CRT parsing the
|
||||||
|
// binary uses.
|
||||||
|
//
|
||||||
|
// Layout when installed via `npm install -g @github/copilot` (verified
|
||||||
|
// against @github/copilot 1.0.77):
|
||||||
|
//
|
||||||
|
// <prefix>\copilot.cmd (shim)
|
||||||
|
// <prefix>\node_modules\@github\copilot\npm-loader.js (JS entry)
|
||||||
|
// <prefix>\node_modules\@github\copilot\node_modules\@github\copilot-win32-x64\copilot.exe (native)
|
||||||
|
//
|
||||||
|
// npm keeps the platform package nested under the parent package; older npm
|
||||||
|
// versions and other package managers hoist it to the top-level
|
||||||
|
// node_modules instead, so both locations are probed.
|
||||||
|
//
|
||||||
|
// statFn is injected so this is testable on non-Windows hosts.
|
||||||
|
func resolveCopilotNativeFromShim(shimPath string, statFn func(string) (os.FileInfo, error)) string {
|
||||||
|
if !strings.EqualFold(filepath.Ext(shimPath), ".cmd") {
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
prefix := filepath.Dir(shimPath)
|
||||||
|
for _, pkg := range copilotWindowsPackageCandidates(runtime.GOARCH) {
|
||||||
|
candidates := []string{
|
||||||
|
// npm's current layout: optional platform dep nested under the parent.
|
||||||
|
filepath.Join(prefix, "node_modules", "@github", "copilot", "node_modules", "@github", pkg, "copilot.exe"),
|
||||||
|
// Hoisted layout used by older npm versions and other installers.
|
||||||
|
filepath.Join(prefix, "node_modules", "@github", pkg, "copilot.exe"),
|
||||||
|
}
|
||||||
|
for _, candidate := range candidates {
|
||||||
|
if _, err := statFn(candidate); err == nil {
|
||||||
|
return candidate
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
|
||||||
|
// copilotWindowsPackageCandidates returns the npm platform package names that
|
||||||
|
// may host the bundled copilot.exe, ordered so the most likely match for the
|
||||||
|
// given GOARCH comes first. ARM64 hosts try the arm64 build first; everything
|
||||||
|
// else tries x64 first, because an x64 Node running under emulation on an
|
||||||
|
// ARM64 host installs the x64 platform package. Cost is one extra statFn call
|
||||||
|
// per miss when the GOARCH-preferred package isn't installed.
|
||||||
|
func copilotWindowsPackageCandidates(goarch string) []string {
|
||||||
|
switch goarch {
|
||||||
|
case "arm64":
|
||||||
|
return []string{"copilot-win32-arm64", "copilot-win32-x64"}
|
||||||
|
default:
|
||||||
|
return []string{"copilot-win32-x64", "copilot-win32-arm64"}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -34,3 +34,121 @@ func TestChooseCopilotInvocation_PassthroughForNonLauncher(t *testing.T) {
|
|||||||
t.Errorf("argv changed unexpectedly:\n got %#v\n want %#v", gotArgs, args)
|
t.Errorf("argv changed unexpectedly:\n got %#v\n want %#v", gotArgs, args)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ── Windows native-binary resolution tests ──
|
||||||
|
//
|
||||||
|
// These run on every platform: resolveCopilotNativeFromShim takes statFn as a
|
||||||
|
// parameter precisely so the Windows layout can be asserted from macOS/Linux CI.
|
||||||
|
|
||||||
|
// TestResolveCopilotNativeFromShim_NestedNpmLayout covers the layout npm
|
||||||
|
// actually produces today (verified against @github/copilot 1.0.77): the
|
||||||
|
// platform package stays nested under the parent package rather than being
|
||||||
|
// hoisted.
|
||||||
|
func TestResolveCopilotNativeFromShim_NestedNpmLayout(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
prefix := filepath.Join("C:\\Users", "dev", "AppData", "Roaming", "npm")
|
||||||
|
shim := filepath.Join(prefix, "copilot.cmd")
|
||||||
|
native := filepath.Join(prefix, "node_modules", "@github", "copilot", "node_modules", "@github", "copilot-win32-x64", "copilot.exe")
|
||||||
|
|
||||||
|
if got := resolveCopilotNativeFromShim(shim, fakeStat(native)); got != native {
|
||||||
|
t.Errorf("got %q, want %q", got, native)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestResolveCopilotNativeFromShim_HoistedNpmLayout covers older npm versions
|
||||||
|
// and other installers that hoist the optional platform dep to the top-level
|
||||||
|
// node_modules.
|
||||||
|
func TestResolveCopilotNativeFromShim_HoistedNpmLayout(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
prefix := filepath.Join("C:\\Users", "dev", "AppData", "Roaming", "npm")
|
||||||
|
shim := filepath.Join(prefix, "copilot.cmd")
|
||||||
|
native := filepath.Join(prefix, "node_modules", "@github", "copilot-win32-x64", "copilot.exe")
|
||||||
|
|
||||||
|
if got := resolveCopilotNativeFromShim(shim, fakeStat(native)); got != native {
|
||||||
|
t.Errorf("got %q, want %q", got, native)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestResolveCopilotNativeFromShim_FindsArm64Package covers Windows-on-ARM
|
||||||
|
// hosts, where npm installs @github/copilot-win32-arm64 instead. The resolver
|
||||||
|
// must find it regardless of which arch this test binary was built for, since
|
||||||
|
// the arch list only controls probe order, not membership.
|
||||||
|
func TestResolveCopilotNativeFromShim_FindsArm64Package(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
prefix := filepath.Join("C:\\Users", "dev", "AppData", "Roaming", "npm")
|
||||||
|
shim := filepath.Join(prefix, "copilot.cmd")
|
||||||
|
native := filepath.Join(prefix, "node_modules", "@github", "copilot", "node_modules", "@github", "copilot-win32-arm64", "copilot.exe")
|
||||||
|
|
||||||
|
if got := resolveCopilotNativeFromShim(shim, fakeStat(native)); got != native {
|
||||||
|
t.Errorf("got %q, want %q", got, native)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestResolveCopilotNativeFromShim_ReturnsEmptyWhenNativeMissing covers a
|
||||||
|
// partial install or a Copilot build predating the platform packages. The
|
||||||
|
// caller must fall back to the PowerShell launcher rather than spawn a path
|
||||||
|
// that doesn't exist.
|
||||||
|
func TestResolveCopilotNativeFromShim_ReturnsEmptyWhenNativeMissing(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
shim := filepath.Join("C:\\Users", "dev", "AppData", "Roaming", "npm", "copilot.cmd")
|
||||||
|
|
||||||
|
if got := resolveCopilotNativeFromShim(shim, fakeStat()); got != "" {
|
||||||
|
t.Errorf("got %q, want empty (missing native binary)", got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestResolveCopilotNativeFromShim_SkipsNonCmdPath keeps macOS/Linux and
|
||||||
|
// direct-binary Windows launches on the untouched passthrough path.
|
||||||
|
func TestResolveCopilotNativeFromShim_SkipsNonCmdPath(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
for _, p := range []string{
|
||||||
|
"/usr/local/bin/copilot",
|
||||||
|
"C:\\Users\\dev\\AppData\\Roaming\\npm\\copilot.exe",
|
||||||
|
"C:\\Users\\dev\\AppData\\Roaming\\npm\\copilot.ps1",
|
||||||
|
"",
|
||||||
|
} {
|
||||||
|
if got := resolveCopilotNativeFromShim(p, fakeStat("anything")); got != "" {
|
||||||
|
t.Errorf("path %q: got %q, want empty", p, got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestResolveCopilotNativeFromShim_AcceptsUppercaseExtension guards the
|
||||||
|
// PATHEXT case: Windows filesystems are case-insensitive and exec.LookPath
|
||||||
|
// can return either case.
|
||||||
|
func TestResolveCopilotNativeFromShim_AcceptsUppercaseExtension(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
prefix := filepath.Join("C:\\Users", "dev", "AppData", "Roaming", "npm")
|
||||||
|
shim := filepath.Join(prefix, "copilot.CMD")
|
||||||
|
native := filepath.Join(prefix, "node_modules", "@github", "copilot", "node_modules", "@github", "copilot-win32-x64", "copilot.exe")
|
||||||
|
|
||||||
|
if got := resolveCopilotNativeFromShim(shim, fakeStat(native)); got != native {
|
||||||
|
t.Errorf("got %q, want %q", got, native)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestCopilotWindowsPackageCandidates_ArchOrdering pins the probe order: the
|
||||||
|
// host's own arch first, but never at the cost of dropping the other one — an
|
||||||
|
// x64 Node running under emulation on an ARM64 host installs the x64 package.
|
||||||
|
func TestCopilotWindowsPackageCandidates_ArchOrdering(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
cases := []struct {
|
||||||
|
goarch string
|
||||||
|
want []string
|
||||||
|
}{
|
||||||
|
{"arm64", []string{"copilot-win32-arm64", "copilot-win32-x64"}},
|
||||||
|
{"amd64", []string{"copilot-win32-x64", "copilot-win32-arm64"}},
|
||||||
|
}
|
||||||
|
for _, tc := range cases {
|
||||||
|
if got := copilotWindowsPackageCandidates(tc.goarch); !reflect.DeepEqual(got, tc.want) {
|
||||||
|
t.Errorf("goarch %q: got %#v, want %#v", tc.goarch, got, tc.want)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -2,12 +2,31 @@
|
|||||||
|
|
||||||
package agent
|
package agent
|
||||||
|
|
||||||
import "log/slog"
|
import (
|
||||||
|
"log/slog"
|
||||||
|
"os"
|
||||||
|
)
|
||||||
|
|
||||||
// platformCopilotInvocation rewrites copilot.cmd → PowerShell -File
|
// platformCopilotInvocation keeps the multi-line -p prompt built by
|
||||||
// copilot.ps1 on Windows to avoid cmd.exe %* re-tokenisation mangling
|
// buildCopilotArgs intact on Windows by spawning the native copilot.exe
|
||||||
// the multi-line -p prompt built by buildCopilotArgs.
|
// bundled in the npm package, so neither cmd.exe's %* expansion nor
|
||||||
// powerShellLookup and rewriteCmdToPS1 are defined in cursor_invocation_windows.go.
|
// PowerShell's $args re-serialisation can re-tokenise it (see
|
||||||
|
// resolveCopilotNativeFromShim for why neither launcher can be made safe).
|
||||||
|
//
|
||||||
|
// When the native binary can't be located — a partial install, or a Copilot
|
||||||
|
// build old enough to predate the platform packages — fall back to
|
||||||
|
// `powershell -File copilot.ps1`, which still avoids the strictly worse
|
||||||
|
// cmd.exe layer. powerShellLookup and rewriteCmdToPS1 are defined in
|
||||||
|
// cursor_invocation_windows.go.
|
||||||
func platformCopilotInvocation(lookedUp string, args []string, logger *slog.Logger) (string, []string, bool) {
|
func platformCopilotInvocation(lookedUp string, args []string, logger *slog.Logger) (string, []string, bool) {
|
||||||
|
if native := resolveCopilotNativeFromShim(lookedUp, os.Stat); native != "" {
|
||||||
|
if logger != nil {
|
||||||
|
logger.Info("copilot: spawning the bundled native binary to keep argv intact",
|
||||||
|
"shim", lookedUp,
|
||||||
|
"native", native,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
return native, args, true
|
||||||
|
}
|
||||||
return rewriteCmdToPS1("copilot", lookedUp, args, logger)
|
return rewriteCmdToPS1("copilot", lookedUp, args, logger)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -5,17 +5,66 @@ package agent
|
|||||||
import (
|
import (
|
||||||
"io"
|
"io"
|
||||||
"log/slog"
|
"log/slog"
|
||||||
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"reflect"
|
"reflect"
|
||||||
|
"runtime"
|
||||||
"testing"
|
"testing"
|
||||||
)
|
)
|
||||||
|
|
||||||
// TestPlatformCopilotInvocation_RewritesCmdLauncherToPowerShellFile is the
|
// TestPlatformCopilotInvocation_PrefersBundledNativeBinary is the core Windows
|
||||||
// core Windows test: when LookPath resolves copilot to the npm-installed .cmd
|
// test: when LookPath resolves copilot to the npm .cmd launcher and the npm
|
||||||
// launcher and a sibling copilot.ps1 exists, we should invoke PowerShell with
|
// package ships copilot.exe, we must spawn that binary directly and forward
|
||||||
// -File <ps1> and forward every original arg unchanged — including the
|
// every original arg unchanged. Both launchers re-serialise argv onto a new
|
||||||
// multi-line -p prompt that would otherwise be mangled by cmd.exe's %*
|
// command line and mangle the multi-line, quote-bearing -p prompt, which
|
||||||
// re-expansion inside copilot.cmd.
|
// Copilot then rejects with "your prompt was not quoted".
|
||||||
|
func TestPlatformCopilotInvocation_PrefersBundledNativeBinary(t *testing.T) {
|
||||||
|
dir := t.TempDir()
|
||||||
|
cmdPath := filepath.Join(dir, "copilot.cmd")
|
||||||
|
ps1Path := filepath.Join(dir, "copilot.ps1")
|
||||||
|
writeFile(t, cmdPath, "@echo off\r\npowershell -NoProfile -ExecutionPolicy Bypass -File \"%~dp0copilot.ps1\" %*\r\n")
|
||||||
|
// A usable .ps1 launcher must NOT win over the native binary.
|
||||||
|
writeFile(t, ps1Path, "# fake copilot.ps1\r\n")
|
||||||
|
stubPowerShell(t, filepath.Join(dir, "powershell.exe"), true)
|
||||||
|
|
||||||
|
pkg := "copilot-win32-x64"
|
||||||
|
if runtime.GOARCH == "arm64" {
|
||||||
|
pkg = "copilot-win32-arm64"
|
||||||
|
}
|
||||||
|
nativeDir := filepath.Join(dir, "node_modules", "@github", "copilot", "node_modules", "@github", pkg)
|
||||||
|
if err := os.MkdirAll(nativeDir, 0o755); err != nil {
|
||||||
|
t.Fatalf("mkdir native dir: %v", err)
|
||||||
|
}
|
||||||
|
nativePath := filepath.Join(nativeDir, "copilot.exe")
|
||||||
|
writeFile(t, nativePath, "")
|
||||||
|
|
||||||
|
prompt := "You are running as a local coding agent.\n\n# Context\nRun `go build -ldflags \"-X main.version=x\"`.\n"
|
||||||
|
args := []string{
|
||||||
|
"-p", prompt,
|
||||||
|
"--output-format", "json",
|
||||||
|
"--allow-all",
|
||||||
|
"--no-ask-user",
|
||||||
|
}
|
||||||
|
logger := slog.New(slog.NewTextHandler(io.Discard, nil))
|
||||||
|
|
||||||
|
gotExec, gotArgs, ok := platformCopilotInvocation(cmdPath, args, logger)
|
||||||
|
if !ok {
|
||||||
|
t.Fatalf("expected native rewrite to be applied, got ok=false")
|
||||||
|
}
|
||||||
|
if gotExec != nativePath {
|
||||||
|
t.Errorf("argv0: got %q want %q", gotExec, nativePath)
|
||||||
|
}
|
||||||
|
if !reflect.DeepEqual(gotArgs, args) {
|
||||||
|
t.Errorf("argv must pass through untouched:\n got %#v\n want %#v", gotArgs, args)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestPlatformCopilotInvocation_RewritesCmdLauncherToPowerShellFile covers the
|
||||||
|
// fallback: when the bundled native binary can't be located (partial install,
|
||||||
|
// or a Copilot build predating the platform packages) but a sibling
|
||||||
|
// copilot.ps1 exists, we should invoke PowerShell with -File <ps1> and forward
|
||||||
|
// every original arg unchanged — still better than cmd.exe's %* re-expansion
|
||||||
|
// inside copilot.cmd.
|
||||||
func TestPlatformCopilotInvocation_RewritesCmdLauncherToPowerShellFile(t *testing.T) {
|
func TestPlatformCopilotInvocation_RewritesCmdLauncherToPowerShellFile(t *testing.T) {
|
||||||
dir := t.TempDir()
|
dir := t.TempDir()
|
||||||
cmdPath := filepath.Join(dir, "copilot.cmd")
|
cmdPath := filepath.Join(dir, "copilot.cmd")
|
||||||
|
|||||||
Reference in New Issue
Block a user