From 4abf2066b1d432685e90a9b297d13153bbe3b1da Mon Sep 17 00:00:00 2001 From: Christian Rocha Date: Mon, 8 Jun 2026 09:59:30 -0400 Subject: [PATCH] fix(server): correct socket location test on macOS Co-Authored-By: Charm Crush --- internal/cmd/root.go | 2 +- internal/server/net_other.go | 2 +- internal/server/proto.go | 13 +++++++++---- internal/server/socket_test.go | 16 ++++++++++------ 4 files changed, 21 insertions(+), 12 deletions(-) diff --git a/internal/cmd/root.go b/internal/cmd/root.go index 6533133025b0cc98b1c7e134be5fbb1c806e1cff..4f9ef2d28e823e3f1eec92ccbbcd108d29b38ad9 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -437,7 +437,7 @@ func ensureServer(cmd *cobra.Command, hostURL *url.URL) error { // we detect it with a short DialTimeout and remove the // orphaned file so the normal spawn path can run. if hostURL.Scheme == "unix" { - conn, dialErr := net.DialTimeout( + conn, dialErr := net.DialTimeout( //nolint:noctx hostURL.Scheme, hostURL.Host, 200*time.Millisecond, ) if dialErr == nil { diff --git a/internal/server/net_other.go b/internal/server/net_other.go index 29b7dec2c3a47cb07767e1ddd0e73bf9c9bcbf07..502d1436b4a091bd04a4b42dff82c7fcfafff7a8 100644 --- a/internal/server/net_other.go +++ b/internal/server/net_other.go @@ -32,7 +32,7 @@ func listen(network, address string) (net.Listener, bool, error) { var removedStale bool if network == "unix" && address != "" { if _, err := os.Stat(address); err == nil { - conn, dialErr := net.DialTimeout(network, address, staleSocketDialTimeout) + conn, dialErr := net.DialTimeout(network, address, staleSocketDialTimeout) //nolint:noctx if dialErr == nil { // A live server owns the socket. Fall through to // net.Listen so the caller sees the standard diff --git a/internal/server/proto.go b/internal/server/proto.go index f388d51bb87490a484bcb06b16e4698058bae134..1d0dddece0b549115cb75934589c9ffc40cfaef1 100644 --- a/internal/server/proto.go +++ b/internal/server/proto.go @@ -261,16 +261,21 @@ func (c *controllerV1) handleGetWorkspaceEvents(w http.ResponseWriter, r *http.R if !ok { return } - if err := c.backend.AttachClient(id, clientID); err != nil { + // Subscribe to the event broker BEFORE attaching the client. + // AttachClient bumps the stream count that observers use to + // detect a live subscriber; subscribing first guarantees that + // once a client appears attached, any published event is + // delivered rather than dropped on a not-yet-registered stream. + events, err := c.backend.SubscribeEvents(r.Context(), id) + if err != nil { c.handleError(w, r, err) return } - defer c.backend.DetachClient(id, clientID) - events, err := c.backend.SubscribeEvents(r.Context(), id) - if err != nil { + if err := c.backend.AttachClient(id, clientID); err != nil { c.handleError(w, r, err) return } + defer c.backend.DetachClient(id, clientID) w.Header().Set("Content-Type", "text/event-stream") w.Header().Set("Cache-Control", "no-cache") diff --git a/internal/server/socket_test.go b/internal/server/socket_test.go index 494a1a0460c2f4d0b18541e49d98f995b3a1764d..38e3255866f09f86171f50d102dfd6f1cdd4ec8b 100644 --- a/internal/server/socket_test.go +++ b/internal/server/socket_test.go @@ -71,8 +71,12 @@ func TestDefaultHost_XDGRuntimeDir(t *testing.T) { path := strings.TrimPrefix(host, "unix://") // The composed path may exceed maxUnixSocketPathLen and fall back - // to /tmp; only assert containment when it did not. - if len(filepath.Join(dir, "crush.sock")) <= maxUnixSocketPathLen { + // to /tmp; only assert containment when it did not. Recompose the + // path under dir (rather than checking the returned path length, + // which is short again after a /tmp fallback) to decide whether a + // fallback happened. The socket is named crush-.sock. + composed := filepath.Join(dir, filepath.Base(path)) + if len(composed) <= maxUnixSocketPathLen { require.True(t, strings.HasPrefix(path, dir), "socket path %q should live under %q", path, dir) } @@ -104,7 +108,7 @@ func TestDefaultHost_FallbackTemp(t *testing.T) { // it. A leftover file is best-effort removed via t.Cleanup. func staleSocketPath(t *testing.T, path string) { t.Helper() - ln, err := net.Listen("unix", path) + ln, err := net.Listen("unix", path) //nolint:noctx require.NoError(t, err) ul, ok := ln.(*net.UnixListener) require.True(t, ok, "expected *net.UnixListener, got %T", ln) @@ -112,7 +116,7 @@ func staleSocketPath(t *testing.T, path string) { require.NoError(t, ul.Close()) // Verify it is actually stale: dialing should fail. - conn, dialErr := net.DialTimeout("unix", path, 200*time.Millisecond) + conn, dialErr := net.DialTimeout("unix", path, 200*time.Millisecond) //nolint:noctx if dialErr == nil { conn.Close() t.Fatalf("expected stale socket at %q to refuse connections", path) @@ -150,7 +154,7 @@ func TestListen_LiveSocketNotRemoved(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "s.sock") - ln1, err := net.Listen("unix", path) + ln1, err := net.Listen("unix", path) //nolint:noctx require.NoError(t, err) // Drain accepts so the listener stays alive and responsive without @@ -183,7 +187,7 @@ func TestListen_LiveSocketNotRemoved(t *testing.T) { // The live socket file must still be on disk and dialable. _, statErr := os.Stat(path) require.NoError(t, statErr, "live socket file should still exist") - conn, dialErr := net.DialTimeout("unix", path, 200*time.Millisecond) + conn, dialErr := net.DialTimeout("unix", path, 200*time.Millisecond) //nolint:noctx require.NoError(t, dialErr, "live socket should still accept dials") _ = conn.Close() }