Skip to content

[Go][tests] E2E harness picks the first @github/copilot-* package instead of the current platform package #2116

Description

@nytron88

Summary

The Go test harness resolves the Copilot CLI by globbing nodejs/node_modules/@github/copilot-*/index.js and taking matches[0] — the alphabetically first result — rather than the package matching the current OS/architecture. This logic lives in go/internal/e2e/testharness/context.go:36-41 and affects every E2E test that calls testharness.CLIPath().

This is the same defect #2103 reports for the Python harness, and the same one PR #2093 fixed for .NET.

// go/internal/e2e/testharness/context.go
base := RepoPath("nodejs", "node_modules", "@github")
matches, _ := filepath.Glob(filepath.Join(base, "copilot-*", "index.js"))
if len(matches) > 0 {
    cliPath = matches[0]
    return
}

Why This Is a Bug

filepath.Glob sorts its results, so matches[0] is whichever platform package sorts first — copilot-darwin-arm64 beats copilot-linux-x64 on a Linux host. The Node SDK resolves this correctly via getCliPlatformPackageNames() in nodejs/src/client.ts:357, which builds the candidate names from platform and architecture and tries both Linux libc variants (linux vs linuxmusl).

Two further problems in the same block:

  1. The glob is too loose. copilot-* also matches non-platform packages under @github, such as copilot-language-server. Any of those shipping an index.js becomes a CLI candidate.
  2. The no-match diagnostic is duplicated and uninformative. CLIPath() returns "" rather than an error, so every caller re-implements the same guard — NewTestContext (context.go:121-124), plus client_e2e_test.go:12-15, rpc_e2e_test.go:13-16, agent_and_compact_rpc_e2e_test.go:15-18 and inprocess_ffi_e2e_test.go:29-32, which each repeat if cliPath == "" { t.Fatal("CLI not found. Run 'npm install' in the nodejs directory first.") }. Nothing fails silently, but no message says which package was looked for, so "you never ran npm install" and "you have a node_modules for the wrong platform" — the situation this bug creates — are indistinguishable. NewTestContext's variant also interpolates the empty path, printing "CLI not found at .".

Where this actually bites

Worth noting for prioritisation: CI does not currently exercise this path. .github/workflows/go-sdk-tests.yml sets COPILOT_CLI_PATH from the setup-copilot action, and go/test.sh sets it too when unset, so CLIPath() returns from its env-var branch before reaching the glob. The bug shows up in local development — running go test ./internal/e2e/... directly from an editor or terminal without COPILOT_CLI_PATH.

Both of those env-var producers use the same loose pattern and have the same ordering defect:

  • go/test.sh:22ls "$SCRIPT_DIR"/../nodejs/node_modules/@github/copilot-*/index.js | head -n1
  • .github/actions/setup-copilot/action.yml:29cli_path=$(ls "$(pwd)"/nodejs/node_modules/@github/copilot-*/index.js | head -n1)

go/test.sh is in scope here. The composite action is shared with the Java and Rust test workflows and the Java publish workflows, so it probably deserves its own change rather than riding along with a Go fix.

Repro Steps

  1. Leave COPILOT_CLI_PATH unset
  2. Have more than one @github/copilot-* package present in nodejs/node_modules/@github — a stale tree, a node_modules cache restored from a different runner, or a cross-platform container mount
  3. Run the Go E2E tests directly: cd go && go test ./internal/e2e/...
  4. CLIPath() returns the alphabetically first platform package
  5. The test process launches an entrypoint built for another OS/arch

Expected Behavior

Build the candidate package names from the current platform and architecture, distinguishing the Linux glibc and musl variants, and probe those exact names — mirroring nodejs/src/client.ts.

Go already has this mapping. ffihost.PrebuildsFolder() in go/internal/ffihost/resolve.go:34-59 returns exactly the npm platform string the package directory is named after (linux-x64, linuxmusl-x64, darwin-arm64, win32-x64, …), including the musl probe, and returns "" on unsupported platforms. resolve.go is the only file in that package without the copilot_inprocess build tag and it imports nothing outside the standard library, so the harness can import it without pulling in the FFI machinery:

base := RepoPath("nodejs", "node_modules", "@github")
for _, name := range cliPlatformPackageNames() {
    candidate := filepath.Join(base, name, "index.js")
    if info, err := os.Stat(candidate); err == nil && !info.IsDir() {
        cliPath = candidate
        return
    }
}
// no match: report the directory searched, the candidates tried, and COPILOT_CLI_PATH

where cliPlatformPackageNames() returns "copilot-" + ffihost.PrebuildsFolder() plus, on Linux, the sibling libc variant — npm installs exactly one of them, and isMusl() can come up empty in minimal containers.

For the no-match case, giving CLIPath a testing.TB parameter so it can t.Fatalf with the directory searched and the candidates tried would let all five call sites drop their duplicated guard and would report the wrong-platform case distinctly. It's an internal package, so the signature change costs nothing outside the repo.

Related Issues

The Java harness (TestUtil.findCliPath) already builds copilot-<platform>-<cpuArch> explicitly and is unaffected.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions