fix(server): correct socket location test on macOS

Christian Rocha and Charm Crush created 1 week ago

Co-Authored-By: Charm Crush <[e-mail]>

Change summary

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(-)

Detailed changes

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 {

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

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")

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-<uid>.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()
 }