From 048ab8dcd36255589c7fb5ca4a41a796fc22e52c Mon Sep 17 00:00:00 2001 From: Ali Al Dallal Date: Tue, 18 Aug 2026 07:14:09 -0400 Subject: [PATCH] fix: bridge smoke liveness probe -- Signal(nil) declared every launch dead Process.Signal's Unix implementation type-asserts its argument to syscall.Signal; a nil interface fails that assertion and returns an error for a perfectly alive process, so waitForBridge's early-exit branch fired on the first poll of every launch -- the identical 'connection refused' failure on CI hosted runners and locally. syscall.Signal(0) is the real kill(2) liveness probe. Regression test pins a live child probing false and a reaped child probing true. testing.md's parity verdict rewritten: the headless-windowing lead was never tested (the harness failed first); remaining work is registry calibration against Mill's real multi-window shape, tracked as a goal. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01FW5GkkAG8du7tNdYLk2zSd --- .claude/rules/testing.md | 40 +++++++++++++----------- internal/webviewbridgesmoke/main.go | 12 +++++-- internal/webviewbridgesmoke/main_test.go | 27 ++++++++++++++++ 3 files changed, 57 insertions(+), 22 deletions(-) diff --git a/.claude/rules/testing.md b/.claude/rules/testing.md index d8940f9d..42239cd4 100644 --- a/.claude/rules/testing.md +++ b/.claude/rules/testing.md @@ -144,25 +144,27 @@ layer per capability," never "a seed per thing": probe only, never CI-badged as parity. Revisit trigger: grow the check registry when a WebKit-only bug escapes it, same discipline as the manual-only registry below. - **CI status: non-required/informational, first live attempt - failed.** The `webview-bridge-smoke` job in `ci.yml` ran on a real - `macos-latest` GitHub-hosted runner and failed launching the app: - `app process exited before the MCP bridge became reachable: dial - tcp 127.0.0.1:9099: connect: connection refused`, with an EMPTY - captured stderr tail — the go/npm builds both succeeded, but the - spawned app process produced no diagnostic output at all before - exiting. First investigation lead: a silent, near-instant exit with - zero stderr is consistent with an early native-windowing failure - (no real console/WindowServer session available the way the - precedent research assumed from Electron's own hosted-runner CI — - Electron's windowing stack may not need the same session a genuine - Wails/Cocoa `NSWindow` does), though the log alone doesn't prove - that specific cause; config-dir/signing weren't ruled in or out - either. Not iterated further pending a dedicated investigation - pass. Until resolved, the OPERATIVE parity gate is the LOCAL - pre-release run (`scripts/webview-bridge-smoke.sh`, run by hand - before a release) — the CI job stays wired and green-or-red - visible, but nothing depends on it passing yet. + **CI status: non-required/informational; the launch failure was + the harness's own liveness probe, now fixed.** Every "app process + exited before the MCP bridge became reachable" failure (CI and + local alike) traced to `procExited` calling `Process.Signal(nil)`: + the Unix implementation type-asserts its argument to + `syscall.Signal`, a nil interface fails that assertion, and the + probe declared every perfectly-alive launch dead on its first + poll. The earlier headless-windowing lead was never actually + tested — the harness failed before the app could boot. With + `syscall.Signal(0)` the local run connects and drives the real + registry (the click-model and selection-ring checks have passed + against the real WKWebView). What remains before the job means + anything: the CHECK REGISTRY was written blind against a + single-window assumption — Mill's desktop build opens multiple + windows (main + the Quick Panel's second window), so + `app-info-window-sane` and the bridge's window-ambiguous clicks + and badge locator need calibrating against the app's real window + shape (tracked as its own goal). Until that lands, the OPERATIVE + parity gate remains the LOCAL run, read with the registry's + current miscalibration in mind; the CI job stays wired, + non-required. - **Manual-only registry** — OS-bound checks (hotkey delivery, real clipboard, tray) listed explicitly with reasons, never silently absent (see goal 0010's enforcement). Non-seed instance: the diff --git a/internal/webviewbridgesmoke/main.go b/internal/webviewbridgesmoke/main.go index d65c6e69..7e1d641e 100644 --- a/internal/webviewbridgesmoke/main.go +++ b/internal/webviewbridgesmoke/main.go @@ -22,6 +22,7 @@ import ( "os/exec" "path/filepath" "strconv" + "syscall" "time" ) @@ -206,10 +207,15 @@ func waitForBridge(client *mcpClient, proc *os.Process, timeout time.Duration) e } func procExited(proc *os.Process) bool { - // A nil signal probes liveness without affecting the process -- + // Signal 0 probes liveness without affecting the process -- // documented Unix kill(2) behaviour, exec.Process exposes no - // higher-level equivalent. - return proc.Signal(nil) != nil + // higher-level equivalent. It must be syscall.Signal(0), never a + // nil os.Signal: Signal's Unix implementation type-asserts its + // argument to syscall.Signal, and a nil interface fails that + // assertion, so Signal(nil) errors for a perfectly alive process + // and this probe would declare every launch dead on its first + // poll. + return proc.Signal(syscall.Signal(0)) != nil } // stopProcess quits exactly the PID this script itself launched -- diff --git a/internal/webviewbridgesmoke/main_test.go b/internal/webviewbridgesmoke/main_test.go index 116d1a85..839c1fe4 100644 --- a/internal/webviewbridgesmoke/main_test.go +++ b/internal/webviewbridgesmoke/main_test.go @@ -5,6 +5,7 @@ import ( "errors" "net" "os" + "os/exec" "path/filepath" "testing" ) @@ -135,3 +136,29 @@ func TestPortInUse(t *testing.T) { } }) } + +// Regression: the liveness probe must report a LIVE process as alive. +// Signal(nil) fails the Unix implementation's syscall.Signal type +// assertion and errors for any process, which made waitForBridge +// declare every app launch dead on its first poll. +func TestProcExited(t *testing.T) { + cmd := exec.CommandContext(context.Background(), "sleep", "30") + if err := cmd.Start(); err != nil { + t.Fatalf("start sleep: %v", err) + } + defer func() { _ = cmd.Process.Kill(); _, _ = cmd.Process.Wait() }() + + if procExited(cmd.Process) { + t.Fatal("procExited reported a live process as exited") + } + + if err := cmd.Process.Kill(); err != nil { + t.Fatalf("kill: %v", err) + } + if _, err := cmd.Process.Wait(); err != nil { + t.Fatalf("wait: %v", err) + } + if !procExited(cmd.Process) { + t.Fatal("procExited reported a reaped process as alive") + } +}