Add in-process (FFI) transport to the Go SDK#1976
Draft
SteveSandersonMS wants to merge 3 commits into
Draft
Conversation
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/<platform>/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
This comment has been minimized.
This comment has been minimized.
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
This comment has been minimized.
This comment has been minimized.
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
Contributor
SDK Consistency Review✅ This PR maintains and improves cross-SDK consistency. It brings the Go SDK to parity with the Node.js, Python, .NET, and Rust SDKs by adding the in-process (FFI) transport. Feature coverage after this PR
Notes
No action required from a consistency standpoint — this PR is a net improvement.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Brings the Go SDK to parity with the .NET, Rust, TypeScript, and Python SDKs by adding an in-process (FFI) transport. Instead of spawning a CLI child and talking JSON-RPC over stdio/TCP, the SDK loads the native runtime library (
runtime.node) and pumps LSP-framed JSON-RPC across its C ABI. The runtime'shost_startexport spawns the worker itself (<entrypoint> --embedded-host --no-auto-update).The only intentional divergence from the other SDKs is the Go-specific FFI mechanism:
ebitengine/purego, which loads the cdylib at runtime with CGO disabled — preserving Go's pure build and cross-compilation.What's included
go/internal/ffihost/— purego FFI host: C ABI binding, single-load-per-process guard, cdylib resolution (flat name next to the CLI binary, with a devprebuilds/<platform>/runtime.nodefallback), plus musl and Windows loaders.InProcessConnection{Path, Args}(markedExperimental) implementingRuntimeConnection; connection-levelEnvonStdioConnection/TCPConnectionvia a sharedchildProcessConnectioninterface.client.go—startInProcesswired intoStart/Stop/ForceStop/killProcess, with no PATH fallback for entrypoint resolution (InProcessConnection.Path>COPILOT_CLI_PATH> embedded),--no-auto-updateargv, and host ownership before the blocking handshake for cancellation safety.validateEnvironmentOptionsrejectsEnv/WorkingDirectory/Telemetryin-process, and rejects env set in both client- and connection-level for child transports.runtime.node(conditional per platform, like .NET'sCondition="Exists(...)"), install it next to the CLI binary with SHA-256 verification (fail closed), and exposeRuntimeLibPath().COPILOT_SDK_DEFAULT_CONNECTION=inprocesssupport that swaps only the default stdio connection, mirrors the merged env + cwd onto the process, and leaves TCP/URI/custom-stdio/telemetry tests on their original transport. New in-processpingsmoke test..github/workflows/go-sdk-tests.ymlgains atransport: [default, inprocess]matrix dimension and a "Select inprocess transport" step (mirrors the Python/Node/.NET workflows).Consistency with other SDKs
--embedded-host --no-auto-updateworker argvValidation
go build ./...clean,gofmtclean,golangci-lintclean on changed packages.pingsmoke e2e passes.defaultandinprocess: snapshot session, cwd-override, env-append/provider-endpoint, telemetry (stays stdio), TCP connection-token (stays TCP).Notes / follow-ups
inprocesson all three OSes is validated locally only via representative tests (CI now runs the full matrix). Individual tests that assert child-process-specific behavior may later need atestharness.IsInProcessTransport()skip.