diff --git a/go/internal/e2e/auto_tier_e2e_test.go b/go/internal/e2e/auto_tier_e2e_test.go index eb7fda1351..588e075052 100644 --- a/go/internal/e2e/auto_tier_e2e_test.go +++ b/go/internal/e2e/auto_tier_e2e_test.go @@ -164,6 +164,9 @@ func TestAutoTierE2E(t *testing.T) { }) t.Run("should restore and override fast auto tier on cold resume", func(t *testing.T) { + if testharness.RunInIsolatedProcess(t) { + return + } ctx, client := newAutoClient(t) fastSession, err := client.CreateSession(t.Context(), &copilot.SessionConfig{ Model: "auto", @@ -287,6 +290,9 @@ func TestAutoTierE2E(t *testing.T) { }) t.Run("should commit fast auto tier after successful turn", func(t *testing.T) { + if testharness.RunInIsolatedProcess(t) { + return + } _, client := newAutoClient(t) session, err := client.CreateSession(t.Context(), &copilot.SessionConfig{ Model: "auto", @@ -358,6 +364,9 @@ func TestAutoTierE2E(t *testing.T) { }) t.Run("should preserve effective tier when fast activation fails", func(t *testing.T) { + if testharness.RunInIsolatedProcess(t) { + return + } ctx, client := newAutoClient(t) session, err := client.CreateSession(t.Context(), &copilot.SessionConfig{ Model: "auto", diff --git a/go/internal/e2e/mcp_oauth_e2e_test.go b/go/internal/e2e/mcp_oauth_e2e_test.go index 9e10fbbda9..eb9c72593c 100644 --- a/go/internal/e2e/mcp_oauth_e2e_test.go +++ b/go/internal/e2e/mcp_oauth_e2e_test.go @@ -397,6 +397,7 @@ func startOAuthMCPServer(t *testing.T, cimdSupported ...bool) string { if cmd.ProcessState != nil && cmd.ProcessState.Exited() { return } + testharness.PrepareForProcessWait() _ = cmd.Process.Kill() _, _ = cmd.Process.Wait() }) diff --git a/go/internal/e2e/testharness/inprocess_cleanup_disabled.go b/go/internal/e2e/testharness/inprocess_cleanup_disabled.go index 404bcd9285..d5fb4db466 100644 --- a/go/internal/e2e/testharness/inprocess_cleanup_disabled.go +++ b/go/internal/e2e/testharness/inprocess_cleanup_disabled.go @@ -5,3 +5,6 @@ package testharness func waitForInProcessCleanup() error { return nil } + +// PrepareForProcessWait is a no-op when the in-process runtime is unavailable. +func PrepareForProcessWait() {} diff --git a/go/internal/e2e/testharness/inprocess_cleanup_enabled.go b/go/internal/e2e/testharness/inprocess_cleanup_enabled.go index c65ae15324..83ca9f584a 100644 --- a/go/internal/e2e/testharness/inprocess_cleanup_enabled.go +++ b/go/internal/e2e/testharness/inprocess_cleanup_enabled.go @@ -16,3 +16,9 @@ func waitForInProcessCleanup() error { } return nil } + +// PrepareForProcessWait repairs signal handlers before the harness stops or +// waits for a child process while an in-process runtime is active. +func PrepareForProcessWait() { + ffihost.PrepareForChildProcessWait() +} diff --git a/go/internal/e2e/testharness/proxy.go b/go/internal/e2e/testharness/proxy.go index 990c066eea..f120a0f8ba 100644 --- a/go/internal/e2e/testharness/proxy.go +++ b/go/internal/e2e/testharness/proxy.go @@ -130,6 +130,8 @@ func (p *CapiProxy) StopWithOptions(skipWritingCache bool) error { p.proxyURL = "" }() + PrepareForProcessWait() + // Send stop request to the server if p.proxyURL != "" { stopURL := p.proxyURL + "/stop" diff --git a/go/internal/ffihost/ffihost.go b/go/internal/ffihost/ffihost.go index afdfd7c89f..c98b5f4847 100644 --- a/go/internal/ffihost/ffihost.go +++ b/go/internal/ffihost/ffihost.go @@ -207,6 +207,18 @@ type Host struct { callbackToken uintptr } +func (h *Host) rearmForeignSignalHandlers() { + if h.cliEntrypoint != "" { + rearmForeignSignalHandlers(h.lib.handle) + } +} + +// PrepareForChildProcessWait repairs signal handlers that the embedded runtime +// may have replaced before the Go process stops or waits for its own child. +func PrepareForChildProcessWait() { + rearmForeignSignalHandlers(0) +} + // Create resolves the native library and prepares the host. environment and // args contain SDK-managed runtime options. func Create(runtimeEntrypoint, cliEntrypoint string, environment map[string]string, args []string) (*Host, error) { @@ -260,10 +272,8 @@ func (h *Host) Start() error { return fmt.Errorf("copilot_runtime_host_start failed (library %q)", h.libraryPath) } - if h.cliEntrypoint != "" { - // A legacy embedded host may install a SIGCHLD handler without SA_ONSTACK. - rearmForeignSignalHandlers(h.lib.handle) - } + // An embedded host may install a SIGCHLD handler without SA_ONSTACK. + h.rearmForeignSignalHandlers() callbackHandle := sharedOutboundCallback() callbackToken := uintptr(nextOutboundToken.Add(1)) @@ -273,13 +283,15 @@ func (h *Host) Start() error { if h.connectionID == 0 { outboundTargets.Delete(callbackToken) h.callbackToken = 0 + h.rearmForeignSignalHandlers() h.lib.hostShutdown(h.serverID) - if h.cliEntrypoint != "" { - rearmForeignSignalHandlers(h.lib.handle) - } + h.rearmForeignSignalHandlers() h.serverID = 0 return fmt.Errorf("copilot_runtime_connection_open failed") } + // Connection initialization may install Tokio's SIGCHLD handler after + // host startup, so repair it again before any child process can exit. + h.rearmForeignSignalHandlers() return nil } @@ -360,6 +372,9 @@ func (h *Host) writeFrame(frame []byte) (int, error) { if len(frame) == 0 { return 0, nil } + // A prior runtime request may have installed or restored Tokio's SIGCHLD + // handler. Repair it before another request can stop a native child process. + h.rearmForeignSignalHandlers() ok := h.lib.connectionWrite(connID, unsafe.Pointer(&frame[0]), uintptr(len(frame))) runtime.KeepAlive(frame) if !ok { @@ -392,6 +407,7 @@ func (h *Host) tryFinalizeCleanupLocked() bool { connID := h.connectionID if connID != 0 { + h.rearmForeignSignalHandlers() if !h.lib.connectionClose(connID) { return false } @@ -403,17 +419,15 @@ func (h *Host) tryFinalizeCleanupLocked() bool { if callbackToken != 0 { outboundTargets.Delete(callbackToken) } - serverID := h.serverID if serverID != 0 { + h.rearmForeignSignalHandlers() if !h.lib.hostShutdown(serverID) { log.Printf("FfiRuntimeHost: host_shutdown did not recognize server %d", serverID) } h.serverID = 0 - if h.cliEntrypoint != "" { - // A legacy host may restore its saved SIGCHLD action during shutdown. - rearmForeignSignalHandlers(h.lib.handle) - } + // A host may restore its saved SIGCHLD action during shutdown. + h.rearmForeignSignalHandlers() } return true } diff --git a/go/internal/ffihost/sigonstack_darwin.go b/go/internal/ffihost/sigonstack_darwin.go index 2e7d2a99b7..0663f17063 100644 --- a/go/internal/ffihost/sigonstack_darwin.go +++ b/go/internal/ffihost/sigonstack_darwin.go @@ -26,15 +26,13 @@ const ( ) // 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 (enforced by the Go runtime on -// both macOS and Linux; the Linux variant lives in sigonstack_linux.go). +// installed by the native runtime that omitted it. Tokio's process-global +// SIGCHLD registration through signal-hook-registry chains Go's handler but +// replaces its flags without SA_ONSTACK. The Go runtime aborts with "non-Go code +// set up signal handler without SA_ONSTACK flag" when SIGCHLD (signal 20 on +// Darwin) is delivered while a Go-managed child process is reaped. // -// We preserve each foreign handler and merely OR in SA_ONSTACK, so libuv's child +// We preserve each foreign handler and merely OR in SA_ONSTACK, so Tokio'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 diff --git a/go/internal/ffihost/sigonstack_linux.go b/go/internal/ffihost/sigonstack_linux.go index 668cf67055..6c422fd96a 100644 --- a/go/internal/ffihost/sigonstack_linux.go +++ b/go/internal/ffihost/sigonstack_linux.go @@ -26,14 +26,13 @@ type linuxSigaction struct { } // 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. +// installed by the native runtime that omitted it. Tokio's process-global +// SIGCHLD registration through signal-hook-registry chains Go's handler but +// replaces its flags without SA_ONSTACK. The Go runtime aborts with "non-Go code +// set up signal handler without SA_ONSTACK flag" when SIGCHLD (signal 17 on +// Linux) is delivered while a Go-managed child process is reaped. // -// We preserve each foreign handler and merely OR in SA_ONSTACK, so libuv's child +// We preserve each foreign handler and merely OR in SA_ONSTACK, so Tokio'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 diff --git a/go/internal/ffihost/sigonstack_linux_test.go b/go/internal/ffihost/sigonstack_linux_test.go index 3a382385f2..2f40026899 100644 --- a/go/internal/ffihost/sigonstack_linux_test.go +++ b/go/internal/ffihost/sigonstack_linux_test.go @@ -7,6 +7,7 @@ import ( "os/signal" "syscall" "testing" + "unsafe" ) func TestRearmForeignSignalHandlersAddsOnStack(t *testing.T) { @@ -36,3 +37,82 @@ func TestRearmForeignSignalHandlersAddsOnStack(t *testing.T) { t.Fatal("SA_ONSTACK was not restored") } } + +func TestHostRearmsSignalHandlersAroundNativeOperations(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) + + host := &Host{ + cliEntrypoint: "copilot", + lib: &ffiLibrary{ + hostStart: func(unsafe.Pointer, uintptr, unsafe.Pointer, uintptr) uint32 { + return 1 + }, + connectionOpen: func(uint32, uintptr, uintptr, unsafe.Pointer, uintptr, unsafe.Pointer, uintptr, unsafe.Pointer, uintptr) uint32 { + withoutOnStack := original + withoutOnStack.flags &^= linuxSaOnStack + if !linuxSetSigaction(int(syscall.SIGUSR1), &withoutOnStack) { + t.Fatal("failed to clear SA_ONSTACK during connection initialization") + } + return 2 + }, + connectionWrite: func(uint32, unsafe.Pointer, uintptr) bool { + assertSignalHandlerOnStack(t, "connection write") + return true + }, + connectionClose: func(uint32) bool { + assertSignalHandlerOnStack(t, "connection close") + return true + }, + hostShutdown: func(uint32) bool { + assertSignalHandlerOnStack(t, "host shutdown") + return true + }, + }, + recv: newReceiveBuffer(), + } + if err := host.Start(); err != nil { + t.Fatal(err) + } + defer host.Dispose() + + 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 after connection initialization") + } + + withoutOnStack := original + withoutOnStack.flags &^= linuxSaOnStack + if !linuxSetSigaction(int(syscall.SIGUSR1), &withoutOnStack) { + t.Fatal("failed to clear SA_ONSTACK before connection write") + } + if _, err := host.writeFrame([]byte("request")); err != nil { + t.Fatal(err) + } + + if !linuxSetSigaction(int(syscall.SIGUSR1), &withoutOnStack) { + t.Fatal("failed to clear SA_ONSTACK before disposal") + } + host.Dispose() +} + +func assertSignalHandlerOnStack(t *testing.T, operation string) { + t.Helper() + var action linuxSigaction + if !linuxGetSigaction(int(syscall.SIGUSR1), &action) { + t.Fatalf("failed to read SIGUSR1 action before %s", operation) + } + if action.flags&linuxSaOnStack == 0 { + t.Fatalf("SA_ONSTACK was not restored before %s", operation) + } +} diff --git a/go/internal/ffihost/sigonstack_other.go b/go/internal/ffihost/sigonstack_other.go index 9da78255f6..6f40992883 100644 --- a/go/internal/ffihost/sigonstack_other.go +++ b/go/internal/ffihost/sigonstack_other.go @@ -6,5 +6,5 @@ package ffihost // 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. +// installed by Tokio that the Go runtime rejects; Windows is unaffected. func rearmForeignSignalHandlers(_ uintptr) {} diff --git a/nodejs/package.json b/nodejs/package.json index 043751ff14..1c255f7e09 100644 --- a/nodejs/package.json +++ b/nodejs/package.json @@ -5,7 +5,7 @@ "url": "https://github.com/github/copilot-sdk.git" }, "version": "0.0.0-dev", - "copilotCliVersion": "1.0.85", + "copilotCliVersion": "1.0.86-0", "description": "TypeScript SDK for programmatic control of GitHub Copilot CLI via JSON-RPC", "main": "./dist/cjs/index.js", "types": "./dist/index.d.ts", diff --git a/nodejs/src/cliVersion.ts b/nodejs/src/cliVersion.ts index bdaffb487f..3eb4b744ab 100644 --- a/nodejs/src/cliVersion.ts +++ b/nodejs/src/cliVersion.ts @@ -1,3 +1,3 @@ -export const COPILOT_CLI_VERSION = "1.0.85"; +export const COPILOT_CLI_VERSION = "1.0.86-0"; export const COPILOT_CLI_USE_NPM_PACKAGE = false;