From 18ccad4bb66632da811c0ca42c21c173b4fdc5af Mon Sep 17 00:00:00 2001 From: seal Date: Sat, 15 Aug 2026 23:45:32 -0400 Subject: [PATCH] feat(compass-app): per-window frame routing in the bridge service (SEA-2035 M3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Route each compass_rpc call's response frames to only its originating webview window instead of broadcasting app-wide, so a stream never reaches a non-owning window once more than one is open. The originating window is captured off the still-live bound-method ctx in register (application.WindowKey) and stored on the inflightCall; the pump callback emits through a per-call sink (emitFrame) that dispatches to that window via the windowDispatcher seam, falling back to the app-wide eventEmitter when no window is in context. A frame for a call whose window has closed is dropped by DispatchWailsEvent's isDestroyed no-op (A4), not fallback-broadcast. The application package compiles only under -tags gtk3 on this toolchain, so the ctx read + Wails adapter live in a build-tagged helper (bridge_service_window_gtk3.go / _nogtk3.go), mirroring the main.go/main_nogtk3.go split; bridge_service.go stays //go:build unix and imports no application, keeping the forwarding path testable behind the seam. newBridgeService, CompassRPCCancel, register's context derivation, finish, and the mutex discipline are unchanged. Tests (unix-tagged, no webview): per-window delivery, a concurrent single-service two-window isolation arm (§M3, no cross-window delivery), the preserved no-window fallback, and a destroyed-window drop. All event-gated, race-clean. Spec-impact: implements docs/designs/product/compass-multi-window/design.md §A4 + §M3 (per-window frame routing); no spec change. Ledger-impact: none. Co-authored-by: Matt Wilkinson --- go/cmd/compass-app/bridge_service.go | 73 +++++- go/cmd/compass-app/bridge_service_test.go | 243 ++++++++++++++++++ .../compass-app/bridge_service_window_gtk3.go | 50 ++++ .../bridge_service_window_nogtk3.go | 21 ++ 4 files changed, 385 insertions(+), 2 deletions(-) create mode 100644 go/cmd/compass-app/bridge_service_window_gtk3.go create mode 100644 go/cmd/compass-app/bridge_service_window_nogtk3.go diff --git a/go/cmd/compass-app/bridge_service.go b/go/cmd/compass-app/bridge_service.go index c04c6e8a..c0f59ba9 100644 --- a/go/cmd/compass-app/bridge_service.go +++ b/go/cmd/compass-app/bridge_service.go @@ -43,6 +43,30 @@ type eventEmitter interface { Emit(name string, data ...any) bool } +// windowDispatcher is the per-window analogue of the eventEmitter seam: it +// delivers one response frame to a SINGLE originating webview window, rather +// than app-wide. The real Wails *application.WebviewWindow satisfies it via +// DispatchWailsEvent (webview_window.go:1372); a test substitutes a fake that +// records deliveries with no webview. +// +// The seam is why this file stays //go:build unix and imports no +// github.com/wailsapp/wails/v3/pkg/application: that package only compiles under +// -tags gtk3 on this toolchain (the default GTK4/WebKit6.0 path has no +// pkg-config here; main_nogtk3.go documents it repo-wide), so a direct import +// would break the untagged module build and the unix-tagged tests. The concrete +// window handle is captured from the bound-method ctx by windowFromContext, a +// build-tagged helper (bridge_service_window_gtk3.go reads application.WindowKey +// and returns the window; the nogtk3 stub returns nil), mirroring the +// main.go / main_nogtk3.go split. §A4/§M3 mandate exactly this: per-window +// routing behind the existing eventEmitter seam. +type windowDispatcher interface { + // dispatch delivers one frame to this window's webview under the given event + // name. It no-ops for a destroyed window (the real DispatchWailsEvent guards + // isDestroyed(), webview_window.go:1373), so a frame for a closed window's + // call is dropped rather than broadcast (A4). + dispatch(name string, resp responseFrame) +} + // bridgeService binds the compass_rpc / compass_rpc_cancel IPC methods and // forwards each call through the pump. In-flight calls are tracked by requestId // so compass_rpc_cancel can tear one down; the map is mutex-guarded because the @@ -86,6 +110,17 @@ type bridgeService struct { // never has its live entry mis-deleted by a prior call's deferred finish. type inflightCall struct { cancel context.CancelFunc + + // window is the originating webview window for this call, captured from the + // bound-method ctx by windowFromContext at register time — it is gone from + // ctx by the time the pump goroutine emits, so it is captured up front and + // stored here (record §M3:354). This call's response frames route to this + // window only, via the windowDispatcher seam; nil means no originating window + // was in context (a windowless transport, a non-gtk3 build, or a direct test + // call), and the frames fall back to the app-wide eventEmitter. The interface + // value is comparable, so it is also the index M3b's close-time + // cancel-all-for-window keys on. + window windowDispatcher } // newBridgeService builds a bridge service that forwards against pump and emits @@ -287,9 +322,20 @@ func (s *bridgeService) Connect(ctx context.Context, req connectRequest) connect // bound-method invocation's lifetime — the stream must outlive CompassRPC's // return and is torn down only by a terminal frame or an explicit // compass_rpc_cancel, never by Wails reclaiming the call context. +// +// The originating window is captured HERE, off the still-live bound-method ctx, +// by windowFromContext (a build-tagged helper: the gtk3 build reads +// application.WindowKey, set by Wails at messageprocessor_call.go:136, and +// returns the window; the nogtk3 build returns nil). WithoutCancel would preserve +// the value on callCtx too, but the frames emit on the pump goroutine after +// CompassRPC has returned, so the handle is read synchronously up front and +// stored (record §M3:354). A nil result (no window in ctx — a windowless +// transport, a non-gtk3 build, or a direct test call) routes frames to the +// app-wide fallback. func (s *bridgeService) register(ctx context.Context, requestID string) (context.Context, *inflightCall) { + win := windowFromContext(ctx) callCtx, cancel := context.WithCancel(context.WithoutCancel(ctx)) - call := &inflightCall{cancel: cancel} + call := &inflightCall{cancel: cancel, window: win} s.mu.Lock() if prev, ok := s.inflight[requestID]; ok { prev.cancel() @@ -314,10 +360,33 @@ func (s *bridgeService) run(callCtx context.Context, call *inflightCall, req rpc Body: req.Body, } s.pump.Do(callCtx, rpc, func(f bridge.Frame) { - s.events.Emit(eventName, frameToResponse(f)) + s.emitFrame(call, eventName, frameToResponse(f)) }) } +// emitFrame is the per-call response-frame sink: it routes one frame to the +// call's originating window when one was captured, else to the app-wide emitter. +// +// A captured window gets the frame via the windowDispatcher seam — the real one +// is *application.WebviewWindow.DispatchWailsEvent (per-window delivery, +// webview_window.go:1372), so the app-wide broadcast (transport_event_ipc.go) is +// bypassed and a frame never reaches a non-owning window. That method internally +// no-ops once the window isDestroyed() (webview_window.go:1373), so a frame for a +// call whose window has since closed is silently dropped rather than broadcast — +// M3 routes and inherits that drop; it adds no destroyed check of its own (A4). +// +// A nil window (no window in the caller's ctx — a windowless transport, a +// non-gtk3 build, or a direct test call) falls back to the app-wide eventEmitter +// seam (main.go:104), preserving both non-window callers and the fake-emitter +// test path (§M3). +func (s *bridgeService) emitFrame(call *inflightCall, name string, resp responseFrame) { + if call.window != nil { + call.window.dispatch(name, resp) + return + } + s.events.Emit(name, resp) +} + // finish drops the in-flight entry for a completed call, then cancels its own // context (idempotent). Deletion is guarded by pointer identity: it removes the // entry only if the CURRENT entry under requestID is still THIS call, so a call diff --git a/go/cmd/compass-app/bridge_service_test.go b/go/cmd/compass-app/bridge_service_test.go index 6f885e8f..6c4471af 100644 --- a/go/cmd/compass-app/bridge_service_test.go +++ b/go/cmd/compass-app/bridge_service_test.go @@ -540,3 +540,246 @@ func TestAccountIDBoundGetter(t *testing.T) { t.Errorf("AccountID = %q, want acc-resolved", got) } } + +// fakeWindow is a windowDispatcher test double: it records each per-window +// delivery (event name + decoded frame) onto a buffered channel, the per-window +// analogue of fakeEmitter. It stands in for a real *application.WebviewWindow so +// the routed frame path is verified without the GTK webview stack (which does +// not compile under the unix test tag). The service consults call.window through +// the windowDispatcher seam, so a test injects one by setting inflightCall.window +// directly — windowFromContext returns nil in the non-gtk3 test build. +type fakeWindow struct { + ch chan emitted +} + +func newFakeWindow() *fakeWindow { + return &fakeWindow{ch: make(chan emitted, 64)} +} + +func (w *fakeWindow) dispatch(name string, resp responseFrame) { + w.ch <- emitted{name: name, frame: resp} +} + +// recvWindow receives one delivery on a fake window's channel or fails on +// timeout (event-gated, no sleeps), mirroring recv for the emitter path. +func recvWindow(t *testing.T, w *fakeWindow) emitted { + t.Helper() + select { + case ev := <-w.ch: + return ev + case <-time.After(testTimeout): + t.Fatal("timed out waiting for a per-window delivery") + return emitted{} + } +} + +// assertNoEmit fails if the app-wide emitter received any frame within a short +// window — proving a windowed call did NOT fall back to the broadcast path. It +// is the negative half of per-window routing: frames go to the window's channel +// ONLY. A short deadline bounds the check; the frames it guards against are +// emitted synchronously on the same pump goroutine that already delivered to the +// window, so by the time the window's terminal frame is observed, any stray Emit +// would already be buffered. +func assertNoEmit(t *testing.T, e *fakeEmitter) { + t.Helper() + select { + case ev := <-e.ch: + t.Fatalf("app-wide Emit received %q frame for a windowed call; want per-window only", ev.frame.Kind) + case <-time.After(50 * time.Millisecond): + } +} + +// runWindowed drives one call synchronously with its window handle injected on +// the inflightCall, returning after every frame has been routed and the entry +// cleared (the deterministic single-shot pattern of TestCompassRPCUnaryRoundTrip). +// The window is set on the registered call directly because windowFromContext +// returns nil in the non-gtk3 test build; the seam field is exactly what the +// routed sink (emitFrame) consults. +func runWindowed(svc *bridgeService, win windowDispatcher, req rpcRequest) { + callCtx, call := svc.register(context.Background(), req.RequestID) + call.window = win + svc.run(callCtx, call, req) +} + +// TestCompassRPCRoutesToOriginatingWindow proves M3's core behavior: a call with +// a captured window routes ALL its frames to THAT window's dispatcher (per-window +// delivery), and NOT to the app-wide emitter — so with more than one window open +// a frame never broadcasts to a non-owning window. +func TestCompassRPCRoutesToOriginatingWindow(t *testing.T) { + respBody := []byte("windowed-response") + socket := stubServer(t, func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/grpc-web+proto") + w.WriteHeader(http.StatusOK) + _, _ = w.Write(respBody) + }) + + svc, emitter := newService(socket) + win := newFakeWindow() + const requestID = "req-windowed" + runWindowed(svc, win, rpcRequest{RequestID: requestID, Path: "/compass.v1.Service/Method"}) + + // head -> body -> end, all on the WINDOW's channel under the per-id event. + head := recvWindow(t, win) + if head.name != "compass_rpc:"+requestID { + t.Errorf("window delivery name = %q, want per-requestId key", head.name) + } + if head.frame.Kind != frameKindHead { + t.Fatalf("frame[0].kind = %q, want head", head.frame.Kind) + } + if body := recvWindow(t, win); body.frame.Kind != frameKindBody { + t.Fatalf("frame[1].kind = %q, want body", body.frame.Kind) + } + if end := recvWindow(t, win); end.frame.Kind != frameKindEnd { + t.Fatalf("frame[2].kind = %q, want end", end.frame.Kind) + } + + // The app-wide emitter saw nothing: a windowed call never broadcasts. + assertNoEmit(t, emitter) + assertNotInflight(t, svc, requestID) +} + +// TestCompassRPCConcurrentTwoWindowIsolation is the §M3 two-window arm on the +// concurrent-distinct-ids shape: ONE bridgeService (the production shape — a +// single service is created once in launch()) drives two calls CONCURRENTLY, +// each carrying a DIFFERENT originating window, sharing the one mutex-guarded +// inflight map. Each call's frames must land ONLY on its own window's channel, +// never the other's and never the app-wide emitter. This is the arm that would +// actually catch a routing regression: a service-scoped (rather than per-call) +// window handle, or a closure that captured the wrong call, would cross-deliver +// here — where two independent services could not. Event-gated, no sleeps. +func TestCompassRPCConcurrentTwoWindowIsolation(t *testing.T) { + handler := func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/grpc-web+proto") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("ok")) + } + socket := stubServer(t, handler) + + svc, emitter := newService(socket) + winA := newFakeWindow() + winB := newFakeWindow() + const idA = "req-winA" + const idB = "req-winB" + + // Drive both calls on the ONE service concurrently, mirroring CompassRPC's + // own register-then-go-run, with each call's originating window injected on + // the registered inflightCall (windowFromContext returns nil in this + // non-gtk3 test build, so the seam field is set directly). The two run + // goroutines share svc.inflight — the concurrency the arm exists to stress. + // Each run signals done on return (after its deferred finish clears the + // inflight entry), so the not-inflight assertion is gated on completion + // rather than racing the goroutine — event-gated, no sleeps. + launch := func(win windowDispatcher, req rpcRequest) chan struct{} { + callCtx, call := svc.register(context.Background(), req.RequestID) + call.window = win + done := make(chan struct{}) + go func() { + defer close(done) + svc.run(callCtx, call, req) + }() + return done + } + doneA := launch(winA, rpcRequest{RequestID: idA, Path: "/a"}) + doneB := launch(winB, rpcRequest{RequestID: idB, Path: "/b"}) + + // Each window receives its own stream through the terminal frame; every + // frame must carry that window's own per-id event name (no cross-delivery). + drainWindow(t, winA, idA) + drainWindow(t, winB, idB) + + // Both runs returned (deferred finish cleared each entry); assert teardown. + waitDone(t, doneA) + waitDone(t, doneB) + + // Neither concurrent call fell back to the app-wide broadcast. + assertNoEmit(t, emitter) + assertNotInflight(t, svc, idA) + assertNotInflight(t, svc, idB) +} + +// drainWindow reads frames off a window's channel through the terminal end frame, +// asserting every one carries the expected per-id event name (no cross-delivery). +func drainWindow(t *testing.T, w *fakeWindow, requestID string) { + t.Helper() + want := "compass_rpc:" + requestID + for { + ev := recvWindow(t, w) + if ev.name != want { + t.Errorf("cross-window delivery: got %q, want %q", ev.name, want) + } + if ev.frame.Kind == frameKindEnd || ev.frame.Kind == frameKindError { + return + } + } +} + +// waitDone blocks until a run goroutine signals completion, or fails on timeout +// (event-gated, no sleeps) — the completion gate for a concurrently-driven call. +func waitDone(t *testing.T, done <-chan struct{}) { + t.Helper() + select { + case <-done: + case <-time.After(testTimeout): + t.Fatal("timed out waiting for a run goroutine to finish") + } +} + +// TestCompassRPCNoWindowFallsBackToEmit pins the preserved fallback: a call with +// no captured window emits through the app-wide eventEmitter seam (the fakeEmitter +// path), exactly as before M3. This is the invariant the existing no-window tests +// (which drive CompassRPC with a plain context) depend on. +func TestCompassRPCNoWindowFallsBackToEmit(t *testing.T) { + socket := stubServer(t, func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/grpc-web+proto") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("ok")) + }) + svc, emitter := newService(socket) + const requestID = "req-nowindow" + // No window injected (call.window stays nil), matching windowFromContext's + // nil result for a windowless caller. + callCtx, call := svc.register(context.Background(), requestID) + if call.window != nil { + t.Fatalf("no-window register captured a window %v, want nil", call.window) + } + svc.run(callCtx, call, rpcRequest{RequestID: requestID, Path: "/x"}) + + if head := recv(t, emitter); head.name != "compass_rpc:"+requestID || head.frame.Kind != frameKindHead { + t.Fatalf("head = (%q,%q), want (compass_rpc:%s, head)", head.name, head.frame.Kind, requestID) + } + assertNotInflight(t, svc, requestID) +} + +// TestCompassRPCDestroyedWindowDropsFrames pins A4: a frame for a call whose +// window has closed is DROPPED, not fallback-broadcast. The real +// *application.WebviewWindow.DispatchWailsEvent no-ops once isDestroyed() +// (webview_window.go:1373); through the windowDispatcher seam a "destroyed" +// window is one whose dispatch is a no-op. The call must still complete and tear +// down cleanly, with NO frame delivered anywhere (not the window, not the +// app-wide emitter) and no panic. +func TestCompassRPCDestroyedWindowDropsFrames(t *testing.T) { + socket := stubServer(t, func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/grpc-web+proto") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("dropped")) + }) + svc, emitter := newService(socket) + const requestID = "req-destroyed" + // A destroyed window: dispatch is a no-op, modeling DispatchWailsEvent's + // isDestroyed() guard. Routing still targets it (not the fallback), so the + // frames are dropped rather than broadcast app-wide. + runWindowed(svc, destroyedWindow{}, rpcRequest{RequestID: requestID, Path: "/gone"}) + + // Nothing reached the app-wide emitter: the drop is NOT a fallback broadcast. + assertNoEmit(t, emitter) + // The call finished and cleaned up despite every frame being dropped. + assertNotInflight(t, svc, requestID) +} + +// destroyedWindow is a windowDispatcher whose dispatch is a no-op, modeling a +// window whose DispatchWailsEvent has begun no-opping because the window +// isDestroyed() (webview_window.go:1373). Frames routed to it are silently +// dropped — the A4 destroyed-window behavior. +type destroyedWindow struct{} + +func (destroyedWindow) dispatch(string, responseFrame) {} diff --git a/go/cmd/compass-app/bridge_service_window_gtk3.go b/go/cmd/compass-app/bridge_service_window_gtk3.go new file mode 100644 index 00000000..91066e33 --- /dev/null +++ b/go/cmd/compass-app/bridge_service_window_gtk3.go @@ -0,0 +1,50 @@ +//go:build unix && gtk3 + +// The gtk3 build's binding of the bridge service's per-window frame routing +// (M3) to the real Wails window API. It is split out of bridge_service.go +// (//go:build unix) so that file — and its unix-tagged tests — never import +// github.com/wailsapp/wails/v3/pkg/application, which only compiles under +// -tags gtk3 on this toolchain (the default GTK4/WebKit6.0 path has no +// pkg-config here; main_nogtk3.go documents it repo-wide). This mirrors the +// main.go / main_nogtk3.go split: the real shell is the gtk3 binary, and the +// windowDispatcher seam (bridge_service.go) is what keeps the forwarding path +// testable without the GTK stack. +package main + +import ( + "context" + + "github.com/wailsapp/wails/v3/pkg/application" +) + +// windowFromContext extracts the originating webview window from a bound-method +// call context. Wails puts the calling window under application.WindowKey +// (messageprocessor_call.go:16,136); the comma-ok assertion yields a nil window +// when no key is present (a windowless transport, or a direct test call). +// +// It returns an UNTYPED-nil windowDispatcher when no window is present — never a +// typed nil wrapping a nil *application.WebviewWindow — so the nil check in +// emitFrame (call.window != nil) correctly selects the app-wide fallback. +func windowFromContext(ctx context.Context) windowDispatcher { + win, ok := ctx.Value(application.WindowKey).(application.Window) + if !ok || win == nil { + return nil + } + return wailsWindowDispatcher{win: win} +} + +// wailsWindowDispatcher adapts an application.Window to the windowDispatcher +// seam: dispatch delivers one response frame to that window's webview via +// DispatchWailsEvent (webview_window.go:1372), the per-window analogue of the +// app-wide eventEmitter.Emit. The struct is comparable (a single interface +// field), so an inflightCall.window value can index M3b's cancel-all-for-window. +type wailsWindowDispatcher struct { + win application.Window +} + +// dispatch routes one frame to this window's webview only. DispatchWailsEvent +// internally no-ops once the window isDestroyed() (webview_window.go:1373), so a +// frame for a call whose window has since closed is dropped, not broadcast (A4). +func (d wailsWindowDispatcher) dispatch(name string, resp responseFrame) { + d.win.DispatchWailsEvent(&application.CustomEvent{Name: name, Data: resp}) +} diff --git a/go/cmd/compass-app/bridge_service_window_nogtk3.go b/go/cmd/compass-app/bridge_service_window_nogtk3.go new file mode 100644 index 00000000..8478f354 --- /dev/null +++ b/go/cmd/compass-app/bridge_service_window_nogtk3.go @@ -0,0 +1,21 @@ +//go:build unix && !gtk3 + +// The non-gtk3 build's stub for the bridge service's per-window frame routing +// (M3). The untagged module build (`go build ./...`) and the unix-tagged bridge +// tests compile without the GTK stack, where github.com/wailsapp/wails/v3/pkg/ +// application is unavailable (main_nogtk3.go documents why). There is no webview +// window to route to in this build, so windowFromContext always returns a nil +// dispatcher and every call falls back to the app-wide eventEmitter — the same +// behavior the bridge had before M3. The real per-window routing lives in +// bridge_service_window_gtk3.go, compiled into the shipped gtk3 shell. +package main + +import "context" + +// windowFromContext returns a nil windowDispatcher in the non-gtk3 build: there +// is no Wails window API here, so no call carries an originating window and the +// frame sink uses the app-wide eventEmitter fallback (bridge_service.go +// emitFrame). Matching windowFromContext in bridge_service_window_gtk3.go. +func windowFromContext(_ context.Context) windowDispatcher { + return nil +}