From 6902d21ade6b1a11f540706d427fba4d8f8c2f9b Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Mon, 13 Jul 2026 13:42:57 +0000 Subject: [PATCH 01/14] Add in-process (FFI) transport to the Go SDK Bring the Go SDK to parity with the .NET, Rust, TypeScript, and Python SDKs by adding an in-process transport that hosts the runtime via the native FFI library (runtime.node) instead of spawning a child process for the runtime server. - ffihost: pure-Go FFI host using ebitengine/purego (CGO stays off), C ABI binding, single-load-per-process guard, and cdylib resolution (flat name next to the CLI, or prebuilds//runtime.node), including musl and Windows loaders. - API: new InProcessConnection{Path, Args} (Experimental) plus connection-level Env on Stdio/TCP via a shared childProcessConnection interface. - client: startInProcess wiring into Start/Stop/ForceStop/killProcess with no-PATH entrypoint resolution, --no-auto-update, and host ownership before the blocking handshake for cancellation safety. validateEnvironmentOptions rejects Env/WorkingDirectory/Telemetry in-process and env set in both client and connection for child transports. - bundler + embeddedcli: extract and embed runtime.node (conditional per platform) and install it next to the CLI binary with SHA-256 verification; add RuntimeLibPath(). - e2e: COPILOT_SDK_DEFAULT_CONNECTION=inprocess harness support that swaps only the default stdio connection, mirrors the merged env and cwd onto the process, and leaves TCP/URI/custom-stdio/telemetry tests on their transport; new in-process ping smoke test. - CI: add transport: [default, inprocess] matrix to the Go workflow. - Docs and unit tests for the new connection and validation semantics. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: a96d1a82-c80e-40f1-a3df-a9708429b68b --- .github/workflows/go-sdk-tests.yml | 8 +- go/README.md | 39 ++- go/client.go | 156 ++++++++++- go/client_test.go | 94 +++++++ go/cmd/bundler/main.go | 166 ++++++++--- go/go.mod | 1 + go/go.sum | 2 + go/internal/e2e/inprocess_ffi_e2e_test.go | 51 ++++ go/internal/e2e/testharness/context.go | 125 ++++++++- go/internal/embeddedcli/embeddedcli.go | 95 +++++++ go/internal/ffihost/buffer.go | 65 +++++ go/internal/ffihost/ffihost.go | 321 ++++++++++++++++++++++ go/internal/ffihost/loader_other.go | 15 + go/internal/ffihost/loader_windows.go | 18 ++ go/internal/ffihost/resolve.go | 118 ++++++++ go/types.go | 63 ++++- 16 files changed, 1286 insertions(+), 51 deletions(-) create mode 100644 go/internal/e2e/inprocess_ffi_e2e_test.go create mode 100644 go/internal/ffihost/buffer.go create mode 100644 go/internal/ffihost/ffihost.go create mode 100644 go/internal/ffihost/loader_other.go create mode 100644 go/internal/ffihost/loader_windows.go create mode 100644 go/internal/ffihost/resolve.go diff --git a/.github/workflows/go-sdk-tests.yml b/.github/workflows/go-sdk-tests.yml index e262961096..c7ce3dbee9 100644 --- a/.github/workflows/go-sdk-tests.yml +++ b/.github/workflows/go-sdk-tests.yml @@ -29,7 +29,7 @@ permissions: jobs: test: - name: "Go SDK Tests" + name: "Go SDK Tests (${{ matrix.os }}, ${{ matrix.transport }})" if: github.event.repository.fork == false env: POWERSHELL_UPDATECHECK: Off @@ -37,6 +37,7 @@ jobs: fail-fast: false matrix: os: [ubuntu-latest, macos-latest, windows-latest] + transport: ["default", "inprocess"] runs-on: ${{ matrix.os }} defaults: run: @@ -78,6 +79,11 @@ jobs: if: runner.os == 'Windows' run: pwsh.exe -Command "Write-Host 'PowerShell ready'" + - name: Select inprocess transport + if: matrix.transport == 'inprocess' + run: | + echo "COPILOT_SDK_DEFAULT_CONNECTION=inprocess" >> "$GITHUB_ENV" + - name: Run Go SDK tests env: COPILOT_HMAC_KEY: ${{ secrets.COPILOT_DEVELOPER_CLI_INTEGRATION_HMAC_KEY }} diff --git a/go/README.md b/go/README.md index 5787e5a53e..edde8697bd 100644 --- a/go/README.md +++ b/go/README.md @@ -101,6 +101,38 @@ Follow these steps to embed the CLI: That's it! When your application calls `copilot.NewClient` without a `Connection` field (or with an empty `StdioConnection{}`) and no `COPILOT_CLI_PATH` environment variable, the SDK will automatically install the embedded CLI to a cache directory and use it for all operations. +The bundler also embeds the native in-process runtime library (`runtime.node`) alongside the CLI binary when the target platform's Copilot package ships one, so the [in-process transport](#in-process-transport-experimental) works out of the box for embedded builds. + +## In-process transport (Experimental) + +> **Experimental:** the in-process API may change in a future release. + +By default the SDK spawns the Copilot CLI as a child process and talks JSON-RPC over stdio or TCP. The **in-process** transport instead loads the native runtime library (`runtime.node`) into your process and hosts the runtime there; the native host spawns the CLI worker itself. This avoids a separate long-lived child process for the runtime server. + +```go +client := copilot.NewClient(&copilot.ClientOptions{ + Connection: copilot.InProcessConnection{Path: "/path/to/copilot"}, +}) +if err := client.Start(context.Background()); err != nil { + log.Fatal(err) +} +defer client.Stop() +``` + +Resolution and requirements: + +- The entrypoint is resolved with **no `PATH` lookup**: explicit `InProcessConnection.Path`, then `COPILOT_CLI_PATH`, then the bundled embedded CLI. It must be an on-disk path. +- The runtime library is loaded from next to the resolved entrypoint (the flat `libcopilot_runtime.*` name installed by the bundler, or the package's `prebuilds//runtime.node`). Start fails loudly if it cannot be found. +- The runtime library is loaded once per process; loading a different library path in the same process is an error. + +The in-process transport rejects options that cannot be honored by a runtime hosted in your shared process (each panics at `NewClient`): + +- `Env` — the host process has a single environment block. Set variables on the host process environment instead. +- `WorkingDirectory` — the worker is spawned without a working-directory parameter. Change the process working directory before creating the client. +- `Telemetry` — per-client telemetry is lowered to native-runtime environment variables. Use a child-process transport for per-client telemetry. + +Implemented with pure-Go FFI (via [purego](https://github.com/ebitengine/purego)), so `CGO_ENABLED=0` and cross-compilation are preserved; no C toolchain is required. + ## API Reference ### Client @@ -142,11 +174,14 @@ Event types: `SessionLifecycleCreated`, `SessionLifecycleDeleted`, `SessionLifec **ClientOptions:** - `Connection` (RuntimeConnection): How the SDK connects to the runtime. Construct via one of: - - `StdioConnection{Path, Args}` — spawn a runtime over stdio (the default if `Connection` is nil) - - `TCPConnection{Port, ConnectionToken, Path, Args}` — spawn a runtime that listens on TCP + - `StdioConnection{Path, Args, Env}` — spawn a runtime over stdio (the default if `Connection` is nil) + - `TCPConnection{Port, ConnectionToken, Path, Args, Env}` — spawn a runtime that listens on TCP - `URIConnection{URL, ConnectionToken}` — connect to an already-running runtime (no process spawned) + - `InProcessConnection{Path, Args}` — **Experimental.** Host the runtime in-process via the native FFI library instead of spawning a child process. See [In-process transport](#in-process-transport-experimental) below. When `Path` is empty for stdio/tcp, the SDK uses the bundled CLI (or `COPILOT_CLI_PATH` env var). + + `StdioConnection` and `TCPConnection` accept an optional connection-level `Env`. Set environment variables via **either** the client-level `Env` option or the connection's `Env`, not both (setting both panics); prefer the connection-level `Env`. - `WorkingDirectory` (string): Working directory for the runtime process - `BaseDirectory` (string): Base directory for Copilot data (session state, config, etc.). Sets `COPILOT_HOME` on the spawned runtime. When empty, the runtime defaults to `~/.copilot`. Ignored with `URIConnection`. This does **not** affect where the Go SDK extracts the embedded CLI binary; use `embeddedcli.Config.Dir` for the extraction/cache location. - `LogLevel` (string): Log level. When empty (default), the runtime uses its own default level (the SDK does not pass `--log-level`). diff --git a/go/client.go b/go/client.go index b57864288d..a72f6446cf 100644 --- a/go/client.go +++ b/go/client.go @@ -48,6 +48,7 @@ import ( "github.com/google/uuid" "github.com/github/copilot-sdk/go/internal/embeddedcli" + "github.com/github/copilot-sdk/go/internal/ffihost" "github.com/github/copilot-sdk/go/internal/jsonrpc2" "github.com/github/copilot-sdk/go/internal/truncbuffer" "github.com/github/copilot-sdk/go/rpc" @@ -93,6 +94,37 @@ func validateSessionFSConfig(config *SessionFSConfig) error { return nil } +// validateEnvironmentOptions enforces the transport-specific rules for +// per-client environment, working directory, and telemetry. It panics (fails +// loud) on a misconfiguration, matching the other SDKs. +// +// The in-process transport loads the native runtime into this process, whose +// single environment block and process-global working directory cannot carry +// per-client values, and whose telemetry lowers to shared process-global env +// vars — so options that depend on them are rejected there. Child-process +// transports each own their OS process, so per-connection env is allowed, but +// setting it in both the client-level option and the connection is rejected. +func validateEnvironmentOptions(connection RuntimeConnection, opts *ClientOptions) { + if _, ok := connection.(InProcessConnection); ok { + if opts.Env != nil { + panic("Env is not supported with InProcessConnection: the in-process transport loads the native runtime into the shared host process, whose single environment block cannot carry per-client values. Set the variables on the host process environment instead.") + } + if opts.WorkingDirectory != "" { + panic("WorkingDirectory is not supported with InProcessConnection: the in-process transport hosts the runtime in the shared host process and spawns the worker without a working-directory parameter. Use a child-process transport, or set the process working directory before creating the client.") + } + if opts.Telemetry != nil { + panic("Telemetry is not supported with InProcessConnection: telemetry configuration is lowered to environment variables read by native runtime code running in the shared host process, so per-client telemetry cannot be honored in-process. Configure telemetry via the host process environment, or use a child-process transport.") + } + return + } + + if cp, ok := connection.(childProcessConnection); ok { + if cp.connEnv() != nil && opts.Env != nil { + panic("Set environment variables via either the client-level Env option or the connection's Env, not both. Prefer the connection-level Env for child-process transports.") + } + } +} + // Client manages the connection to the Copilot CLI server and provides session management. // // The Client can either spawn a CLI server process or connect to an existing server. @@ -124,6 +156,8 @@ type Client struct { isExternalServer bool conn net.Conn // stores net.Conn for external TCP connections useStdio bool // resolved value from options + useInProcess bool // true for InProcessConnection (FFI transport) + ffiHost *ffihost.Host // resolved process options for the spawned runtime (zero values for URIConnection) cliPath string cliArgs []string @@ -223,32 +257,58 @@ func NewClient(options *ClientOptions) *Client { client.isExternalServer = true client.useStdio = false client.tcpConnectionToken = conn.ConnectionToken + case InProcessConnection: + client.useStdio = false + client.useInProcess = true + client.cliPath = conn.Path + if len(conn.Args) > 0 { + client.cliArgs = append([]string{}, conn.Args...) + } default: panic(fmt.Sprintf("unknown RuntimeConnection type: %T", connection)) } + // Validate transport-specific option constraints (fail loud). The in-process + // transport loads the runtime into this process, whose single environment + // block, process-global working directory, and shared telemetry state cannot + // carry per-client values. Child-process transports may set env via either + // the client-level option or the connection, but not both. + validateEnvironmentOptions(connection, &opts) + // Validate auth options when connecting to an external runtime. if client.isExternalServer && (opts.GitHubToken != "" || opts.UseLoggedInUser != nil) { panic("GitHubToken and UseLoggedInUser cannot be used with URIConnection (external runtime manages its own auth)") } + // For child-process transports, a connection-level env takes precedence over + // the client-level env (setting both was rejected above). Resolve it before + // defaulting so an explicit empty connection env stays authoritative. + if cp, ok := connection.(childProcessConnection); ok { + if env := cp.connEnv(); env != nil { + opts.Env = env + } + } + // Default Env to current environment if not set if opts.Env == nil { opts.Env = os.Environ() } - // Check effective environment for CLI path (only if not explicitly set via options) - if client.cliPath == "" { + // Check effective environment for CLI path (only if not explicitly set via + // options). The in-process transport never falls back to PATH, so its + // entrypoint is resolved separately in startInProcess. + if client.cliPath == "" && !client.useInProcess { if cliPath := getEnvValue(opts.Env, "COPILOT_CLI_PATH"); cliPath != "" { client.cliPath = cliPath } } // Resolve the effective connection token: explicit value if set; else if the SDK - // spawns its own runtime in TCP mode, generate a UUID; otherwise empty. + // spawns its own runtime in TCP mode, generate a UUID; otherwise empty. The + // in-process transport uses no socket, so it needs no connection token. if client.tcpConnectionToken != "" { client.effectiveConnectionToken = client.tcpConnectionToken - } else if !client.useStdio && !client.isExternalServer { + } else if !client.useStdio && !client.isExternalServer && !client.useInProcess { client.effectiveConnectionToken = uuid.NewString() } @@ -451,7 +511,7 @@ func (c *Client) Stop() error { c.startStopMux.Lock() defer c.startStopMux.Unlock() - if c.process != nil && !c.isExternalServer && c.RPC != nil { + if (c.process != nil || c.ffiHost != nil) && !c.isExternalServer && c.RPC != nil { rpcClient := c.RPC runtimeShutdownStart := time.Now() shutdownDone := make(chan error, 1) @@ -486,6 +546,13 @@ func (c *Client) Stop() error { } c.process = nil + // Tear down the in-process FFI host (closes the connection and shuts down the + // native runtime). No child process to reap in this mode. + if c.ffiHost != nil { + c.ffiHost.Dispose() + c.ffiHost = nil + } + // Close external TCP connection if exists if c.isExternalServer && c.conn != nil { if err := c.conn.Close(); err != nil { @@ -567,6 +634,12 @@ func (c *Client) ForceStop() { } c.process = nil + // Dispose the in-process FFI host (if any) without waiting on graceful shutdown. + if c.ffiHost != nil { + c.ffiHost.Dispose() + c.ffiHost = nil + } + // Close external TCP connection if exists if c.isExternalServer && c.conn != nil { _ = c.conn.Close() // Ignore errors @@ -1753,6 +1826,10 @@ const stderrBufferSize = 64 * 1024 // This spawns the CLI server as a subprocess using the configured transport // mode (stdio or TCP). func (c *Client) startCLIServer(ctx context.Context) error { + if c.useInProcess { + return c.startInProcess(ctx) + } + cliPath := c.cliPath if cliPath == "" { // If no CLI path is provided, attempt to use the embedded CLI if available @@ -1962,7 +2039,72 @@ func (c *Client) startCLIServer(ctx context.Context) error { } } +// startInProcess hosts the runtime in-process via the native FFI library. It +// resolves the CLI entrypoint (no PATH fallback — the entrypoint must be an +// explicit path, COPILOT_CLI_PATH, or the bundled runtime), loads the sibling +// cdylib, lets the native host spawn the worker, and wires the JSON-RPC client +// to the FFI byte streams exactly like the stdio transport. +func (c *Client) startInProcess(ctx context.Context) error { + entrypoint := c.cliPath + if entrypoint == "" { + // The in-process transport does not resolve a bare command name from PATH + // (unlike the child-process transport); fall back only to COPILOT_CLI_PATH + // and then the bundled runtime, all absolute on-disk paths. + if p := getEnvValue(c.options.Env, "COPILOT_CLI_PATH"); p != "" { + entrypoint = p + } + } + if entrypoint == "" { + entrypoint = embeddedcli.Path() + } + if entrypoint == "" { + return errors.New("in-process transport requires the Copilot CLI: set InProcessConnection.Path or COPILOT_CLI_PATH, or build with the bundled embedded runtime") + } + + host, err := ffihost.Create(entrypoint, nil, c.cliArgs) + if err != nil { + return err + } + // Own the host before the blocking handshake so a cancelled or failed start + // leaves it disposable by Stop/ForceStop rather than leaking (host.Start runs + // on its own goroutine and cannot be interrupted once the native call begins). + c.ffiHost = host + + errCh := make(chan error, 1) + go func() { errCh <- host.Start() }() + select { + case err := <-errCh: + if err != nil { + return err + } + case <-ctx.Done(): + return ctx.Err() + } + + c.client = jsonrpc2.NewClient(host.Writer(), host.Reader()) + c.client.SetOnClose(func() { + // Run in a goroutine to avoid deadlocking with Stop/ForceStop, which hold + // startStopMux while waiting for readLoop to finish. + go func() { + c.startStopMux.Lock() + defer c.startStopMux.Unlock() + c.state = stateDisconnected + }() + }) + c.RPC = rpc.NewServerRPC(c.client) + c.internalRPC = rpc.NewInternalServerRPC(c.client) + c.setupNotificationHandler() + c.client.Start() + return nil +} + func (c *Client) killProcess() error { + // Tear down the in-process FFI host on error paths that reuse killProcess to + // abort a start (there is no OS process to kill in that mode). + if c.ffiHost != nil { + c.ffiHost.Dispose() + c.ffiHost = nil + } if p := c.osProcess.Swap(nil); p != nil { if err := p.Kill(); err != nil { return fmt.Errorf("failed to kill CLI process: %w", err) @@ -2024,8 +2166,8 @@ func (c *Client) monitorProcess() { // connectToServer establishes a connection to the server. func (c *Client) connectToServer(ctx context.Context) error { - if c.useStdio { - // Already connected via stdio in startCLIServer + if c.useStdio || c.useInProcess { + // Already connected: stdio in startCLIServer, FFI streams in startInProcess. return nil } diff --git a/go/client_test.go b/go/client_test.go index 20f8821d53..df92d76cce 100644 --- a/go/client_test.go +++ b/go/client_test.go @@ -625,6 +625,100 @@ func TestClient_EnvOptions(t *testing.T) { }) } +func TestClient_InProcessConnection(t *testing.T) { + t.Run("uses in-process transport", func(t *testing.T) { + client := NewClient(&ClientOptions{Connection: InProcessConnection{Path: "/tmp/copilot"}}) + if !client.useInProcess { + t.Error("Expected useInProcess=true for InProcessConnection") + } + if client.useStdio { + t.Error("Expected useStdio=false for InProcessConnection") + } + if client.isExternalServer { + t.Error("Expected isExternalServer=false for InProcessConnection") + } + if client.cliPath != "/tmp/copilot" { + t.Errorf("Expected cliPath to be '/tmp/copilot', got %q", client.cliPath) + } + }) + + t.Run("does not resolve COPILOT_CLI_PATH into cliPath at construction", func(t *testing.T) { + t.Setenv("COPILOT_CLI_PATH", "/from/env/copilot") + client := NewClient(&ClientOptions{Connection: InProcessConnection{}}) + if client.cliPath != "" { + t.Errorf("Expected in-process cliPath to stay empty at construction, got %q", client.cliPath) + } + }) + + t.Run("panics when Env is set", func(t *testing.T) { + defer func() { + if r := recover(); r == nil { + t.Error("Expected panic when Env is set with InProcessConnection") + } + }() + NewClient(&ClientOptions{ + Connection: InProcessConnection{}, + Env: []string{"FOO=bar"}, + }) + }) + + t.Run("panics when WorkingDirectory is set", func(t *testing.T) { + defer func() { + if r := recover(); r == nil { + t.Error("Expected panic when WorkingDirectory is set with InProcessConnection") + } + }() + NewClient(&ClientOptions{ + Connection: InProcessConnection{}, + WorkingDirectory: "/tmp/work", + }) + }) + + t.Run("panics when Telemetry is set", func(t *testing.T) { + defer func() { + if r := recover(); r == nil { + t.Error("Expected panic when Telemetry is set with InProcessConnection") + } + }() + NewClient(&ClientOptions{ + Connection: InProcessConnection{}, + Telemetry: &TelemetryConfig{ExporterType: "file"}, + }) + }) +} + +func TestClient_ConnectionLevelEnv(t *testing.T) { + t.Run("rejects env set on both client and connection", func(t *testing.T) { + defer func() { + if r := recover(); r == nil { + t.Error("Expected panic when env is set on both client and connection") + } + }() + NewClient(&ClientOptions{ + Connection: StdioConnection{Env: []string{"A=1"}}, + Env: []string{"B=2"}, + }) + }) + + t.Run("stdio connection env is used when client env is unset", func(t *testing.T) { + client := NewClient(&ClientOptions{ + Connection: StdioConnection{Env: []string{"ONLY=conn"}}, + }) + if len(client.options.Env) != 1 || client.options.Env[0] != "ONLY=conn" { + t.Errorf("Expected connection-level Env to be used, got %v", client.options.Env) + } + }) + + t.Run("tcp connection env is used when client env is unset", func(t *testing.T) { + client := NewClient(&ClientOptions{ + Connection: TCPConnection{Port: 9000, Env: []string{"ONLY=conn"}}, + }) + if len(client.options.Env) != 1 || client.options.Env[0] != "ONLY=conn" { + t.Errorf("Expected connection-level Env to be used, got %v", client.options.Env) + } + }) +} + func TestClient_SessionIdleTimeoutSeconds(t *testing.T) { t.Run("should store SessionIdleTimeoutSeconds option", func(t *testing.T) { client := NewClient(&ClientOptions{ diff --git a/go/cmd/bundler/main.go b/go/cmd/bundler/main.go index 1e5f5ecd8b..5b42e25771 100644 --- a/go/cmd/bundler/main.go +++ b/go/cmd/bundler/main.go @@ -91,14 +91,14 @@ func main() { fmt.Printf("Building bundle for %s (CLI version %s)\n", *platform, version) - binaryPath, sha256Hash, err := buildBundle(info, version, outputPath) + binaryPath, sha256Hash, runtimeArtifactPath, runtimeHash, err := buildBundle(info, version, outputPath, goos, goarch) if err != nil { fmt.Fprintf(os.Stderr, "Error: %v\n", err) os.Exit(1) } // Generate the Go file with embed directive - if err := generateGoFile(goos, goarch, binaryPath, version, sha256Hash, "main"); err != nil { + if err := generateGoFile(goos, goarch, binaryPath, version, sha256Hash, runtimeArtifactPath, runtimeHash, "main"); err != nil { fmt.Fprintf(os.Stderr, "Error: %v\n", err) os.Exit(1) } @@ -253,12 +253,16 @@ func isHex(s string) bool { return true } -// buildBundle downloads the CLI binary and writes it to outputPath. -func buildBundle(info platformInfo, cliVersion, outputPath string) (string, []byte, error) { +// buildBundle downloads the CLI binary (and, when the CLI package ships it, the +// native in-process runtime library) and writes them to outputPath's directory. +// It returns the CLI bundle path and hash, plus the runtime-library artifact path +// and hash (both empty when the package does not ship the runtime library). +func buildBundle(info platformInfo, cliVersion, outputPath, goos, goarch string) (string, []byte, string, []byte, error) { outputDir := filepath.Dir(outputPath) if outputDir == "" { outputDir = "." } + runtimeArtifactPath := filepath.Join(outputDir, runtimeLibArtifactName(cliVersion, goos, goarch)) // Check if output already exists if _, err := os.Stat(outputPath); err == nil { @@ -266,49 +270,102 @@ func buildBundle(info platformInfo, cliVersion, outputPath string) (string, []by fmt.Printf("Output %s already exists, skipping download\n", outputPath) sha256Hash, err := sha256FileFromCompressed(outputPath) if err != nil { - return "", nil, fmt.Errorf("failed to hash existing output: %w", err) + return "", nil, "", nil, fmt.Errorf("failed to hash existing output: %w", err) } if err := downloadCLILicense(cliVersion, outputPath); err != nil { - return "", nil, fmt.Errorf("failed to download CLI license: %w", err) + return "", nil, "", nil, fmt.Errorf("failed to download CLI license: %w", err) } - return outputPath, sha256Hash, nil + // Reuse an existing runtime-library artifact if present. + if _, err := os.Stat(runtimeArtifactPath); err == nil { + runtimeHash, err := sha256FileFromCompressed(runtimeArtifactPath) + if err != nil { + return "", nil, "", nil, fmt.Errorf("failed to hash existing runtime library: %w", err) + } + return outputPath, sha256Hash, runtimeArtifactPath, runtimeHash, nil + } + return outputPath, sha256Hash, "", nil, nil } // Create temp directory for download tempDir, err := os.MkdirTemp("", "copilot-bundler-*") if err != nil { - return "", nil, fmt.Errorf("failed to create temp dir: %w", err) + return "", nil, "", nil, fmt.Errorf("failed to create temp dir: %w", err) } defer os.RemoveAll(tempDir) // Download the binary - binaryPath, err := downloadCLIBinary(info.npmPlatform, info.binaryName, cliVersion, tempDir) + binaryPath, tarballPath, err := downloadCLIBinary(info.npmPlatform, info.binaryName, cliVersion, tempDir) if err != nil { - return "", nil, fmt.Errorf("failed to download CLI binary: %w", err) + return "", nil, "", nil, fmt.Errorf("failed to download CLI binary: %w", err) } // Create output directory if needed if outputDir != "." { if err := os.MkdirAll(outputDir, 0755); err != nil { - return "", nil, fmt.Errorf("failed to create output directory: %w", err) + return "", nil, "", nil, fmt.Errorf("failed to create output directory: %w", err) } } sha256Hash, err := sha256File(binaryPath) if err != nil { - return "", nil, fmt.Errorf("failed to hash output binary: %w", err) + return "", nil, "", nil, fmt.Errorf("failed to hash output binary: %w", err) } if err := compressZstdFile(binaryPath, outputPath); err != nil { - return "", nil, fmt.Errorf("failed to write output binary: %w", err) + return "", nil, "", nil, fmt.Errorf("failed to write output binary: %w", err) } if err := downloadCLILicense(cliVersion, outputPath); err != nil { - return "", nil, fmt.Errorf("failed to download CLI license: %w", err) + return "", nil, "", nil, fmt.Errorf("failed to download CLI license: %w", err) + } + + // Extract the native in-process runtime library from the same tarball, if the + // package ships it (older CLI versions do not). Missing is not an error — the + // generated file simply omits the runtime embed for that platform. + rawLibPath := filepath.Join(tempDir, "runtime.node") + found, err := extractOptionalFileFromTarball(tarballPath, tempDir, + "package/prebuilds/"+info.npmPlatform+"/runtime.node", "runtime.node") + if err != nil { + return "", nil, "", nil, fmt.Errorf("failed to extract runtime library: %w", err) } + var runtimeHash []byte + returnedRuntimeArtifact := "" + if found { + runtimeHash, err = sha256File(rawLibPath) + if err != nil { + return "", nil, "", nil, fmt.Errorf("failed to hash runtime library: %w", err) + } + if err := compressZstdFile(rawLibPath, runtimeArtifactPath); err != nil { + return "", nil, "", nil, fmt.Errorf("failed to write runtime library: %w", err) + } + returnedRuntimeArtifact = runtimeArtifactPath + fmt.Printf("Successfully created %s\n", runtimeArtifactPath) + } else { + fmt.Printf("Package %s does not ship a runtime library; in-process transport unavailable for this platform bundle\n", info.npmPlatform) + } + fmt.Printf("Successfully created %s\n", outputPath) - return outputPath, sha256Hash, nil + return outputPath, sha256Hash, returnedRuntimeArtifact, runtimeHash, nil +} + +// runtimeLibArtifactName builds the compressed runtime-library artifact filename. +func runtimeLibArtifactName(version, goos, goarch string) string { + return fmt.Sprintf("zcopilotruntime_%s_%s_%s.%s.zst", version, goos, goarch, runtimeLibExt(goos)) +} + +// runtimeLibExt returns the shared-library extension for the target OS. +func runtimeLibExt(goos string) string { + switch goos { + case "windows": + return "dll" + case "darwin": + return "dylib" + default: + return "so" + } } // generateGoFile creates a Go source file that embeds the binary and metadata. -func generateGoFile(goos, goarch, binaryPath, cliVersion string, sha256Hash []byte, pkgName string) error { +// When runtimeArtifactPath is non-empty, it also embeds the native in-process +// runtime library and wires it into the embeddedcli configuration. +func generateGoFile(goos, goarch, binaryPath, cliVersion string, sha256Hash []byte, runtimeArtifactPath string, runtimeHash []byte, pkgName string) error { // Generate Go file path: zcopilot_linux_amd64.go (without version) binaryName := filepath.Base(binaryPath) licenseName := licenseFileName(binaryName) @@ -319,6 +376,34 @@ func generateGoFile(goos, goarch, binaryPath, cliVersion string, sha256Hash []by hashBase64 = base64.StdEncoding.EncodeToString(sha256Hash) } + // Build the optional runtime-library embed fragments. When the platform + // package does not ship the runtime, these stay empty and the generated file + // omits the RuntimeLib config (in-process transport is then unavailable for + // that platform bundle, mirroring the other SDKs' conditional native embed). + runtimeEmbed := "" + runtimeConfig := "" + runtimeReader := "" + if runtimeArtifactPath != "" { + runtimeArtifactName := filepath.Base(runtimeArtifactPath) + runtimeHashBase64 := base64.StdEncoding.EncodeToString(runtimeHash) + runtimeEmbed = fmt.Sprintf(` +//go:embed %s +var localEmbeddedCopilotRuntimeLib []byte +`, runtimeArtifactName) + runtimeConfig = fmt.Sprintf(` + RuntimeLib: runtimeLibReader(), + RuntimeLibHash: mustDecodeBase64(%q),`, runtimeHashBase64) + runtimeReader = ` +func runtimeLibReader() io.Reader { + r, err := zstd.NewReader(bytes.NewReader(localEmbeddedCopilotRuntimeLib)) + if err != nil { + panic("failed to create zstd reader: " + err.Error()) + } + return r +} +` + } + content := fmt.Sprintf(`// Code generated by copilot-sdk bundler; DO NOT EDIT. package %s @@ -338,14 +423,14 @@ var localEmbeddedCopilotCLI []byte //go:embed %s var localEmbeddedCopilotCLILicense []byte - +%s func init() { embeddedcli.Setup(embeddedcli.Config{ Cli: cliReader(), License: localEmbeddedCopilotCLILicense, Version: %q, - CliHash: mustDecodeBase64(%q), + CliHash: mustDecodeBase64(%q),%s }) } @@ -356,7 +441,7 @@ func cliReader() io.Reader { } return r } - +%s func mustDecodeBase64(s string) []byte { b, err := base64.StdEncoding.DecodeString(s) if err != nil { @@ -364,7 +449,7 @@ func mustDecodeBase64(s string) []byte { } return b } -`, pkgName, binaryName, licenseName, cliVersion, hashBase64) +`, pkgName, binaryName, licenseName, runtimeEmbed, cliVersion, hashBase64, runtimeConfig, runtimeReader) if err := os.WriteFile(goFilePath, []byte(content), 0644); err != nil { return err @@ -374,63 +459,65 @@ func mustDecodeBase64(s string) []byte { return nil } -// downloadCLIBinary downloads the npm tarball and extracts the CLI binary. -func downloadCLIBinary(npmPlatform, binaryName, cliVersion, destDir string) (string, error) { +// downloadCLIBinary downloads the npm tarball and extracts the CLI binary. It +// returns the extracted binary path and the downloaded tarball path (retained so +// callers can extract additional files, such as the runtime library). +func downloadCLIBinary(npmPlatform, binaryName, cliVersion, destDir string) (string, string, error) { tarballURL := fmt.Sprintf(tarballURLFmt, npmPlatform, npmPlatform, cliVersion) fmt.Printf("Downloading from %s...\n", tarballURL) resp, err := http.Get(tarballURL) if err != nil { - return "", fmt.Errorf("failed to download: %w", err) + return "", "", fmt.Errorf("failed to download: %w", err) } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - return "", fmt.Errorf("failed to download: %s", resp.Status) + return "", "", fmt.Errorf("failed to download: %s", resp.Status) } // Save tarball to temp file tarballPath := filepath.Join(destDir, fmt.Sprintf("copilot-%s-%s.tgz", npmPlatform, cliVersion)) tarballFile, err := os.Create(tarballPath) if err != nil { - return "", fmt.Errorf("failed to create tarball file: %w", err) + return "", "", fmt.Errorf("failed to create tarball file: %w", err) } if _, err := io.Copy(tarballFile, resp.Body); err != nil { tarballFile.Close() - return "", fmt.Errorf("failed to save tarball: %w", err) + return "", "", fmt.Errorf("failed to save tarball: %w", err) } if err := tarballFile.Close(); err != nil { - return "", fmt.Errorf("failed to close tarball file: %w", err) + return "", "", fmt.Errorf("failed to close tarball file: %w", err) } // Extract only the CLI binary to avoid unpacking the full package tree. binaryPath := filepath.Join(destDir, binaryName) if err := extractFileFromTarball(tarballPath, destDir, "package/"+binaryName, binaryName); err != nil { - return "", fmt.Errorf("failed to extract binary: %w", err) + return "", "", fmt.Errorf("failed to extract binary: %w", err) } // Verify binary exists if _, err := os.Stat(binaryPath); err != nil { - return "", fmt.Errorf("binary not found after extraction: %w", err) + return "", "", fmt.Errorf("binary not found after extraction: %w", err) } // Make executable on Unix if !strings.HasSuffix(binaryName, ".exe") { if err := os.Chmod(binaryPath, 0755); err != nil { - return "", fmt.Errorf("failed to chmod binary: %w", err) + return "", "", fmt.Errorf("failed to chmod binary: %w", err) } } stat, err := os.Stat(binaryPath) if err != nil { - return "", fmt.Errorf("failed to stat binary: %w", err) + return "", "", fmt.Errorf("failed to stat binary: %w", err) } sizeMB := float64(stat.Size()) / 1024 / 1024 fmt.Printf("Downloaded %s (%.1f MB)\n", binaryName, sizeMB) - return binaryPath, nil + return binaryPath, tarballPath, nil } // downloadCLILicense downloads the @github/copilot package and writes its license next to outputPath. @@ -561,6 +648,21 @@ func extractFileFromTarball(tarballPath, destDir, targetPath, outputName string) return fmt.Errorf("file %q not found in tarball", targetPath) } +// extractOptionalFileFromTarball extracts a single file from a .tgz into destDir +// like extractFileFromTarball, but returns (false, nil) instead of an error when +// the file is absent. Used for the runtime library, which older CLI packages do +// not ship. +func extractOptionalFileFromTarball(tarballPath, destDir, targetPath, outputName string) (bool, error) { + err := extractFileFromTarball(tarballPath, destDir, targetPath, outputName) + if err == nil { + return true, nil + } + if strings.Contains(err.Error(), "not found in tarball") { + return false, nil + } + return false, err +} + // compressZstdFile compresses src into dst using zstd. func compressZstdFile(src, dst string) error { srcFile, err := os.Open(src) diff --git a/go/go.mod b/go/go.mod index 586a5d3360..ba0f4feb73 100644 --- a/go/go.mod +++ b/go/go.mod @@ -9,6 +9,7 @@ require ( require ( github.com/coder/websocket v1.8.15 + github.com/ebitengine/purego v0.10.1 github.com/google/uuid v1.6.0 go.opentelemetry.io/otel v1.35.0 go.opentelemetry.io/otel/trace v1.35.0 diff --git a/go/go.sum b/go/go.sum index e7ac53d5a4..cab5b6aabe 100644 --- a/go/go.sum +++ b/go/go.sum @@ -2,6 +2,8 @@ github.com/coder/websocket v1.8.15 h1:6B2JPeOGlpff2Uz6vOEH1Vzpi0iUz20A+lPVhPHtNU github.com/coder/websocket v1.8.15/go.mod h1:NX3SzP+inril6yawo5CQXx8+fk145lPDC6pumgx0mVg= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/ebitengine/purego v0.10.1 h1:dewVBCBT2GaMu1SrNTYxQhgQBethzfhiwvZiLGP/qyY= +github.com/ebitengine/purego v0.10.1/go.mod h1:iIjxzd6CiRiOG0UyXP+V1+jWqUXVjPKLAI0mRfJZTmQ= github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= github.com/go-logr/logr v1.4.3 h1:CjnDlHq8ikf6E492q6eKboGOC0T8CDaOvkHCIg8idEI= github.com/go-logr/logr v1.4.3/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= diff --git a/go/internal/e2e/inprocess_ffi_e2e_test.go b/go/internal/e2e/inprocess_ffi_e2e_test.go new file mode 100644 index 0000000000..c965d74d44 --- /dev/null +++ b/go/internal/e2e/inprocess_ffi_e2e_test.go @@ -0,0 +1,51 @@ +package e2e + +import ( + "testing" + + copilot "github.com/github/copilot-sdk/go" + "github.com/github/copilot-sdk/go/internal/e2e/testharness" +) + +// TestInProcessFfiE2E is a smoke test for the in-process (FFI) transport. It +// starts a client that loads the native runtime cdylib next to the resolved CLI +// entrypoint, lets the native host spawn the worker, performs a purely local +// "ping" round-trip through the runtime, and stops cleanly. No auth or replay +// proxy is involved, so it needs no snapshot. +// +// Mirrors python/e2e/test_inprocess_ffi_e2e.py and +// nodejs/test/e2e/inprocess_ffi.e2e.test.ts. +func TestInProcessFfiE2E(t *testing.T) { + cliPath := testharness.CLIPath() + if cliPath == "" { + t.Fatal("CLI not found. Run 'npm install' in the nodejs directory first.") + } + + t.Run("should start and connect over in-process FFI", func(t *testing.T) { + client := copilot.NewClient(&copilot.ClientOptions{ + Connection: copilot.InProcessConnection{Path: cliPath}, + }) + t.Cleanup(func() { client.ForceStop() }) + + if err := client.Start(t.Context()); err != nil { + t.Fatalf("Failed to start client over in-process FFI: %v", err) + } + + pong, err := client.Ping(t.Context(), "ffi message") + if err != nil { + t.Fatalf("Failed to ping: %v", err) + } + + if pong.Message != "pong: ffi message" { + t.Errorf("Expected pong.message to be 'pong: ffi message', got %q", pong.Message) + } + + if pong.Timestamp.IsZero() { + t.Errorf("Expected non-zero pong.timestamp, got %s", pong.Timestamp) + } + + if err := client.Stop(); err != nil { + t.Errorf("Expected no errors on stop, got %v", err) + } + }) +} diff --git a/go/internal/e2e/testharness/context.go b/go/internal/e2e/testharness/context.go index 6b6463749f..63d09dabaa 100644 --- a/go/internal/e2e/testharness/context.go +++ b/go/internal/e2e/testharness/context.go @@ -53,6 +53,36 @@ type TestContext struct { ProxyURL string proxy *CapiProxy + + // In-process transport state. When the inprocess CI matrix cell is active the + // worker inherits this process's ambient env and cwd (per-client env/working + // directory are rejected in-process), so the isolated test env/cwd are mirrored + // onto the real process and restored on Close. + inProcess bool + restoreEnv []envRestore + restoreCwd string +} + +// envRestore captures a single environment variable's prior value so the +// in-process ambient mirror can be undone during teardown. +type envRestore struct { + key string + prev string + had bool +} + +// isInProcessTransport reports whether the in-process (FFI) transport is selected +// for E2E tests via COPILOT_SDK_DEFAULT_CONNECTION=inprocess. Mirrors the +// Node/Python/.NET harnesses. +func isInProcessTransport() bool { + return strings.EqualFold(os.Getenv("COPILOT_SDK_DEFAULT_CONNECTION"), "inprocess") +} + +// IsInProcessTransport reports whether E2E tests run under the in-process (FFI) +// transport. Tests that configure options unsupported in-process (e.g. per-client +// telemetry) should skip when this returns true. +func IsInProcessTransport() bool { + return isInProcessTransport() } // NewTestContext creates a new test context with isolated directories and a replaying proxy. @@ -106,11 +136,12 @@ func NewTestContext(t *testing.T) *TestContext { } ctx := &TestContext{ - CLIPath: cliPath, - HomeDir: homeDir, - WorkDir: workDir, - ProxyURL: proxyURL, - proxy: proxy, + CLIPath: cliPath, + HomeDir: homeDir, + WorkDir: workDir, + ProxyURL: proxyURL, + proxy: proxy, + inProcess: isInProcessTransport(), } t.Cleanup(func() { @@ -172,6 +203,7 @@ func (c *TestContext) ConfigureWithoutSnapshot(t *testing.T) { // Close cleans up the test context resources. func (c *TestContext) Close(testFailed bool) { + c.restoreInProcessEnvironment() if c.proxy != nil { c.proxy.StopWithOptions(testFailed) } @@ -183,6 +215,60 @@ func (c *TestContext) Close(testFailed bool) { } } +// applyInProcessEnvironment mirrors the isolated test environment onto the real +// process for in-process hosting: the worker inherits this process's env and cwd +// at spawn, so per-test redirects must live on os.Environ and the process cwd. +// Auth flows via GH_TOKEN/GITHUB_TOKEN (the FFI argv omits the stdio auth-token +// wiring) and HMAC is disabled so host-side auth matches the replay snapshots. +// mergedEnv is the effective per-client env (harness defaults plus any per-test +// additions); workDir is the effective working directory. Values are restored in +// Close. Safe to call more than once (restores unwind in reverse). +func (c *TestContext) applyInProcessEnvironment(mergedEnv []string, workDir string) { + inprocessEnv := map[string]string{} + for _, kv := range mergedEnv { + if key, value, ok := strings.Cut(kv, "="); ok { + inprocessEnv[key] = value + } + } + // Auth flows via GH_TOKEN/GITHUB_TOKEN and HMAC is disabled for the in-process + // host, overriding any inherited values. + inprocessEnv["GH_TOKEN"] = defaultGitHubToken + inprocessEnv["GITHUB_TOKEN"] = defaultGitHubToken + inprocessEnv["COPILOT_HMAC_KEY"] = "" + inprocessEnv["CAPI_HMAC_KEY"] = "" + + for key, value := range inprocessEnv { + prev, had := os.LookupEnv(key) + c.restoreEnv = append(c.restoreEnv, envRestore{key: key, prev: prev, had: had}) + os.Setenv(key, value) + } + if workDir != "" { + if c.restoreCwd == "" { + if cwd, err := os.Getwd(); err == nil { + c.restoreCwd = cwd + } + } + os.Chdir(workDir) + } +} + +// restoreInProcessEnvironment undoes applyInProcessEnvironment during teardown. +func (c *TestContext) restoreInProcessEnvironment() { + for i := len(c.restoreEnv) - 1; i >= 0; i-- { + r := c.restoreEnv[i] + if r.had { + os.Setenv(r.key, r.prev) + } else { + os.Unsetenv(r.key) + } + } + c.restoreEnv = nil + if c.restoreCwd != "" { + os.Chdir(c.restoreCwd) + c.restoreCwd = "" + } +} + // GetExchanges retrieves the captured HTTP exchanges from the proxy. func (c *TestContext) GetExchanges() ([]ParsedHttpExchange, error) { return c.proxy.GetExchanges() @@ -261,9 +347,38 @@ func (c *TestContext) NewClient(opts ...func(*copilot.ClientOptions)) *copilot.C options.GitHubToken = defaultGitHubToken } + // Under the inprocess matrix cell, host the default stdio connection in-process. + // The worker inherits this process's ambient env/cwd (per-client env and working + // directory are rejected in-process), so mirror the effective (merged) env and + // cwd onto the real process and drop those options. Tests that pin a specific + // transport (TCP/URI/custom stdio) or configure per-client telemetry are left on + // their transport, mirroring the Node/.NET harnesses. + if c.inProcess && c.shouldUseInProcess(options) { + c.applyInProcessEnvironment(options.Env, options.WorkingDirectory) + options.Connection = copilot.InProcessConnection{Path: c.CLIPath} + options.Env = nil + options.WorkingDirectory = "" + } + return copilot.NewClient(options) } +// shouldUseInProcess reports whether a client built from options should be hosted +// in-process for the inprocess matrix cell. Only the harness default stdio +// connection is swapped; a test that pins a custom stdio path/args/env, a TCP/URI +// connection, or configures per-client telemetry (which cannot be carried +// in-process) is exercising behavior that must stay on its own transport. +func (c *TestContext) shouldUseInProcess(options *copilot.ClientOptions) bool { + if options.Telemetry != nil { + return false + } + s, ok := options.Connection.(copilot.StdioConnection) + if !ok { + return false + } + return s.Path == c.CLIPath && len(s.Args) == 0 && s.Env == nil +} + func fileExists(path string) bool { _, err := os.Stat(path) return err == nil diff --git a/go/internal/embeddedcli/embeddedcli.go b/go/internal/embeddedcli/embeddedcli.go index 0866a3f811..9f26451ba4 100644 --- a/go/internal/embeddedcli/embeddedcli.go +++ b/go/internal/embeddedcli/embeddedcli.go @@ -21,12 +21,20 @@ import ( // system cache directory. Version is used to suffix the installed binary name to // allow multiple versions to coexist. License, when provided, is written next // to the installed binary. +// +// RuntimeLib and RuntimeLibHash are optional: when set, the native in-process +// runtime library (cdylib) is installed next to the CLI binary under its natural +// platform name so the in-process (FFI) transport can load it. They are omitted +// for CLI packages that do not ship the native runtime. type Config struct { Cli io.Reader CliHash []byte License []byte + RuntimeLib io.Reader + RuntimeLibHash []byte + Dir string Version string } @@ -61,11 +69,22 @@ var Path = sync.OnceValue(func() string { return path }) +// RuntimeLibPath returns the on-disk path to the installed native in-process +// runtime library (cdylib), or "" when no runtime library was bundled or the +// CLI could not be installed. It ensures the embedded CLI is installed first. +func RuntimeLibPath() string { + Path() + setupMu.Lock() + defer setupMu.Unlock() + return runtimeLibPath +} + var ( config Config setupMu sync.Mutex setupDone bool pathInitialized bool + runtimeLibPath string ) func install() (path string) { @@ -155,9 +174,85 @@ func installAt(installDir string) (string, error) { return "", fmt.Errorf("writing license file: %w", err) } } + + // Install the native in-process runtime library (if bundled) next to the CLI + // binary under its natural platform name, so the FFI transport resolves it via + // the flat-name lookup. Fail closed on any hash mismatch — never place + // unverified native code. + if config.RuntimeLib != nil { + libPath, err := installRuntimeLib(installDir) + if err != nil { + return "", err + } + runtimeLibPath = libPath + } + return finalPath, nil } +// installRuntimeLib writes the embedded runtime cdylib into installDir under its +// natural platform file name, verifying its SHA-256. It is idempotent: an +// existing file with a matching hash is reused; a mismatch is a hard error. +func installRuntimeLib(installDir string) (string, error) { + if len(config.RuntimeLibHash) != sha256.Size { + return "", fmt.Errorf("RuntimeLibHash must be a SHA-256 hash (%d bytes), got %d bytes", sha256.Size, len(config.RuntimeLibHash)) + } + libPath := filepath.Join(installDir, naturalRuntimeLibName()) + + if _, err := os.Stat(libPath); err == nil { + existingHash, err := hashFile(libPath) + if err != nil { + return "", fmt.Errorf("hashing existing runtime library: %w", err) + } + if !bytes.Equal(existingHash, config.RuntimeLibHash) { + return "", fmt.Errorf("existing runtime library hash mismatch") + } + return libPath, nil + } + + // Write to a temp file in the same directory, verify, then atomically rename. + tmp, err := os.CreateTemp(installDir, ".copilot-runtime-*.tmp") + if err != nil { + return "", fmt.Errorf("creating temp runtime library: %w", err) + } + tmpPath := tmp.Name() + h := sha256.New() + _, err = io.Copy(io.MultiWriter(tmp, h), config.RuntimeLib) + if err1 := tmp.Close(); err1 != nil && err == nil { + err = err1 + } + if closer, ok := config.RuntimeLib.(io.Closer); ok { + closer.Close() + } + if err != nil { + os.Remove(tmpPath) + return "", fmt.Errorf("writing runtime library: %w", err) + } + if !bytes.Equal(h.Sum(nil), config.RuntimeLibHash) { + os.Remove(tmpPath) + return "", fmt.Errorf("runtime library hash mismatch") + } + if err := os.Rename(tmpPath, libPath); err != nil { + os.Remove(tmpPath) + return "", fmt.Errorf("installing runtime library: %w", err) + } + return libPath, nil +} + +// naturalRuntimeLibName is the flat platform file name for the runtime cdylib, +// matching ffihost.NaturalLibraryName (kept in sync; embeddedcli stays +// dependency-free for use by generated embed files). +func naturalRuntimeLibName() string { + switch runtime.GOOS { + case "windows": + return "copilot_runtime.dll" + case "darwin": + return "libcopilot_runtime.dylib" + default: + return "libcopilot_runtime.so" + } +} + // versionedBinaryPath builds the unpacked binary filename with an optional version suffix. func versionedBinaryPath(dir, binaryName, version string) string { if version == "" { diff --git a/go/internal/ffihost/buffer.go b/go/internal/ffihost/buffer.go new file mode 100644 index 0000000000..48774de5c1 --- /dev/null +++ b/go/internal/ffihost/buffer.go @@ -0,0 +1,65 @@ +package ffihost + +import ( + "io" + "sync" +) + +// receiveBuffer is a thread-safe byte buffer that feeds blocking Read from a +// producer thread. The native outbound callback (invoked on a foreign runtime +// thread) appends frames via feed without ever blocking; the JSON-RPC reader +// goroutine drains them via Read, which blocks until data or EOF. +// +// It implements io.ReadCloser so it can be handed to jsonrpc2.NewClient as the +// server → client stream. +type receiveBuffer struct { + mu sync.Mutex + cond *sync.Cond + buf []byte + closed bool +} + +func newReceiveBuffer() *receiveBuffer { + rb := &receiveBuffer{} + rb.cond = sync.NewCond(&rb.mu) + return rb +} + +func (rb *receiveBuffer) feed(data []byte) { + rb.mu.Lock() + defer rb.mu.Unlock() + if rb.closed { + return + } + rb.buf = append(rb.buf, data...) + rb.cond.Broadcast() +} + +// Read blocks until at least one byte is available or the buffer is closed. +// It returns io.EOF only once the buffer is closed and fully drained. +func (rb *receiveBuffer) Read(p []byte) (int, error) { + if len(p) == 0 { + return 0, nil + } + rb.mu.Lock() + defer rb.mu.Unlock() + for len(rb.buf) == 0 && !rb.closed { + rb.cond.Wait() + } + if len(rb.buf) == 0 { + return 0, io.EOF + } + n := copy(p, rb.buf) + rb.buf = rb.buf[n:] + return n, nil +} + +// Close marks the buffer closed; subsequent Reads drain remaining bytes then +// return io.EOF. Idempotent. +func (rb *receiveBuffer) Close() error { + rb.mu.Lock() + defer rb.mu.Unlock() + rb.closed = true + rb.cond.Broadcast() + return nil +} diff --git a/go/internal/ffihost/ffihost.go b/go/internal/ffihost/ffihost.go new file mode 100644 index 0000000000..df613fbc97 --- /dev/null +++ b/go/internal/ffihost/ffihost.go @@ -0,0 +1,321 @@ +// Package ffihost hosts the Copilot runtime in-process by loading the native +// runtime library (runtime.node — a Rust cdylib) and driving JSON-RPC over its +// C ABI (FFI) instead of spawning the CLI as a child process and talking over +// stdio/TCP. +// +// The native copilot_runtime_host_start export spawns the residual CLI worker +// itself (` --embedded-host --no-auto-update`), so the SDK never +// launches the worker directly; it only pumps opaque LSP `Content-Length:`-framed +// JSON-RPC bytes across the boundary: +// +// - client → server frames go to copilot_runtime_connection_write +// - server → client frames arrive on a native callback that feeds a +// thread-safe receive buffer read by the JSON-RPC client +// +// The existing internal/jsonrpc2 client handles framing unchanged — this is a +// transport swap, not a new protocol. Host exposes an io.WriteCloser (client → +// server) and io.ReadCloser (server → client) that plug straight into +// jsonrpc2.NewClient. +// +// The C ABI (shared with the .NET, Node.js, Python, and Rust SDKs): +// +// uint32 copilot_runtime_host_start(uint8 *argv, size_t argv_len, +// uint8 *env, size_t env_len); +// bool copilot_runtime_host_shutdown(uint32 server_id); +// uint32 copilot_runtime_connection_open(uint32 server_id, outbound cb, +// void *user_data, +// uint8 *a, size_t a_len, +// uint8 *b, size_t b_len, +// uint8 *c, size_t c_len); +// bool copilot_runtime_connection_write(uint32 conn_id, uint8 *bytes, size_t len); +// bool copilot_runtime_connection_close(uint32 conn_id); +// // outbound callback: +// void outbound(void *user_data, uint8 *bytes, size_t len); +// +// The native binding uses github.com/ebitengine/purego so the library is loaded +// at runtime with CGO disabled, preserving the SDK's pure-Go build and +// cross-compilation. +package ffihost + +import ( + "encoding/json" + "fmt" + "io" + "runtime" + "strings" + "sync" + "unsafe" + + "github.com/ebitengine/purego" +) + +const symbolPrefix = "copilot_runtime_" + +// ffiLibrary binds the copilot_runtime_* C ABI exports of a loaded cdylib. +type ffiLibrary struct { + hostStart func(argv unsafe.Pointer, argvLen uintptr, env unsafe.Pointer, envLen uintptr) uint32 + hostShutdown func(serverID uint32) bool + connectionOpen func(serverID uint32, cb uintptr, userData uintptr, a unsafe.Pointer, aLen uintptr, b unsafe.Pointer, bLen uintptr, c unsafe.Pointer, cLen uintptr) uint32 + connectionWrite func(connID uint32, bytes unsafe.Pointer, length uintptr) bool + connectionClose func(connID uint32) bool +} + +// The cdylib may only be loaded once per process; a second load of a different +// path is unsupported (matches the .NET/Node/Python/Rust hosts). Guard it here. +var ( + loadMu sync.Mutex + loadedLibrary *ffiLibrary + loadedLibraryPath string +) + +func loadLibrary(libraryPath string) (lib *ffiLibrary, err error) { + loadMu.Lock() + defer loadMu.Unlock() + + if loadedLibrary != nil { + if loadedLibraryPath != libraryPath { + return nil, fmt.Errorf( + "an in-process FFI runtime library is already loaded from %q; loading a different library from %q in the same process is not supported", + loadedLibraryPath, libraryPath) + } + return loadedLibrary, nil + } + + handle, err := openLibrary(libraryPath) + if err != nil { + return nil, fmt.Errorf("loading FFI runtime library %q: %w", libraryPath, err) + } + + // RegisterLibFunc panics if a symbol is missing; convert that to an error so + // callers get a clean failure instead of a crash. + defer func() { + if r := recover(); r != nil { + err = fmt.Errorf("binding FFI runtime library %q: %v", libraryPath, r) + lib = nil + } + }() + + bound := &ffiLibrary{} + purego.RegisterLibFunc(&bound.hostStart, handle, symbolPrefix+"host_start") + purego.RegisterLibFunc(&bound.hostShutdown, handle, symbolPrefix+"host_shutdown") + purego.RegisterLibFunc(&bound.connectionOpen, handle, symbolPrefix+"connection_open") + purego.RegisterLibFunc(&bound.connectionWrite, handle, symbolPrefix+"connection_write") + purego.RegisterLibFunc(&bound.connectionClose, handle, symbolPrefix+"connection_close") + + loadedLibrary = bound + loadedLibraryPath = libraryPath + return bound, nil +} + +// Host hosts the Copilot runtime in-process via its native C ABI. +// +// Construct with Create, call Start to spawn the worker and open the FFI +// connection, wire Writer/Reader into jsonrpc2.NewClient, and call Dispose to +// tear everything down. +type Host struct { + libraryPath string + cliEntrypoint string + environment map[string]string + extraArgs []string + lib *ffiLibrary + + serverID uint32 + connectionID uint32 + + recv *receiveBuffer + + disposeMu sync.Mutex + disposed bool + + // callbackHandle is the purego callback trampoline address. purego keeps the + // underlying Go closure alive for the process lifetime, so it (and this Host, + // which the closure captures) must not be relied upon to be freed. + callbackHandle uintptr + // Serializes teardown against in-flight native callbacks so the receive + // buffer is not fed after it is closed. + callbackMu sync.Mutex + activeCallbacks int +} + +// Create resolves the cdylib next to the CLI entrypoint and prepares the host. +// It does not spawn the worker; call Start for that. environment may be nil. +func Create(cliEntrypoint string, environment map[string]string, extraArgs []string) (*Host, error) { + libraryPath, err := ResolveLibraryPath(cliEntrypoint) + if err != nil { + return nil, err + } + lib, err := loadLibrary(libraryPath) + if err != nil { + return nil, err + } + return &Host{ + libraryPath: libraryPath, + cliEntrypoint: cliEntrypoint, + environment: environment, + extraArgs: append([]string(nil), extraArgs...), + lib: lib, + recv: newReceiveBuffer(), + }, nil +} + +// Start spawns the worker and opens the FFI connection. It blocks until the +// worker connects back and signals readiness (up to ~30s), so callers should +// run it off any latency-sensitive goroutine. +func (h *Host) Start() error { + argv := h.buildArgv() + env := h.buildEnv() + + var argvPtr, envPtr unsafe.Pointer + if len(argv) > 0 { + argvPtr = unsafe.Pointer(&argv[0]) + } + if len(env) > 0 { + envPtr = unsafe.Pointer(&env[0]) + } + + h.serverID = h.lib.hostStart(argvPtr, uintptr(len(argv)), envPtr, uintptr(len(env))) + // Keep the JSON buffers alive across the (synchronous) native call. + runtime.KeepAlive(argv) + runtime.KeepAlive(env) + if h.serverID == 0 { + return fmt.Errorf("copilot_runtime_host_start failed (library %q, entrypoint %q)", h.libraryPath, h.cliEntrypoint) + } + + h.callbackHandle = purego.NewCallback(h.onOutbound) + h.connectionID = h.lib.connectionOpen(h.serverID, h.callbackHandle, 0, nil, 0, nil, 0, nil, 0) + if h.connectionID == 0 { + h.lib.hostShutdown(h.serverID) + h.serverID = 0 + return fmt.Errorf("copilot_runtime_connection_open failed") + } + return nil +} + +// Writer returns the client → server frame sink (plug into jsonrpc2 as stdin). +func (h *Host) Writer() io.WriteCloser { return hostWriter{h} } + +// Reader returns the server → client frame source (plug into jsonrpc2 as stdout). +func (h *Host) Reader() io.ReadCloser { return h.recv } + +func (h *Host) buildArgv() []byte { + // A `.js` entrypoint (dev) is launched via node; the packaged single-file CLI + // embeds its own Node and is invoked directly. `--no-auto-update` pins the + // worker to the runtime package matching the loaded cdylib (avoids ABI skew). + var argv []string + if strings.HasSuffix(strings.ToLower(h.cliEntrypoint), ".js") { + argv = []string{"node", h.cliEntrypoint, "--embedded-host", "--no-auto-update"} + } else { + argv = []string{h.cliEntrypoint, "--embedded-host", "--no-auto-update"} + } + argv = append(argv, h.extraArgs...) + b, _ := json.Marshal(argv) + return b +} + +func (h *Host) buildEnv() []byte { + if len(h.environment) == 0 { + return nil + } + b, _ := json.Marshal(h.environment) + return b +} + +// onOutbound is the native server → client callback, invoked on a foreign +// runtime thread. The native pointer is only valid for this call, so the bytes +// are copied out before returning. Nothing may panic across the FFI boundary. +func (h *Host) onOutbound(userData uintptr, bytesPtr uintptr, bytesLen uintptr) uintptr { + _ = userData + h.callbackMu.Lock() + if h.disposed { + h.callbackMu.Unlock() + return 0 + } + h.activeCallbacks++ + h.callbackMu.Unlock() + + defer func() { + h.callbackMu.Lock() + h.activeCallbacks-- + h.callbackMu.Unlock() + // Never let a panic unwind into native code. + _ = recover() + }() + + if bytesPtr != 0 && bytesLen > 0 { + // The native runtime delivers the outbound frame as a raw buffer address + // (uintptr) plus length. Materialize a slice over it just long enough to + // copy the bytes into Go-owned memory before returning to native code. + //nolint:govet // FFI callback receives the buffer address as an integer; converting it to a pointer to copy out is the intended, checked-length use. + src := unsafe.Slice((*byte)(unsafe.Pointer(bytesPtr)), int(bytesLen)) + buf := make([]byte, len(src)) + copy(buf, src) + h.recv.feed(buf) + } + return 0 +} + +func (h *Host) writeFrame(frame []byte) (int, error) { + h.disposeMu.Lock() + closed := h.disposed || h.connectionID == 0 + connID := h.connectionID + h.disposeMu.Unlock() + if closed { + return 0, fmt.Errorf("the in-process runtime connection is closed") + } + if len(frame) == 0 { + return 0, nil + } + ok := h.lib.connectionWrite(connID, unsafe.Pointer(&frame[0]), uintptr(len(frame))) + runtime.KeepAlive(frame) + if !ok { + return 0, fmt.Errorf("failed to write a frame to the in-process runtime connection") + } + return len(frame), nil +} + +// Dispose closes the FFI connection, shuts down the native host, and releases +// resources. It is idempotent and waits for any in-flight outbound callback to +// finish before closing the receive buffer. +func (h *Host) Dispose() { + h.disposeMu.Lock() + if h.disposed { + h.disposeMu.Unlock() + return + } + h.disposed = true + connID := h.connectionID + serverID := h.serverID + h.connectionID = 0 + h.serverID = 0 + h.disposeMu.Unlock() + + // Stop accepting new callbacks and wait for in-flight ones to drain before + // closing the receive buffer they feed. + for { + h.callbackMu.Lock() + if h.activeCallbacks == 0 { + h.callbackMu.Unlock() + break + } + h.callbackMu.Unlock() + runtime.Gosched() + } + + if connID != 0 { + h.lib.connectionClose(connID) + } + if serverID != 0 { + h.lib.hostShutdown(serverID) + } + h.recv.Close() +} + +// hostWriter adapts Host into the io.WriteCloser jsonrpc2 writes request frames to. +type hostWriter struct{ h *Host } + +func (w hostWriter) Write(p []byte) (int, error) { return w.h.writeFrame(p) } + +func (w hostWriter) Close() error { + w.h.Dispose() + return nil +} diff --git a/go/internal/ffihost/loader_other.go b/go/internal/ffihost/loader_other.go new file mode 100644 index 0000000000..f0ca809d83 --- /dev/null +++ b/go/internal/ffihost/loader_other.go @@ -0,0 +1,15 @@ +// SPDX-License-Identifier: MIT + +//go:build !windows + +package ffihost + +import "github.com/ebitengine/purego" + +// openLibrary loads the shared library at path and returns an opaque handle. +// RTLD_NOW surfaces any load problem here (eager binding) rather than at first +// call, matching the .NET/Python hosts; RTLD_LOCAL keeps the runtime's symbols +// private to this handle. +func openLibrary(path string) (uintptr, error) { + return purego.Dlopen(path, purego.RTLD_NOW|purego.RTLD_LOCAL) +} diff --git a/go/internal/ffihost/loader_windows.go b/go/internal/ffihost/loader_windows.go new file mode 100644 index 0000000000..701f4f611f --- /dev/null +++ b/go/internal/ffihost/loader_windows.go @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: MIT + +//go:build windows + +package ffihost + +import "syscall" + +// openLibrary loads the DLL at path and returns its module handle. purego's +// RegisterLibFunc resolves exports from this handle via GetProcAddress, so the +// standard-library loader is sufficient and keeps CGO disabled. +func openLibrary(path string) (uintptr, error) { + handle, err := syscall.LoadLibrary(path) + if err != nil { + return 0, err + } + return uintptr(handle), nil +} diff --git a/go/internal/ffihost/resolve.go b/go/internal/ffihost/resolve.go new file mode 100644 index 0000000000..3868dd1a66 --- /dev/null +++ b/go/internal/ffihost/resolve.go @@ -0,0 +1,118 @@ +package ffihost + +import ( + "fmt" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "sync" +) + +// NaturalLibraryName is the natural platform shared-library file name for the +// runtime cdylib — the `.node` file renamed to what a Rust cdylib would be +// called on this OS. The library is loaded by absolute path, so the on-disk name +// is ours to choose; this matches the flat name the bundler installs next to the +// CLI binary and the name the other SDKs use. +func NaturalLibraryName() string { + switch runtime.GOOS { + case "windows": + return "copilot_runtime.dll" + case "darwin": + return "libcopilot_runtime.dylib" + default: + return "libcopilot_runtime.so" + } +} + +// PrebuildsFolder returns the napi-rs `-` folder name the +// runtime package ships under prebuilds/ (e.g. linux-x64, darwin-arm64, +// win32-x64, including the musl variant on Alpine). Returns "" for unsupported +// platforms. +func PrebuildsFolder() string { + var platform string + switch runtime.GOOS { + case "linux": + if isMusl() { + platform = "linuxmusl" + } else { + platform = "linux" + } + case "darwin": + platform = "darwin" + case "windows": + platform = "win32" + default: + return "" + } + + var arch string + switch runtime.GOARCH { + case "amd64": + arch = "x64" + case "arm64": + arch = "arm64" + default: + return "" + } + return platform + "-" + arch +} + +// ResolveLibraryPath resolves the native runtime library next to the given CLI +// entrypoint. It checks, in order: +// +// 1. The natural platform library name next to the CLI (bundled/flat layout, +// what the embedded-CLI installer writes). +// 2. prebuilds//runtime.node next to the CLI (dev/package layout). +// +// It returns an error when neither exists. +func ResolveLibraryPath(cliEntrypoint string) (string, error) { + abs, err := filepath.Abs(cliEntrypoint) + if err != nil { + abs = cliEntrypoint + } + dir := filepath.Dir(abs) + + flat := filepath.Join(dir, NaturalLibraryName()) + if fileExists(flat) { + return flat, nil + } + + if folder := PrebuildsFolder(); folder != "" { + prebuilt := filepath.Join(dir, "prebuilds", folder, "runtime.node") + if fileExists(prebuilt) { + return prebuilt, nil + } + } + + return "", fmt.Errorf( + "in-process FFI runtime library not found next to %q (looked for %q and prebuilds/%s/runtime.node); "+ + "use a runtime package that ships the native library", + abs, NaturalLibraryName(), PrebuildsFolder()) +} + +func fileExists(path string) bool { + info, err := os.Stat(path) + return err == nil && !info.IsDir() +} + +var ( + muslOnce sync.Once + muslResult bool +) + +// isMusl reports whether the current Linux system uses musl libc (e.g. Alpine), +// which ships the runtime under the linuxmusl- prebuilds folder. +func isMusl() bool { + muslOnce.Do(func() { + if runtime.GOOS != "linux" { + return + } + // `ldd --version` prints "musl libc" on musl systems and errors/glibc text + // elsewhere; a best-effort check is enough to pick the prebuilds folder. + out, _ := exec.Command("ldd", "--version").CombinedOutput() + muslResult = strings.Contains(strings.ToLower(string(out)), "musl") + }) + return muslResult +} diff --git a/go/types.go b/go/types.go index 029ee2b36e..f9386c2f66 100644 --- a/go/types.go +++ b/go/types.go @@ -20,14 +20,23 @@ const ( // RuntimeConnection describes how a [Client] connects to the Copilot runtime. // -// Construct one with a [StdioConnection], [TCPConnection], or [URIConnection] -// literal and pass it via [ClientOptions.Connection]. When [ClientOptions.Connection] -// is nil, the default is an empty [StdioConnection] (the SDK spawns the bundled -// runtime and communicates over stdin/stdout). +// Construct one with a [StdioConnection], [TCPConnection], [URIConnection], or +// [InProcessConnection] literal and pass it via [ClientOptions.Connection]. When +// [ClientOptions.Connection] is nil, the default is an empty [StdioConnection] +// (the SDK spawns the bundled runtime and communicates over stdin/stdout). type RuntimeConnection interface { runtimeConnection() } +// childProcessConnection is implemented by the connection types that spawn a +// runtime child process ([StdioConnection] and [TCPConnection]). It exposes the +// per-connection environment so the client can resolve and validate it uniformly +// regardless of the specific child-process transport. +type childProcessConnection interface { + RuntimeConnection + connEnv() []string +} + // StdioConnection spawns a runtime child process and communicates over its // stdin/stdout pipes. This is the default when no connection is configured. type StdioConnection struct { @@ -35,10 +44,17 @@ type StdioConnection struct { Path string // Args are extra command-line arguments inserted before SDK-managed args. Args []string + // Env are the environment variables for the runtime process, each of the + // form "KEY=VALUE". When set, these take precedence over + // [ClientOptions.Env]; setting both is rejected. When nil, the client-level + // env (or the current process environment) is used. + Env []string } func (StdioConnection) runtimeConnection() {} +func (c StdioConnection) connEnv() []string { return c.Env } + // TCPConnection spawns a runtime child process that listens on a TCP socket // and connects to it. type TCPConnection struct { @@ -54,10 +70,17 @@ type TCPConnection struct { Path string // Args are extra command-line arguments inserted before SDK-managed args. Args []string + // Env are the environment variables for the runtime process, each of the + // form "KEY=VALUE". When set, these take precedence over + // [ClientOptions.Env]; setting both is rejected. When nil, the client-level + // env (or the current process environment) is used. + Env []string } func (TCPConnection) runtimeConnection() {} +func (c TCPConnection) connEnv() []string { return c.Env } + // URIConnection connects to an already-running runtime at the given URL. // The SDK does not spawn a process in this mode. type URIConnection struct { @@ -71,6 +94,32 @@ type URIConnection struct { func (URIConnection) runtimeConnection() {} +// InProcessConnection hosts the Copilot runtime in-process by loading its native +// runtime library (a Rust cdylib) and driving JSON-RPC over the library's C ABI, +// instead of spawning a runtime child process. The native host spawns the +// residual worker itself; the SDK never launches it directly. +// +// Because the runtime is loaded into the calling process, per-client +// environment, working directory, and telemetry cannot be represented and are +// rejected by [NewClient] (see [ClientOptions]). Set those via the host process +// environment instead, or use a child-process transport ([StdioConnection] / +// [TCPConnection]). +// +// Experimental: the in-process transport is experimental and its API and +// behavior may change in a future release. +type InProcessConnection struct { + // Path is the runtime executable used to locate and launch the worker and + // to resolve the sibling native library. When empty, the client resolves + // COPILOT_CLI_PATH and then the bundled runtime. Unlike the child-process + // transports, no executable is looked up on PATH. + Path string + // Args are extra command-line arguments passed to the residual worker, + // appended after the SDK-managed args. + Args []string +} + +func (InProcessConnection) runtimeConnection() {} + // ClientOptions configures the [Client]. type ClientOptions struct { // Connection describes how to connect to the Copilot runtime. When nil, @@ -95,6 +144,12 @@ type ClientOptions struct { // Env are the environment variables for the runtime process (default: // inherits from current process). Each entry is of the form "KEY=VALUE". // If Env contains duplicate keys, only the last value for each key is used. + // + // For child-process transports ([StdioConnection] / [TCPConnection]) the + // per-connection Env, when set, takes precedence over this field; setting + // both is rejected. Env is not supported with [InProcessConnection] (the + // runtime shares this process's single environment block) and is rejected + // by [NewClient]. Env []string // GitHubToken is the GitHub token to use for authentication. // When provided, the token is passed to the runtime via environment From 8bb0a99e0f18e3cd56fe0193ff52b3e3bf6b03a8 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Mon, 13 Jul 2026 15:17:28 +0000 Subject: [PATCH 02/14] Fix Go FFI e2e CI failures (inprocess matrix) Address three failure modes in the transport:[default,inprocess] matrix: 1. Snapshot path was cwd-relative. ConfigureForTest resolved snapshots via filepath.Abs, which broke once the in-process path chdir'd into the temp work dir, yielding empty snapshots ("No cached response"). Anchor the path to the caller's source directory instead. 2. Process-global LLM inference provider / per-client telemetry. The shared in-process runtime refuses a second provider client and ignores per-client telemetry. Rather than silently downgrade the transport, the 8 affected tests now skip explicitly via testharness.SkipIfInProcess (they still run over stdio in the default cell), mirroring Rust's skip_inprocess. 3. macOS SA_ONSTACK crash. libuv installs a SIGCHLD handler without SA_ONSTACK on first uv_spawn (during host_start); Go's runtime then aborts when it reaps its own os/exec children. Re-arm the foreign handler with SA_ONSTACK after host_start (sigonstack_darwin.go; no-op elsewhere), and gate the FFI smoke test to the inprocess cell so default cells never load libnode. Also unset ambient HMAC keys at package init when inprocess (issue #1934). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: a96d1a82-c80e-40f1-a3df-a9708429b68b --- .../byok_bearer_token_provider_e2e_test.go | 1 + .../copilot_request_cancel_error_e2e_test.go | 2 + .../e2e/copilot_request_handler_e2e_test.go | 1 + .../copilot_request_session_id_e2e_test.go | 1 + go/internal/e2e/inprocess_ffi_e2e_test.go | 10 +++ go/internal/e2e/session_config_e2e_test.go | 1 + go/internal/e2e/subagent_hooks_e2e_test.go | 1 + go/internal/e2e/telemetry_e2e_test.go | 1 + go/internal/e2e/testharness/context.go | 70 ++++++++++++---- go/internal/ffihost/ffihost.go | 8 ++ go/internal/ffihost/sigonstack_darwin.go | 81 +++++++++++++++++++ go/internal/ffihost/sigonstack_other.go | 10 +++ 12 files changed, 172 insertions(+), 15 deletions(-) create mode 100644 go/internal/ffihost/sigonstack_darwin.go create mode 100644 go/internal/ffihost/sigonstack_other.go diff --git a/go/internal/e2e/byok_bearer_token_provider_e2e_test.go b/go/internal/e2e/byok_bearer_token_provider_e2e_test.go index 2f298596d1..33e32b1322 100644 --- a/go/internal/e2e/byok_bearer_token_provider_e2e_test.go +++ b/go/internal/e2e/byok_bearer_token_provider_e2e_test.go @@ -111,6 +111,7 @@ func (rt *byokCapturingRoundTripper) reset() { // 3. per-provider dispatch routes each provider's turn to its own callback, and // the resulting token reaches that provider's endpoint. func TestBYOKBearerTokenProvider(t *testing.T) { + testharness.SkipIfInProcess(t, "an LLM inference provider is process-global in-process") ctx := testharness.NewTestContext(t) rt := &byokCapturingRoundTripper{} handler := &copilot.CopilotRequestHandler{Transport: rt} diff --git a/go/internal/e2e/copilot_request_cancel_error_e2e_test.go b/go/internal/e2e/copilot_request_cancel_error_e2e_test.go index c48a617021..46091d5ac7 100644 --- a/go/internal/e2e/copilot_request_cancel_error_e2e_test.go +++ b/go/internal/e2e/copilot_request_cancel_error_e2e_test.go @@ -54,6 +54,7 @@ func (tr *throwingTransport) RoundTrip(req *http.Request) (*http.Response, error } func TestCopilotRequestError(t *testing.T) { + testharness.SkipIfInProcess(t, "an LLM inference provider is process-global in-process") ctx := testharness.NewTestContext(t) transport := &throwingTransport{} handler := &copilot.CopilotRequestHandler{Transport: transport} @@ -132,6 +133,7 @@ func waitFor(t *testing.T, predicate func() bool, timeout time.Duration) { } func TestCopilotRequestCancel(t *testing.T) { + testharness.SkipIfInProcess(t, "an LLM inference provider is process-global in-process") ctx := testharness.NewTestContext(t) transport := newCancellingTransport() handler := &copilot.CopilotRequestHandler{Transport: transport} diff --git a/go/internal/e2e/copilot_request_handler_e2e_test.go b/go/internal/e2e/copilot_request_handler_e2e_test.go index 052a1bdebc..a0cfcb63ea 100644 --- a/go/internal/e2e/copilot_request_handler_e2e_test.go +++ b/go/internal/e2e/copilot_request_handler_e2e_test.go @@ -119,6 +119,7 @@ func (rt *rewritingRoundTripper) RoundTrip(req *http.Request) (*http.Response, e } func TestCopilotRequestHandler(t *testing.T) { + testharness.SkipIfInProcess(t, "an LLM inference provider is process-global in-process") ctx := testharness.NewTestContext(t) counters := &handlerCounters{} httpURL, wsURL := startFakeUpstreams(t, counters) diff --git a/go/internal/e2e/copilot_request_session_id_e2e_test.go b/go/internal/e2e/copilot_request_session_id_e2e_test.go index e3569d3806..f7673bd457 100644 --- a/go/internal/e2e/copilot_request_session_id_e2e_test.go +++ b/go/internal/e2e/copilot_request_session_id_e2e_test.go @@ -90,6 +90,7 @@ func assertAgentMetadata(t *testing.T, r interceptedRequest) { } func TestCopilotRequestSessionID(t *testing.T) { + testharness.SkipIfInProcess(t, "an LLM inference provider is process-global in-process") ctx := testharness.NewTestContext(t) transport := &recordingTransport{} handler := &copilot.CopilotRequestHandler{Transport: transport} diff --git a/go/internal/e2e/inprocess_ffi_e2e_test.go b/go/internal/e2e/inprocess_ffi_e2e_test.go index c965d74d44..d2d85c8b9a 100644 --- a/go/internal/e2e/inprocess_ffi_e2e_test.go +++ b/go/internal/e2e/inprocess_ffi_e2e_test.go @@ -16,6 +16,16 @@ import ( // Mirrors python/e2e/test_inprocess_ffi_e2e.py and // nodejs/test/e2e/inprocess_ffi.e2e.test.ts. func TestInProcessFfiE2E(t *testing.T) { + // Loading the native runtime cdylib (libnode) into this test process installs + // foreign signal handlers. On macOS the Go runtime then aborts when it reaps + // its own os/exec children (see ffihost signal re-arming). The in-process + // matrix cell already loads libnode for the whole suite and re-arms those + // handlers; the default (child-process) cell must never load it, so restrict + // this dedicated FFI smoke test to the in-process cell. + if !testharness.IsInProcessTransport() { + t.Skip("in-process FFI smoke test runs only under the inprocess transport cell") + } + cliPath := testharness.CLIPath() if cliPath == "" { t.Fatal("CLI not found. Run 'npm install' in the nodejs directory first.") diff --git a/go/internal/e2e/session_config_e2e_test.go b/go/internal/e2e/session_config_e2e_test.go index 6dc1c59ad4..672a905dc0 100644 --- a/go/internal/e2e/session_config_e2e_test.go +++ b/go/internal/e2e/session_config_e2e_test.go @@ -404,6 +404,7 @@ func TestSessionConfigNewOptionsE2E(t *testing.T) { } func TestSessionConfigNewOptionsCopilotRequestE2E(t *testing.T) { + testharness.SkipIfInProcess(t, "an LLM inference provider is process-global in-process") t.Run("should enable citations for Anthropic file attachments on create", func(t *testing.T) { ctx := testharness.NewTestContext(t) transport := &recordingTransport{} diff --git a/go/internal/e2e/subagent_hooks_e2e_test.go b/go/internal/e2e/subagent_hooks_e2e_test.go index 6a5000a2c4..0e2fde9f86 100644 --- a/go/internal/e2e/subagent_hooks_e2e_test.go +++ b/go/internal/e2e/subagent_hooks_e2e_test.go @@ -77,6 +77,7 @@ func assertSubagentRequestMetadata(t *testing.T, records []subagentRequestRecord } func TestSubagentHooksE2E(t *testing.T) { + testharness.SkipIfInProcess(t, "an LLM inference provider is process-global in-process") ctx := testharness.NewTestContext(t) transport := newRecordingForwardingTransport() client := ctx.NewClient(func(o *copilot.ClientOptions) { diff --git a/go/internal/e2e/telemetry_e2e_test.go b/go/internal/e2e/telemetry_e2e_test.go index eff384d020..4567817fd9 100644 --- a/go/internal/e2e/telemetry_e2e_test.go +++ b/go/internal/e2e/telemetry_e2e_test.go @@ -14,6 +14,7 @@ import ( // Mirrors dotnet/test/TelemetryExportTests.cs (snapshot category "telemetry"). func TestTelemetryE2E(t *testing.T) { + testharness.SkipIfInProcess(t, "telemetry configuration is not honored in-process") t.Run("should export file telemetry for sdk interactions", func(t *testing.T) { ctx := testharness.NewTestContext(t) ctx.ConfigureForTest(t) diff --git a/go/internal/e2e/testharness/context.go b/go/internal/e2e/testharness/context.go index 63d09dabaa..04953b590e 100644 --- a/go/internal/e2e/testharness/context.go +++ b/go/internal/e2e/testharness/context.go @@ -78,6 +78,25 @@ func isInProcessTransport() bool { return strings.EqualFold(os.Getenv("COPILOT_SDK_DEFAULT_CONNECTION"), "inprocess") } +// init neutralizes any ambient HMAC signing key as early as package load when the +// in-process transport is selected. Host-side auth resolution ranks the HMAC key +// above the GitHub token, so an ambient COPILOT_HMAC_KEY (CI injects one as a +// job-level credential) would be picked over the token the replay snapshots +// expect, producing request signatures that miss the recorded exchanges. Because +// the runtime is hosted in this process, the key must be removed before the native +// library is loaded and captures it — a later, per-client override is too late and +// setting it to an empty value is still treated as a signing key. Out-of-process +// children resolve auth in their own process where the token already outranks the +// HMAC key, so this is scoped to the in-process cell. Mirrors the analogous +// module-load neutralization in the Node/Python/.NET harnesses. +// See https://github.com/github/copilot-sdk/issues/1934. +func init() { + if isInProcessTransport() { + os.Unsetenv("COPILOT_HMAC_KEY") + os.Unsetenv("CAPI_HMAC_KEY") + } +} + // IsInProcessTransport reports whether E2E tests run under the in-process (FFI) // transport. Tests that configure options unsupported in-process (e.g. per-client // telemetry) should skip when this returns true. @@ -85,6 +104,18 @@ func IsInProcessTransport() bool { return isInProcessTransport() } +// SkipIfInProcess skips the test when E2E tests run under the in-process (FFI) +// transport, for behavior the shared in-process runtime cannot support (e.g. a +// process-global LLM inference provider, or per-client telemetry). The reason is +// surfaced in the test log so the skip is explicit rather than a silent transport +// downgrade. Such tests still run over stdio in the default matrix cell. +func SkipIfInProcess(t *testing.T, reason string) { + t.Helper() + if isInProcessTransport() { + t.Skipf("unsupported over the in-process (FFI) transport: %s", reason) + } +} + // NewTestContext creates a new test context with isolated directories and a replaying proxy. func NewTestContext(t *testing.T) *TestContext { t.Helper() @@ -177,7 +208,14 @@ func (c *TestContext) ConfigureForTest(t *testing.T) { t.Fatalf("Expected test name with subtest, got: %s", testName) } sanitizedName := strings.ToLower(regexp.MustCompile(`[^a-zA-Z0-9]`).ReplaceAllString(parts[1], "_")) - snapshotPath := filepath.Join("..", "..", "..", "test", "snapshots", testFile, sanitizedName+".yaml") + // Anchor the snapshot path to the caller's source directory rather than the + // process working directory: the in-process transport chdir's into the test's + // isolated work dir (the worker inherits the process cwd), so a cwd-relative + // path would resolve against the wrong root for every subtest after the first. + // All e2e test files live in go/internal/e2e, so the repo root is three levels + // up from the caller's directory. + repoRoot := filepath.Join(filepath.Dir(callerFile), "..", "..", "..") + snapshotPath := filepath.Join(repoRoot, "test", "snapshots", testFile, sanitizedName+".yaml") absSnapshotPath, err := filepath.Abs(snapshotPath) if err != nil { @@ -219,10 +257,11 @@ func (c *TestContext) Close(testFailed bool) { // process for in-process hosting: the worker inherits this process's env and cwd // at spawn, so per-test redirects must live on os.Environ and the process cwd. // Auth flows via GH_TOKEN/GITHUB_TOKEN (the FFI argv omits the stdio auth-token -// wiring) and HMAC is disabled so host-side auth matches the replay snapshots. -// mergedEnv is the effective per-client env (harness defaults plus any per-test -// additions); workDir is the effective working directory. Values are restored in -// Close. Safe to call more than once (restores unwind in reverse). +// wiring); the ambient HMAC signing key is removed process-wide at package load +// (see init) so host-side auth matches the replay snapshots. mergedEnv is the +// effective per-client env (harness defaults plus any per-test additions); workDir +// is the effective working directory. Values are restored in Close. Safe to call +// more than once (restores unwind in reverse). func (c *TestContext) applyInProcessEnvironment(mergedEnv []string, workDir string) { inprocessEnv := map[string]string{} for _, kv := range mergedEnv { @@ -230,12 +269,12 @@ func (c *TestContext) applyInProcessEnvironment(mergedEnv []string, workDir stri inprocessEnv[key] = value } } - // Auth flows via GH_TOKEN/GITHUB_TOKEN and HMAC is disabled for the in-process - // host, overriding any inherited values. + // Auth flows via GH_TOKEN/GITHUB_TOKEN for the in-process host, overriding any + // inherited values. The HMAC key is neutralized process-wide at package load. inprocessEnv["GH_TOKEN"] = defaultGitHubToken inprocessEnv["GITHUB_TOKEN"] = defaultGitHubToken - inprocessEnv["COPILOT_HMAC_KEY"] = "" - inprocessEnv["CAPI_HMAC_KEY"] = "" + delete(inprocessEnv, "COPILOT_HMAC_KEY") + delete(inprocessEnv, "CAPI_HMAC_KEY") for key, value := range inprocessEnv { prev, had := os.LookupEnv(key) @@ -365,13 +404,14 @@ func (c *TestContext) NewClient(opts ...func(*copilot.ClientOptions)) *copilot.C // shouldUseInProcess reports whether a client built from options should be hosted // in-process for the inprocess matrix cell. Only the harness default stdio -// connection is swapped; a test that pins a custom stdio path/args/env, a TCP/URI -// connection, or configures per-client telemetry (which cannot be carried -// in-process) is exercising behavior that must stay on its own transport. +// connection is swapped; a test that pins a custom stdio path/args/env or a +// TCP/URI connection is exercising behavior that must stay on its own transport. +// +// Options the in-process runtime cannot support (per-client telemetry, an LLM +// inference provider) are NOT silently downgraded here — the affected tests skip +// explicitly via testharness.SkipIfInProcess so the limitation is visible rather +// than masked by a quiet transport swap. func (c *TestContext) shouldUseInProcess(options *copilot.ClientOptions) bool { - if options.Telemetry != nil { - return false - } s, ok := options.Connection.(copilot.StdioConnection) if !ok { return false diff --git a/go/internal/ffihost/ffihost.go b/go/internal/ffihost/ffihost.go index df613fbc97..4697f6fdd3 100644 --- a/go/internal/ffihost/ffihost.go +++ b/go/internal/ffihost/ffihost.go @@ -181,6 +181,14 @@ func (h *Host) Start() error { return fmt.Errorf("copilot_runtime_host_start failed (library %q, entrypoint %q)", h.libraryPath, h.cliEntrypoint) } + // host_start spawned the worker child via libuv's uv_spawn, which installs a + // SIGCHLD handler without SA_ONSTACK on its first call. On macOS the Go runtime + // aborts ("non-Go code set up signal handler without SA_ONSTACK flag") when it + // later reaps one of its own os/exec children, so re-add SA_ONSTACK to that + // foreign handler now that it exists (no-op off Darwin, and before the first + // spawn there is nothing to fix — hence here rather than at library load). + rearmForeignSignalHandlers() + h.callbackHandle = purego.NewCallback(h.onOutbound) h.connectionID = h.lib.connectionOpen(h.serverID, h.callbackHandle, 0, nil, 0, nil, 0, nil, 0) if h.connectionID == 0 { diff --git a/go/internal/ffihost/sigonstack_darwin.go b/go/internal/ffihost/sigonstack_darwin.go new file mode 100644 index 0000000000..4b0f3fd87d --- /dev/null +++ b/go/internal/ffihost/sigonstack_darwin.go @@ -0,0 +1,81 @@ +// SPDX-License-Identifier: MIT + +//go:build darwin + +package ffihost + +import ( + "encoding/binary" + "unsafe" + + "github.com/ebitengine/purego" +) + +// Darwin `struct sigaction` layout (16 bytes, little-endian on amd64/arm64): +// +// offset 0: union __sigaction_u sa_handler/sa_sigaction (8 bytes, pointer) +// offset 8: sigset_t sa_mask (4 bytes, uint32) +// offset 12: int sa_flags (4 bytes) +const ( + darwinSigactionSize = 16 + darwinFlagsOffset = 12 + saOnStack = 0x0001 // SA_ONSTACK on Darwin + sigDfl = 0 // SIG_DFL + sigIgn = 1 // SIG_IGN + maxSignal = 31 // NSIG-1 on Darwin +) + +// rearmForeignSignalHandlers re-adds the SA_ONSTACK flag to any signal handler +// installed by the native runtime (libnode/libuv, loaded via dlopen) that +// omitted it. The Go runtime aborts with "non-Go code set up signal handler +// without SA_ONSTACK flag" when such a signal (notably SIGCHLD, signal 20 on +// Darwin) is delivered while a Go-managed child process is reaped. libuv +// installs a SIGCHLD handler without SA_ONSTACK, which poisons every subsequent +// os/exec child reaped by Go in the same process (only observed on macOS; Linux +// does not enforce this). +// +// We preserve each foreign handler and merely OR in SA_ONSTACK, so libuv's child +// watching keeps working while the Go runtime stays happy. Handlers left at +// SIG_DFL/SIG_IGN and Go's own handlers (which already carry SA_ONSTACK) are +// untouched. Best-effort: any failure is silently ignored, since the worst case +// is the pre-existing crash. +func rearmForeignSignalHandlers() { + handle, err := purego.Dlopen("/usr/lib/libSystem.B.dylib", purego.RTLD_NOW|purego.RTLD_GLOBAL) + if err != nil || handle == 0 { + return + } + + var sigaction func(sig int32, act, oact unsafe.Pointer) int32 + if !bindSigaction(handle, &sigaction) { + return + } + + for sig := int32(1); sig <= maxSignal; sig++ { + var cur [darwinSigactionSize]byte + if sigaction(sig, nil, unsafe.Pointer(&cur[0])) != 0 { + continue + } + handler := binary.LittleEndian.Uint64(cur[0:8]) + if handler == sigDfl || handler == sigIgn { + continue + } + flags := binary.LittleEndian.Uint32(cur[darwinFlagsOffset : darwinFlagsOffset+4]) + if flags&saOnStack != 0 { + continue + } + binary.LittleEndian.PutUint32(cur[darwinFlagsOffset:darwinFlagsOffset+4], flags|saOnStack) + sigaction(sig, unsafe.Pointer(&cur[0]), nil) + } +} + +// bindSigaction resolves libc's sigaction into fn, converting the panic +// RegisterLibFunc raises on a missing symbol into a false return. +func bindSigaction(handle uintptr, fn *func(sig int32, act, oact unsafe.Pointer) int32) (ok bool) { + defer func() { + if recover() != nil { + ok = false + } + }() + purego.RegisterLibFunc(fn, handle, "sigaction") + return true +} diff --git a/go/internal/ffihost/sigonstack_other.go b/go/internal/ffihost/sigonstack_other.go new file mode 100644 index 0000000000..e53a77fc5c --- /dev/null +++ b/go/internal/ffihost/sigonstack_other.go @@ -0,0 +1,10 @@ +// SPDX-License-Identifier: MIT + +//go:build !darwin + +package ffihost + +// rearmForeignSignalHandlers is a no-op off Darwin. The Go runtime only enforces +// the SA_ONSTACK requirement that libuv's SIGCHLD handler violates on macOS; +// Linux and Windows are unaffected, so no signal re-arming is needed. +func rearmForeignSignalHandlers() {} From 9a1f3a4b85706eeba54d032cedd34543572d5ea7 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Mon, 13 Jul 2026 16:12:39 +0000 Subject: [PATCH 03/14] Fix Go FFI inprocess e2e CI failures Resolve the remaining inprocess-cell failures in the go-sdk-tests matrix: - Anchor all repo-relative test paths to source dirs, not the process cwd. The in-process transport os.Chdir's the whole test process into a per-test temp workdir, so cwd-relative resolution of the replay proxy (server.ts) and MCP server scripts (*.mjs) broke for every test after the first in-process one. Add testharness.RepoPath and use it for the proxy, CLI, and MCP paths. - Fix a data race in ffihost.Host: `disposed` was written under disposeMu but read under callbackMu. Unify all disposed/serverID/connectionID/activeCallbacks access under a single mutex, which also fixes a real drain-ordering bug where a late onOutbound callback could feed the receive buffer after it was closed. - Re-arm foreign signal handlers with SA_ONSTACK on linux too (not just darwin). libuv's SA_ONSTACK-less SIGCHLD handler makes the Go runtime abort when the test process reaps its own os/exec child (e.g. an MCP OAuth server) while libnode is loaded. Resolve sigaction through the runtime handle (with a libc dlopen fallback). - Make the OAuth MCP server's stderr buffer thread-safe (syncBuffer): os/exec writes to it on a goroutine while the test reads String(), a -race violation. - Skip only the readCheckpoint "unknown checkpoint" scenario in-process, where the native runtime decodes the id as u32 and rejects the large sentinel, matching Rust's explicit skip. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: a96d1a82-c80e-40f1-a3df-a9708429b68b --- go/internal/e2e/mcp_and_agents_e2e_test.go | 5 +- go/internal/e2e/mcp_oauth_e2e_test.go | 28 ++++- go/internal/e2e/mcp_server_helpers_test.go | 6 +- .../e2e/pre_mcp_tool_call_hook_e2e_test.go | 2 +- .../e2e/rpc_workspace_checkpoints_e2e_test.go | 5 + go/internal/e2e/testharness/context.go | 12 +- go/internal/e2e/testharness/helper.go | 21 ++++ go/internal/e2e/testharness/proxy.go | 7 +- go/internal/ffihost/ffihost.go | 62 +++++----- go/internal/ffihost/sigonstack_darwin.go | 6 +- go/internal/ffihost/sigonstack_linux.go | 108 ++++++++++++++++++ go/internal/ffihost/sigonstack_other.go | 10 +- 12 files changed, 213 insertions(+), 59 deletions(-) create mode 100644 go/internal/ffihost/sigonstack_linux.go diff --git a/go/internal/e2e/mcp_and_agents_e2e_test.go b/go/internal/e2e/mcp_and_agents_e2e_test.go index e4bd34e268..71a152ecaf 100644 --- a/go/internal/e2e/mcp_and_agents_e2e_test.go +++ b/go/internal/e2e/mcp_and_agents_e2e_test.go @@ -115,10 +115,7 @@ func TestMCPServersE2E(t *testing.T) { t.Run("should pass literal env values to MCP server subprocess", func(t *testing.T) { ctx.ConfigureForTest(t) - mcpServerPath, err := filepath.Abs("../../../test/harness/test-mcp-server.mjs") - if err != nil { - t.Fatalf("Failed to resolve test-mcp-server path: %v", err) - } + mcpServerPath := testharness.RepoPath("test", "harness", "test-mcp-server.mjs") mcpServerDir := filepath.Dir(mcpServerPath) mcpServers := map[string]copilot.MCPServerConfig{ diff --git a/go/internal/e2e/mcp_oauth_e2e_test.go b/go/internal/e2e/mcp_oauth_e2e_test.go index 7959043063..1e08b839c8 100644 --- a/go/internal/e2e/mcp_oauth_e2e_test.go +++ b/go/internal/e2e/mcp_oauth_e2e_test.go @@ -6,7 +6,6 @@ import ( "net/http" "os" "os/exec" - "path/filepath" "slices" "strings" "sync" @@ -306,17 +305,14 @@ type oauthMCPRequest struct { func startOAuthMCPServer(t *testing.T) string { t.Helper() - serverPath, err := filepath.Abs("../../../test/harness/test-mcp-oauth-server.mjs") - if err != nil { - t.Fatalf("Failed to resolve OAuth MCP server path: %v", err) - } + serverPath := testharness.RepoPath("test", "harness", "test-mcp-oauth-server.mjs") cmd := exec.Command("node", serverPath) cmd.Env = append(os.Environ(), "EXPECTED_TOKEN="+expectedMCPOAuthToken) stdout, err := cmd.StdoutPipe() if err != nil { t.Fatalf("Failed to pipe OAuth MCP server stdout: %v", err) } - var stderr strings.Builder + var stderr syncBuffer cmd.Stderr = &stderr if err := cmd.Start(); err != nil { t.Fatalf("Failed to start OAuth MCP server: %v", err) @@ -362,6 +358,26 @@ func stringValue(value *string) string { return *value } +// syncBuffer is a minimal io.Writer whose contents can be read concurrently. +// os/exec writes to cmd.Stderr on a separate goroutine, so reading a plain +// strings.Builder while the process is running is a data race (caught by -race). +type syncBuffer struct { + mu sync.Mutex + buf strings.Builder +} + +func (b *syncBuffer) Write(p []byte) (int, error) { + b.mu.Lock() + defer b.mu.Unlock() + return b.buf.Write(p) +} + +func (b *syncBuffer) String() string { + b.mu.Lock() + defer b.mu.Unlock() + return b.buf.String() +} + func fetchOAuthMCPRequests(t *testing.T, baseURL string) []oauthMCPRequest { t.Helper() diff --git a/go/internal/e2e/mcp_server_helpers_test.go b/go/internal/e2e/mcp_server_helpers_test.go index e7269b1e26..68e72b18bc 100644 --- a/go/internal/e2e/mcp_server_helpers_test.go +++ b/go/internal/e2e/mcp_server_helpers_test.go @@ -8,16 +8,14 @@ import ( "time" copilot "github.com/github/copilot-sdk/go" + "github.com/github/copilot-sdk/go/internal/e2e/testharness" "github.com/github/copilot-sdk/go/rpc" ) func testMCPServers(t *testing.T, serverNames ...string) map[string]copilot.MCPServerConfig { t.Helper() - mcpServerPath, err := filepath.Abs("../../../test/harness/test-mcp-server.mjs") - if err != nil { - t.Fatalf("Failed to resolve test-mcp-server path: %v", err) - } + mcpServerPath := testharness.RepoPath("test", "harness", "test-mcp-server.mjs") mcpServerDir := filepath.Dir(mcpServerPath) mcpServers := make(map[string]copilot.MCPServerConfig, len(serverNames)) diff --git a/go/internal/e2e/pre_mcp_tool_call_hook_e2e_test.go b/go/internal/e2e/pre_mcp_tool_call_hook_e2e_test.go index 1847270922..111cfb86a4 100644 --- a/go/internal/e2e/pre_mcp_tool_call_hook_e2e_test.go +++ b/go/internal/e2e/pre_mcp_tool_call_hook_e2e_test.go @@ -15,7 +15,7 @@ func TestPreMCPToolCallHookE2E(t *testing.T) { client := ctx.NewClient() t.Cleanup(func() { client.ForceStop() }) - testHarnessDir, _ := filepath.Abs("../../../test/harness") + testHarnessDir := testharness.RepoPath("test", "harness") metaEchoServer := filepath.Join(testHarnessDir, "test-mcp-meta-echo-server.mjs") metaEchoConfig := func() map[string]copilot.MCPServerConfig { diff --git a/go/internal/e2e/rpc_workspace_checkpoints_e2e_test.go b/go/internal/e2e/rpc_workspace_checkpoints_e2e_test.go index 87c3c8da83..849e3a5fa6 100644 --- a/go/internal/e2e/rpc_workspace_checkpoints_e2e_test.go +++ b/go/internal/e2e/rpc_workspace_checkpoints_e2e_test.go @@ -33,6 +33,11 @@ func TestRPCWorkspaceCheckpointsE2E(t *testing.T) { }) t.Run("should return nil or empty content for unknown checkpoint", func(t *testing.T) { + // In-process, session.workspaces.readCheckpoint is answered by the native + // runtime, which decodes the checkpoint number as a u32 and rejects the + // large sentinel this test uses. Covered by the default (stdio) transport. + // Mirrors Rust's should_return_null_or_empty_content_for_unknown_checkpoint. + testharness.SkipIfInProcess(t, "readCheckpoint decodes the id as u32 in-process") session := createWorkspaceRPCSession(t, client) defer session.Disconnect() diff --git a/go/internal/e2e/testharness/context.go b/go/internal/e2e/testharness/context.go index 04953b590e..bc1f47cbff 100644 --- a/go/internal/e2e/testharness/context.go +++ b/go/internal/e2e/testharness/context.go @@ -33,13 +33,11 @@ func CLIPath() string { // 1.0.64-1 the @github/copilot package is a thin loader; the runnable // index.js ships in the installed platform package // (e.g. @github/copilot-linux-x64). - base, err := filepath.Abs("../../../nodejs/node_modules/@github") - if err == nil { - matches, _ := filepath.Glob(filepath.Join(base, "copilot-*", "index.js")) - if len(matches) > 0 { - cliPath = matches[0] - return - } + base := RepoPath("nodejs", "node_modules", "@github") + matches, _ := filepath.Glob(filepath.Join(base, "copilot-*", "index.js")) + if len(matches) > 0 { + cliPath = matches[0] + return } }) return cliPath diff --git a/go/internal/e2e/testharness/helper.go b/go/internal/e2e/testharness/helper.go index ca94d03adf..af08b2dbcc 100644 --- a/go/internal/e2e/testharness/helper.go +++ b/go/internal/e2e/testharness/helper.go @@ -3,11 +3,32 @@ package testharness import ( "context" "errors" + "path/filepath" + "runtime" "time" copilot "github.com/github/copilot-sdk/go" ) +// RepoPath resolves a path relative to the repository root, anchored to this +// source file's directory rather than the process working directory. The +// in-process (FFI) transport os.Chdir's the whole test process into a per-test +// temp workdir (the shared runtime host inherits the process cwd), so any +// cwd-relative resolution (e.g. filepath.Abs("../../../test/...")) would break +// for every test after the first in-process one. This helper stays correct +// regardless of the current working directory. +func RepoPath(elem ...string) string { + _, callerFile, _, ok := runtime.Caller(0) + if !ok { + // Fall back to a cwd-relative join; only correct before any chdir. + return filepath.Join(append([]string{"..", "..", ".."}, elem...)...) + } + // This file lives at go/internal/e2e/testharness/, so the repo root is four + // levels up from its directory. + repoRoot := filepath.Join(filepath.Dir(callerFile), "..", "..", "..", "..") + return filepath.Join(append([]string{repoRoot}, elem...)...) +} + // GetFinalAssistantMessage waits for and returns the final assistant message from a session turn. // If alreadyIdle is true, skip waiting for session.idle (useful for resumed sessions where the // idle event was ephemeral and not persisted in the event history). diff --git a/go/internal/e2e/testharness/proxy.go b/go/internal/e2e/testharness/proxy.go index 8933183c87..6c0a6d99dc 100644 --- a/go/internal/e2e/testharness/proxy.go +++ b/go/internal/e2e/testharness/proxy.go @@ -38,8 +38,11 @@ func (p *CapiProxy) Start() (string, error) { return p.proxyURL, nil } - // The harness server is in the shared test directory - serverPath := "../../../test/harness/server.ts" + // The harness server is in the shared test directory. Anchor the path to + // the repo root (not the process cwd), because the in-process (FFI) + // transport os.Chdir's into a per-test temp workdir, which would otherwise + // break the cwd-relative resolution. + serverPath := RepoPath("test", "harness", "server.ts") p.cmd = exec.Command("npx", "tsx", serverPath) p.cmd.Dir = "." // Will be resolved relative to test execution diff --git a/go/internal/ffihost/ffihost.go b/go/internal/ffihost/ffihost.go index 4697f6fdd3..1a0945240c 100644 --- a/go/internal/ffihost/ffihost.go +++ b/go/internal/ffihost/ffihost.go @@ -53,6 +53,7 @@ const symbolPrefix = "copilot_runtime_" // ffiLibrary binds the copilot_runtime_* C ABI exports of a loaded cdylib. type ffiLibrary struct { + handle uintptr hostStart func(argv unsafe.Pointer, argvLen uintptr, env unsafe.Pointer, envLen uintptr) uint32 hostShutdown func(serverID uint32) bool connectionOpen func(serverID uint32, cb uintptr, userData uintptr, a unsafe.Pointer, aLen uintptr, b unsafe.Pointer, bLen uintptr, c unsafe.Pointer, cLen uintptr) uint32 @@ -95,7 +96,7 @@ func loadLibrary(libraryPath string) (lib *ffiLibrary, err error) { } }() - bound := &ffiLibrary{} + bound := &ffiLibrary{handle: handle} purego.RegisterLibFunc(&bound.hostStart, handle, symbolPrefix+"host_start") purego.RegisterLibFunc(&bound.hostShutdown, handle, symbolPrefix+"host_shutdown") purego.RegisterLibFunc(&bound.connectionOpen, handle, symbolPrefix+"connection_open") @@ -119,22 +120,24 @@ type Host struct { extraArgs []string lib *ffiLibrary + // mu guards disposed, activeCallbacks, serverID, and connectionID. A single + // mutex is used so that publishing disposed=true and draining in-flight + // callbacks are serialized against onOutbound's disposed-check/increment; + // this prevents a callback from feeding the receive buffer after it is + // closed (and avoids a data race on disposed observed by the -race detector). + mu sync.Mutex serverID uint32 connectionID uint32 + disposed bool + // activeCallbacks counts outbound native callbacks currently executing. + activeCallbacks int recv *receiveBuffer - disposeMu sync.Mutex - disposed bool - // callbackHandle is the purego callback trampoline address. purego keeps the // underlying Go closure alive for the process lifetime, so it (and this Host, // which the closure captures) must not be relied upon to be freed. callbackHandle uintptr - // Serializes teardown against in-flight native callbacks so the receive - // buffer is not fed after it is closed. - callbackMu sync.Mutex - activeCallbacks int } // Create resolves the cdylib next to the CLI entrypoint and prepares the host. @@ -182,12 +185,14 @@ func (h *Host) Start() error { } // host_start spawned the worker child via libuv's uv_spawn, which installs a - // SIGCHLD handler without SA_ONSTACK on its first call. On macOS the Go runtime - // aborts ("non-Go code set up signal handler without SA_ONSTACK flag") when it - // later reaps one of its own os/exec children, so re-add SA_ONSTACK to that - // foreign handler now that it exists (no-op off Darwin, and before the first - // spawn there is nothing to fix — hence here rather than at library load). - rearmForeignSignalHandlers() + // SIGCHLD handler without SA_ONSTACK on its first call. The Go runtime aborts + // ("non-Go code set up signal handler without SA_ONSTACK flag") when it later + // reaps one of its own os/exec children (e.g. a test-spawned MCP server) and + // the delivered SIGCHLD lands on a non-signal stack. Re-add SA_ONSTACK to that + // foreign handler now that it exists (implemented on darwin+linux; a no-op on + // other platforms, and before the first spawn there is nothing to fix — hence + // here rather than at library load). + rearmForeignSignalHandlers(h.lib.handle) h.callbackHandle = purego.NewCallback(h.onOutbound) h.connectionID = h.lib.connectionOpen(h.serverID, h.callbackHandle, 0, nil, 0, nil, 0, nil, 0) @@ -233,18 +238,18 @@ func (h *Host) buildEnv() []byte { // are copied out before returning. Nothing may panic across the FFI boundary. func (h *Host) onOutbound(userData uintptr, bytesPtr uintptr, bytesLen uintptr) uintptr { _ = userData - h.callbackMu.Lock() + h.mu.Lock() if h.disposed { - h.callbackMu.Unlock() + h.mu.Unlock() return 0 } h.activeCallbacks++ - h.callbackMu.Unlock() + h.mu.Unlock() defer func() { - h.callbackMu.Lock() + h.mu.Lock() h.activeCallbacks-- - h.callbackMu.Unlock() + h.mu.Unlock() // Never let a panic unwind into native code. _ = recover() }() @@ -263,10 +268,10 @@ func (h *Host) onOutbound(userData uintptr, bytesPtr uintptr, bytesLen uintptr) } func (h *Host) writeFrame(frame []byte) (int, error) { - h.disposeMu.Lock() + h.mu.Lock() closed := h.disposed || h.connectionID == 0 connID := h.connectionID - h.disposeMu.Unlock() + h.mu.Unlock() if closed { return 0, fmt.Errorf("the in-process runtime connection is closed") } @@ -285,27 +290,30 @@ func (h *Host) writeFrame(frame []byte) (int, error) { // resources. It is idempotent and waits for any in-flight outbound callback to // finish before closing the receive buffer. func (h *Host) Dispose() { - h.disposeMu.Lock() + h.mu.Lock() if h.disposed { - h.disposeMu.Unlock() + h.mu.Unlock() return } + // Publish disposed under the same lock onOutbound uses to check it, so no new + // callback can pass the check and increment activeCallbacks after the drain + // loop below observes zero. h.disposed = true connID := h.connectionID serverID := h.serverID h.connectionID = 0 h.serverID = 0 - h.disposeMu.Unlock() + h.mu.Unlock() // Stop accepting new callbacks and wait for in-flight ones to drain before // closing the receive buffer they feed. for { - h.callbackMu.Lock() + h.mu.Lock() if h.activeCallbacks == 0 { - h.callbackMu.Unlock() + h.mu.Unlock() break } - h.callbackMu.Unlock() + h.mu.Unlock() runtime.Gosched() } diff --git a/go/internal/ffihost/sigonstack_darwin.go b/go/internal/ffihost/sigonstack_darwin.go index 4b0f3fd87d..61b1e751ae 100644 --- a/go/internal/ffihost/sigonstack_darwin.go +++ b/go/internal/ffihost/sigonstack_darwin.go @@ -31,15 +31,15 @@ const ( // without SA_ONSTACK flag" when such a signal (notably SIGCHLD, signal 20 on // Darwin) is delivered while a Go-managed child process is reaped. libuv // installs a SIGCHLD handler without SA_ONSTACK, which poisons every subsequent -// os/exec child reaped by Go in the same process (only observed on macOS; Linux -// does not enforce this). +// os/exec child reaped by Go in the same process (enforced by the Go runtime on +// both macOS and Linux; the Linux variant lives in sigonstack_linux.go). // // We preserve each foreign handler and merely OR in SA_ONSTACK, so libuv's child // watching keeps working while the Go runtime stays happy. Handlers left at // SIG_DFL/SIG_IGN and Go's own handlers (which already carry SA_ONSTACK) are // untouched. Best-effort: any failure is silently ignored, since the worst case // is the pre-existing crash. -func rearmForeignSignalHandlers() { +func rearmForeignSignalHandlers(_ uintptr) { handle, err := purego.Dlopen("/usr/lib/libSystem.B.dylib", purego.RTLD_NOW|purego.RTLD_GLOBAL) if err != nil || handle == 0 { return diff --git a/go/internal/ffihost/sigonstack_linux.go b/go/internal/ffihost/sigonstack_linux.go new file mode 100644 index 0000000000..d8357a30b4 --- /dev/null +++ b/go/internal/ffihost/sigonstack_linux.go @@ -0,0 +1,108 @@ +// SPDX-License-Identifier: MIT + +//go:build linux + +package ffihost + +import ( + "encoding/binary" + "unsafe" + + "github.com/ebitengine/purego" +) + +// Linux `struct sigaction` layout for glibc and musl on amd64/arm64 +// (little-endian): +// +// offset 0: sa_handler/sa_sigaction (8 bytes, pointer) +// offset 8: sigset_t sa_mask (128 bytes) +// offset 136: int sa_flags (4 bytes) +// offset 144: sa_restorer (8 bytes) +// +// Both glibc and musl use a 128-byte sigset_t here, so sa_flags is at offset 136 +// and the whole struct is 152 bytes. We over-allocate slightly for safety. +const ( + linuxSigactionSize = 160 + linuxFlagsOffset = 136 + linuxSaOnStack = 0x08000000 // SA_ONSTACK on Linux + linuxSigDfl = 0 // SIG_DFL + linuxSigIgn = 1 // SIG_IGN + linuxMaxSignal = 31 // standard signals; covers SIGCHLD (17) +) + +// libcCandidates are dlopen names to try when resolving sigaction fails through +// the already-loaded runtime library handle (e.g. when its libc symbols are not +// reachable via that handle). Covers glibc and musl. +var libcCandidates = []string{"libc.so.6", "libc.so", "libc.musl-x86_64.so.1", "libc.musl-aarch64.so.1"} + +// rearmForeignSignalHandlers re-adds the SA_ONSTACK flag to any signal handler +// installed by the native runtime (libnode/libuv, loaded via dlopen) that +// omitted it. The Go runtime aborts with "non-Go code set up signal handler +// without SA_ONSTACK flag" when such a signal (notably SIGCHLD, signal 17 on +// Linux) is delivered while a Go-managed child process is reaped. libuv installs +// a SIGCHLD handler without SA_ONSTACK, which poisons every subsequent os/exec +// child reaped by Go in the same process. +// +// We preserve each foreign handler and merely OR in SA_ONSTACK, so libuv's child +// watching keeps working while the Go runtime stays happy. Handlers left at +// SIG_DFL/SIG_IGN and Go's own handlers (which already carry SA_ONSTACK) are +// untouched. Best-effort: any failure is silently ignored, since the worst case +// is the pre-existing crash. +// +// runtimeHandle is the dlopen handle of the native runtime; sigaction is +// resolved through it (dlsym searches the library's libc dependency) so we avoid +// guessing the libc path, with an explicit libc dlopen as a fallback. +func rearmForeignSignalHandlers(runtimeHandle uintptr) { + var sigaction func(sig int32, act, oact unsafe.Pointer) int32 + if !resolveLinuxSigaction(runtimeHandle, &sigaction) { + return + } + + for sig := int32(1); sig <= linuxMaxSignal; sig++ { + var cur [linuxSigactionSize]byte + if sigaction(sig, nil, unsafe.Pointer(&cur[0])) != 0 { + continue + } + handler := binary.LittleEndian.Uint64(cur[0:8]) + if handler == linuxSigDfl || handler == linuxSigIgn { + continue + } + flags := binary.LittleEndian.Uint32(cur[linuxFlagsOffset : linuxFlagsOffset+4]) + if flags&linuxSaOnStack != 0 { + continue + } + binary.LittleEndian.PutUint32(cur[linuxFlagsOffset:linuxFlagsOffset+4], flags|linuxSaOnStack) + sigaction(sig, unsafe.Pointer(&cur[0]), nil) + } +} + +// resolveLinuxSigaction binds libc's sigaction into fn, first via the runtime +// library handle (whose libc dependency exports it) and then via an explicit +// libc dlopen. Returns false if no candidate resolves the symbol. +func resolveLinuxSigaction(runtimeHandle uintptr, fn *func(sig int32, act, oact unsafe.Pointer) int32) bool { + if runtimeHandle != 0 && bindLinuxSigaction(runtimeHandle, fn) { + return true + } + for _, name := range libcCandidates { + handle, err := purego.Dlopen(name, purego.RTLD_NOW|purego.RTLD_GLOBAL) + if err != nil || handle == 0 { + continue + } + if bindLinuxSigaction(handle, fn) { + return true + } + } + return false +} + +// bindLinuxSigaction resolves sigaction from handle into fn, converting the +// panic RegisterLibFunc raises on a missing symbol into a false return. +func bindLinuxSigaction(handle uintptr, fn *func(sig int32, act, oact unsafe.Pointer) int32) (ok bool) { + defer func() { + if recover() != nil { + ok = false + } + }() + purego.RegisterLibFunc(fn, handle, "sigaction") + return true +} diff --git a/go/internal/ffihost/sigonstack_other.go b/go/internal/ffihost/sigonstack_other.go index e53a77fc5c..273837eb9b 100644 --- a/go/internal/ffihost/sigonstack_other.go +++ b/go/internal/ffihost/sigonstack_other.go @@ -1,10 +1,10 @@ // SPDX-License-Identifier: MIT -//go:build !darwin +//go:build !darwin && !linux package ffihost -// rearmForeignSignalHandlers is a no-op off Darwin. The Go runtime only enforces -// the SA_ONSTACK requirement that libuv's SIGCHLD handler violates on macOS; -// Linux and Windows are unaffected, so no signal re-arming is needed. -func rearmForeignSignalHandlers() {} +// rearmForeignSignalHandlers is a no-op on platforms other than darwin and +// linux. Only those Unix platforms deliver the SA_ONSTACK-less SIGCHLD handler +// (installed by libuv) that the Go runtime rejects; Windows is unaffected. +func rearmForeignSignalHandlers(_ uintptr) {} From d53279ac5a362cd5e07a8ecfb5dc6f36dd31ec77 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 14 Jul 2026 10:36:09 +0100 Subject: [PATCH 04/14] Fix Go in-process transport parity Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 447feac4-35df-4c0b-ab34-00789ad78da7 --- go/README.md | 5 +- go/client.go | 30 ++++++++++- go/client_test.go | 53 +++++++++++++++++++ go/embeddedcli/installer.go | 6 +-- go/internal/embeddedcli/embeddedcli.go | 36 ++++++++----- go/internal/embeddedcli/embeddedcli_test.go | 41 +++++++++++++++ go/internal/ffihost/ffihost.go | 46 ++++++++++++++--- go/internal/ffihost/ffihost_test.go | 18 +++++++ go/internal/ffihost/resolve.go | 31 +++++++++-- go/internal/ffihost/resolve_test.go | 57 +++++++++++++++++++++ go/types.go | 8 +-- 11 files changed, 296 insertions(+), 35 deletions(-) create mode 100644 go/internal/ffihost/ffihost_test.go create mode 100644 go/internal/ffihost/resolve_test.go diff --git a/go/README.md b/go/README.md index edde8697bd..8a357007fc 100644 --- a/go/README.md +++ b/go/README.md @@ -121,8 +121,11 @@ defer client.Stop() Resolution and requirements: +- Set `COPILOT_SDK_DEFAULT_CONNECTION=inprocess` to select the in-process + transport when `ClientOptions.Connection` is nil. An explicit connection + always takes precedence. - The entrypoint is resolved with **no `PATH` lookup**: explicit `InProcessConnection.Path`, then `COPILOT_CLI_PATH`, then the bundled embedded CLI. It must be an on-disk path. -- The runtime library is loaded from next to the resolved entrypoint (the flat `libcopilot_runtime.*` name installed by the bundler, or the package's `prebuilds//runtime.node`). Start fails loudly if it cannot be found. +- The runtime library is loaded from next to the resolved entrypoint (the matching versioned `libcopilot_runtime_*` name installed for an embedded CLI, a flat package library name, or the package's `prebuilds//runtime.node`). Start fails loudly if it cannot be found. - The runtime library is loaded once per process; loading a different library path in the same process is an error. The in-process transport rejects options that cannot be honored by a runtime hosted in your shared process (each panics at `NewClient`): diff --git a/go/client.go b/go/client.go index a72f6446cf..dc589f3d4c 100644 --- a/go/client.go +++ b/go/client.go @@ -227,10 +227,15 @@ func NewClient(options *ClientOptions) *Client { opts = *options } - // Resolve the connection. nil defaults to an empty StdioConnection. + // Resolve the connection. An explicit connection always wins; otherwise + // honor the same process/environment override as the other SDKs. connection := opts.Connection if connection == nil { - connection = StdioConnection{} + env := opts.Env + if env == nil { + env = os.Environ() + } + connection = resolveDefaultConnection(env) } switch conn := connection.(type) { case StdioConnection: @@ -326,6 +331,27 @@ func NewClient(options *ClientOptions) *Client { return client } +const defaultConnectionEnvVar = "COPILOT_SDK_DEFAULT_CONNECTION" + +// resolveDefaultConnection selects the transport when no explicit connection +// was supplied. The override is primarily used by hosts and the E2E transport +// matrix; explicit connection options always take precedence. +func resolveDefaultConnection(env []string) RuntimeConnection { + value := getEnvValue(env, defaultConnectionEnvVar) + switch { + case value == "", strings.EqualFold(value, "stdio"): + return StdioConnection{} + case strings.EqualFold(value, "inprocess"): + return InProcessConnection{} + default: + panic(fmt.Sprintf( + "invalid %s value %q: expected \"inprocess\", \"stdio\", or unset", + defaultConnectionEnvVar, + value, + )) + } +} + // getEnvValue looks up a key in an environment slice ([]string of "KEY=VALUE"). // Returns the value if found, or empty string otherwise. func getEnvValue(env []string, key string) string { diff --git a/go/client_test.go b/go/client_test.go index df92d76cce..4980bcbe1a 100644 --- a/go/client_test.go +++ b/go/client_test.go @@ -687,6 +687,59 @@ func TestClient_InProcessConnection(t *testing.T) { }) } +func TestClient_DefaultConnection(t *testing.T) { + t.Run("defaults to stdio when override is unset", func(t *testing.T) { + t.Setenv(defaultConnectionEnvVar, "") + + client := NewClient(nil) + + if !client.useStdio || client.useInProcess { + t.Fatalf("Expected stdio default, got useStdio=%v useInProcess=%v", client.useStdio, client.useInProcess) + } + }) + + t.Run("selects in-process case-insensitively", func(t *testing.T) { + t.Setenv(defaultConnectionEnvVar, "InPrOcEsS") + + client := NewClient(nil) + + if !client.useInProcess || client.useStdio { + t.Fatalf("Expected in-process default, got useStdio=%v useInProcess=%v", client.useStdio, client.useInProcess) + } + }) + + t.Run("accepts explicit stdio override", func(t *testing.T) { + t.Setenv(defaultConnectionEnvVar, "STDIO") + + client := NewClient(nil) + + if !client.useStdio || client.useInProcess { + t.Fatalf("Expected stdio default, got useStdio=%v useInProcess=%v", client.useStdio, client.useInProcess) + } + }) + + t.Run("explicit connection takes precedence", func(t *testing.T) { + t.Setenv(defaultConnectionEnvVar, "inprocess") + + client := NewClient(&ClientOptions{Connection: TCPConnection{Port: 1234}}) + + if client.useInProcess || client.useStdio || client.port != 1234 { + t.Fatalf("Expected explicit TCP connection to win, got useStdio=%v useInProcess=%v port=%d", client.useStdio, client.useInProcess, client.port) + } + }) + + t.Run("panics for invalid override", func(t *testing.T) { + t.Setenv(defaultConnectionEnvVar, "tcp") + + defer func() { + if r := recover(); r == nil { + t.Fatal("Expected invalid default connection override to panic") + } + }() + NewClient(nil) + }) +} + func TestClient_ConnectionLevelEnv(t *testing.T) { t.Run("rejects env set on both client and connection", func(t *testing.T) { defer func() { diff --git a/go/embeddedcli/installer.go b/go/embeddedcli/installer.go index 6edddf281a..59e23c6fcc 100644 --- a/go/embeddedcli/installer.go +++ b/go/embeddedcli/installer.go @@ -5,9 +5,9 @@ import "github.com/github/copilot-sdk/go/internal/embeddedcli" // Config defines the inputs used to install and locate the embedded Copilot CLI. // // Cli and CliHash are required. If Dir is empty, the CLI is installed into the -// system cache directory. Version is used to suffix the installed binary name to -// allow multiple versions to coexist. License, when provided, is written next -// to the installed binary. +// system cache directory. Version is used to suffix the installed binary and +// runtime-library names so multiple versions can coexist. License, when +// provided, is written next to the installed binary. type Config = embeddedcli.Config // Setup sets the embedded GitHub Copilot CLI install configuration. diff --git a/go/internal/embeddedcli/embeddedcli.go b/go/internal/embeddedcli/embeddedcli.go index 9f26451ba4..a61d253c04 100644 --- a/go/internal/embeddedcli/embeddedcli.go +++ b/go/internal/embeddedcli/embeddedcli.go @@ -18,13 +18,13 @@ import ( // Config defines the inputs used to install and locate the embedded Copilot CLI. // // Cli and CliHash are required. If Dir is empty, the CLI is installed into the -// system cache directory. Version is used to suffix the installed binary name to -// allow multiple versions to coexist. License, when provided, is written next -// to the installed binary. +// system cache directory. Version is used to suffix the installed binary and +// runtime-library names so multiple versions can coexist. License, when +// provided, is written next to the installed binary. // // RuntimeLib and RuntimeLibHash are optional: when set, the native in-process -// runtime library (cdylib) is installed next to the CLI binary under its natural -// platform name so the in-process (FFI) transport can load it. They are omitted +// runtime library (cdylib) is installed next to the CLI binary with the same +// version suffix so the in-process (FFI) transport can load it. They are omitted // for CLI packages that do not ship the native runtime. type Config struct { Cli io.Reader @@ -176,11 +176,10 @@ func installAt(installDir string) (string, error) { } // Install the native in-process runtime library (if bundled) next to the CLI - // binary under its natural platform name, so the FFI transport resolves it via - // the flat-name lookup. Fail closed on any hash mismatch — never place - // unverified native code. + // binary with the same version suffix. Fail closed on any hash mismatch — + // never place unverified native code. if config.RuntimeLib != nil { - libPath, err := installRuntimeLib(installDir) + libPath, err := installRuntimeLib(installDir, version) if err != nil { return "", err } @@ -190,14 +189,14 @@ func installAt(installDir string) (string, error) { return finalPath, nil } -// installRuntimeLib writes the embedded runtime cdylib into installDir under its -// natural platform file name, verifying its SHA-256. It is idempotent: an +// installRuntimeLib writes the embedded runtime cdylib into installDir under a +// versioned platform file name, verifying its SHA-256. It is idempotent: an // existing file with a matching hash is reused; a mismatch is a hard error. -func installRuntimeLib(installDir string) (string, error) { +func installRuntimeLib(installDir, version string) (string, error) { if len(config.RuntimeLibHash) != sha256.Size { return "", fmt.Errorf("RuntimeLibHash must be a SHA-256 hash (%d bytes), got %d bytes", sha256.Size, len(config.RuntimeLibHash)) } - libPath := filepath.Join(installDir, naturalRuntimeLibName()) + libPath := versionedRuntimeLibPath(installDir, naturalRuntimeLibName(), version) if _, err := os.Stat(libPath); err == nil { existingHash, err := hashFile(libPath) @@ -263,6 +262,17 @@ func versionedBinaryPath(dir, binaryName, version string) string { return filepath.Join(dir, fmt.Sprintf("%s_%s%s", base, version, ext)) } +// versionedRuntimeLibPath builds the runtime-library filename with the same +// optional version suffix as the embedded CLI. +func versionedRuntimeLibPath(dir, libraryName, version string) string { + if version == "" { + return filepath.Join(dir, libraryName) + } + base := strings.TrimSuffix(libraryName, filepath.Ext(libraryName)) + ext := filepath.Ext(libraryName) + return filepath.Join(dir, fmt.Sprintf("%s_%s%s", base, version, ext)) +} + // sanitizeVersion makes a version string safe for filenames. func sanitizeVersion(version string) string { if version == "" { diff --git a/go/internal/embeddedcli/embeddedcli_test.go b/go/internal/embeddedcli/embeddedcli_test.go index 0453f7293d..8cc969fda1 100644 --- a/go/internal/embeddedcli/embeddedcli_test.go +++ b/go/internal/embeddedcli/embeddedcli_test.go @@ -16,6 +16,7 @@ func resetGlobals() { config = Config{} setupDone = false pathInitialized = false + runtimeLibPath = "" } func mustPanic(t *testing.T, fn func()) { @@ -134,3 +135,43 @@ func TestVersionedBinaryPath(t *testing.T) { t.Fatalf("versionedBinaryPath() = %q want %q", got, want) } } + +func TestInstallAtAllowsMultipleRuntimeVersions(t *testing.T) { + resetGlobals() + tempDir := t.TempDir() + + installVersion := func(version string, cliContent, runtimeContent []byte) (string, string) { + t.Helper() + cliHash := sha256.Sum256(cliContent) + runtimeHash := sha256.Sum256(runtimeContent) + config = Config{ + Cli: bytes.NewReader(cliContent), + CliHash: cliHash[:], + RuntimeLib: bytes.NewReader(runtimeContent), + RuntimeLibHash: runtimeHash[:], + Version: version, + } + + cliPath, err := installAt(tempDir) + if err != nil { + t.Fatalf("install version %s: %v", version, err) + } + return cliPath, runtimeLibPath + } + + cli1, runtime1 := installVersion("1.0.0", []byte("cli-one"), []byte("runtime-one")) + cli2, runtime2 := installVersion("2.0.0", []byte("cli-two"), []byte("runtime-two")) + + if cli1 == cli2 { + t.Fatalf("Expected versioned CLI paths to differ, got %q", cli1) + } + if runtime1 == runtime2 { + t.Fatalf("Expected versioned runtime paths to differ, got %q", runtime1) + } + if got, err := os.ReadFile(runtime1); err != nil || string(got) != "runtime-one" { + t.Fatalf("Unexpected first runtime: content=%q err=%v", got, err) + } + if got, err := os.ReadFile(runtime2); err != nil || string(got) != "runtime-two" { + t.Fatalf("Unexpected second runtime: content=%q err=%v", got, err) + } +} diff --git a/go/internal/ffihost/ffihost.go b/go/internal/ffihost/ffihost.go index 1a0945240c..acfa8d005c 100644 --- a/go/internal/ffihost/ffihost.go +++ b/go/internal/ffihost/ffihost.go @@ -44,6 +44,7 @@ import ( "runtime" "strings" "sync" + "sync/atomic" "unsafe" "github.com/ebitengine/purego" @@ -69,6 +70,28 @@ var ( loadedLibraryPath string ) +var ( + outboundCallbackOnce sync.Once + outboundCallbackHandle uintptr + outboundTargets sync.Map + nextOutboundToken atomic.Uint64 +) + +func sharedOutboundCallback() uintptr { + outboundCallbackOnce.Do(func() { + outboundCallbackHandle = purego.NewCallback(routeOutbound) + }) + return outboundCallbackHandle +} + +func routeOutbound(userData uintptr, bytesPtr uintptr, bytesLen uintptr) uintptr { + target, ok := outboundTargets.Load(userData) + if !ok { + return 0 + } + return target.(*Host).onOutbound(bytesPtr, bytesLen) +} + func loadLibrary(libraryPath string) (lib *ffiLibrary, err error) { loadMu.Lock() defer loadMu.Unlock() @@ -134,10 +157,7 @@ type Host struct { recv *receiveBuffer - // callbackHandle is the purego callback trampoline address. purego keeps the - // underlying Go closure alive for the process lifetime, so it (and this Host, - // which the closure captures) must not be relied upon to be freed. - callbackHandle uintptr + callbackToken uintptr } // Create resolves the cdylib next to the CLI entrypoint and prepares the host. @@ -194,9 +214,14 @@ func (h *Host) Start() error { // here rather than at library load). rearmForeignSignalHandlers(h.lib.handle) - h.callbackHandle = purego.NewCallback(h.onOutbound) - h.connectionID = h.lib.connectionOpen(h.serverID, h.callbackHandle, 0, nil, 0, nil, 0, nil, 0) + callbackHandle := sharedOutboundCallback() + callbackToken := uintptr(nextOutboundToken.Add(1)) + outboundTargets.Store(callbackToken, h) + h.callbackToken = callbackToken + h.connectionID = h.lib.connectionOpen(h.serverID, callbackHandle, callbackToken, nil, 0, nil, 0, nil, 0) if h.connectionID == 0 { + outboundTargets.Delete(callbackToken) + h.callbackToken = 0 h.lib.hostShutdown(h.serverID) h.serverID = 0 return fmt.Errorf("copilot_runtime_connection_open failed") @@ -236,8 +261,7 @@ func (h *Host) buildEnv() []byte { // onOutbound is the native server → client callback, invoked on a foreign // runtime thread. The native pointer is only valid for this call, so the bytes // are copied out before returning. Nothing may panic across the FFI boundary. -func (h *Host) onOutbound(userData uintptr, bytesPtr uintptr, bytesLen uintptr) uintptr { - _ = userData +func (h *Host) onOutbound(bytesPtr uintptr, bytesLen uintptr) uintptr { h.mu.Lock() if h.disposed { h.mu.Unlock() @@ -301,10 +325,16 @@ func (h *Host) Dispose() { h.disposed = true connID := h.connectionID serverID := h.serverID + callbackToken := h.callbackToken h.connectionID = 0 h.serverID = 0 + h.callbackToken = 0 h.mu.Unlock() + if callbackToken != 0 { + outboundTargets.Delete(callbackToken) + } + // Stop accepting new callbacks and wait for in-flight ones to drain before // closing the receive buffer they feed. for { diff --git a/go/internal/ffihost/ffihost_test.go b/go/internal/ffihost/ffihost_test.go new file mode 100644 index 0000000000..b3020ddd8c --- /dev/null +++ b/go/internal/ffihost/ffihost_test.go @@ -0,0 +1,18 @@ +package ffihost + +import "testing" + +func TestDisposeUnregistersOutboundTarget(t *testing.T) { + token := uintptr(nextOutboundToken.Add(1)) + host := &Host{ + recv: newReceiveBuffer(), + callbackToken: token, + } + outboundTargets.Store(token, host) + + host.Dispose() + + if _, ok := outboundTargets.Load(token); ok { + t.Fatal("Expected disposed host to be removed from outbound callback registry") + } +} diff --git a/go/internal/ffihost/resolve.go b/go/internal/ffihost/resolve.go index 3868dd1a66..13f4916706 100644 --- a/go/internal/ffihost/resolve.go +++ b/go/internal/ffihost/resolve.go @@ -62,9 +62,9 @@ func PrebuildsFolder() string { // ResolveLibraryPath resolves the native runtime library next to the given CLI // entrypoint. It checks, in order: // -// 1. The natural platform library name next to the CLI (bundled/flat layout, -// what the embedded-CLI installer writes). -// 2. prebuilds//runtime.node next to the CLI (dev/package layout). +// 1. A versioned platform library name matching a versioned embedded CLI. +// 2. The natural platform library name next to the CLI (flat package layout). +// 3. prebuilds//runtime.node next to the CLI (dev/package layout). // // It returns an error when neither exists. func ResolveLibraryPath(cliEntrypoint string) (string, error) { @@ -74,6 +74,10 @@ func ResolveLibraryPath(cliEntrypoint string) (string, error) { } dir := filepath.Dir(abs) + if versioned := versionedLibraryPathForEntrypoint(abs); versioned != "" && fileExists(versioned) { + return versioned, nil + } + flat := filepath.Join(dir, NaturalLibraryName()) if fileExists(flat) { return flat, nil @@ -87,11 +91,30 @@ func ResolveLibraryPath(cliEntrypoint string) (string, error) { } return "", fmt.Errorf( - "in-process FFI runtime library not found next to %q (looked for %q and prebuilds/%s/runtime.node); "+ + "in-process FFI runtime library not found next to %q (looked for a matching versioned library, %q, and prebuilds/%s/runtime.node); "+ "use a runtime package that ships the native library", abs, NaturalLibraryName(), PrebuildsFolder()) } +func versionedLibraryPathForEntrypoint(cliEntrypoint string) string { + name := filepath.Base(cliEntrypoint) + stem := name + if strings.HasSuffix(strings.ToLower(stem), ".exe") { + stem = stem[:len(stem)-len(".exe")] + } + version, ok := strings.CutPrefix(stem, "copilot_") + if !ok || version == "" { + return "" + } + + libraryName := NaturalLibraryName() + libraryStem := strings.TrimSuffix(libraryName, filepath.Ext(libraryName)) + return filepath.Join( + filepath.Dir(cliEntrypoint), + libraryStem+"_"+version+filepath.Ext(libraryName), + ) +} + func fileExists(path string) bool { info, err := os.Stat(path) return err == nil && !info.IsDir() diff --git a/go/internal/ffihost/resolve_test.go b/go/internal/ffihost/resolve_test.go new file mode 100644 index 0000000000..df3b327252 --- /dev/null +++ b/go/internal/ffihost/resolve_test.go @@ -0,0 +1,57 @@ +package ffihost + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestVersionedLibraryPathForEntrypoint(t *testing.T) { + dir := t.TempDir() + cliName := "copilot_1.2.3" + if filepath.Ext(NaturalLibraryName()) == ".dll" { + cliName += ".exe" + } + + got := versionedLibraryPathForEntrypoint(filepath.Join(dir, cliName)) + libraryName := NaturalLibraryName() + want := filepath.Join( + dir, + strings.TrimSuffix(libraryName, filepath.Ext(libraryName))+"_1.2.3"+filepath.Ext(libraryName), + ) + if got != want { + t.Fatalf("versionedLibraryPathForEntrypoint() = %q, want %q", got, want) + } +} + +func TestResolveLibraryPathPrefersMatchingVersion(t *testing.T) { + dir := t.TempDir() + cliName := "copilot_1.2.3" + if filepath.Ext(NaturalLibraryName()) == ".dll" { + cliName += ".exe" + } + cliPath := filepath.Join(dir, cliName) + versionedPath := versionedLibraryPathForEntrypoint(cliPath) + flatPath := filepath.Join(dir, NaturalLibraryName()) + + for _, path := range []string{cliPath, versionedPath, flatPath} { + if err := os.WriteFile(path, []byte("test"), 0600); err != nil { + t.Fatalf("WriteFile(%q): %v", path, err) + } + } + + got, err := ResolveLibraryPath(cliPath) + if err != nil { + t.Fatalf("ResolveLibraryPath() error: %v", err) + } + if got != versionedPath { + t.Fatalf("ResolveLibraryPath() = %q, want %q", got, versionedPath) + } +} + +func TestVersionedLibraryPathForUnversionedEntrypoint(t *testing.T) { + if got := versionedLibraryPathForEntrypoint(filepath.Join(t.TempDir(), "copilot")); got != "" { + t.Fatalf("Expected no versioned library path, got %q", got) + } +} diff --git a/go/types.go b/go/types.go index f9386c2f66..35b5d996f7 100644 --- a/go/types.go +++ b/go/types.go @@ -22,8 +22,8 @@ const ( // // Construct one with a [StdioConnection], [TCPConnection], [URIConnection], or // [InProcessConnection] literal and pass it via [ClientOptions.Connection]. When -// [ClientOptions.Connection] is nil, the default is an empty [StdioConnection] -// (the SDK spawns the bundled runtime and communicates over stdin/stdout). +// [ClientOptions.Connection] is nil, COPILOT_SDK_DEFAULT_CONNECTION may select +// "inprocess" or "stdio"; when unset, the default is an empty [StdioConnection]. type RuntimeConnection interface { runtimeConnection() } @@ -123,8 +123,8 @@ func (InProcessConnection) runtimeConnection() {} // ClientOptions configures the [Client]. type ClientOptions struct { // Connection describes how to connect to the Copilot runtime. When nil, - // defaults to an empty [StdioConnection] (spawn the bundled runtime over - // stdio). + // COPILOT_SDK_DEFAULT_CONNECTION may select "inprocess" or "stdio"; + // when unset, defaults to an empty [StdioConnection]. Connection RuntimeConnection // WorkingDirectory is the working directory for the runtime process. // If empty, inherits the current process's working directory. From 820cf7d11c69a8bed09fabdf471f702e172e77c3 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 14 Jul 2026 10:40:02 +0100 Subject: [PATCH 05/14] Require matching FFI runtime versions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 447feac4-35df-4c0b-ab34-00789ad78da7 --- go/internal/ffihost/resolve.go | 12 +++++++++--- go/internal/ffihost/resolve_test.go | 26 +++++++++++++++++++++++++- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/go/internal/ffihost/resolve.go b/go/internal/ffihost/resolve.go index 13f4916706..e1b7d3daee 100644 --- a/go/internal/ffihost/resolve.go +++ b/go/internal/ffihost/resolve.go @@ -66,7 +66,8 @@ func PrebuildsFolder() string { // 2. The natural platform library name next to the CLI (flat package layout). // 3. prebuilds//runtime.node next to the CLI (dev/package layout). // -// It returns an error when neither exists. +// Versioned embedded CLIs require their exactly matching versioned library. +// Unversioned entrypoints fall back to the flat and package layouts. func ResolveLibraryPath(cliEntrypoint string) (string, error) { abs, err := filepath.Abs(cliEntrypoint) if err != nil { @@ -74,8 +75,13 @@ func ResolveLibraryPath(cliEntrypoint string) (string, error) { } dir := filepath.Dir(abs) - if versioned := versionedLibraryPathForEntrypoint(abs); versioned != "" && fileExists(versioned) { - return versioned, nil + if versioned := versionedLibraryPathForEntrypoint(abs); versioned != "" { + if fileExists(versioned) { + return versioned, nil + } + return "", fmt.Errorf( + "in-process FFI runtime library matching versioned CLI %q not found at %q", + abs, versioned) } flat := filepath.Join(dir, NaturalLibraryName()) diff --git a/go/internal/ffihost/resolve_test.go b/go/internal/ffihost/resolve_test.go index df3b327252..fe34cb035e 100644 --- a/go/internal/ffihost/resolve_test.go +++ b/go/internal/ffihost/resolve_test.go @@ -25,7 +25,7 @@ func TestVersionedLibraryPathForEntrypoint(t *testing.T) { } } -func TestResolveLibraryPathPrefersMatchingVersion(t *testing.T) { +func TestResolveLibraryPathRequiresMatchingVersion(t *testing.T) { dir := t.TempDir() cliName := "copilot_1.2.3" if filepath.Ext(NaturalLibraryName()) == ".dll" { @@ -50,6 +50,30 @@ func TestResolveLibraryPathPrefersMatchingVersion(t *testing.T) { } } +func TestResolveLibraryPathRejectsFlatLibraryForVersionedCLI(t *testing.T) { + dir := t.TempDir() + cliName := "copilot_1.2.3" + if filepath.Ext(NaturalLibraryName()) == ".dll" { + cliName += ".exe" + } + cliPath := filepath.Join(dir, cliName) + flatPath := filepath.Join(dir, NaturalLibraryName()) + + for _, path := range []string{cliPath, flatPath} { + if err := os.WriteFile(path, []byte("test"), 0600); err != nil { + t.Fatalf("WriteFile(%q): %v", path, err) + } + } + + _, err := ResolveLibraryPath(cliPath) + if err == nil { + t.Fatal("ResolveLibraryPath() succeeded with a flat library for a versioned CLI") + } + if !strings.Contains(err.Error(), filepath.Base(versionedLibraryPathForEntrypoint(cliPath))) { + t.Fatalf("ResolveLibraryPath() error = %q, want matching versioned library path", err) + } +} + func TestVersionedLibraryPathForUnversionedEntrypoint(t *testing.T) { if got := versionedLibraryPathForEntrypoint(filepath.Join(t.TempDir(), "copilot")); got != "" { t.Fatalf("Expected no versioned library path, got %q", got) From 6d92b09e19c4afbcc34795e81ac5edf3ad6ce0c4 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 14 Jul 2026 10:47:03 +0100 Subject: [PATCH 06/14] Isolate embedded CLI versions by directory Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 447feac4-35df-4c0b-ab34-00789ad78da7 --- go/README.md | 2 +- go/embeddedcli/installer.go | 6 +- go/internal/embeddedcli/embeddedcli.go | 74 +++++++++------------ go/internal/embeddedcli/embeddedcli_test.go | 73 ++++++++++++++++---- go/internal/ffihost/resolve.go | 38 ++--------- go/internal/ffihost/resolve_test.go | 71 ++++++-------------- 6 files changed, 120 insertions(+), 144 deletions(-) diff --git a/go/README.md b/go/README.md index 8a357007fc..7c5a6ce1eb 100644 --- a/go/README.md +++ b/go/README.md @@ -125,7 +125,7 @@ Resolution and requirements: transport when `ClientOptions.Connection` is nil. An explicit connection always takes precedence. - The entrypoint is resolved with **no `PATH` lookup**: explicit `InProcessConnection.Path`, then `COPILOT_CLI_PATH`, then the bundled embedded CLI. It must be an on-disk path. -- The runtime library is loaded from next to the resolved entrypoint (the matching versioned `libcopilot_runtime_*` name installed for an embedded CLI, a flat package library name, or the package's `prebuilds//runtime.node`). Start fails loudly if it cannot be found. +- The runtime library is loaded from next to the resolved entrypoint (the natural platform library name installed for an embedded CLI or flat package, or the package's `prebuilds//runtime.node`). Embedded CLI versions are isolated in separate cache directories so the CLI and runtime library always remain paired. Start fails loudly if the library cannot be found. - The runtime library is loaded once per process; loading a different library path in the same process is an error. The in-process transport rejects options that cannot be honored by a runtime hosted in your shared process (each panics at `NewClient`): diff --git a/go/embeddedcli/installer.go b/go/embeddedcli/installer.go index 59e23c6fcc..75c9b2c6b4 100644 --- a/go/embeddedcli/installer.go +++ b/go/embeddedcli/installer.go @@ -5,9 +5,9 @@ import "github.com/github/copilot-sdk/go/internal/embeddedcli" // Config defines the inputs used to install and locate the embedded Copilot CLI. // // Cli and CliHash are required. If Dir is empty, the CLI is installed into the -// system cache directory. Version is used to suffix the installed binary and -// runtime-library names so multiple versions can coexist. License, when -// provided, is written next to the installed binary. +// system cache directory. When Version is set, the CLI and runtime library are +// installed into a version-specific child directory so multiple versions can +// coexist. License, when provided, is written next to the installed binary. type Config = embeddedcli.Config // Setup sets the embedded GitHub Copilot CLI install configuration. diff --git a/go/internal/embeddedcli/embeddedcli.go b/go/internal/embeddedcli/embeddedcli.go index a61d253c04..faa9d79070 100644 --- a/go/internal/embeddedcli/embeddedcli.go +++ b/go/internal/embeddedcli/embeddedcli.go @@ -18,14 +18,14 @@ import ( // Config defines the inputs used to install and locate the embedded Copilot CLI. // // Cli and CliHash are required. If Dir is empty, the CLI is installed into the -// system cache directory. Version is used to suffix the installed binary and -// runtime-library names so multiple versions can coexist. License, when -// provided, is written next to the installed binary. +// system cache directory. When Version is set, the CLI is installed into a +// version-specific child directory so multiple versions can coexist. License, +// when provided, is written next to the installed binary. // // RuntimeLib and RuntimeLibHash are optional: when set, the native in-process -// runtime library (cdylib) is installed next to the CLI binary with the same -// version suffix so the in-process (FFI) transport can load it. They are omitted -// for CLI packages that do not ship the native runtime. +// runtime library (cdylib) is installed next to the CLI binary so the in-process +// (FFI) transport can load it. They are omitted for CLI packages that do not +// ship the native runtime. type Config struct { Cli io.Reader CliHash []byte @@ -123,17 +123,16 @@ func install() (path string) { } func installAt(installDir string) (string, error) { - if err := os.MkdirAll(installDir, 0755); err != nil { - return "", fmt.Errorf("creating install directory: %w", err) - } version := sanitizeVersion(config.Version) - lockName := ".copilot-cli.lock" if version != "" { - lockName = fmt.Sprintf(".copilot-cli-%s.lock", version) + installDir = filepath.Join(installDir, version) + } + if err := os.MkdirAll(installDir, 0755); err != nil { + return "", fmt.Errorf("creating install directory: %w", err) } // Best effort to prevent concurrent installs. - if release, _ := flock.Acquire(filepath.Join(installDir, lockName)); release != nil { + if release, _ := flock.Acquire(filepath.Join(installDir, ".copilot-cli.lock")); release != nil { defer release() } @@ -141,7 +140,7 @@ func installAt(installDir string) (string, error) { if runtime.GOOS == "windows" { binaryName += ".exe" } - finalPath := versionedBinaryPath(installDir, binaryName, version) + finalPath := filepath.Join(installDir, binaryName) if _, err := os.Stat(finalPath); err == nil { existingHash, err := hashFile(finalPath) @@ -151,6 +150,13 @@ func installAt(installDir string) (string, error) { if !bytes.Equal(existingHash, config.CliHash) { return "", fmt.Errorf("existing binary hash mismatch") } + if config.RuntimeLib != nil { + libPath, err := installRuntimeLib(installDir) + if err != nil { + return "", err + } + runtimeLibPath = libPath + } return finalPath, nil } @@ -175,11 +181,10 @@ func installAt(installDir string) (string, error) { } } - // Install the native in-process runtime library (if bundled) next to the CLI - // binary with the same version suffix. Fail closed on any hash mismatch — - // never place unverified native code. + // Install the native in-process runtime library (if bundled) next to the CLI. + // Fail closed on any hash mismatch; never place unverified native code. if config.RuntimeLib != nil { - libPath, err := installRuntimeLib(installDir, version) + libPath, err := installRuntimeLib(installDir) if err != nil { return "", err } @@ -189,14 +194,14 @@ func installAt(installDir string) (string, error) { return finalPath, nil } -// installRuntimeLib writes the embedded runtime cdylib into installDir under a -// versioned platform file name, verifying its SHA-256. It is idempotent: an +// installRuntimeLib writes the embedded runtime cdylib into installDir under its +// natural platform file name, verifying its SHA-256. It is idempotent: an // existing file with a matching hash is reused; a mismatch is a hard error. -func installRuntimeLib(installDir, version string) (string, error) { +func installRuntimeLib(installDir string) (string, error) { if len(config.RuntimeLibHash) != sha256.Size { return "", fmt.Errorf("RuntimeLibHash must be a SHA-256 hash (%d bytes), got %d bytes", sha256.Size, len(config.RuntimeLibHash)) } - libPath := versionedRuntimeLibPath(installDir, naturalRuntimeLibName(), version) + libPath := filepath.Join(installDir, naturalRuntimeLibName()) if _, err := os.Stat(libPath); err == nil { existingHash, err := hashFile(libPath) @@ -252,27 +257,6 @@ func naturalRuntimeLibName() string { } } -// versionedBinaryPath builds the unpacked binary filename with an optional version suffix. -func versionedBinaryPath(dir, binaryName, version string) string { - if version == "" { - return filepath.Join(dir, binaryName) - } - base := strings.TrimSuffix(binaryName, filepath.Ext(binaryName)) - ext := filepath.Ext(binaryName) - return filepath.Join(dir, fmt.Sprintf("%s_%s%s", base, version, ext)) -} - -// versionedRuntimeLibPath builds the runtime-library filename with the same -// optional version suffix as the embedded CLI. -func versionedRuntimeLibPath(dir, libraryName, version string) string { - if version == "" { - return filepath.Join(dir, libraryName) - } - base := strings.TrimSuffix(libraryName, filepath.Ext(libraryName)) - ext := filepath.Ext(libraryName) - return filepath.Join(dir, fmt.Sprintf("%s_%s%s", base, version, ext)) -} - // sanitizeVersion makes a version string safe for filenames. func sanitizeVersion(version string) string { if version == "" { @@ -293,7 +277,11 @@ func sanitizeVersion(version string) string { b.WriteRune('_') } } - return b.String() + sanitized := b.String() + if sanitized == "." || sanitized == ".." { + return strings.Repeat("_", len(sanitized)) + } + return sanitized } // hashFile returns the SHA-256 hash of a file on disk. diff --git a/go/internal/embeddedcli/embeddedcli_test.go b/go/internal/embeddedcli/embeddedcli_test.go index 8cc969fda1..42cdedd9e3 100644 --- a/go/internal/embeddedcli/embeddedcli_test.go +++ b/go/internal/embeddedcli/embeddedcli_test.go @@ -66,7 +66,7 @@ func TestInstallAtWritesBinaryAndLicense(t *testing.T) { path := Path() - expectedPath := versionedBinaryPath(tempDir, binaryNameForOS(), "1.2.3") + expectedPath := filepath.Join(tempDir, "1.2.3", binaryNameForOS()) if path != expectedPath { t.Fatalf("unexpected path: got %q want %q", path, expectedPath) } @@ -100,7 +100,7 @@ func TestInstallAtWritesBinaryAndLicense(t *testing.T) { func TestInstallAtExistingBinaryHashMismatch(t *testing.T) { resetGlobals() tempDir := t.TempDir() - binaryPath := versionedBinaryPath(tempDir, binaryNameForOS(), "") + binaryPath := filepath.Join(tempDir, binaryNameForOS()) if err := os.MkdirAll(filepath.Dir(binaryPath), 0755); err != nil { t.Fatalf("mkdir: %v", err) } @@ -121,18 +121,15 @@ func TestInstallAtExistingBinaryHashMismatch(t *testing.T) { } func TestSanitizeVersion(t *testing.T) { - got := sanitizeVersion("v1.2.3+build/abc") - want := "v1.2.3_build_abc" - if got != want { - t.Fatalf("sanitizeVersion() = %q want %q", got, want) - } -} - -func TestVersionedBinaryPath(t *testing.T) { - got := versionedBinaryPath("/tmp", "copilot.exe", "1.0.0") - want := filepath.Join("/tmp", "copilot_1.0.0.exe") - if got != want { - t.Fatalf("versionedBinaryPath() = %q want %q", got, want) + tests := map[string]string{ + "v1.2.3+build/abc": "v1.2.3_build_abc", + ".": "_", + "..": "__", + } + for input, want := range tests { + if got := sanitizeVersion(input); got != want { + t.Errorf("sanitizeVersion(%q) = %q want %q", input, got, want) + } } } @@ -168,6 +165,18 @@ func TestInstallAtAllowsMultipleRuntimeVersions(t *testing.T) { if runtime1 == runtime2 { t.Fatalf("Expected versioned runtime paths to differ, got %q", runtime1) } + if got, want := filepath.Base(cli1), binaryNameForOS(); got != want { + t.Fatalf("First CLI filename = %q, want %q", got, want) + } + if got, want := filepath.Base(runtime1), naturalRuntimeLibName(); got != want { + t.Fatalf("First runtime filename = %q, want %q", got, want) + } + if got, want := filepath.Base(filepath.Dir(cli1)), "1.0.0"; got != want { + t.Fatalf("First CLI version directory = %q, want %q", got, want) + } + if filepath.Dir(cli1) != filepath.Dir(runtime1) { + t.Fatalf("CLI and runtime were installed in different directories: %q and %q", cli1, runtime1) + } if got, err := os.ReadFile(runtime1); err != nil || string(got) != "runtime-one" { t.Fatalf("Unexpected first runtime: content=%q err=%v", got, err) } @@ -175,3 +184,39 @@ func TestInstallAtAllowsMultipleRuntimeVersions(t *testing.T) { t.Fatalf("Unexpected second runtime: content=%q err=%v", got, err) } } + +func TestInstallAtExistingBinaryInstallsMissingRuntime(t *testing.T) { + resetGlobals() + tempDir := t.TempDir() + versionDir := filepath.Join(tempDir, "1.2.3") + if err := os.MkdirAll(versionDir, 0755); err != nil { + t.Fatalf("mkdir: %v", err) + } + + cliContent := []byte("cli") + cliPath := filepath.Join(versionDir, binaryNameForOS()) + if err := os.WriteFile(cliPath, cliContent, 0755); err != nil { + t.Fatalf("write CLI: %v", err) + } + cliHash := sha256.Sum256(cliContent) + runtimeContent := []byte("runtime") + runtimeHash := sha256.Sum256(runtimeContent) + config = Config{ + Cli: bytes.NewReader(cliContent), + CliHash: cliHash[:], + RuntimeLib: bytes.NewReader(runtimeContent), + RuntimeLibHash: runtimeHash[:], + Version: "1.2.3", + } + + gotCLIPath, err := installAt(tempDir) + if err != nil { + t.Fatalf("installAt(): %v", err) + } + if gotCLIPath != cliPath { + t.Fatalf("installAt() = %q, want %q", gotCLIPath, cliPath) + } + if got, err := os.ReadFile(filepath.Join(versionDir, naturalRuntimeLibName())); err != nil || string(got) != "runtime" { + t.Fatalf("Unexpected runtime: content=%q err=%v", got, err) + } +} diff --git a/go/internal/ffihost/resolve.go b/go/internal/ffihost/resolve.go index e1b7d3daee..c8d4052322 100644 --- a/go/internal/ffihost/resolve.go +++ b/go/internal/ffihost/resolve.go @@ -62,12 +62,10 @@ func PrebuildsFolder() string { // ResolveLibraryPath resolves the native runtime library next to the given CLI // entrypoint. It checks, in order: // -// 1. A versioned platform library name matching a versioned embedded CLI. -// 2. The natural platform library name next to the CLI (flat package layout). -// 3. prebuilds//runtime.node next to the CLI (dev/package layout). +// 1. The natural platform library name next to the CLI (bundled/flat layout). +// 2. prebuilds//runtime.node next to the CLI (dev/package layout). // -// Versioned embedded CLIs require their exactly matching versioned library. -// Unversioned entrypoints fall back to the flat and package layouts. +// It returns an error when neither exists. func ResolveLibraryPath(cliEntrypoint string) (string, error) { abs, err := filepath.Abs(cliEntrypoint) if err != nil { @@ -75,15 +73,6 @@ func ResolveLibraryPath(cliEntrypoint string) (string, error) { } dir := filepath.Dir(abs) - if versioned := versionedLibraryPathForEntrypoint(abs); versioned != "" { - if fileExists(versioned) { - return versioned, nil - } - return "", fmt.Errorf( - "in-process FFI runtime library matching versioned CLI %q not found at %q", - abs, versioned) - } - flat := filepath.Join(dir, NaturalLibraryName()) if fileExists(flat) { return flat, nil @@ -97,30 +86,11 @@ func ResolveLibraryPath(cliEntrypoint string) (string, error) { } return "", fmt.Errorf( - "in-process FFI runtime library not found next to %q (looked for a matching versioned library, %q, and prebuilds/%s/runtime.node); "+ + "in-process FFI runtime library not found next to %q (looked for %q and prebuilds/%s/runtime.node); "+ "use a runtime package that ships the native library", abs, NaturalLibraryName(), PrebuildsFolder()) } -func versionedLibraryPathForEntrypoint(cliEntrypoint string) string { - name := filepath.Base(cliEntrypoint) - stem := name - if strings.HasSuffix(strings.ToLower(stem), ".exe") { - stem = stem[:len(stem)-len(".exe")] - } - version, ok := strings.CutPrefix(stem, "copilot_") - if !ok || version == "" { - return "" - } - - libraryName := NaturalLibraryName() - libraryStem := strings.TrimSuffix(libraryName, filepath.Ext(libraryName)) - return filepath.Join( - filepath.Dir(cliEntrypoint), - libraryStem+"_"+version+filepath.Ext(libraryName), - ) -} - func fileExists(path string) bool { info, err := os.Stat(path) return err == nil && !info.IsDir() diff --git a/go/internal/ffihost/resolve_test.go b/go/internal/ffihost/resolve_test.go index fe34cb035e..df3a668dfe 100644 --- a/go/internal/ffihost/resolve_test.go +++ b/go/internal/ffihost/resolve_test.go @@ -3,39 +3,15 @@ package ffihost import ( "os" "path/filepath" - "strings" "testing" ) -func TestVersionedLibraryPathForEntrypoint(t *testing.T) { +func TestResolveLibraryPathUsesNaturalLibraryNextToCLI(t *testing.T) { dir := t.TempDir() - cliName := "copilot_1.2.3" - if filepath.Ext(NaturalLibraryName()) == ".dll" { - cliName += ".exe" - } - - got := versionedLibraryPathForEntrypoint(filepath.Join(dir, cliName)) - libraryName := NaturalLibraryName() - want := filepath.Join( - dir, - strings.TrimSuffix(libraryName, filepath.Ext(libraryName))+"_1.2.3"+filepath.Ext(libraryName), - ) - if got != want { - t.Fatalf("versionedLibraryPathForEntrypoint() = %q, want %q", got, want) - } -} + cliPath := filepath.Join(dir, "copilot") + libraryPath := filepath.Join(dir, NaturalLibraryName()) -func TestResolveLibraryPathRequiresMatchingVersion(t *testing.T) { - dir := t.TempDir() - cliName := "copilot_1.2.3" - if filepath.Ext(NaturalLibraryName()) == ".dll" { - cliName += ".exe" - } - cliPath := filepath.Join(dir, cliName) - versionedPath := versionedLibraryPathForEntrypoint(cliPath) - flatPath := filepath.Join(dir, NaturalLibraryName()) - - for _, path := range []string{cliPath, versionedPath, flatPath} { + for _, path := range []string{cliPath, libraryPath} { if err := os.WriteFile(path, []byte("test"), 0600); err != nil { t.Fatalf("WriteFile(%q): %v", path, err) } @@ -45,37 +21,34 @@ func TestResolveLibraryPathRequiresMatchingVersion(t *testing.T) { if err != nil { t.Fatalf("ResolveLibraryPath() error: %v", err) } - if got != versionedPath { - t.Fatalf("ResolveLibraryPath() = %q, want %q", got, versionedPath) + if got != libraryPath { + t.Fatalf("ResolveLibraryPath() = %q, want %q", got, libraryPath) } } -func TestResolveLibraryPathRejectsFlatLibraryForVersionedCLI(t *testing.T) { - dir := t.TempDir() - cliName := "copilot_1.2.3" - if filepath.Ext(NaturalLibraryName()) == ".dll" { - cliName += ".exe" +func TestResolveLibraryPathFallsBackToPrebuilds(t *testing.T) { + folder := PrebuildsFolder() + if folder == "" { + t.Skip("unsupported platform") } - cliPath := filepath.Join(dir, cliName) - flatPath := filepath.Join(dir, NaturalLibraryName()) - for _, path := range []string{cliPath, flatPath} { + dir := t.TempDir() + cliPath := filepath.Join(dir, "copilot") + libraryPath := filepath.Join(dir, "prebuilds", folder, "runtime.node") + if err := os.MkdirAll(filepath.Dir(libraryPath), 0755); err != nil { + t.Fatalf("MkdirAll(): %v", err) + } + for _, path := range []string{cliPath, libraryPath} { if err := os.WriteFile(path, []byte("test"), 0600); err != nil { t.Fatalf("WriteFile(%q): %v", path, err) } } - _, err := ResolveLibraryPath(cliPath) - if err == nil { - t.Fatal("ResolveLibraryPath() succeeded with a flat library for a versioned CLI") - } - if !strings.Contains(err.Error(), filepath.Base(versionedLibraryPathForEntrypoint(cliPath))) { - t.Fatalf("ResolveLibraryPath() error = %q, want matching versioned library path", err) + got, err := ResolveLibraryPath(cliPath) + if err != nil { + t.Fatalf("ResolveLibraryPath() error: %v", err) } -} - -func TestVersionedLibraryPathForUnversionedEntrypoint(t *testing.T) { - if got := versionedLibraryPathForEntrypoint(filepath.Join(t.TempDir(), "copilot")); got != "" { - t.Fatalf("Expected no versioned library path, got %q", got) + if got != libraryPath { + t.Fatalf("ResolveLibraryPath() = %q, want %q", got, libraryPath) } } From 343b742305d57c78fb4f21a4c05746449503e8cd Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 14 Jul 2026 11:30:31 +0100 Subject: [PATCH 07/14] Hide in-process runtime implementation details Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 447feac4-35df-4c0b-ab34-00789ad78da7 --- go/README.md | 16 ++--- go/client.go | 36 ++++------ go/client_test.go | 6 +- go/internal/e2e/inprocess_ffi_e2e_test.go | 3 +- go/internal/e2e/testharness/context.go | 3 +- go/internal/ffihost/ffihost.go | 5 +- go/types.go | 11 +-- python/README.md | 16 +++-- python/copilot/client.py | 88 +++++++---------------- python/e2e/test_inprocess_ffi_e2e.py | 9 +-- python/e2e/testharness/context.py | 14 ++-- python/test_client.py | 9 +++ rust/README.md | 12 ++-- rust/src/lib.rs | 45 +++++++----- rust/tests/e2e/rpc_mcp_and_skills.rs | 36 +++++++--- rust/tests/e2e/support.rs | 20 ++---- 16 files changed, 157 insertions(+), 172 deletions(-) diff --git a/go/README.md b/go/README.md index 7c5a6ce1eb..0a266ab534 100644 --- a/go/README.md +++ b/go/README.md @@ -101,17 +101,17 @@ Follow these steps to embed the CLI: That's it! When your application calls `copilot.NewClient` without a `Connection` field (or with an empty `StdioConnection{}`) and no `COPILOT_CLI_PATH` environment variable, the SDK will automatically install the embedded CLI to a cache directory and use it for all operations. -The bundler also embeds the native in-process runtime library (`runtime.node`) alongside the CLI binary when the target platform's Copilot package ships one, so the [in-process transport](#in-process-transport-experimental) works out of the box for embedded builds. +The bundler also embeds the native runtime library required by the [in-process transport](#in-process-transport-experimental), so it works out of the box for embedded builds. ## In-process transport (Experimental) > **Experimental:** the in-process API may change in a future release. -By default the SDK spawns the Copilot CLI as a child process and talks JSON-RPC over stdio or TCP. The **in-process** transport instead loads the native runtime library (`runtime.node`) into your process and hosts the runtime there; the native host spawns the CLI worker itself. This avoids a separate long-lived child process for the runtime server. +By default the SDK starts the runtime as a child process and talks JSON-RPC over stdio or TCP. The **in-process** transport instead loads a native runtime library directly into your process. ```go client := copilot.NewClient(&copilot.ClientOptions{ - Connection: copilot.InProcessConnection{Path: "/path/to/copilot"}, + Connection: copilot.InProcessConnection{}, }) if err := client.Start(context.Background()); err != nil { log.Fatal(err) @@ -124,14 +124,14 @@ Resolution and requirements: - Set `COPILOT_SDK_DEFAULT_CONNECTION=inprocess` to select the in-process transport when `ClientOptions.Connection` is nil. An explicit connection always takes precedence. -- The entrypoint is resolved with **no `PATH` lookup**: explicit `InProcessConnection.Path`, then `COPILOT_CLI_PATH`, then the bundled embedded CLI. It must be an on-disk path. -- The runtime library is loaded from next to the resolved entrypoint (the natural platform library name installed for an embedded CLI or flat package, or the package's `prebuilds//runtime.node`). Embedded CLI versions are isolated in separate cache directories so the CLI and runtime library always remain paired. Start fails loudly if the library cannot be found. -- The runtime library is loaded once per process; loading a different library path in the same process is an error. +- Set `COPILOT_CLI_PATH` only when using an externally provisioned compatible runtime package; otherwise the bundled runtime is used. No `PATH` lookup is performed. +- Embedded runtime versions are isolated in separate cache directories. Start fails loudly if the native runtime is unavailable. +- Only one native runtime version may be loaded per process. The in-process transport rejects options that cannot be honored by a runtime hosted in your shared process (each panics at `NewClient`): - `Env` — the host process has a single environment block. Set variables on the host process environment instead. -- `WorkingDirectory` — the worker is spawned without a working-directory parameter. Change the process working directory before creating the client. +- `WorkingDirectory` — the runtime shares the host process's working directory. Change the process working directory before creating the client. - `Telemetry` — per-client telemetry is lowered to native-runtime environment variables. Use a child-process transport for per-client telemetry. Implemented with pure-Go FFI (via [purego](https://github.com/ebitengine/purego)), so `CGO_ENABLED=0` and cross-compilation are preserved; no C toolchain is required. @@ -180,7 +180,7 @@ Event types: `SessionLifecycleCreated`, `SessionLifecycleDeleted`, `SessionLifec - `StdioConnection{Path, Args, Env}` — spawn a runtime over stdio (the default if `Connection` is nil) - `TCPConnection{Port, ConnectionToken, Path, Args, Env}` — spawn a runtime that listens on TCP - `URIConnection{URL, ConnectionToken}` — connect to an already-running runtime (no process spawned) - - `InProcessConnection{Path, Args}` — **Experimental.** Host the runtime in-process via the native FFI library instead of spawning a child process. See [In-process transport](#in-process-transport-experimental) below. + - `InProcessConnection{}` — **Experimental.** Host the runtime in-process via the native FFI library instead of spawning a child process. See [In-process transport](#in-process-transport-experimental) below. When `Path` is empty for stdio/tcp, the SDK uses the bundled CLI (or `COPILOT_CLI_PATH` env var). diff --git a/go/client.go b/go/client.go index dc589f3d4c..2cea020025 100644 --- a/go/client.go +++ b/go/client.go @@ -110,7 +110,7 @@ func validateEnvironmentOptions(connection RuntimeConnection, opts *ClientOption panic("Env is not supported with InProcessConnection: the in-process transport loads the native runtime into the shared host process, whose single environment block cannot carry per-client values. Set the variables on the host process environment instead.") } if opts.WorkingDirectory != "" { - panic("WorkingDirectory is not supported with InProcessConnection: the in-process transport hosts the runtime in the shared host process and spawns the worker without a working-directory parameter. Use a child-process transport, or set the process working directory before creating the client.") + panic("WorkingDirectory is not supported with InProcessConnection: the native runtime shares the host process working directory. Use a child-process transport, or set the process working directory before creating the client.") } if opts.Telemetry != nil { panic("Telemetry is not supported with InProcessConnection: telemetry configuration is lowered to environment variables read by native runtime code running in the shared host process, so per-client telemetry cannot be honored in-process. Configure telemetry via the host process environment, or use a child-process transport.") @@ -265,10 +265,6 @@ func NewClient(options *ClientOptions) *Client { case InProcessConnection: client.useStdio = false client.useInProcess = true - client.cliPath = conn.Path - if len(conn.Args) > 0 { - client.cliArgs = append([]string{}, conn.Args...) - } default: panic(fmt.Sprintf("unknown RuntimeConnection type: %T", connection)) } @@ -299,9 +295,7 @@ func NewClient(options *ClientOptions) *Client { opts.Env = os.Environ() } - // Check effective environment for CLI path (only if not explicitly set via - // options). The in-process transport never falls back to PATH, so its - // entrypoint is resolved separately in startInProcess. + // Check the effective environment for a child-process runtime override. if client.cliPath == "" && !client.useInProcess { if cliPath := getEnvValue(opts.Env, "COPILOT_CLI_PATH"); cliPath != "" { client.cliPath = cliPath @@ -2065,29 +2059,25 @@ func (c *Client) startCLIServer(ctx context.Context) error { } } -// startInProcess hosts the runtime in-process via the native FFI library. It -// resolves the CLI entrypoint (no PATH fallback — the entrypoint must be an -// explicit path, COPILOT_CLI_PATH, or the bundled runtime), loads the sibling -// cdylib, lets the native host spawn the worker, and wires the JSON-RPC client -// to the FFI byte streams exactly like the stdio transport. +// startInProcess loads the native runtime library and wires the JSON-RPC client +// to its FFI byte streams. func (c *Client) startInProcess(ctx context.Context) error { - entrypoint := c.cliPath - if entrypoint == "" { + runtimePath := c.cliPath + if runtimePath == "" { // The in-process transport does not resolve a bare command name from PATH - // (unlike the child-process transport); fall back only to COPILOT_CLI_PATH - // and then the bundled runtime, all absolute on-disk paths. + // (unlike the child-process transport). if p := getEnvValue(c.options.Env, "COPILOT_CLI_PATH"); p != "" { - entrypoint = p + runtimePath = p } } - if entrypoint == "" { - entrypoint = embeddedcli.Path() + if runtimePath == "" { + runtimePath = embeddedcli.Path() } - if entrypoint == "" { - return errors.New("in-process transport requires the Copilot CLI: set InProcessConnection.Path or COPILOT_CLI_PATH, or build with the bundled embedded runtime") + if runtimePath == "" { + return errors.New("in-process runtime unavailable: set COPILOT_CLI_PATH to a compatible runtime package or build with the bundled embedded runtime") } - host, err := ffihost.Create(entrypoint, nil, c.cliArgs) + host, err := ffihost.Create(runtimePath, nil) if err != nil { return err } diff --git a/go/client_test.go b/go/client_test.go index 4980bcbe1a..0d27b77415 100644 --- a/go/client_test.go +++ b/go/client_test.go @@ -627,7 +627,7 @@ func TestClient_EnvOptions(t *testing.T) { func TestClient_InProcessConnection(t *testing.T) { t.Run("uses in-process transport", func(t *testing.T) { - client := NewClient(&ClientOptions{Connection: InProcessConnection{Path: "/tmp/copilot"}}) + client := NewClient(&ClientOptions{Connection: InProcessConnection{}}) if !client.useInProcess { t.Error("Expected useInProcess=true for InProcessConnection") } @@ -637,8 +637,8 @@ func TestClient_InProcessConnection(t *testing.T) { if client.isExternalServer { t.Error("Expected isExternalServer=false for InProcessConnection") } - if client.cliPath != "/tmp/copilot" { - t.Errorf("Expected cliPath to be '/tmp/copilot', got %q", client.cliPath) + if client.cliPath != "" { + t.Errorf("Expected in-process cliPath to stay empty at construction, got %q", client.cliPath) } }) diff --git a/go/internal/e2e/inprocess_ffi_e2e_test.go b/go/internal/e2e/inprocess_ffi_e2e_test.go index d2d85c8b9a..6923384a28 100644 --- a/go/internal/e2e/inprocess_ffi_e2e_test.go +++ b/go/internal/e2e/inprocess_ffi_e2e_test.go @@ -30,10 +30,11 @@ func TestInProcessFfiE2E(t *testing.T) { if cliPath == "" { t.Fatal("CLI not found. Run 'npm install' in the nodejs directory first.") } + t.Setenv("COPILOT_CLI_PATH", cliPath) t.Run("should start and connect over in-process FFI", func(t *testing.T) { client := copilot.NewClient(&copilot.ClientOptions{ - Connection: copilot.InProcessConnection{Path: cliPath}, + Connection: copilot.InProcessConnection{}, }) t.Cleanup(func() { client.ForceStop() }) diff --git a/go/internal/e2e/testharness/context.go b/go/internal/e2e/testharness/context.go index bc1f47cbff..b73153dfff 100644 --- a/go/internal/e2e/testharness/context.go +++ b/go/internal/e2e/testharness/context.go @@ -271,6 +271,7 @@ func (c *TestContext) applyInProcessEnvironment(mergedEnv []string, workDir stri // inherited values. The HMAC key is neutralized process-wide at package load. inprocessEnv["GH_TOKEN"] = defaultGitHubToken inprocessEnv["GITHUB_TOKEN"] = defaultGitHubToken + inprocessEnv["COPILOT_CLI_PATH"] = c.CLIPath delete(inprocessEnv, "COPILOT_HMAC_KEY") delete(inprocessEnv, "CAPI_HMAC_KEY") @@ -392,7 +393,7 @@ func (c *TestContext) NewClient(opts ...func(*copilot.ClientOptions)) *copilot.C // their transport, mirroring the Node/.NET harnesses. if c.inProcess && c.shouldUseInProcess(options) { c.applyInProcessEnvironment(options.Env, options.WorkingDirectory) - options.Connection = copilot.InProcessConnection{Path: c.CLIPath} + options.Connection = copilot.InProcessConnection{} options.Env = nil options.WorkingDirectory = "" } diff --git a/go/internal/ffihost/ffihost.go b/go/internal/ffihost/ffihost.go index acfa8d005c..2321146286 100644 --- a/go/internal/ffihost/ffihost.go +++ b/go/internal/ffihost/ffihost.go @@ -140,7 +140,6 @@ type Host struct { libraryPath string cliEntrypoint string environment map[string]string - extraArgs []string lib *ffiLibrary // mu guards disposed, activeCallbacks, serverID, and connectionID. A single @@ -162,7 +161,7 @@ type Host struct { // Create resolves the cdylib next to the CLI entrypoint and prepares the host. // It does not spawn the worker; call Start for that. environment may be nil. -func Create(cliEntrypoint string, environment map[string]string, extraArgs []string) (*Host, error) { +func Create(cliEntrypoint string, environment map[string]string) (*Host, error) { libraryPath, err := ResolveLibraryPath(cliEntrypoint) if err != nil { return nil, err @@ -175,7 +174,6 @@ func Create(cliEntrypoint string, environment map[string]string, extraArgs []str libraryPath: libraryPath, cliEntrypoint: cliEntrypoint, environment: environment, - extraArgs: append([]string(nil), extraArgs...), lib: lib, recv: newReceiveBuffer(), }, nil @@ -245,7 +243,6 @@ func (h *Host) buildArgv() []byte { } else { argv = []string{h.cliEntrypoint, "--embedded-host", "--no-auto-update"} } - argv = append(argv, h.extraArgs...) b, _ := json.Marshal(argv) return b } diff --git a/go/types.go b/go/types.go index 35b5d996f7..d434ad47e1 100644 --- a/go/types.go +++ b/go/types.go @@ -96,8 +96,7 @@ func (URIConnection) runtimeConnection() {} // InProcessConnection hosts the Copilot runtime in-process by loading its native // runtime library (a Rust cdylib) and driving JSON-RPC over the library's C ABI, -// instead of spawning a runtime child process. The native host spawns the -// residual worker itself; the SDK never launches it directly. +// instead of spawning a runtime child process. // // Because the runtime is loaded into the calling process, per-client // environment, working directory, and telemetry cannot be represented and are @@ -108,14 +107,6 @@ func (URIConnection) runtimeConnection() {} // Experimental: the in-process transport is experimental and its API and // behavior may change in a future release. type InProcessConnection struct { - // Path is the runtime executable used to locate and launch the worker and - // to resolve the sibling native library. When empty, the client resolves - // COPILOT_CLI_PATH and then the bundled runtime. Unlike the child-process - // transports, no executable is looked up on PATH. - Path string - // Args are extra command-line arguments passed to the residual worker, - // appended after the SDK-managed args. - Args []string } func (InProcessConnection) runtimeConnection() {} diff --git a/python/README.md b/python/README.md index 30dc964e30..3089c36699 100644 --- a/python/README.md +++ b/python/README.md @@ -39,9 +39,9 @@ To pre-provision the native library required by the in-process (FFI) transport python -m copilot download-runtime --in-process ``` -This additionally fetches the native runtime shared library and places it next to -the cached binary. Stdio/TCP users never download it. When omitted, the library is -downloaded lazily on first use of the in-process transport. +This additionally fetches the native runtime library into the versioned runtime +cache. Stdio/TCP users never download it. When omitted, it is downloaded +lazily on first use of the in-process transport. | Platform | Cache path | |----------|-----------| @@ -216,7 +216,7 @@ All options are kw-only parameters: - `RuntimeConnection.for_stdio(path=None, args=None)` — spawn a local CLI process and talk over stdio. - `RuntimeConnection.for_tcp(port=0, connection_token=None, path=None, args=None)` — spawn a local CLI in TCP mode. - `RuntimeConnection.for_uri(url, connection_token=None)` — connect to an existing CLI server (e.g. `"localhost:8080"`). -- `RuntimeConnection.for_inprocess(path=None, args=None)` — host the runtime in-process via its native C ABI (FFI). See [In-process (FFI) transport](#in-process-ffi-transport). +- `RuntimeConnection.for_inprocess()` — host the runtime in-process via its native C ABI (FFI). See [In-process (FFI) transport](#in-process-ffi-transport). Child-process connections (`for_stdio`/`for_tcp`) also expose a per-connection `env` field for the spawned process. Set it on the returned connection instead of @@ -232,8 +232,7 @@ client = CopilotClient(connection=conn) # do NOT also pass env=... here > ⚠️ **Experimental.** The in-process transport loads the runtime's native shared > library into your process and drives JSON-RPC over its C ABI (via stdlib -> `ctypes`), instead of spawning a child process. The native host spawns the -> residual worker itself. +> `ctypes`), instead of spawning a child process. ```python from copilot import CopilotClient, RuntimeConnection @@ -249,9 +248,12 @@ finally: **Requirements & behavior:** -- Requires the native runtime library next to the CLI. Pre-provision it with +- Pre-provision the native runtime with `python -m copilot download-runtime --in-process`, or let the SDK download it lazily on first use of this transport. +- Set `COPILOT_CLI_PATH` only when using an externally provisioned compatible + runtime package. In-process connections do not accept per-connection paths + or raw process arguments. - Because the runtime shares this single host process, per-client options that lower to environment variables or a working directory **cannot** be honored and are rejected: `env`, `telemetry`, and `working_directory` all raise `ValueError` diff --git a/python/copilot/client.py b/python/copilot/client.py index 1127710dae..40201fbc53 100644 --- a/python/copilot/client.py +++ b/python/copilot/client.py @@ -362,11 +362,7 @@ def for_uri(url: str, *, connection_token: str | None = None) -> UriRuntimeConne return UriRuntimeConnection(url=url, connection_token=connection_token) @staticmethod - def for_inprocess( - *, - path: str | None = None, - args: Sequence[str] = (), - ) -> InProcessRuntimeConnection: + def for_inprocess() -> InProcessRuntimeConnection: """Host the runtime **in-process** via its native C ABI (FFI). **Experimental.** The in-process (FFI) transport is experimental and its @@ -374,7 +370,7 @@ def for_inprocess( Instead of spawning the runtime as a child process, the SDK loads the runtime's native shared library into this process and drives JSON-RPC - over its C ABI. The native host spawns the residual worker itself. + over its C ABI. Because the runtime loads into this single shared process, per-client options that lower to environment variables or a working directory @@ -382,17 +378,15 @@ def for_inprocess( :attr:`CopilotClientOptions.telemetry`, and :attr:`CopilotClientOptions.working_directory` are rejected with this transport. Set those on the host process before creating the client. - - Args: - path: Path to the runtime executable used to resolve the native - library location. When ``None``, uses the bundled binary. - args: Extra command-line arguments passed to the worker. + Set ``COPILOT_CLI_PATH`` only when using an externally provisioned + compatible runtime package. Note: - The native runtime library must be available next to the CLI - (download it with ``python -m copilot download-runtime --in-process``). + Pre-provision the native runtime with + ``python -m copilot download-runtime --in-process`` when automatic + downloads are disabled. """ - return InProcessRuntimeConnection(path=path, args=tuple(args)) + return InProcessRuntimeConnection() @dataclass @@ -461,17 +455,9 @@ class InProcessRuntimeConnection(RuntimeConnection): Construct via :meth:`RuntimeConnection.for_inprocess`. The runtime's native shared library is loaded into this process and JSON-RPC is driven over its - C ABI; the native host spawns the residual worker itself. + C ABI. """ - path: str | None = None - """Path to the runtime executable used to resolve the native library. - - ``None`` uses the bundled binary.""" - - args: Sequence[str] = () - """Extra command-line arguments passed to the worker.""" - class _GitHubTelemetryAdapter: """Adapts a user-provided ``on_github_telemetry`` callback to the generated @@ -1117,7 +1103,7 @@ def _get_or_download_cli(*, include_runtime_lib: bool = False) -> str | None: with no pinned version, or auto-download disabled). When ``include_runtime_lib`` is set, also ensures the native in-process FFI - runtime library is present next to the CLI (downloading it on first use). + runtime is available (downloading it on first use). """ from ._cli_download import get_or_download_cli @@ -1217,9 +1203,9 @@ def _validate_environment_options( if options.working_directory is not None: raise ValueError( "working_directory is not supported with RuntimeConnection.for_inprocess(): " - "the in-process transport hosts the runtime in the shared host process and " - "spawns the worker without a working-directory parameter, so a per-client " - "working directory cannot be honored in-process. Use a child-process " + "the native runtime shares the host process working directory, so a " + "per-client working directory cannot be honored in-process. Use a " + "child-process " "transport, or set the process working directory before creating the client." ) return @@ -1392,6 +1378,7 @@ def __init__( self._is_external_server: bool = isinstance(connection, UriRuntimeConnection) self._cli_path_source: str | None = None self._ffi_host: FfiRuntimeHost | None = None + self._inprocess_runtime_path: str | None = None if isinstance(connection, UriRuntimeConnection): if connection.connection_token is not None and len(connection.connection_token) == 0: @@ -1400,14 +1387,11 @@ def __init__( self._runtime_port: int | None = actual_port self._effective_connection_token: str | None = connection.connection_token elif isinstance(connection, InProcessRuntimeConnection): - # In-process (FFI): no child process and no per-connection token; the - # native host spawns the worker itself. Resolve the runtime entrypoint - # exactly like stdio (explicit > COPILOT_CLI_PATH > downloaded), and - # ensure the native runtime library is present alongside it. + # In-process (FFI): no child process and no per-connection token. self._runtime_port = None self._effective_connection_token = None - connection.path = self._resolve_runtime_entrypoint( - connection.path, include_runtime_lib=True + self._inprocess_runtime_path = self._resolve_runtime_entrypoint( + None, include_runtime_lib=True ) if options.use_logged_in_user is None: options.use_logged_in_user = not bool(options.github_token) @@ -1502,8 +1486,7 @@ def _resolve_runtime_entrypoint( "auto-downloads the CLI on first use), set COPILOT_CLI_PATH, " "or pass an explicit path via " "RuntimeConnection.for_stdio(path=...) / " - "RuntimeConnection.for_tcp(path=...) / " - "RuntimeConnection.for_inprocess(path=...)." + "RuntimeConnection.for_tcp(path=...)." ) @staticmethod @@ -1791,9 +1774,7 @@ async def stop(self) -> None: async with self._models_cache_lock: self._models_cache = None - # Dispose the in-process FFI host (shuts down the native runtime worker and - # releases the loaded shared library). The process-like adapter's terminate() - # delegates here, but call dispose() explicitly so teardown is unambiguous. + # Dispose the in-process FFI host and release the loaded native library. if self._ffi_host is not None: try: self._ffi_host.dispose() @@ -1895,8 +1876,7 @@ async def force_stop(self) -> None: except Exception: logger.debug("Error while force-stopping Copilot CLI process", exc_info=True) - # Force-dispose the in-process FFI host (kills the native worker and releases - # the shared library) before tearing down the JSON-RPC client. + # Force-dispose the in-process FFI host before tearing down JSON-RPC. if self._ffi_host is not None: try: self._ffi_host.dispose() @@ -3962,36 +3942,23 @@ async def read_port(): async def _start_inprocess_ffi(self) -> None: """Host the runtime in-process via the native FFI library. - Loads the runtime cdylib next to the resolved CLI entrypoint, lets the - native host spawn the worker, and opens the FFI JSON-RPC connection. The - resulting :class:`FfiRuntimeHost` exposes a process-like adapter that the - JSON-RPC client drives exactly like a spawned child process. + Loads the native runtime library and opens the FFI JSON-RPC connection. Raises: RuntimeError: If the native library is missing or startup fails. """ assert isinstance(self._connection, InProcessRuntimeConnection) - conn = self._connection - cli_path = conn.path - assert cli_path is not None # resolved in __init__ + runtime_path = self._inprocess_runtime_path + assert runtime_path is not None # resolved in __init__ - # No PATH lookup here (unlike the child-process transport): the in-process - # entrypoint is always an absolute on-disk path — an explicit path, - # COPILOT_CLI_PATH, or the downloaded CLI — so the native library provisioned - # next to it in __init__ is exactly the one FfiRuntimeHost loads. Resolving a - # bare command name via PATH would look for the library beside a different - # directory than the one it was provisioned into and fail. A packaged - # single-file CLI is invoked directly; a `.js` dev entrypoint is launched via - # node — both handled inside FfiRuntimeHost. logger.info( "CopilotClient._start_inprocess_ffi hosting Copilot runtime in-process", - extra={"cli_path": cli_path, "cli_path_source": self._cli_path_source}, + extra={"runtime_path": runtime_path, "runtime_path_source": self._cli_path_source}, ) # env/telemetry/working_directory are rejected for the in-process transport - # (see _validate_environment_options); the worker inherits this process's - # ambient environment. Pass extra worker args via connection.args. - host = FfiRuntimeHost.create(cli_path, environment=None, args=conn.args) + # because the native runtime shares this process's ambient state. + host = FfiRuntimeHost.create(runtime_path, environment=None, args=()) # Track the host and expose its process-like adapter *before* the blocking # handshake. asyncio.to_thread keeps running host_start after a cancellation @@ -4003,8 +3970,7 @@ async def _start_inprocess_ffi(self) -> None: self._process = host.process ffi_start = time.perf_counter() - # host_start blocks until the worker connects back and signals readiness, - # so run the handshake off the event loop. + # Native startup may block, so run the handshake off the event loop. await asyncio.to_thread(host.start_blocking) log_timing( logger, diff --git a/python/e2e/test_inprocess_ffi_e2e.py b/python/e2e/test_inprocess_ffi_e2e.py index 59a836972c..c119c4ea4e 100644 --- a/python/e2e/test_inprocess_ffi_e2e.py +++ b/python/e2e/test_inprocess_ffi_e2e.py @@ -21,14 +21,15 @@ class TestInProcessFfi: - async def test_should_start_and_connect_over_in_process_ffi(self, ctx: E2ETestContext): + async def test_should_start_and_connect_over_in_process_ffi( + self, ctx: E2ETestContext, monkeypatch: pytest.MonkeyPatch + ): # In-process hosting loads the runtime cdylib next to the resolved CLI # entrypoint and lets the native host spawn the worker. ``ping`` is a # purely local RPC round-trip, so no auth or replay proxy is involved. # If the native library is unavailable, start() raises and the test fails. - client = CopilotClient( - connection=RuntimeConnection.for_inprocess(path=get_cli_path_for_tests()), - ) + monkeypatch.setenv("COPILOT_CLI_PATH", get_cli_path_for_tests()) + client = CopilotClient(connection=RuntimeConnection.for_inprocess()) await client.start() try: diff --git a/python/e2e/testharness/context.py b/python/e2e/testharness/context.py index 0e53605025..679a36e28c 100644 --- a/python/e2e/testharness/context.py +++ b/python/e2e/testharness/context.py @@ -66,6 +66,7 @@ def __init__(self): self._proxy: CapiProxy | None = None self._client: CopilotClient | None = None self._inprocess: bool = is_inprocess_transport() + self._client_inprocess: bool = False self._restore_env: list[tuple[str, str | None]] = [] self._restore_cwd: str | None = None @@ -103,13 +104,11 @@ async def setup(self, cli_args: list[str] | None = None): # (os.environ writes reach native getenv on CPython) and chdir into the # work dir, then create the client without working_directory/env. This # matches the Node/.NET in-process harnesses. - if self._inprocess: + self._client_inprocess = self._inprocess and not cli_args + if self._client_inprocess: self._apply_inprocess_environment() self._client = CopilotClient( - connection=RuntimeConnection.for_inprocess( - path=self.cli_path, - args=tuple(cli_args or []), - ), + connection=RuntimeConnection.for_inprocess(), github_token=DEFAULT_GITHUB_TOKEN, ) else: @@ -137,6 +136,7 @@ def _apply_inprocess_environment(self) -> None: { "GH_TOKEN": DEFAULT_GITHUB_TOKEN, "GITHUB_TOKEN": DEFAULT_GITHUB_TOKEN, + "COPILOT_CLI_PATH": self.cli_path, "COPILOT_HMAC_KEY": "", "CAPI_HMAC_KEY": "", } @@ -156,7 +156,7 @@ def add_runtime_env(self, key: str, value: str) -> None: live on ``os.environ`` (and be restored in teardown). Must be called before the runtime starts (i.e., before the first ``create_session``). """ - if self._inprocess: + if self._client_inprocess: self._restore_env.append((key, os.environ.get(key))) os.environ[key] = value else: @@ -191,7 +191,7 @@ async def teardown(self, test_failed: bool = False): pass # stop() completes all cleanup before raising; safe to ignore in teardown self._client = None - if self._inprocess: + if self._client_inprocess: self._restore_inprocess_environment() if self._proxy: diff --git a/python/test_client.py b/python/test_client.py index 2c3db552a6..9cb00d809f 100644 --- a/python/test_client.py +++ b/python/test_client.py @@ -5,6 +5,7 @@ """ import asyncio +import inspect from datetime import UTC, datetime from unittest.mock import AsyncMock, Mock, patch @@ -42,6 +43,14 @@ from e2e.testharness import CLI_PATH +def test_inprocess_connection_has_no_child_process_options(): + connection = RuntimeConnection.for_inprocess() + + assert list(inspect.signature(RuntimeConnection.for_inprocess).parameters) == [] + assert not hasattr(connection, "path") + assert not hasattr(connection, "args") + + class TestClientShutdown: @pytest.mark.asyncio async def test_stop_requests_runtime_shutdown_for_owned_process(self): diff --git a/rust/README.md b/rust/README.md index 7dda184838..254a2b2167 100644 --- a/rust/README.md +++ b/rust/README.md @@ -776,15 +776,19 @@ none of them are scheduled for removal. ## Embedded CLI -The SDK provisions the Copilot CLI binary at build time. By default the -`bundled-cli` feature embeds only the verified CLI executable in your compiled -crate. Enable `bundled-in-process` to additionally embed the native -runtime library and use `Transport::InProcess`: +The SDK provisions its runtime at build time. By default the `bundled-cli` +feature embeds the verified child-process runtime in your compiled crate. +Enable `bundled-in-process` to additionally embed the native runtime library +and use `Transport::InProcess`: ```toml github-copilot-sdk = { version = "0.1", features = ["bundled-in-process"] } ``` +`CliProgram::Path` and raw `ClientOptions::extra_args` apply only to +child-process transports. Set `COPILOT_CLI_PATH` only when using an externally +provisioned compatible runtime package with in-process transport. + For builds that prefer a smaller artifact, disable the `bundled-cli` feature: ```toml diff --git a/rust/src/lib.rs b/rust/src/lib.rs index 8e5685ea2b..d901ef7a45 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -124,15 +124,14 @@ pub enum Transport { Stdio, /// Host the runtime in-process over FFI (no child process). /// - /// Loads the native runtime library next to the resolved CLI entrypoint - /// and speaks JSON-RPC over its C ABI. The runtime spawns its - /// own worker; the SDK never launches a CLI child process. This is - /// **experimental**. Per-client [`ClientOptions::working_directory`], + /// Loads the native runtime library and speaks JSON-RPC over its C ABI. + /// This is **experimental**. Per-client [`ClientOptions::program`], + /// [`ClientOptions::extra_args`], [`ClientOptions::working_directory`], /// [`ClientOptions::env`]/[`ClientOptions::env_remove`], /// [`ClientOptions::telemetry`], and [`ClientOptions::github_token`] are /// not supported because native runtime code shares the host process. /// [`ClientOptions::base_directory`] remains supported because it is - /// passed to the spawned worker as `COPILOT_HOME`. + /// applied to the native runtime as `COPILOT_HOME`. /// /// Requires the `bundled-in-process` Cargo feature. InProcess, @@ -227,7 +226,7 @@ pub fn install_bundled_cli() -> Option { /// This skips auto-resolution entirely. #[non_exhaustive] pub struct ClientOptions { - /// How to locate the CLI binary. + /// How to locate the child-process runtime. pub program: CliProgram, /// Arguments prepended before `--server` (e.g. the script path for node). pub prefix_args: Vec, @@ -239,7 +238,7 @@ pub struct ClientOptions { pub env: Vec<(OsString, OsString)>, /// Environment variable names to remove from the child process. pub env_remove: Vec, - /// Extra CLI flags appended after the transport-specific arguments. + /// Extra flags for child-process transports. pub extra_args: Vec, /// Transport mode used to communicate with the CLI server. pub transport: Transport, @@ -658,7 +657,7 @@ impl ClientOptions { Self::default() } - /// How to locate the CLI binary. See [`CliProgram`]. + /// How to locate the child-process runtime. See [`CliProgram`]. pub fn with_program(mut self, program: impl Into) -> Self { self.program = program.into(); self @@ -913,6 +912,21 @@ fn resolve_default_transport_value(value: Option<&str>) -> Result { #[cfg(any(feature = "bundled-in-process", test))] fn validate_inprocess_options(options: &ClientOptions) -> Result<()> { + if !matches!(&options.program, CliProgram::Resolve) { + return Err(Error::with_message( + ErrorKind::InvalidConfig, + "ClientOptions::program is not supported with Transport::InProcess; \ + set COPILOT_CLI_PATH only when using an externally provisioned runtime package", + )); + } + if !options.extra_args.is_empty() { + return Err(Error::with_message( + ErrorKind::InvalidConfig, + "ClientOptions::extra_args is not supported with Transport::InProcess; \ + use typed client options instead", + )); + } + let unsupported = if !options.working_directory.as_os_str().is_empty() { Some("working_directory") } else if !options.env.is_empty() { @@ -964,7 +978,7 @@ struct ClientInner { child: parking_lot::Mutex>, #[cfg(feature = "bundled-in-process")] /// In-process FFI runtime host, set only for [`Transport::InProcess`]. - /// Closing it tears down the FFI connection and worker. + /// Closing it tears down the native runtime connection. ffi_host: parking_lot::Mutex>>, rpc: JsonRpcClient, cwd: PathBuf, @@ -1218,7 +1232,7 @@ impl Client { Transport::InProcess => { #[cfg(feature = "bundled-in-process")] { - info!(entrypoint = %program.display(), "hosting copilot runtime in-process (FFI)"); + info!(runtime_path = %program.display(), "hosting copilot runtime in-process (FFI)"); let mut environment = Vec::new(); if let Some(base_directory) = &options.base_directory { let value = base_directory.to_str().ok_or_else(|| { @@ -1243,7 +1257,6 @@ impl Client { if options.use_logged_in_user == Some(false) { args.push("--no-auto-login".to_string()); } - args.extend(options.extra_args.clone()); let host = crate::ffi::FfiHost::create(&program, environment, args)?; let (reader, writer, shared) = host.start().await?; let client = Self::from_transport( @@ -2304,7 +2317,7 @@ impl Client { } } - // The runtime.shutdown RPC above already asked the worker to clean up; + // The runtime.shutdown RPC above already asked the runtime to clean up; // closing here tears down the transport. #[cfg(feature = "bundled-in-process")] { @@ -2519,6 +2532,8 @@ mod tests { ClientOptions::new().with_telemetry(TelemetryConfig::default()), ClientOptions::new().with_github_token("token"), ClientOptions::new().with_prefix_args(["index.js"]), + ClientOptions::new().with_program(CliProgram::Path("copilot".into())), + ClientOptions::new().with_extra_args(["--verbose"]), ]; for options in invalid { @@ -2527,14 +2542,13 @@ mod tests { } #[test] - fn inprocess_allows_worker_and_rpc_options() { + fn inprocess_allows_typed_runtime_options() { let options = ClientOptions::new() .with_base_directory("state") .with_log_level(LogLevel::Debug) .with_session_idle_timeout_seconds(10) .with_use_logged_in_user(false) - .with_enable_remote_sessions(true) - .with_extra_args(["--verbose"]); + .with_enable_remote_sessions(true); assert!(validate_inprocess_options(&options).is_ok()); } @@ -2544,7 +2558,6 @@ mod tests { async fn inprocess_requires_cargo_feature() { let error = Client::start( ClientOptions::new() - .with_program(CliProgram::Path("copilot".into())) .with_transport(Transport::InProcess), ) .await diff --git a/rust/tests/e2e/rpc_mcp_and_skills.rs b/rust/tests/e2e/rpc_mcp_and_skills.rs index bd6298974a..eb8368ebcd 100644 --- a/rust/tests/e2e/rpc_mcp_and_skills.rs +++ b/rust/tests/e2e/rpc_mcp_and_skills.rs @@ -9,7 +9,8 @@ use github_copilot_sdk::rpc::{ McpAppsSetHostContextRequest, McpCancelSamplingExecutionParams, McpDisableRequest, McpEnableRequest, McpExecuteSamplingParams, McpExecuteSamplingRequest, McpOauthLoginRequest, McpResourcesReadRequest, McpSamplingExecutionAction, McpSetEnvValueModeDetails, - McpSetEnvValueModeParams, SkillsDisableRequest, SkillsEnableRequest, + McpSetEnvValueModeParams, PermissionsAllowAllMode, PermissionsSetAllowAllRequest, + SkillsDisableRequest, SkillsEnableRequest, }; use github_copilot_sdk::{IndexMap, McpServerConfig, McpStdioServerConfig}; @@ -339,14 +340,22 @@ async fn should_list_extensions() { with_e2e_context("rpc_mcp_and_skills", "should_list_extensions", |ctx| { Box::pin(async move { ctx.set_default_copilot_user(); - let client = - github_copilot_sdk::Client::start(ctx.client_options().with_extra_args(["--yolo"])) - .await - .expect("start yolo client"); + let client = ctx.start_client().await; let session = client .create_session(ctx.approve_all_session_config()) .await .expect("create session"); + session + .rpc() + .permissions() + .set_allow_all(PermissionsSetAllowAllRequest { + enabled: None, + mode: Some(PermissionsAllowAllMode::On), + model: None, + source: None, + }) + .await + .expect("enable allow-all"); let result = session .rpc() @@ -677,15 +686,22 @@ async fn should_report_error_when_extensions_are_not_available() { |ctx| { Box::pin(async move { ctx.set_default_copilot_user(); - let client = github_copilot_sdk::Client::start( - ctx.client_options().with_extra_args(["--yolo"]), - ) - .await - .expect("start client"); + let client = ctx.start_client().await; let session = client .create_session(ctx.approve_all_session_config()) .await .expect("create session"); + session + .rpc() + .permissions() + .set_allow_all(PermissionsSetAllowAllRequest { + enabled: None, + mode: Some(PermissionsAllowAllMode::On), + model: None, + source: None, + }) + .await + .expect("enable allow-all"); expect_err_contains( session.rpc().extensions().enable(ExtensionsEnableRequest { diff --git a/rust/tests/e2e/support.rs b/rust/tests/e2e/support.rs index 7479baeeba..539734f58f 100644 --- a/rust/tests/e2e/support.rs +++ b/rust/tests/e2e/support.rs @@ -207,7 +207,6 @@ impl E2eContext { pub async fn start_inprocess_client(&self) -> Client { let options = ClientOptions::new() .with_use_logged_in_user(false) - .with_program(CliProgram::Path(self.cli_path.clone())) .with_transport(Transport::InProcess); Client::start(options) .await @@ -627,8 +626,12 @@ impl InProcessEnvGuard { } let mut pairs: Vec<(OsString, OsString)> = ctx.environment(); pairs.push(("COPILOT_SDK_AUTH_TOKEN".into(), "".into())); + pairs.push(( + "COPILOT_CLI_PATH".into(), + ctx.cli_path.clone().into_os_string(), + )); // Some tests opt into gated runtime APIs via per-client `options.env`, which the - // in-process transport does not pass to the shared worker (see issue #1934). + // in-process transport does not pass to the shared native runtime (see issue #1934). // These are process-global runtime gates (not per-client behavior), so applying // them to the host process for the serial in-process suite is equivalent and // inert for tests that don't exercise the gated API. @@ -759,17 +762,8 @@ fn client_options_for_cli( cwd: &Path, env: Vec<(OsString, OsString)>, ) -> ClientOptions { - // When the in-process FFI transport is the default (matrix cell that sets - // COPILOT_SDK_DEFAULT_CONNECTION=inprocess), pass the CLI entrypoint - // directly: the FFI host builds the `node --embedded-host` - // argv itself and loads the sibling runtime cdylib. Splitting a `.js` - // entrypoint into node + prefix_args (the stdio layout) would point the - // library resolver at node's directory instead. - let inprocess_default = std::env::var("COPILOT_SDK_DEFAULT_CONNECTION") - .map(|value| value.eq_ignore_ascii_case("inprocess")) - .unwrap_or(false); - if inprocess_default { - return ClientOptions::new().with_program(CliProgram::Path(cli_path.to_path_buf())); + if is_inprocess_default() { + return ClientOptions::new().with_use_logged_in_user(false); } let options = ClientOptions::new() .with_cwd(cwd) From 0dc1884f52b42210b5855da30ae8a51ba5616bf9 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 14 Jul 2026 11:53:55 +0100 Subject: [PATCH 08/14] Gate Go in-process runtime behind build tag Exclude the native runtime payload and purego implementation from normal Go application builds while enabling them with copilot_inprocess. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 447feac4-35df-4c0b-ab34-00789ad78da7 --- .github/workflows/go-sdk-tests.yml | 1 + go/README.md | 9 +++- go/client.go | 9 ++-- go/client_test.go | 12 +++++ go/cmd/bundler/main.go | 65 ++++++++++++++++-------- go/cmd/bundler/main_test.go | 46 +++++++++++++++++ go/inprocess.go | 10 ++++ go/inprocess_disabled.go | 11 ++++ go/inprocess_enabled.go | 11 ++++ go/internal/ffihost/ffihost.go | 2 + go/internal/ffihost/ffihost_test.go | 2 + go/internal/ffihost/loader_other.go | 2 +- go/internal/ffihost/loader_windows.go | 2 +- go/internal/ffihost/sigonstack_darwin.go | 2 +- go/internal/ffihost/sigonstack_linux.go | 2 +- go/internal/ffihost/sigonstack_other.go | 2 +- go/types.go | 3 +- 17 files changed, 161 insertions(+), 30 deletions(-) create mode 100644 go/cmd/bundler/main_test.go create mode 100644 go/inprocess.go create mode 100644 go/inprocess_disabled.go create mode 100644 go/inprocess_enabled.go diff --git a/.github/workflows/go-sdk-tests.yml b/.github/workflows/go-sdk-tests.yml index c7ce3dbee9..365e60373f 100644 --- a/.github/workflows/go-sdk-tests.yml +++ b/.github/workflows/go-sdk-tests.yml @@ -83,6 +83,7 @@ jobs: if: matrix.transport == 'inprocess' run: | echo "COPILOT_SDK_DEFAULT_CONNECTION=inprocess" >> "$GITHUB_ENV" + echo "GOFLAGS=-tags=copilot_inprocess" >> "$GITHUB_ENV" - name: Run Go SDK tests env: diff --git a/go/README.md b/go/README.md index 0a266ab534..a597f7899a 100644 --- a/go/README.md +++ b/go/README.md @@ -101,7 +101,7 @@ Follow these steps to embed the CLI: That's it! When your application calls `copilot.NewClient` without a `Connection` field (or with an empty `StdioConnection{}`) and no `COPILOT_CLI_PATH` environment variable, the SDK will automatically install the embedded CLI to a cache directory and use it for all operations. -The bundler also embeds the native runtime library required by the [in-process transport](#in-process-transport-experimental), so it works out of the box for embedded builds. +The bundler prepares the native runtime library required by the [in-process transport](#in-process-transport-experimental). It is included in the application only when building with the `copilot_inprocess` build tag. ## In-process transport (Experimental) @@ -109,6 +109,12 @@ The bundler also embeds the native runtime library required by the [in-process t By default the SDK starts the runtime as a child process and talks JSON-RPC over stdio or TCP. The **in-process** transport instead loads a native runtime library directly into your process. +Build your application with the `copilot_inprocess` build tag: + +```sh +go build -tags copilot_inprocess +``` + ```go client := copilot.NewClient(&copilot.ClientOptions{ Connection: copilot.InProcessConnection{}, @@ -121,6 +127,7 @@ defer client.Stop() Resolution and requirements: +- The application must be built with the `copilot_inprocess` build tag. - Set `COPILOT_SDK_DEFAULT_CONNECTION=inprocess` to select the in-process transport when `ClientOptions.Connection` is nil. An explicit connection always takes precedence. diff --git a/go/client.go b/go/client.go index 2cea020025..979f8a0ca9 100644 --- a/go/client.go +++ b/go/client.go @@ -48,7 +48,6 @@ import ( "github.com/google/uuid" "github.com/github/copilot-sdk/go/internal/embeddedcli" - "github.com/github/copilot-sdk/go/internal/ffihost" "github.com/github/copilot-sdk/go/internal/jsonrpc2" "github.com/github/copilot-sdk/go/internal/truncbuffer" "github.com/github/copilot-sdk/go/rpc" @@ -157,7 +156,7 @@ type Client struct { conn net.Conn // stores net.Conn for external TCP connections useStdio bool // resolved value from options useInProcess bool // true for InProcessConnection (FFI transport) - ffiHost *ffihost.Host + ffiHost inProcessHost // resolved process options for the spawned runtime (zero values for URIConnection) cliPath string cliArgs []string @@ -2062,6 +2061,10 @@ func (c *Client) startCLIServer(ctx context.Context) error { // startInProcess loads the native runtime library and wires the JSON-RPC client // to its FFI byte streams. func (c *Client) startInProcess(ctx context.Context) error { + if !inProcessAvailable { + return errors.New("in-process transport unavailable: rebuild with -tags copilot_inprocess on a supported platform") + } + runtimePath := c.cliPath if runtimePath == "" { // The in-process transport does not resolve a bare command name from PATH @@ -2077,7 +2080,7 @@ func (c *Client) startInProcess(ctx context.Context) error { return errors.New("in-process runtime unavailable: set COPILOT_CLI_PATH to a compatible runtime package or build with the bundled embedded runtime") } - host, err := ffihost.Create(runtimePath, nil) + host, err := createInProcessHost(runtimePath) if err != nil { return err } diff --git a/go/client_test.go b/go/client_test.go index 0d27b77415..c995cc9bde 100644 --- a/go/client_test.go +++ b/go/client_test.go @@ -626,6 +626,18 @@ func TestClient_EnvOptions(t *testing.T) { } func TestClient_InProcessConnection(t *testing.T) { + t.Run("requires build tag", func(t *testing.T) { + if inProcessAvailable { + t.Skip("in-process transport is enabled") + } + + client := NewClient(&ClientOptions{Connection: InProcessConnection{}}) + err := client.Start(context.Background()) + if err == nil || !strings.Contains(err.Error(), "-tags copilot_inprocess") { + t.Fatalf("Expected build-tag error, got %v", err) + } + }) + t.Run("uses in-process transport", func(t *testing.T) { client := NewClient(&ClientOptions{Connection: InProcessConnection{}}) if !client.useInProcess { diff --git a/go/cmd/bundler/main.go b/go/cmd/bundler/main.go index 5b42e25771..069092a1cf 100644 --- a/go/cmd/bundler/main.go +++ b/go/cmd/bundler/main.go @@ -362,24 +362,54 @@ func runtimeLibExt(goos string) string { } } -// generateGoFile creates a Go source file that embeds the binary and metadata. -// When runtimeArtifactPath is non-empty, it also embeds the native in-process -// runtime library and wires it into the embeddedcli configuration. +// generateGoFile creates separate source files for normal and in-process builds. +// Both embed the CLI, while only the copilot_inprocess-tagged file embeds the +// native runtime library. func generateGoFile(goos, goarch, binaryPath, cliVersion string, sha256Hash []byte, runtimeArtifactPath string, runtimeHash []byte, pkgName string) error { - // Generate Go file path: zcopilot_linux_amd64.go (without version) binaryName := filepath.Base(binaryPath) licenseName := licenseFileName(binaryName) - goFileName := fmt.Sprintf("zcopilot_%s_%s.go", goos, goarch) - goFilePath := filepath.Join(filepath.Dir(binaryPath), goFileName) hashBase64 := "" if len(sha256Hash) > 0 { hashBase64 = base64.StdEncoding.EncodeToString(sha256Hash) } - // Build the optional runtime-library embed fragments. When the platform - // package does not ship the runtime, these stay empty and the generated file - // omits the RuntimeLib config (in-process transport is then unavailable for - // that platform bundle, mirroring the other SDKs' conditional native embed). + outputDir := filepath.Dir(binaryPath) + defaultPath := filepath.Join(outputDir, fmt.Sprintf("zcopilot_%s_%s.go", goos, goarch)) + defaultContent := generatedGoFileContent( + "!copilot_inprocess", + pkgName, + binaryName, + licenseName, + cliVersion, + hashBase64, + "", + nil, + ) + if err := os.WriteFile(defaultPath, []byte(defaultContent), 0644); err != nil { + return err + } + + inProcessPath := filepath.Join(outputDir, fmt.Sprintf("zcopilot_inprocess_%s_%s.go", goos, goarch)) + inProcessContent := generatedGoFileContent( + "copilot_inprocess", + pkgName, + binaryName, + licenseName, + cliVersion, + hashBase64, + runtimeArtifactPath, + runtimeHash, + ) + if err := os.WriteFile(inProcessPath, []byte(inProcessContent), 0644); err != nil { + return err + } + + fmt.Printf("Generated %s\n", defaultPath) + fmt.Printf("Generated %s\n", inProcessPath) + return nil +} + +func generatedGoFileContent(buildConstraint, pkgName, binaryName, licenseName, cliVersion, hashBase64, runtimeArtifactPath string, runtimeHash []byte) string { runtimeEmbed := "" runtimeConfig := "" runtimeReader := "" @@ -404,15 +434,17 @@ func runtimeLibReader() io.Reader { ` } - content := fmt.Sprintf(`// Code generated by copilot-sdk bundler; DO NOT EDIT. + return fmt.Sprintf(`//go:build %s + +// Code generated by copilot-sdk bundler; DO NOT EDIT. package %s import ( "bytes" - "io" "encoding/base64" _ "embed" + "io" "github.com/github/copilot-sdk/go/embeddedcli" "github.com/klauspost/compress/zstd" @@ -449,14 +481,7 @@ func mustDecodeBase64(s string) []byte { } return b } -`, pkgName, binaryName, licenseName, runtimeEmbed, cliVersion, hashBase64, runtimeConfig, runtimeReader) - - if err := os.WriteFile(goFilePath, []byte(content), 0644); err != nil { - return err - } - - fmt.Printf("Generated %s\n", goFilePath) - return nil +`, buildConstraint, pkgName, binaryName, licenseName, runtimeEmbed, cliVersion, hashBase64, runtimeConfig, runtimeReader) } // downloadCLIBinary downloads the npm tarball and extracts the CLI binary. It diff --git a/go/cmd/bundler/main_test.go b/go/cmd/bundler/main_test.go new file mode 100644 index 0000000000..1b5cf6de2f --- /dev/null +++ b/go/cmd/bundler/main_test.go @@ -0,0 +1,46 @@ +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestGenerateGoFileGatesRuntimeEmbed(t *testing.T) { + dir := t.TempDir() + binaryPath := filepath.Join(dir, "copilot.zst") + runtimePath := filepath.Join(dir, "runtime.node.zst") + for _, path := range []string{binaryPath, licensePathForOutput(binaryPath), runtimePath} { + if err := os.WriteFile(path, []byte("test"), 0644); err != nil { + t.Fatal(err) + } + } + + hash := make([]byte, 32) + if err := generateGoFile("linux", "amd64", binaryPath, "1.2.3", hash, runtimePath, hash, "main"); err != nil { + t.Fatal(err) + } + + defaultSource, err := os.ReadFile(filepath.Join(dir, "zcopilot_linux_amd64.go")) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(defaultSource), "//go:build !copilot_inprocess") { + t.Fatal("default embed file does not exclude copilot_inprocess builds") + } + if strings.Contains(string(defaultSource), "localEmbeddedCopilotRuntimeLib") { + t.Fatal("default embed file includes the native runtime") + } + + inProcessSource, err := os.ReadFile(filepath.Join(dir, "zcopilot_inprocess_linux_amd64.go")) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(inProcessSource), "//go:build copilot_inprocess") { + t.Fatal("in-process embed file does not require the copilot_inprocess tag") + } + if !strings.Contains(string(inProcessSource), "localEmbeddedCopilotRuntimeLib") { + t.Fatal("in-process embed file does not include the native runtime") + } +} diff --git a/go/inprocess.go b/go/inprocess.go new file mode 100644 index 0000000000..1d23bc94c1 --- /dev/null +++ b/go/inprocess.go @@ -0,0 +1,10 @@ +package copilot + +import "io" + +type inProcessHost interface { + Start() error + Writer() io.WriteCloser + Reader() io.ReadCloser + Dispose() +} diff --git a/go/inprocess_disabled.go b/go/inprocess_disabled.go new file mode 100644 index 0000000000..b312c44a59 --- /dev/null +++ b/go/inprocess_disabled.go @@ -0,0 +1,11 @@ +//go:build !copilot_inprocess || (!darwin && !linux && !windows) + +package copilot + +import "errors" + +const inProcessAvailable = false + +func createInProcessHost(string) (inProcessHost, error) { + return nil, errors.New("in-process transport unavailable") +} diff --git a/go/inprocess_enabled.go b/go/inprocess_enabled.go new file mode 100644 index 0000000000..3dc44dc8a1 --- /dev/null +++ b/go/inprocess_enabled.go @@ -0,0 +1,11 @@ +//go:build copilot_inprocess && (darwin || linux || windows) + +package copilot + +import "github.com/github/copilot-sdk/go/internal/ffihost" + +const inProcessAvailable = true + +func createInProcessHost(runtimePath string) (inProcessHost, error) { + return ffihost.Create(runtimePath, nil) +} diff --git a/go/internal/ffihost/ffihost.go b/go/internal/ffihost/ffihost.go index 2321146286..d4cd8d9e86 100644 --- a/go/internal/ffihost/ffihost.go +++ b/go/internal/ffihost/ffihost.go @@ -1,3 +1,5 @@ +//go:build copilot_inprocess && (darwin || linux || windows) + // Package ffihost hosts the Copilot runtime in-process by loading the native // runtime library (runtime.node — a Rust cdylib) and driving JSON-RPC over its // C ABI (FFI) instead of spawning the CLI as a child process and talking over diff --git a/go/internal/ffihost/ffihost_test.go b/go/internal/ffihost/ffihost_test.go index b3020ddd8c..8440eab7bb 100644 --- a/go/internal/ffihost/ffihost_test.go +++ b/go/internal/ffihost/ffihost_test.go @@ -1,3 +1,5 @@ +//go:build copilot_inprocess && (darwin || linux || windows) + package ffihost import "testing" diff --git a/go/internal/ffihost/loader_other.go b/go/internal/ffihost/loader_other.go index f0ca809d83..0b7bcc40b9 100644 --- a/go/internal/ffihost/loader_other.go +++ b/go/internal/ffihost/loader_other.go @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -//go:build !windows +//go:build copilot_inprocess && (darwin || linux) package ffihost diff --git a/go/internal/ffihost/loader_windows.go b/go/internal/ffihost/loader_windows.go index 701f4f611f..4ac2789f98 100644 --- a/go/internal/ffihost/loader_windows.go +++ b/go/internal/ffihost/loader_windows.go @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -//go:build windows +//go:build copilot_inprocess && windows package ffihost diff --git a/go/internal/ffihost/sigonstack_darwin.go b/go/internal/ffihost/sigonstack_darwin.go index 61b1e751ae..2e7d2a99b7 100644 --- a/go/internal/ffihost/sigonstack_darwin.go +++ b/go/internal/ffihost/sigonstack_darwin.go @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -//go:build darwin +//go:build copilot_inprocess && darwin package ffihost diff --git a/go/internal/ffihost/sigonstack_linux.go b/go/internal/ffihost/sigonstack_linux.go index d8357a30b4..a5567f152b 100644 --- a/go/internal/ffihost/sigonstack_linux.go +++ b/go/internal/ffihost/sigonstack_linux.go @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -//go:build linux +//go:build copilot_inprocess && linux package ffihost diff --git a/go/internal/ffihost/sigonstack_other.go b/go/internal/ffihost/sigonstack_other.go index 273837eb9b..9da78255f6 100644 --- a/go/internal/ffihost/sigonstack_other.go +++ b/go/internal/ffihost/sigonstack_other.go @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -//go:build !darwin && !linux +//go:build copilot_inprocess && windows package ffihost diff --git a/go/types.go b/go/types.go index d434ad47e1..c9dd71d19b 100644 --- a/go/types.go +++ b/go/types.go @@ -105,7 +105,8 @@ func (URIConnection) runtimeConnection() {} // [TCPConnection]). // // Experimental: the in-process transport is experimental and its API and -// behavior may change in a future release. +// behavior may change in a future release. Build the application with the +// copilot_inprocess build tag to enable this transport. type InProcessConnection struct { } From 27402a08c69c992374a0aa1a14e6ae196c6f7a27 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 14 Jul 2026 12:13:59 +0100 Subject: [PATCH 09/14] Fix in-process CI failures Use the Linux rt_sigaction ABI for SA_ONSTACK repair, keep Go plugin test state alive through runtime shutdown, remove HMAC signing keys from Rust in-process tests, and apply the required Rust formatting. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 447feac4-35df-4c0b-ab34-00789ad78da7 --- .../e2e/rpc_server_plugins_e2e_test.go | 5 +- go/internal/ffihost/sigonstack_linux.go | 112 +++++++----------- go/internal/ffihost/sigonstack_linux_test.go | 38 ++++++ rust/src/lib.rs | 9 +- rust/tests/e2e/support.rs | 12 +- 5 files changed, 99 insertions(+), 77 deletions(-) create mode 100644 go/internal/ffihost/sigonstack_linux_test.go diff --git a/go/internal/e2e/rpc_server_plugins_e2e_test.go b/go/internal/e2e/rpc_server_plugins_e2e_test.go index 9dbd17a672..1e24a769f0 100644 --- a/go/internal/e2e/rpc_server_plugins_e2e_test.go +++ b/go/internal/e2e/rpc_server_plugins_e2e_test.go @@ -311,7 +311,10 @@ func newStartedPortedClient(t *testing.T, ctx *testharness.TestContext, opts ... func newStartedIsolatedPortedClient(t *testing.T, ctx *testharness.TestContext) *copilot.Client { t.Helper() - home := t.TempDir() + home, err := os.MkdirTemp(ctx.WorkDir, "plugin-home-") + if err != nil { + t.Fatalf("Failed to create isolated plugin home: %v", err) + } return newStartedPortedClient(t, ctx, func(opts *copilot.ClientOptions) { opts.Env = append(opts.Env, "COPILOT_HOME="+home, diff --git a/go/internal/ffihost/sigonstack_linux.go b/go/internal/ffihost/sigonstack_linux.go index a5567f152b..668cf67055 100644 --- a/go/internal/ffihost/sigonstack_linux.go +++ b/go/internal/ffihost/sigonstack_linux.go @@ -5,35 +5,25 @@ package ffihost import ( - "encoding/binary" + "syscall" "unsafe" - - "github.com/ebitengine/purego" ) -// Linux `struct sigaction` layout for glibc and musl on amd64/arm64 -// (little-endian): -// -// offset 0: sa_handler/sa_sigaction (8 bytes, pointer) -// offset 8: sigset_t sa_mask (128 bytes) -// offset 136: int sa_flags (4 bytes) -// offset 144: sa_restorer (8 bytes) -// -// Both glibc and musl use a 128-byte sigset_t here, so sa_flags is at offset 136 -// and the whole struct is 152 bytes. We over-allocate slightly for safety. const ( - linuxSigactionSize = 160 - linuxFlagsOffset = 136 - linuxSaOnStack = 0x08000000 // SA_ONSTACK on Linux - linuxSigDfl = 0 // SIG_DFL - linuxSigIgn = 1 // SIG_IGN - linuxMaxSignal = 31 // standard signals; covers SIGCHLD (17) + linuxSaOnStack = 0x08000000 + linuxSigDfl = 0 + linuxSigIgn = 1 + linuxMaxSignal = 31 ) -// libcCandidates are dlopen names to try when resolving sigaction fails through -// the already-loaded runtime library handle (e.g. when its libc symbols are not -// reachable via that handle). Covers glibc and musl. -var libcCandidates = []string{"libc.so.6", "libc.so", "libc.musl-x86_64.so.1", "libc.musl-aarch64.so.1"} +// linuxSigaction matches the kernel rt_sigaction ABI used by the Go runtime on +// Linux amd64 and arm64. +type linuxSigaction struct { + handler uintptr + flags uint64 + restorer uintptr + mask uint64 +} // rearmForeignSignalHandlers re-adds the SA_ONSTACK flag to any signal handler // installed by the native runtime (libnode/libuv, loaded via dlopen) that @@ -48,61 +38,45 @@ var libcCandidates = []string{"libc.so.6", "libc.so", "libc.musl-x86_64.so.1", " // SIG_DFL/SIG_IGN and Go's own handlers (which already carry SA_ONSTACK) are // untouched. Best-effort: any failure is silently ignored, since the worst case // is the pre-existing crash. -// -// runtimeHandle is the dlopen handle of the native runtime; sigaction is -// resolved through it (dlsym searches the library's libc dependency) so we avoid -// guessing the libc path, with an explicit libc dlopen as a fallback. -func rearmForeignSignalHandlers(runtimeHandle uintptr) { - var sigaction func(sig int32, act, oact unsafe.Pointer) int32 - if !resolveLinuxSigaction(runtimeHandle, &sigaction) { - return - } - - for sig := int32(1); sig <= linuxMaxSignal; sig++ { - var cur [linuxSigactionSize]byte - if sigaction(sig, nil, unsafe.Pointer(&cur[0])) != 0 { +func rearmForeignSignalHandlers(_ uintptr) { + for sig := 1; sig <= linuxMaxSignal; sig++ { + var action linuxSigaction + if !linuxGetSigaction(sig, &action) { continue } - handler := binary.LittleEndian.Uint64(cur[0:8]) - if handler == linuxSigDfl || handler == linuxSigIgn { + if action.handler == linuxSigDfl || action.handler == linuxSigIgn { continue } - flags := binary.LittleEndian.Uint32(cur[linuxFlagsOffset : linuxFlagsOffset+4]) - if flags&linuxSaOnStack != 0 { + if action.flags&linuxSaOnStack != 0 { continue } - binary.LittleEndian.PutUint32(cur[linuxFlagsOffset:linuxFlagsOffset+4], flags|linuxSaOnStack) - sigaction(sig, unsafe.Pointer(&cur[0]), nil) + action.flags |= linuxSaOnStack + linuxSetSigaction(sig, &action) } } -// resolveLinuxSigaction binds libc's sigaction into fn, first via the runtime -// library handle (whose libc dependency exports it) and then via an explicit -// libc dlopen. Returns false if no candidate resolves the symbol. -func resolveLinuxSigaction(runtimeHandle uintptr, fn *func(sig int32, act, oact unsafe.Pointer) int32) bool { - if runtimeHandle != 0 && bindLinuxSigaction(runtimeHandle, fn) { - return true - } - for _, name := range libcCandidates { - handle, err := purego.Dlopen(name, purego.RTLD_NOW|purego.RTLD_GLOBAL) - if err != nil || handle == 0 { - continue - } - if bindLinuxSigaction(handle, fn) { - return true - } - } - return false +func linuxGetSigaction(signal int, action *linuxSigaction) bool { + _, _, errno := syscall.RawSyscall6( + syscall.SYS_RT_SIGACTION, + uintptr(signal), + 0, + uintptr(unsafe.Pointer(action)), + unsafe.Sizeof(action.mask), + 0, + 0, + ) + return errno == 0 } -// bindLinuxSigaction resolves sigaction from handle into fn, converting the -// panic RegisterLibFunc raises on a missing symbol into a false return. -func bindLinuxSigaction(handle uintptr, fn *func(sig int32, act, oact unsafe.Pointer) int32) (ok bool) { - defer func() { - if recover() != nil { - ok = false - } - }() - purego.RegisterLibFunc(fn, handle, "sigaction") - return true +func linuxSetSigaction(signal int, action *linuxSigaction) bool { + _, _, errno := syscall.RawSyscall6( + syscall.SYS_RT_SIGACTION, + uintptr(signal), + uintptr(unsafe.Pointer(action)), + 0, + unsafe.Sizeof(action.mask), + 0, + 0, + ) + return errno == 0 } diff --git a/go/internal/ffihost/sigonstack_linux_test.go b/go/internal/ffihost/sigonstack_linux_test.go new file mode 100644 index 0000000000..3a382385f2 --- /dev/null +++ b/go/internal/ffihost/sigonstack_linux_test.go @@ -0,0 +1,38 @@ +//go:build copilot_inprocess && linux + +package ffihost + +import ( + "os" + "os/signal" + "syscall" + "testing" +) + +func TestRearmForeignSignalHandlersAddsOnStack(t *testing.T) { + signals := make(chan os.Signal, 1) + signal.Notify(signals, syscall.SIGUSR1) + defer signal.Stop(signals) + + var original linuxSigaction + if !linuxGetSigaction(int(syscall.SIGUSR1), &original) { + t.Fatal("failed to read SIGUSR1 action") + } + defer linuxSetSigaction(int(syscall.SIGUSR1), &original) + + withoutOnStack := original + withoutOnStack.flags &^= linuxSaOnStack + if !linuxSetSigaction(int(syscall.SIGUSR1), &withoutOnStack) { + t.Fatal("failed to clear SA_ONSTACK") + } + + rearmForeignSignalHandlers(0) + + var rearmed linuxSigaction + if !linuxGetSigaction(int(syscall.SIGUSR1), &rearmed) { + t.Fatal("failed to read rearmed SIGUSR1 action") + } + if rearmed.flags&linuxSaOnStack == 0 { + t.Fatal("SA_ONSTACK was not restored") + } +} diff --git a/rust/src/lib.rs b/rust/src/lib.rs index d901ef7a45..389d66a73e 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -2556,12 +2556,9 @@ mod tests { #[cfg(not(feature = "bundled-in-process"))] #[tokio::test] async fn inprocess_requires_cargo_feature() { - let error = Client::start( - ClientOptions::new() - .with_transport(Transport::InProcess), - ) - .await - .unwrap_err(); + let error = Client::start(ClientOptions::new().with_transport(Transport::InProcess)) + .await + .unwrap_err(); assert!(error.to_string().contains("bundled-in-process")); } diff --git a/rust/tests/e2e/support.rs b/rust/tests/e2e/support.rs index 539734f58f..1cc87ccb4c 100644 --- a/rust/tests/e2e/support.rs +++ b/rust/tests/e2e/support.rs @@ -1,4 +1,4 @@ -use std::ffi::OsString; +use std::ffi::{OsStr, OsString}; use std::future::Future; use std::io::{BufRead, BufReader, Read, Write}; use std::net::TcpStream; @@ -625,6 +625,10 @@ impl InProcessEnvGuard { return None; } let mut pairs: Vec<(OsString, OsString)> = ctx.environment(); + pairs.retain(|(key, _)| { + key.as_os_str() != OsStr::new("COPILOT_HMAC_KEY") + && key.as_os_str() != OsStr::new("CAPI_HMAC_KEY") + }); pairs.push(("COPILOT_SDK_AUTH_TOKEN".into(), "".into())); pairs.push(( "COPILOT_CLI_PATH".into(), @@ -652,6 +656,12 @@ impl InProcessEnvGuard { // other thread races these process-wide env mutations. unsafe { std::env::set_var(key, value) }; } + for key in ["COPILOT_HMAC_KEY", "CAPI_HMAC_KEY"] { + let key = OsString::from(key); + saved.push((key.clone(), std::env::var_os(&key))); + // SAFETY: as above, the in-process suite is serialized. + unsafe { std::env::remove_var(key) }; + } let previous_cwd = std::env::current_dir().expect("read in-process test cwd"); std::env::set_current_dir(ctx.work_dir()).expect("set in-process test cwd"); Some(Self { From bf0d264e88753a02c3797a70be0f0a70ade5a7d1 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 14 Jul 2026 12:16:21 +0100 Subject: [PATCH 10/14] Exclude FFI buffer from untagged lint Apply the in-process build constraint to the private receive buffer so ordinary builds do not lint implementation code whose host is excluded. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 447feac4-35df-4c0b-ab34-00789ad78da7 --- go/internal/ffihost/buffer.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/go/internal/ffihost/buffer.go b/go/internal/ffihost/buffer.go index 48774de5c1..2185ce833d 100644 --- a/go/internal/ffihost/buffer.go +++ b/go/internal/ffihost/buffer.go @@ -1,3 +1,5 @@ +//go:build copilot_inprocess && (darwin || linux || windows) + package ffihost import ( From 0f4aad497a5847f26d9d3afb1661ccf14c7d35fc Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 14 Jul 2026 12:22:04 +0100 Subject: [PATCH 11/14] Allow ambient auth in Rust in-process tests Do not pass --no-auto-login for the in-process harness defaults, so GH_TOKEN and GITHUB_TOKEN can authenticate against the replay proxy. Tests that require unauthenticated behavior still opt out explicitly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 447feac4-35df-4c0b-ab34-00789ad78da7 --- rust/tests/e2e/support.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/rust/tests/e2e/support.rs b/rust/tests/e2e/support.rs index 1cc87ccb4c..3ef849a361 100644 --- a/rust/tests/e2e/support.rs +++ b/rust/tests/e2e/support.rs @@ -205,9 +205,7 @@ impl E2eContext { /// runtime cdylib), so a `.js` entrypoint is not split into node + /// prefix_args here. pub async fn start_inprocess_client(&self) -> Client { - let options = ClientOptions::new() - .with_use_logged_in_user(false) - .with_transport(Transport::InProcess); + let options = ClientOptions::new().with_transport(Transport::InProcess); Client::start(options) .await .expect("start in-process FFI E2E client") @@ -773,7 +771,7 @@ fn client_options_for_cli( env: Vec<(OsString, OsString)>, ) -> ClientOptions { if is_inprocess_default() { - return ClientOptions::new().with_use_logged_in_user(false); + return ClientOptions::new(); } let options = ClientOptions::new() .with_cwd(cwd) From 83584ca660c5c00b6b640062cadd867096f4d5c2 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 14 Jul 2026 13:21:14 +0100 Subject: [PATCH 12/14] Fix cross-language in-process FFI parity Forward typed runtime options, make Go startup cancellation safe, select musl runtime packages, and correct the Go replay harness working directory. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 447feac4-35df-4c0b-ab34-00789ad78da7 --- dotnet/src/Client.cs | 67 +++++++-- dotnet/src/FfiRuntimeHost.cs | 16 ++- dotnet/src/build/GitHub.Copilot.SDK.targets | 5 +- go/README.md | 1 + go/client.go | 52 ++++++- go/client_test.go | 33 +++++ go/cmd/bundler/main.go | 128 ++++++++++++++++-- go/cmd/bundler/main_test.go | 39 +++++- go/embeddedcli/installer.go | 3 +- go/inprocess.go | 5 + go/inprocess_disabled.go | 2 +- go/inprocess_enabled.go | 4 +- go/internal/e2e/testharness/proxy.go | 2 +- go/internal/embeddedcli/embeddedcli.go | 41 ++++++ go/internal/embeddedcli/embeddedcli_test.go | 26 ++++ go/internal/ffihost/ffihost.go | 61 +++++---- go/internal/ffihost/ffihost_test.go | 80 ++++++++++- nodejs/src/client.ts | 73 ++++++++-- nodejs/src/ffiRuntimeHost.ts | 13 +- python/copilot/client.py | 39 +++++- rust/build/in_process.rs | 23 +++- .../snapshot-bundled-in-process-version.sh | 2 + rust/src/lib.rs | 29 ++-- rust/tests/e2e/support.rs | 13 +- 24 files changed, 642 insertions(+), 115 deletions(-) diff --git a/dotnet/src/Client.cs b/dotnet/src/Client.cs index 5607a34b28..97e4f1094d 100644 --- a/dotnet/src/Client.cs +++ b/dotnet/src/Client.cs @@ -349,16 +349,49 @@ async Task StartCoreAsync(CancellationToken ct) { if (_connection is InProcessRuntimeConnection) { - // In-process FFI hosting: load the Rust cdylib and let it spawn - // the CLI worker, instead of the SDK launching a CLI child process. - // The worker reads its configuration (telemetry export, etc.) from - // the environment passed here, so apply the same telemetry-derived - // vars the child-process path sets on its startInfo.Environment. - var ffiEnvironment = _options.Environment?.ToDictionary(kvp => kvp.Key, kvp => (string?)kvp.Value) - ?? new Dictionary(); - ApplyTelemetryEnvironment(ffiEnvironment, _options.Telemetry); - var resolvedFfiEnvironment = ffiEnvironment.ToDictionary(kvp => kvp.Key, kvp => kvp.Value!); - var ffiHost = FfiRuntimeHost.Create(ResolveCliPathForFfi(), GetNapiPrebuildsFolderOrThrow(), resolvedFfiEnvironment, _logger); + var ffiEnvironment = new Dictionary(); + if (!string.IsNullOrEmpty(_options.GitHubToken)) + { + ffiEnvironment["COPILOT_SDK_AUTH_TOKEN"] = _options.GitHubToken!; + } + if (!string.IsNullOrEmpty(_options.BaseDirectory)) + { + ffiEnvironment["COPILOT_HOME"] = _options.BaseDirectory!; + } + if (_options.Mode == CopilotClientMode.Empty) + { + ffiEnvironment["COPILOT_DISABLE_KEYTAR"] = "1"; + } + + var ffiArgs = new List(); + if (_options.LogLevel is { } logLevel && !string.IsNullOrEmpty(logLevel.Value)) + { + ffiArgs.AddRange(["--log-level", logLevel.Value]); + } + if (!string.IsNullOrEmpty(_options.GitHubToken)) + { + ffiArgs.AddRange(["--auth-token-env", "COPILOT_SDK_AUTH_TOKEN"]); + } + var useLoggedInUser = _options.UseLoggedInUser ?? string.IsNullOrEmpty(_options.GitHubToken); + if (!useLoggedInUser) + { + ffiArgs.Add("--no-auto-login"); + } + if (_options.SessionIdleTimeoutSeconds is > 0) + { + ffiArgs.AddRange(["--session-idle-timeout", _options.SessionIdleTimeoutSeconds.Value.ToString(CultureInfo.InvariantCulture)]); + } + if (_options.EnableRemoteSessions) + { + ffiArgs.Add("--remote"); + } + + var ffiHost = FfiRuntimeHost.Create( + ResolveCliPathForFfi(), + GetNapiPrebuildsFolderOrThrow(), + ffiEnvironment, + ffiArgs, + _logger); _ffiHost = ffiHost; await ffiHost.StartAsync(ct); connection = await ConnectToServerAsync(null, null, null, null, ct, ffiHost); @@ -2214,7 +2247,12 @@ private static void ApplyTelemetryEnvironment(IDictionary envir { string os; if (OperatingSystem.IsWindows()) os = "win"; - else if (OperatingSystem.IsLinux()) os = "linux"; + else if (OperatingSystem.IsLinux()) + { + os = RuntimeInformation.RuntimeIdentifier.StartsWith("linux-musl-", StringComparison.Ordinal) + ? "linux-musl" + : "linux"; + } else if (OperatingSystem.IsMacOS()) os = "osx"; else return null; @@ -2261,7 +2299,12 @@ private string ResolveCliPathForFfi() { string platform; if (OperatingSystem.IsWindows()) platform = "win32"; - else if (OperatingSystem.IsLinux()) platform = "linux"; + else if (OperatingSystem.IsLinux()) + { + platform = RuntimeInformation.RuntimeIdentifier.StartsWith("linux-musl-", StringComparison.Ordinal) + ? "linuxmusl" + : "linux"; + } else if (OperatingSystem.IsMacOS()) platform = "darwin"; else return null; diff --git a/dotnet/src/FfiRuntimeHost.cs b/dotnet/src/FfiRuntimeHost.cs index ee838ad276..a838b9fd17 100644 --- a/dotnet/src/FfiRuntimeHost.cs +++ b/dotnet/src/FfiRuntimeHost.cs @@ -44,6 +44,7 @@ internal sealed partial class FfiRuntimeHost : IDisposable private readonly string _cliEntrypoint; private readonly string _libraryPath; private readonly IReadOnlyDictionary? _environment; + private readonly IReadOnlyList _args; private readonly CallbackReceiveStream _receiveStream = new(); private CallbackSendStream? _sendStream; @@ -52,11 +53,12 @@ internal sealed partial class FfiRuntimeHost : IDisposable private uint _connectionId; private bool _disposed; - private FfiRuntimeHost(string libraryPath, string cliEntrypoint, IReadOnlyDictionary? environment, ILogger logger) + private FfiRuntimeHost(string libraryPath, string cliEntrypoint, IReadOnlyDictionary? environment, IReadOnlyList args, ILogger logger) { _libraryPath = libraryPath; _cliEntrypoint = cliEntrypoint; _environment = environment; + _args = args; _logger = logger; } @@ -79,7 +81,7 @@ private FfiRuntimeHost(string libraryPath, string cliEntrypoint, IReadOnlyDictio /// is the napi-rs /// <node-platform>-<arch> folder name (e.g. win32-x64). /// - public static FfiRuntimeHost Create(string cliEntrypoint, string prebuildsFolder, IReadOnlyDictionary? environment, ILogger logger) + public static FfiRuntimeHost Create(string cliEntrypoint, string prebuildsFolder, IReadOnlyDictionary? environment, IReadOnlyList args, ILogger logger) { var fullEntrypoint = Path.GetFullPath(cliEntrypoint); var distDir = Path.GetDirectoryName(fullEntrypoint) @@ -96,7 +98,7 @@ public static FfiRuntimeHost Create(string cliEntrypoint, string prebuildsFolder $"FFI runtime library not found. Looked for '{flatLibraryPath}' and '{prebuildsLibraryPath}'."); PrepareNativeLibrary(libraryPath); - return new FfiRuntimeHost(libraryPath, fullEntrypoint, environment, logger); + return new FfiRuntimeHost(libraryPath, fullEntrypoint, environment, args, logger); } /// @@ -122,7 +124,7 @@ public async Task StartAsync(CancellationToken cancellationToken) // perform the blocking FFI handshake on a background thread. await Task.Run(() => { - var argvJson = BuildArgvJson(_cliEntrypoint); + var argvJson = BuildArgvJson(_cliEntrypoint, _args); var envJson = BuildEnvJson(_environment); _serverId = NativeHostStart(argvJson, envJson); @@ -152,7 +154,7 @@ await Task.Run(() => } } - private static byte[] BuildArgvJson(string cliEntrypoint) + private static byte[] BuildArgvJson(string cliEntrypoint, IReadOnlyList args) { // A .js entrypoint (dev / dist-cli) is launched via node; the packaged // single-file CLI binary embeds its own Node and is invoked directly. @@ -170,6 +172,10 @@ private static byte[] BuildArgvJson(string cliEntrypoint) // Pin the worker to the bundled pkg matching the loaded cdylib, instead of // drifting to a newer version under the user's ~/.copilot/pkg (ABI skew). writer.WriteStringValue("--no-auto-update"); + foreach (var arg in args) + { + writer.WriteStringValue(arg); + } writer.WriteEndArray(); } return stream.ToArray(); diff --git a/dotnet/src/build/GitHub.Copilot.SDK.targets b/dotnet/src/build/GitHub.Copilot.SDK.targets index 5a5e511811..5f7944b2c4 100644 --- a/dotnet/src/build/GitHub.Copilot.SDK.targets +++ b/dotnet/src/build/GitHub.Copilot.SDK.targets @@ -9,6 +9,7 @@ <_CopilotOs Condition="'$(RuntimeIdentifier)' != '' And $(RuntimeIdentifier.StartsWith('win'))">win <_CopilotOs Condition="'$(_CopilotOs)' == '' And '$(RuntimeIdentifier)' != '' And $(RuntimeIdentifier.StartsWith('osx'))">osx <_CopilotOs Condition="'$(_CopilotOs)' == '' And '$(RuntimeIdentifier)' != '' And $(RuntimeIdentifier.StartsWith('maccatalyst'))">osx + <_CopilotOs Condition="'$(_CopilotOs)' == '' And '$(RuntimeIdentifier)' != '' And $(RuntimeIdentifier.StartsWith('linux-musl'))">linux-musl <_CopilotOs Condition="'$(_CopilotOs)' == '' And '$(RuntimeIdentifier)' != ''">linux @@ -22,7 +23,7 @@ - + @@ -31,6 +32,8 @@ <_CopilotPlatform Condition="'$(_CopilotRid)' == 'win-arm64'">win32-arm64 <_CopilotPlatform Condition="'$(_CopilotRid)' == 'linux-x64'">linux-x64 <_CopilotPlatform Condition="'$(_CopilotRid)' == 'linux-arm64'">linux-arm64 + <_CopilotPlatform Condition="'$(_CopilotRid)' == 'linux-musl-x64'">linuxmusl-x64 + <_CopilotPlatform Condition="'$(_CopilotRid)' == 'linux-musl-arm64'">linuxmusl-arm64 <_CopilotPlatform Condition="'$(_CopilotRid)' == 'osx-x64'">darwin-x64 <_CopilotPlatform Condition="'$(_CopilotRid)' == 'osx-arm64'">darwin-arm64 <_CopilotBinary Condition="$(_CopilotRid.StartsWith('win-'))">copilot.exe diff --git a/go/README.md b/go/README.md index a597f7899a..44ba01d66a 100644 --- a/go/README.md +++ b/go/README.md @@ -133,6 +133,7 @@ Resolution and requirements: always takes precedence. - Set `COPILOT_CLI_PATH` only when using an externally provisioned compatible runtime package; otherwise the bundled runtime is used. No `PATH` lookup is performed. - Embedded runtime versions are isolated in separate cache directories. Start fails loudly if the native runtime is unavailable. +- Linux in-process bundles include both glibc and musl runtime packages and select the matching package automatically at startup. - Only one native runtime version may be loaded per process. The in-process transport rejects options that cannot be honored by a runtime hosted in your shared process (each panics at `NewClient`): diff --git a/go/client.go b/go/client.go index 979f8a0ca9..fa7159de74 100644 --- a/go/client.go +++ b/go/client.go @@ -2080,7 +2080,9 @@ func (c *Client) startInProcess(ctx context.Context) error { return errors.New("in-process runtime unavailable: set COPILOT_CLI_PATH to a compatible runtime package or build with the bundled embedded runtime") } - host, err := createInProcessHost(runtimePath) + config := c.inProcessHostConfig() + + host, err := createInProcessHost(runtimePath, config) if err != nil { return err } @@ -2094,9 +2096,16 @@ func (c *Client) startInProcess(ctx context.Context) error { select { case err := <-errCh: if err != nil { + host.Dispose() + c.ffiHost = nil return err } case <-ctx.Done(): + c.ffiHost = nil + go func() { + <-errCh + host.Dispose() + }() return ctx.Err() } @@ -2117,6 +2126,47 @@ func (c *Client) startInProcess(ctx context.Context) error { return nil } +func (c *Client) inProcessHostConfig() inProcessHostConfig { + args := make([]string, 0, 8) + if c.options.LogLevel != "" { + args = append(args, "--log-level", c.options.LogLevel) + } + if c.options.GitHubToken != "" { + args = append(args, "--auth-token-env", "COPILOT_SDK_AUTH_TOKEN") + } + useLoggedInUser := true + if c.options.UseLoggedInUser != nil { + useLoggedInUser = *c.options.UseLoggedInUser + } else if c.options.GitHubToken != "" { + useLoggedInUser = false + } + if !useLoggedInUser { + args = append(args, "--no-auto-login") + } + if c.options.SessionIdleTimeoutSeconds > 0 { + args = append(args, "--session-idle-timeout", strconv.Itoa(c.options.SessionIdleTimeoutSeconds)) + } + if c.options.EnableRemoteSessions { + args = append(args, "--remote") + } + + environment := make(map[string]string) + if c.options.GitHubToken != "" { + environment["COPILOT_SDK_AUTH_TOKEN"] = c.options.GitHubToken + } + if c.options.BaseDirectory != "" { + environment["COPILOT_HOME"] = c.options.BaseDirectory + } + if c.options.Mode == ModeEmpty { + environment["COPILOT_DISABLE_KEYTAR"] = "1" + } + + return inProcessHostConfig{ + Environment: environment, + Args: args, + } +} + func (c *Client) killProcess() error { // Tear down the in-process FFI host on error paths that reuse killProcess to // abort a start (there is no OS process to kill in that mode). diff --git a/go/client_test.go b/go/client_test.go index c995cc9bde..48871e71f6 100644 --- a/go/client_test.go +++ b/go/client_test.go @@ -697,6 +697,39 @@ func TestClient_InProcessConnection(t *testing.T) { Telemetry: &TelemetryConfig{ExporterType: "file"}, }) }) + + t.Run("forwards typed runtime options", func(t *testing.T) { + client := NewClient(&ClientOptions{ + Connection: InProcessConnection{}, + GitHubToken: "test-token", + UseLoggedInUser: Bool(false), + BaseDirectory: "/copilot-home", + LogLevel: "debug", + SessionIdleTimeoutSeconds: 30, + EnableRemoteSessions: true, + Mode: ModeEmpty, + }) + + config := client.inProcessHostConfig() + expectedArgs := []string{ + "--log-level", "debug", + "--auth-token-env", "COPILOT_SDK_AUTH_TOKEN", + "--no-auto-login", + "--session-idle-timeout", "30", + "--remote", + } + if !reflect.DeepEqual(config.Args, expectedArgs) { + t.Fatalf("Expected managed arguments %v, got %v", expectedArgs, config.Args) + } + expectedEnvironment := map[string]string{ + "COPILOT_SDK_AUTH_TOKEN": "test-token", + "COPILOT_HOME": "/copilot-home", + "COPILOT_DISABLE_KEYTAR": "1", + } + if !reflect.DeepEqual(config.Environment, expectedEnvironment) { + t.Fatalf("Expected managed environment %v, got %v", expectedEnvironment, config.Environment) + } + }) } func TestClient_DefaultConnection(t *testing.T) { diff --git a/go/cmd/bundler/main.go b/go/cmd/bundler/main.go index 069092a1cf..e63d1fde66 100644 --- a/go/cmd/bundler/main.go +++ b/go/cmd/bundler/main.go @@ -91,14 +91,47 @@ func main() { fmt.Printf("Building bundle for %s (CLI version %s)\n", *platform, version) - binaryPath, sha256Hash, runtimeArtifactPath, runtimeHash, err := buildBundle(info, version, outputPath, goos, goarch) + binaryPath, sha256Hash, runtimeArtifactPath, runtimeHash, err := buildBundle(info, version, outputPath, goos) if err != nil { fmt.Fprintf(os.Stderr, "Error: %v\n", err) os.Exit(1) } + var muslBinaryPath, muslRuntimeArtifactPath string + var muslBinaryHash, muslRuntimeHash []byte + if goos == "linux" { + muslInfo := platformInfo{ + npmPlatform: strings.Replace(info.npmPlatform, "linux-", "linuxmusl-", 1), + binaryName: info.binaryName, + } + muslOutputPath := filepath.Join(*output, defaultOutputFileName(version, "linuxmusl", goarch, info.binaryName)) + muslBinaryPath, muslBinaryHash, muslRuntimeArtifactPath, muslRuntimeHash, err = buildBundle( + muslInfo, + version, + muslOutputPath, + goos, + ) + if err != nil { + fmt.Fprintf(os.Stderr, "Error: %v\n", err) + os.Exit(1) + } + } + // Generate the Go file with embed directive - if err := generateGoFile(goos, goarch, binaryPath, version, sha256Hash, runtimeArtifactPath, runtimeHash, "main"); err != nil { + if err := generateGoFile( + goos, + goarch, + binaryPath, + version, + sha256Hash, + runtimeArtifactPath, + runtimeHash, + muslBinaryPath, + muslBinaryHash, + muslRuntimeArtifactPath, + muslRuntimeHash, + "main", + ); err != nil { fmt.Fprintf(os.Stderr, "Error: %v\n", err) os.Exit(1) } @@ -257,12 +290,12 @@ func isHex(s string) bool { // native in-process runtime library) and writes them to outputPath's directory. // It returns the CLI bundle path and hash, plus the runtime-library artifact path // and hash (both empty when the package does not ship the runtime library). -func buildBundle(info platformInfo, cliVersion, outputPath, goos, goarch string) (string, []byte, string, []byte, error) { +func buildBundle(info platformInfo, cliVersion, outputPath, goos string) (string, []byte, string, []byte, error) { outputDir := filepath.Dir(outputPath) if outputDir == "" { outputDir = "." } - runtimeArtifactPath := filepath.Join(outputDir, runtimeLibArtifactName(cliVersion, goos, goarch)) + runtimeArtifactPath := filepath.Join(outputDir, runtimeLibArtifactName(cliVersion, info.npmPlatform, goos)) // Check if output already exists if _, err := os.Stat(outputPath); err == nil { @@ -346,8 +379,8 @@ func buildBundle(info platformInfo, cliVersion, outputPath, goos, goarch string) } // runtimeLibArtifactName builds the compressed runtime-library artifact filename. -func runtimeLibArtifactName(version, goos, goarch string) string { - return fmt.Sprintf("zcopilotruntime_%s_%s_%s.%s.zst", version, goos, goarch, runtimeLibExt(goos)) +func runtimeLibArtifactName(version, npmPlatform, goos string) string { + return fmt.Sprintf("zcopilotruntime_%s_%s.%s.zst", version, npmPlatform, runtimeLibExt(goos)) } // runtimeLibExt returns the shared-library extension for the target OS. @@ -365,7 +398,20 @@ func runtimeLibExt(goos string) string { // generateGoFile creates separate source files for normal and in-process builds. // Both embed the CLI, while only the copilot_inprocess-tagged file embeds the // native runtime library. -func generateGoFile(goos, goarch, binaryPath, cliVersion string, sha256Hash []byte, runtimeArtifactPath string, runtimeHash []byte, pkgName string) error { +func generateGoFile( + goos, + goarch, + binaryPath, + cliVersion string, + sha256Hash []byte, + runtimeArtifactPath string, + runtimeHash []byte, + muslBinaryPath string, + muslBinaryHash []byte, + muslRuntimeArtifactPath string, + muslRuntimeHash []byte, + pkgName string, +) error { binaryName := filepath.Base(binaryPath) licenseName := licenseFileName(binaryName) hashBase64 := "" @@ -384,6 +430,10 @@ func generateGoFile(goos, goarch, binaryPath, cliVersion string, sha256Hash []by hashBase64, "", nil, + "", + nil, + "", + nil, ) if err := os.WriteFile(defaultPath, []byte(defaultContent), 0644); err != nil { return err @@ -399,6 +449,10 @@ func generateGoFile(goos, goarch, binaryPath, cliVersion string, sha256Hash []by hashBase64, runtimeArtifactPath, runtimeHash, + muslBinaryPath, + muslBinaryHash, + muslRuntimeArtifactPath, + muslRuntimeHash, ) if err := os.WriteFile(inProcessPath, []byte(inProcessContent), 0644); err != nil { return err @@ -409,7 +463,20 @@ func generateGoFile(goos, goarch, binaryPath, cliVersion string, sha256Hash []by return nil } -func generatedGoFileContent(buildConstraint, pkgName, binaryName, licenseName, cliVersion, hashBase64, runtimeArtifactPath string, runtimeHash []byte) string { +func generatedGoFileContent( + buildConstraint, + pkgName, + binaryName, + licenseName, + cliVersion, + hashBase64, + runtimeArtifactPath string, + runtimeHash []byte, + muslBinaryPath string, + muslBinaryHash []byte, + muslRuntimeArtifactPath string, + muslRuntimeHash []byte, +) string { runtimeEmbed := "" runtimeConfig := "" runtimeReader := "" @@ -434,6 +501,45 @@ func runtimeLibReader() io.Reader { ` } + muslEmbed := "" + muslConfig := "" + muslReaders := "" + if muslBinaryPath != "" && muslRuntimeArtifactPath != "" { + muslBinaryName := filepath.Base(muslBinaryPath) + muslBinaryHashBase64 := base64.StdEncoding.EncodeToString(muslBinaryHash) + muslRuntimeName := filepath.Base(muslRuntimeArtifactPath) + muslRuntimeHashBase64 := base64.StdEncoding.EncodeToString(muslRuntimeHash) + muslEmbed = fmt.Sprintf(` +//go:embed %s +var localEmbeddedCopilotCLILinuxMusl []byte + +//go:embed %s +var localEmbeddedCopilotRuntimeLibLinuxMusl []byte +`, muslBinaryName, muslRuntimeName) + muslConfig = fmt.Sprintf(` + LinuxMuslCli: linuxMuslCLIReader(), + LinuxMuslCliHash: mustDecodeBase64(%q), + LinuxMuslRuntimeLib: linuxMuslRuntimeLibReader(), + LinuxMuslRuntimeLibHash: mustDecodeBase64(%q),`, muslBinaryHashBase64, muslRuntimeHashBase64) + muslReaders = ` +func linuxMuslCLIReader() io.Reader { + r, err := zstd.NewReader(bytes.NewReader(localEmbeddedCopilotCLILinuxMusl)) + if err != nil { + panic("failed to create zstd reader: " + err.Error()) + } + return r +} + +func linuxMuslRuntimeLibReader() io.Reader { + r, err := zstd.NewReader(bytes.NewReader(localEmbeddedCopilotRuntimeLibLinuxMusl)) + if err != nil { + panic("failed to create zstd reader: " + err.Error()) + } + return r +} +` + } + return fmt.Sprintf(`//go:build %s // Code generated by copilot-sdk bundler; DO NOT EDIT. @@ -456,13 +562,14 @@ var localEmbeddedCopilotCLI []byte //go:embed %s var localEmbeddedCopilotCLILicense []byte %s +%s func init() { embeddedcli.Setup(embeddedcli.Config{ Cli: cliReader(), License: localEmbeddedCopilotCLILicense, Version: %q, - CliHash: mustDecodeBase64(%q),%s + CliHash: mustDecodeBase64(%q),%s%s }) } @@ -474,6 +581,7 @@ func cliReader() io.Reader { return r } %s +%s func mustDecodeBase64(s string) []byte { b, err := base64.StdEncoding.DecodeString(s) if err != nil { @@ -481,7 +589,7 @@ func mustDecodeBase64(s string) []byte { } return b } -`, buildConstraint, pkgName, binaryName, licenseName, runtimeEmbed, cliVersion, hashBase64, runtimeConfig, runtimeReader) +`, buildConstraint, pkgName, binaryName, licenseName, runtimeEmbed, muslEmbed, cliVersion, hashBase64, runtimeConfig, muslConfig, runtimeReader, muslReaders) } // downloadCLIBinary downloads the npm tarball and extracts the CLI binary. It diff --git a/go/cmd/bundler/main_test.go b/go/cmd/bundler/main_test.go index 1b5cf6de2f..badc791359 100644 --- a/go/cmd/bundler/main_test.go +++ b/go/cmd/bundler/main_test.go @@ -1,6 +1,8 @@ package main import ( + "go/parser" + "go/token" "os" "path/filepath" "strings" @@ -11,14 +13,35 @@ func TestGenerateGoFileGatesRuntimeEmbed(t *testing.T) { dir := t.TempDir() binaryPath := filepath.Join(dir, "copilot.zst") runtimePath := filepath.Join(dir, "runtime.node.zst") - for _, path := range []string{binaryPath, licensePathForOutput(binaryPath), runtimePath} { + muslBinaryPath := filepath.Join(dir, "copilot-musl.zst") + muslRuntimePath := filepath.Join(dir, "runtime-musl.node.zst") + for _, path := range []string{ + binaryPath, + licensePathForOutput(binaryPath), + runtimePath, + muslBinaryPath, + muslRuntimePath, + } { if err := os.WriteFile(path, []byte("test"), 0644); err != nil { t.Fatal(err) } } hash := make([]byte, 32) - if err := generateGoFile("linux", "amd64", binaryPath, "1.2.3", hash, runtimePath, hash, "main"); err != nil { + if err := generateGoFile( + "linux", + "amd64", + binaryPath, + "1.2.3", + hash, + runtimePath, + hash, + muslBinaryPath, + hash, + muslRuntimePath, + hash, + "main", + ); err != nil { t.Fatal(err) } @@ -32,6 +55,9 @@ func TestGenerateGoFileGatesRuntimeEmbed(t *testing.T) { if strings.Contains(string(defaultSource), "localEmbeddedCopilotRuntimeLib") { t.Fatal("default embed file includes the native runtime") } + if _, err := parser.ParseFile(token.NewFileSet(), "zcopilot_linux_amd64.go", defaultSource, parser.AllErrors); err != nil { + t.Fatalf("default generated source is invalid: %v", err) + } inProcessSource, err := os.ReadFile(filepath.Join(dir, "zcopilot_inprocess_linux_amd64.go")) if err != nil { @@ -43,4 +69,13 @@ func TestGenerateGoFileGatesRuntimeEmbed(t *testing.T) { if !strings.Contains(string(inProcessSource), "localEmbeddedCopilotRuntimeLib") { t.Fatal("in-process embed file does not include the native runtime") } + if !strings.Contains(string(inProcessSource), "localEmbeddedCopilotCLILinuxMusl") { + t.Fatal("in-process embed file does not include the Linux musl CLI") + } + if !strings.Contains(string(inProcessSource), "localEmbeddedCopilotRuntimeLibLinuxMusl") { + t.Fatal("in-process embed file does not include the Linux musl runtime") + } + if _, err := parser.ParseFile(token.NewFileSet(), "zcopilot_inprocess_linux_amd64.go", inProcessSource, parser.AllErrors); err != nil { + t.Fatalf("in-process generated source is invalid: %v", err) + } } diff --git a/go/embeddedcli/installer.go b/go/embeddedcli/installer.go index 75c9b2c6b4..9702b3aec6 100644 --- a/go/embeddedcli/installer.go +++ b/go/embeddedcli/installer.go @@ -7,7 +7,8 @@ import "github.com/github/copilot-sdk/go/internal/embeddedcli" // Cli and CliHash are required. If Dir is empty, the CLI is installed into the // system cache directory. When Version is set, the CLI and runtime library are // installed into a version-specific child directory so multiple versions can -// coexist. License, when provided, is written next to the installed binary. +// coexist. Linux musl alternatives, when provided, are selected automatically. +// License, when provided, is written next to the installed binary. type Config = embeddedcli.Config // Setup sets the embedded GitHub Copilot CLI install configuration. diff --git a/go/inprocess.go b/go/inprocess.go index 1d23bc94c1..c74410e42b 100644 --- a/go/inprocess.go +++ b/go/inprocess.go @@ -8,3 +8,8 @@ type inProcessHost interface { Reader() io.ReadCloser Dispose() } + +type inProcessHostConfig struct { + Environment map[string]string + Args []string +} diff --git a/go/inprocess_disabled.go b/go/inprocess_disabled.go index b312c44a59..b86ed5ca36 100644 --- a/go/inprocess_disabled.go +++ b/go/inprocess_disabled.go @@ -6,6 +6,6 @@ import "errors" const inProcessAvailable = false -func createInProcessHost(string) (inProcessHost, error) { +func createInProcessHost(string, inProcessHostConfig) (inProcessHost, error) { return nil, errors.New("in-process transport unavailable") } diff --git a/go/inprocess_enabled.go b/go/inprocess_enabled.go index 3dc44dc8a1..c20013d8ab 100644 --- a/go/inprocess_enabled.go +++ b/go/inprocess_enabled.go @@ -6,6 +6,6 @@ import "github.com/github/copilot-sdk/go/internal/ffihost" const inProcessAvailable = true -func createInProcessHost(runtimePath string) (inProcessHost, error) { - return ffihost.Create(runtimePath, nil) +func createInProcessHost(runtimePath string, config inProcessHostConfig) (inProcessHost, error) { + return ffihost.Create(runtimePath, config.Environment, config.Args) } diff --git a/go/internal/e2e/testharness/proxy.go b/go/internal/e2e/testharness/proxy.go index 6c0a6d99dc..ec16124bc6 100644 --- a/go/internal/e2e/testharness/proxy.go +++ b/go/internal/e2e/testharness/proxy.go @@ -45,7 +45,7 @@ func (p *CapiProxy) Start() (string, error) { serverPath := RepoPath("test", "harness", "server.ts") p.cmd = exec.Command("npx", "tsx", serverPath) - p.cmd.Dir = "." // Will be resolved relative to test execution + p.cmd.Dir = RepoPath("test", "harness") stdout, err := p.cmd.StdoutPipe() if err != nil { diff --git a/go/internal/embeddedcli/embeddedcli.go b/go/internal/embeddedcli/embeddedcli.go index faa9d79070..cd0be21895 100644 --- a/go/internal/embeddedcli/embeddedcli.go +++ b/go/internal/embeddedcli/embeddedcli.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "os" + "os/exec" "path/filepath" "runtime" "strings" @@ -35,6 +36,13 @@ type Config struct { RuntimeLib io.Reader RuntimeLibHash []byte + // LinuxMuslCli and LinuxMuslRuntimeLib are optional alternatives selected + // automatically when the application runs on a musl-based Linux system. + LinuxMuslCli io.Reader + LinuxMuslCliHash []byte + LinuxMuslRuntimeLib io.Reader + LinuxMuslRuntimeLibHash []byte + Dir string Version string } @@ -46,6 +54,12 @@ func Setup(cfg Config) { if len(cfg.CliHash) != sha256.Size { panic(fmt.Sprintf("CliHash must be a SHA-256 hash (%d bytes), got %d bytes", sha256.Size, len(cfg.CliHash))) } + if cfg.LinuxMuslCli != nil && len(cfg.LinuxMuslCliHash) != sha256.Size { + panic(fmt.Sprintf("LinuxMuslCliHash must be a SHA-256 hash (%d bytes), got %d bytes", sha256.Size, len(cfg.LinuxMuslCliHash))) + } + if cfg.LinuxMuslRuntimeLib != nil && len(cfg.LinuxMuslRuntimeLibHash) != sha256.Size { + panic(fmt.Sprintf("LinuxMuslRuntimeLibHash must be a SHA-256 hash (%d bytes), got %d bytes", sha256.Size, len(cfg.LinuxMuslRuntimeLibHash))) + } setupMu.Lock() defer setupMu.Unlock() if setupDone { @@ -85,9 +99,12 @@ var ( setupDone bool pathInitialized bool runtimeLibPath string + linuxMuslBundle bool ) func install() (path string) { + selectLinuxMuslBundle() + verbose := os.Getenv("COPILOT_CLI_INSTALL_VERBOSE") == "1" logError := func(msg string, err error) { if verbose { @@ -122,11 +139,35 @@ func install() (path string) { return path } +func selectLinuxMuslBundle() { + if runtime.GOOS != "linux" || config.LinuxMuslCli == nil || !isMusl() { + return + } + config = linuxMuslConfig(config) + linuxMuslBundle = true +} + +func linuxMuslConfig(cfg Config) Config { + cfg.Cli = cfg.LinuxMuslCli + cfg.CliHash = cfg.LinuxMuslCliHash + cfg.RuntimeLib = cfg.LinuxMuslRuntimeLib + cfg.RuntimeLibHash = cfg.LinuxMuslRuntimeLibHash + return cfg +} + +func isMusl() bool { + out, _ := exec.Command("ldd", "--version").CombinedOutput() + return strings.Contains(strings.ToLower(string(out)), "musl") +} + func installAt(installDir string) (string, error) { version := sanitizeVersion(config.Version) if version != "" { installDir = filepath.Join(installDir, version) } + if linuxMuslBundle { + installDir = filepath.Join(installDir, "linuxmusl") + } if err := os.MkdirAll(installDir, 0755); err != nil { return "", fmt.Errorf("creating install directory: %w", err) } diff --git a/go/internal/embeddedcli/embeddedcli_test.go b/go/internal/embeddedcli/embeddedcli_test.go index 42cdedd9e3..b0394e0f69 100644 --- a/go/internal/embeddedcli/embeddedcli_test.go +++ b/go/internal/embeddedcli/embeddedcli_test.go @@ -17,6 +17,7 @@ func resetGlobals() { setupDone = false pathInitialized = false runtimeLibPath = "" + linuxMuslBundle = false } func mustPanic(t *testing.T, fn func()) { @@ -37,6 +38,31 @@ func binaryNameForOS() string { return name } +func TestLinuxMuslConfigSelectsAlternativeArtifacts(t *testing.T) { + glibcCLI := strings.NewReader("glibc-cli") + glibcRuntime := strings.NewReader("glibc-runtime") + muslCLI := strings.NewReader("musl-cli") + muslRuntime := strings.NewReader("musl-runtime") + muslCLIHash := bytes.Repeat([]byte{1}, sha256.Size) + muslRuntimeHash := bytes.Repeat([]byte{2}, sha256.Size) + + selected := linuxMuslConfig(Config{ + Cli: glibcCLI, + RuntimeLib: glibcRuntime, + LinuxMuslCli: muslCLI, + LinuxMuslCliHash: muslCLIHash, + LinuxMuslRuntimeLib: muslRuntime, + LinuxMuslRuntimeLibHash: muslRuntimeHash, + }) + + if selected.Cli != muslCLI || selected.RuntimeLib != muslRuntime { + t.Fatal("Expected Linux musl artifacts to replace the glibc artifacts") + } + if !bytes.Equal(selected.CliHash, muslCLIHash) || !bytes.Equal(selected.RuntimeLibHash, muslRuntimeHash) { + t.Fatal("Expected Linux musl hashes to replace the glibc hashes") + } +} + func TestSetupPanicsOnNilCli(t *testing.T) { resetGlobals() mustPanic(t, func() { Setup(Config{}) }) diff --git a/go/internal/ffihost/ffihost.go b/go/internal/ffihost/ffihost.go index d4cd8d9e86..88fe279751 100644 --- a/go/internal/ffihost/ffihost.go +++ b/go/internal/ffihost/ffihost.go @@ -1,14 +1,9 @@ //go:build copilot_inprocess && (darwin || linux || windows) -// Package ffihost hosts the Copilot runtime in-process by loading the native -// runtime library (runtime.node — a Rust cdylib) and driving JSON-RPC over its -// C ABI (FFI) instead of spawning the CLI as a child process and talking over -// stdio/TCP. +// Package ffihost hosts the Copilot runtime in-process by loading its native +// library and driving JSON-RPC over the runtime's C ABI. // -// The native copilot_runtime_host_start export spawns the residual CLI worker -// itself (` --embedded-host --no-auto-update`), so the SDK never -// launches the worker directly; it only pumps opaque LSP `Content-Length:`-framed -// JSON-RPC bytes across the boundary: +// It pumps opaque LSP Content-Length-framed JSON-RPC bytes across the boundary: // // - client → server frames go to copilot_runtime_connection_write // - server → client frames arrive on a native callback that feeds a @@ -135,20 +130,21 @@ func loadLibrary(libraryPath string) (lib *ffiLibrary, err error) { // Host hosts the Copilot runtime in-process via its native C ABI. // -// Construct with Create, call Start to spawn the worker and open the FFI -// connection, wire Writer/Reader into jsonrpc2.NewClient, and call Dispose to -// tear everything down. +// Construct with Create, call Start to open the FFI connection, wire +// Writer/Reader into jsonrpc2.NewClient, and call Dispose to tear everything +// down. type Host struct { libraryPath string cliEntrypoint string environment map[string]string + args []string lib *ffiLibrary - // mu guards disposed, activeCallbacks, serverID, and connectionID. A single - // mutex is used so that publishing disposed=true and draining in-flight - // callbacks are serialized against onOutbound's disposed-check/increment; - // this prevents a callback from feeding the receive buffer after it is - // closed (and avoids a data race on disposed observed by the -race detector). + // lifecycleMu serializes native start/write/shutdown operations. hostStart + // cannot be interrupted, so Dispose waits for it before closing native IDs. + lifecycleMu sync.Mutex + // mu serializes disposal with native callbacks so the receive buffer cannot + // be fed after it is closed. mu sync.Mutex serverID uint32 connectionID uint32 @@ -161,9 +157,9 @@ type Host struct { callbackToken uintptr } -// Create resolves the cdylib next to the CLI entrypoint and prepares the host. -// It does not spawn the worker; call Start for that. environment may be nil. -func Create(cliEntrypoint string, environment map[string]string) (*Host, error) { +// Create resolves the native library and prepares the host. environment and +// args contain SDK-managed runtime options. +func Create(cliEntrypoint string, environment map[string]string, args []string) (*Host, error) { libraryPath, err := ResolveLibraryPath(cliEntrypoint) if err != nil { return nil, err @@ -176,15 +172,25 @@ func Create(cliEntrypoint string, environment map[string]string) (*Host, error) libraryPath: libraryPath, cliEntrypoint: cliEntrypoint, environment: environment, + args: append([]string(nil), args...), lib: lib, recv: newReceiveBuffer(), }, nil } -// Start spawns the worker and opens the FFI connection. It blocks until the -// worker connects back and signals readiness (up to ~30s), so callers should +// Start opens the FFI connection. Native startup may block, so callers should // run it off any latency-sensitive goroutine. func (h *Host) Start() error { + h.lifecycleMu.Lock() + defer h.lifecycleMu.Unlock() + + h.mu.Lock() + if h.disposed { + h.mu.Unlock() + return fmt.Errorf("the in-process runtime host is disposed") + } + h.mu.Unlock() + argv := h.buildArgv() env := h.buildEnv() @@ -245,6 +251,7 @@ func (h *Host) buildArgv() []byte { } else { argv = []string{h.cliEntrypoint, "--embedded-host", "--no-auto-update"} } + argv = append(argv, h.args...) b, _ := json.Marshal(argv) return b } @@ -291,11 +298,14 @@ func (h *Host) onOutbound(bytesPtr uintptr, bytesLen uintptr) uintptr { } func (h *Host) writeFrame(frame []byte) (int, error) { + h.lifecycleMu.Lock() + defer h.lifecycleMu.Unlock() + h.mu.Lock() - closed := h.disposed || h.connectionID == 0 - connID := h.connectionID + disposed := h.disposed h.mu.Unlock() - if closed { + connID := h.connectionID + if disposed || connID == 0 { return 0, fmt.Errorf("the in-process runtime connection is closed") } if len(frame) == 0 { @@ -313,6 +323,9 @@ func (h *Host) writeFrame(frame []byte) (int, error) { // resources. It is idempotent and waits for any in-flight outbound callback to // finish before closing the receive buffer. func (h *Host) Dispose() { + h.lifecycleMu.Lock() + defer h.lifecycleMu.Unlock() + h.mu.Lock() if h.disposed { h.mu.Unlock() diff --git a/go/internal/ffihost/ffihost_test.go b/go/internal/ffihost/ffihost_test.go index 8440eab7bb..bc588fa6a9 100644 --- a/go/internal/ffihost/ffihost_test.go +++ b/go/internal/ffihost/ffihost_test.go @@ -2,7 +2,13 @@ package ffihost -import "testing" +import ( + "encoding/json" + "sync/atomic" + "testing" + "time" + "unsafe" +) func TestDisposeUnregistersOutboundTarget(t *testing.T) { token := uintptr(nextOutboundToken.Add(1)) @@ -18,3 +24,75 @@ func TestDisposeUnregistersOutboundTarget(t *testing.T) { t.Fatal("Expected disposed host to be removed from outbound callback registry") } } + +func TestBuildArgvAppendsManagedOptions(t *testing.T) { + host := &Host{ + cliEntrypoint: "copilot", + args: []string{"--log-level", "debug", "--remote"}, + } + + var argv []string + if err := json.Unmarshal(host.buildArgv(), &argv); err != nil { + t.Fatal(err) + } + + expected := []string{"copilot", "--embedded-host", "--no-auto-update", "--log-level", "debug", "--remote"} + if len(argv) != len(expected) { + t.Fatalf("Expected %d arguments, got %d: %v", len(expected), len(argv), argv) + } + for i := range expected { + if argv[i] != expected[i] { + t.Fatalf("Expected argument %d to be %q, got %q", i, expected[i], argv[i]) + } + } +} + +func TestDisposeWaitsForStartBeforeShuttingDown(t *testing.T) { + started := make(chan struct{}) + releaseStart := make(chan struct{}) + startDone := make(chan error, 1) + disposeDone := make(chan struct{}) + var shutdownID atomic.Uint32 + + host := &Host{ + lib: &ffiLibrary{ + hostStart: func(_ unsafe.Pointer, _ uintptr, _ unsafe.Pointer, _ uintptr) uint32 { + close(started) + <-releaseStart + return 41 + }, + hostShutdown: func(serverID uint32) bool { + shutdownID.Store(serverID) + return true + }, + connectionOpen: func(_ uint32, _ uintptr, _ uintptr, _ unsafe.Pointer, _ uintptr, _ unsafe.Pointer, _ uintptr, _ unsafe.Pointer, _ uintptr) uint32 { + return 42 + }, + connectionClose: func(_ uint32) bool { return true }, + }, + recv: newReceiveBuffer(), + } + + go func() { startDone <- host.Start() }() + <-started + go func() { + host.Dispose() + close(disposeDone) + }() + + select { + case <-disposeDone: + t.Fatal("Dispose returned before native startup completed") + case <-time.After(20 * time.Millisecond): + } + + close(releaseStart) + if err := <-startDone; err != nil { + t.Fatal(err) + } + <-disposeDone + + if got := shutdownID.Load(); got != 41 { + t.Fatalf("Expected shutdown of server 41, got %d", got) + } +} diff --git a/nodejs/src/client.ts b/nodejs/src/client.ts index d7e0f12cd4..65784569cc 100644 --- a/nodejs/src/client.ts +++ b/nodejs/src/client.ts @@ -2558,17 +2558,7 @@ export class CopilotClient { } } - /** - * Start the in-process FFI runtime host: resolve the CLI entrypoint and native - * runtime library, then let the native host spawn the CLI worker. - * - * The worker inherits this host process's ambient environment; per-client options - * that lower to environment variables (`env`, `telemetry`, `gitHubToken`, - * `baseDirectory`) are intentionally not applied here, because the native runtime - * loads into the shared host process and a single env block cannot carry per-client - * values. Configure the in-process runtime via the host process environment instead. - * See https://github.com/github/copilot-sdk/issues/1934. - */ + /** Starts the in-process FFI runtime with SDK-managed typed options. */ private async startInProcessFfi(): Promise { const entrypoint = this.resolveCliPathForFfi(); // Load the FFI host lazily so the native `koffi` addon (and its @@ -2577,7 +2567,40 @@ export class CopilotClient { // The transpiled output is per-file (not bundled), so this resolves the // sibling module at runtime in both the ESM and CJS builds. const { FfiRuntimeHost } = await import("./ffiRuntimeHost.js"); - const host = FfiRuntimeHost.create(entrypoint, CopilotClient.getNapiPrebuildsFolder()); + const environment: Record = {}; + if (this.options.gitHubToken) { + environment.COPILOT_SDK_AUTH_TOKEN = this.options.gitHubToken; + } + if (this.options.baseDirectory) { + environment.COPILOT_HOME = this.options.baseDirectory; + } + if (this.options.mode === "empty") { + environment.COPILOT_DISABLE_KEYTAR = "1"; + } + + const args: string[] = []; + if (this.options.logLevel) { + args.push("--log-level", this.options.logLevel); + } + if (this.options.gitHubToken) { + args.push("--auth-token-env", "COPILOT_SDK_AUTH_TOKEN"); + } + if (!this.options.useLoggedInUser) { + args.push("--no-auto-login"); + } + if (this.options.sessionIdleTimeoutSeconds > 0) { + args.push("--session-idle-timeout", this.options.sessionIdleTimeoutSeconds.toString()); + } + if (this.options.enableRemoteSessions) { + args.push("--remote"); + } + + const host = FfiRuntimeHost.create( + entrypoint, + CopilotClient.getNapiPrebuildsFolder(entrypoint), + environment, + args + ); this.ffiHost = host; await host.start(); } @@ -2611,14 +2634,34 @@ export class CopilotClient { /** * Returns the napi prebuilds folder name for the current host — the * `-` convention (e.g. `win32-x64`, `darwin-arm64`, - * `linux-x64`) under which the runtime ships `prebuilds//runtime.node`. + * `linux-x64`, `linuxmusl-x64`) under which the runtime ships + * `prebuilds//runtime.node`. */ - private static getNapiPrebuildsFolder(): string { + private static getNapiPrebuildsFolder(entrypoint: string): string { const arch = process.arch; if (arch !== "x64" && arch !== "arm64") { throw new Error(`Unsupported architecture '${arch}' for in-process FFI hosting.`); } - return `${process.platform}-${arch}`; + let platform: string = process.platform; + if (platform === "linux" && CopilotClient.isMusl(entrypoint)) { + platform = "linuxmusl"; + } + return `${platform}-${arch}`; + } + + private static isMusl(entrypoint: string): boolean { + if (entrypoint.includes(`copilot-linuxmusl-${process.arch}`)) { + return true; + } + if (entrypoint.includes(`copilot-linux-${process.arch}`)) { + return false; + } + const report = process.report?.getReport(); + const header = + report && "header" in report + ? (report.header as { glibcVersionRuntime?: string }) + : undefined; + return header !== undefined && header.glibcVersionRuntime === undefined; } /** diff --git a/nodejs/src/ffiRuntimeHost.ts b/nodejs/src/ffiRuntimeHost.ts index 05c8c7c72c..a92aa1589a 100644 --- a/nodejs/src/ffiRuntimeHost.ts +++ b/nodejs/src/ffiRuntimeHost.ts @@ -97,7 +97,7 @@ function loadLibrary(libraryPath: string): FfiLibrary { return loadedLibrary; } -function buildArgvJson(cliEntrypoint: string): Buffer { +function buildArgvJson(cliEntrypoint: string, args: readonly string[]): Buffer { // A `.js` entrypoint is launched via node; the packaged single-file CLI binary // embeds its own Node and is invoked directly. `--no-auto-update` pins the worker // to the bundled pkg matching the loaded cdylib, instead of drifting to a newer @@ -105,6 +105,7 @@ function buildArgvJson(cliEntrypoint: string): Buffer { const argv = cliEntrypoint.toLowerCase().endsWith(".js") ? ["node", cliEntrypoint, "--embedded-host", "--no-auto-update"] : [cliEntrypoint, "--embedded-host", "--no-auto-update"]; + argv.push(...args); return Buffer.from(JSON.stringify(argv), "utf8"); } @@ -140,7 +141,8 @@ export class FfiRuntimeHost { private constructor( private readonly libraryPath: string, private readonly cliEntrypoint: string, - private readonly environment?: Record + private readonly environment: Record | undefined, + private readonly args: readonly string[] ) { this.lib = loadLibrary(libraryPath); this.receiveStream = new PassThrough(); @@ -167,7 +169,8 @@ export class FfiRuntimeHost { static create( cliEntrypoint: string, prebuildsFolder: string, - environment?: Record + environment: Record | undefined, + args: readonly string[] ): FfiRuntimeHost { const fullEntrypoint = resolve(cliEntrypoint); const distDir = dirname(fullEntrypoint); @@ -175,7 +178,7 @@ export class FfiRuntimeHost { if (!existsSync(libraryPath)) { throw new Error(`FFI runtime library not found. Looked for '${libraryPath}'.`); } - return new FfiRuntimeHost(libraryPath, fullEntrypoint, environment); + return new FfiRuntimeHost(libraryPath, fullEntrypoint, environment, args); } /** @@ -183,7 +186,7 @@ export class FfiRuntimeHost { * waits for readiness, and opens the FFI JSON-RPC connection. */ async start(): Promise { - const argvJson = buildArgvJson(this.cliEntrypoint); + const argvJson = buildArgvJson(this.cliEntrypoint, this.args); const envJson = buildEnvJson(this.environment); // The native host spawns the CLI worker itself and has no cwd parameter, so the diff --git a/python/copilot/client.py b/python/copilot/client.py index 40201fbc53..94eca4f89c 100644 --- a/python/copilot/client.py +++ b/python/copilot/client.py @@ -1279,10 +1279,12 @@ def __init__( """ Initialize a new CopilotClient. - All process-management options (``working_directory``, ``log_level``, - ``env``, ``github_token``, …) apply only when the SDK spawns the runtime - (stdio / tcp connections). They are ignored when connecting to an - existing runtime via :meth:`RuntimeConnection.for_uri`. + Runtime options apply to locally hosted connections. The in-process + transport supports typed runtime options such as ``log_level``, + ``github_token``, and ``base_directory``, but rejects per-client + ``working_directory``, ``env``, and ``telemetry``. Options are ignored + when connecting to an existing runtime via + :meth:`RuntimeConnection.for_uri`. Args: connection: How to reach the runtime. Defaults to @@ -3956,9 +3958,32 @@ async def _start_inprocess_ffi(self) -> None: extra={"runtime_path": runtime_path, "runtime_path_source": self._cli_path_source}, ) - # env/telemetry/working_directory are rejected for the in-process transport - # because the native runtime shares this process's ambient state. - host = FfiRuntimeHost.create(runtime_path, environment=None, args=()) + opts = self._options + args: list[str] = [] + if opts.log_level: + args.extend(["--log-level", opts.log_level]) + if opts.github_token: + args.extend(["--auth-token-env", "COPILOT_SDK_AUTH_TOKEN"]) + if not opts.use_logged_in_user: + args.append("--no-auto-login") + if opts.session_idle_timeout_seconds is not None and opts.session_idle_timeout_seconds > 0: + args.extend(["--session-idle-timeout", str(opts.session_idle_timeout_seconds)]) + if opts.enable_remote_sessions: + args.append("--remote") + + environment: dict[str, str] = {} + if opts.github_token: + environment["COPILOT_SDK_AUTH_TOKEN"] = opts.github_token + if opts.base_directory: + environment["COPILOT_HOME"] = opts.base_directory + if opts.mode == "empty": + environment["COPILOT_DISABLE_KEYTAR"] = "1" + + host = FfiRuntimeHost.create( + runtime_path, + environment=environment or None, + args=tuple(args), + ) # Track the host and expose its process-like adapter *before* the blocking # handshake. asyncio.to_thread keeps running host_start after a cancellation diff --git a/rust/build/in_process.rs b/rust/build/in_process.rs index 39c9cc22f7..5826fbfa76 100644 --- a/rust/build/in_process.rs +++ b/rust/build/in_process.rs @@ -329,29 +329,38 @@ impl Platform { fn target_platform() -> Option { let os = std::env::var("CARGO_CFG_TARGET_OS").ok()?; let arch = std::env::var("CARGO_CFG_TARGET_ARCH").ok()?; + let target_env = std::env::var("CARGO_CFG_TARGET_ENV").unwrap_or_default(); - match (os.as_str(), arch.as_str()) { - ("macos", "aarch64") => Some(Platform { + match (os.as_str(), arch.as_str(), target_env.as_str()) { + ("macos", "aarch64", _) => Some(Platform { package_name: "copilot-darwin-arm64", binary_name: "copilot", }), - ("macos", "x86_64") => Some(Platform { + ("macos", "x86_64", _) => Some(Platform { package_name: "copilot-darwin-x64", binary_name: "copilot", }), - ("linux", "x86_64") => Some(Platform { + ("linux", "x86_64", "musl") => Some(Platform { + package_name: "copilot-linuxmusl-x64", + binary_name: "copilot", + }), + ("linux", "aarch64", "musl") => Some(Platform { + package_name: "copilot-linuxmusl-arm64", + binary_name: "copilot", + }), + ("linux", "x86_64", _) => Some(Platform { package_name: "copilot-linux-x64", binary_name: "copilot", }), - ("linux", "aarch64") => Some(Platform { + ("linux", "aarch64", _) => Some(Platform { package_name: "copilot-linux-arm64", binary_name: "copilot", }), - ("windows", "x86_64") => Some(Platform { + ("windows", "x86_64", _) => Some(Platform { package_name: "copilot-win32-x64", binary_name: "copilot.exe", }), - ("windows", "aarch64") => Some(Platform { + ("windows", "aarch64", _) => Some(Platform { package_name: "copilot-win32-arm64", binary_name: "copilot.exe", }), diff --git a/rust/scripts/snapshot-bundled-in-process-version.sh b/rust/scripts/snapshot-bundled-in-process-version.sh index 5a4cde73fa..8743f9d17a 100755 --- a/rust/scripts/snapshot-bundled-in-process-version.sh +++ b/rust/scripts/snapshot-bundled-in-process-version.sh @@ -27,6 +27,8 @@ PACKAGES=( "copilot-darwin-x64" "copilot-linux-arm64" "copilot-linux-x64" + "copilot-linuxmusl-arm64" + "copilot-linuxmusl-x64" "copilot-win32-arm64" "copilot-win32-x64" ) diff --git a/rust/src/lib.rs b/rust/src/lib.rs index 389d66a73e..bbef97c284 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -128,10 +128,10 @@ pub enum Transport { /// This is **experimental**. Per-client [`ClientOptions::program`], /// [`ClientOptions::extra_args`], [`ClientOptions::working_directory`], /// [`ClientOptions::env`]/[`ClientOptions::env_remove`], - /// [`ClientOptions::telemetry`], and [`ClientOptions::github_token`] are - /// not supported because native runtime code shares the host process. - /// [`ClientOptions::base_directory`] remains supported because it is - /// applied to the native runtime as `COPILOT_HOME`. + /// and [`ClientOptions::telemetry`] are not supported because native + /// runtime code shares the host process. Typed runtime options such as + /// authentication, log level, and [`ClientOptions::base_directory`] remain + /// supported. /// /// Requires the `bundled-in-process` Cargo feature. InProcess, @@ -935,8 +935,6 @@ fn validate_inprocess_options(options: &ClientOptions) -> Result<()> { Some("env_remove") } else if options.telemetry.is_some() { Some("telemetry") - } else if options.github_token.is_some() { - Some("github_token") } else if !options.prefix_args.is_empty() { Some("prefix_args") } else { @@ -1246,6 +1244,12 @@ impl Client { if options.mode == ClientMode::Empty { environment.push(("COPILOT_DISABLE_KEYTAR".to_string(), "1".to_string())); } + if let Some(github_token) = &options.github_token { + environment.push(( + "COPILOT_SDK_AUTH_TOKEN".to_string(), + github_token.clone(), + )); + } let mut args = Vec::new(); args.extend( Self::log_level_args(&options) @@ -1254,7 +1258,16 @@ impl Client { ); args.extend(Self::session_idle_timeout_args(&options)); args.extend(Self::remote_args(&options)); - if options.use_logged_in_user == Some(false) { + if options.github_token.is_some() { + args.extend([ + "--auth-token-env".to_string(), + "COPILOT_SDK_AUTH_TOKEN".to_string(), + ]); + } + let use_logged_in_user = options + .use_logged_in_user + .unwrap_or(options.github_token.is_none()); + if !use_logged_in_user { args.push("--no-auto-login".to_string()); } let host = crate::ffi::FfiHost::create(&program, environment, args)?; @@ -2530,7 +2543,6 @@ mod tests { ClientOptions::new().with_env([("KEY", "value")]), ClientOptions::new().with_env_remove(["KEY"]), ClientOptions::new().with_telemetry(TelemetryConfig::default()), - ClientOptions::new().with_github_token("token"), ClientOptions::new().with_prefix_args(["index.js"]), ClientOptions::new().with_program(CliProgram::Path("copilot".into())), ClientOptions::new().with_extra_args(["--verbose"]), @@ -2547,6 +2559,7 @@ mod tests { .with_base_directory("state") .with_log_level(LogLevel::Debug) .with_session_idle_timeout_seconds(10) + .with_github_token("token") .with_use_logged_in_user(false) .with_enable_remote_sessions(true); diff --git a/rust/tests/e2e/support.rs b/rust/tests/e2e/support.rs index 3ef849a361..6ad609f58e 100644 --- a/rust/tests/e2e/support.rs +++ b/rust/tests/e2e/support.rs @@ -178,18 +178,7 @@ impl E2eContext { } pub fn client_options_with_github_token(&self, token: &str) -> ClientOptions { - let options = self.client_options(); - if is_inprocess_default() { - // SAFETY: the in-process E2E suite is serialized for the full - // lifetime of InProcessEnvGuard. - unsafe { - std::env::set_var("GH_TOKEN", token); - std::env::set_var("GITHUB_TOKEN", token); - } - options - } else { - options.with_github_token(token) - } + self.client_options().with_github_token(token) } pub async fn start_client(&self) -> Client { From 2ed51a5d554ebc1dd63daa505137ea4a2c865162 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 14 Jul 2026 13:24:35 +0100 Subject: [PATCH 13/14] Apply Rust nightly formatting Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 447feac4-35df-4c0b-ab34-00789ad78da7 --- rust/src/lib.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/rust/src/lib.rs b/rust/src/lib.rs index bbef97c284..65d9dab215 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -1245,10 +1245,8 @@ impl Client { environment.push(("COPILOT_DISABLE_KEYTAR".to_string(), "1".to_string())); } if let Some(github_token) = &options.github_token { - environment.push(( - "COPILOT_SDK_AUTH_TOKEN".to_string(), - github_token.clone(), - )); + environment + .push(("COPILOT_SDK_AUTH_TOKEN".to_string(), github_token.clone())); } let mut args = Vec::new(); args.extend( From dda58bff7dfe4d5d56880b7a538c48ac96c9437f Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 14 Jul 2026 13:30:49 +0100 Subject: [PATCH 14/14] Repair signal handlers after FFI shutdown Reapply SA_ONSTACK after libuv tears down its child watcher so subsequent Go subprocess cleanup cannot crash. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 447feac4-35df-4c0b-ab34-00789ad78da7 --- go/internal/ffihost/ffihost.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/go/internal/ffihost/ffihost.go b/go/internal/ffihost/ffihost.go index 88fe279751..30cd831281 100644 --- a/go/internal/ffihost/ffihost.go +++ b/go/internal/ffihost/ffihost.go @@ -229,6 +229,7 @@ func (h *Host) Start() error { outboundTargets.Delete(callbackToken) h.callbackToken = 0 h.lib.hostShutdown(h.serverID) + rearmForeignSignalHandlers(h.lib.handle) h.serverID = 0 return fmt.Errorf("copilot_runtime_connection_open failed") } @@ -364,6 +365,10 @@ func (h *Host) Dispose() { } if serverID != 0 { h.lib.hostShutdown(serverID) + // libuv may restore a previously saved SIGCHLD action while tearing down + // its final child watcher, so repair the process-wide handler again after + // shutdown before Go reaps another os/exec child. + rearmForeignSignalHandlers(h.lib.handle) } h.recv.Close() }