[codex] Structure loopback port errors

[juliusmarminge]

Summary

• replace the generic optional-cause NetError with distinct Schema errors for listen, address, and release failures
• retain the host, observed address details, reserved port, and immediate Node cause where one exists
• handle synchronous listen failures and close callback failures without inventing causes

Validation

• vp test packages/shared/src/Net.test.ts --no-cache
• vp check (passes with pre-existing warnings)
• vp run typecheck
• vpr typecheck

Overlap audit

No current open PR touches either changed path, including previous filenames.

---

Note

Medium Risk
Breaking change for NetError shape and messages; reserveLoopbackPort failure typing affects startup port selection used by findAvailablePort.

Overview
Replaces the single optional-cause NetError class with three Schema tagged errors—LoopbackPortListenError, LoopbackPortAddressUnavailableError, and LoopbackPortReleaseError—and exposes NetError as a union type for the same failure channel.

reserveLoopbackPort now classifies failures by phase: listen/sync listen throws, unreadable or invalid ephemeral ports (via isUsablePort), and close callback or server error after a valid port was observed. Errors carry host, address/family/port where relevant, and the Node cause when present (no synthetic causes for bad ports).

NetService.make accepts optional createServer for injection (used in new tests with mocked servers). Callers that matched on the old NetError tag or message must narrow on the new union members.

^{Reviewed by Cursor Bugbot for commit ce790f3. Bugbot is set up for automated code reviews on this repo. Configure here.}

Note

Structure loopback port errors into typed variants in NetService

• Replaces the single generic NetError class with a discriminated union of three typed errors: LoopbackPortListenError, LoopbackPortAddressUnavailableError, and LoopbackPortReleaseError, each preserving host, port, address, and original cause fields.
• Rewrites reserveLoopbackPort in Net.ts to classify failures at each stage (listen, address resolution, close) and emit the appropriate typed error.
• Adds an isUsablePort predicate to validate ports as integers in the 1–65535 range; invalid or non-finite ports yield LoopbackPortAddressUnavailableError.
• Extends the make factory to accept an optional createServer injection for testing, used in Net.test.ts to simulate server failure scenarios.
• Behavioral Change: callers catching NetError must now handle a union type; the raw Error cause is preserved on each variant rather than wrapped.

^{Macroscope summarized ce790f3.}

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 3a97f8c7-9831-40f4-a10c-89ea95bcedf4

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/shared-net-errors

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[macroscopeapp]

Approvability

Verdict: Approved

This PR refactors error handling for loopback port reservation by splitting one generic error class into three specific error types with better diagnostic information. The changes are well-tested and self-contained to error classification, without altering the core port reservation functionality.

^{You can customize Macroscope's approvability policy. Learn more.}

Dismissing prior approval to re-evaluate 2581244

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

• ✅ Fixed: Release errors misclassified as listen
  • After a successful listen, the original error listener is now removed and replaced with one that correctly maps release-phase errors to LoopbackPortReleaseError, and the close callback no longer expects an error argument (matching Node.js API behavior).

Or push these changes by commenting:

@cursor push e1ea7d2fdf

Preview (e1ea7d2fdf)

diff --git a/packages/shared/src/Net.test.ts b/packages/shared/src/Net.test.ts
--- a/packages/shared/src/Net.test.ts
+++ b/packages/shared/src/Net.test.ts
@@ -2,9 +2,12 @@
 
 import { assert, describe, it } from "@effect/vitest";
 import * as Effect from "effect/Effect";
+import * as Schema from "effect/Schema";
 
 import * as NetService from "./Net.ts";
 
+const isLoopbackPortListenError = Schema.is(NetService.LoopbackPortListenError);
+
 const closeServer = (server: NodeNet.Server) =>
   Effect.sync(() => {
     try {
@@ -19,19 +22,19 @@
   return typeof address === "object" && address !== null ? address.port : 0;
 };
 
-const openServer = (host?: string): Effect.Effect<NodeNet.Server, NetService.NetError> =>
-  Effect.callback<NodeNet.Server, NetService.NetError>((resume) => {
+const openServer = (host?: string): Effect.Effect<NodeNet.Server, Error> =>
+  Effect.callback<NodeNet.Server, Error>((resume) => {
     const server = NodeNet.createServer();
     let settled = false;
 
-    const settle = (effect: Effect.Effect<NodeNet.Server, NetService.NetError>) => {
+    const settle = (effect: Effect.Effect<NodeNet.Server, Error>) => {
       if (settled) return;
       settled = true;
       resume(effect);
     };
 
     server.once("error", (cause) => {
-      settle(Effect.fail(new NetService.NetError({ host: host ?? "localhost", cause })));
+      settle(Effect.fail(cause));
     });
 
     if (host) {
@@ -45,11 +48,6 @@
 
 it.layer(NetService.layer)("NetService", (it) => {
   describe("Net helpers", () => {
-    it("preserves the loopback reservation error message", () => {
-      const error = new NetService.NetError({ host: "127.0.0.1" });
-      assert.equal(error.message, "Failed to reserve loopback port");
-    });
-
     it.effect("reserveLoopbackPort returns a positive loopback port", () =>
       Effect.gen(function* () {
         const net = yield* NetService.NetService;
@@ -59,6 +57,18 @@
       }),
     );
 
+    it.effect("retains the host and listen cause when reservation fails", () =>
+      Effect.gen(function* () {
+        const net = yield* NetService.NetService;
+        const error = yield* net.reserveLoopbackPort("256.256.256.256").pipe(Effect.flip);
+
+        assert(isLoopbackPortListenError(error));
+        assert.equal(error.host, "256.256.256.256");
+        assert.match(error.message, /256\.256\.256\.256/u);
+        assert.equal((error.cause as NodeJS.ErrnoException).code, "ENOTFOUND");
+      }),
+    );
+
     it.effect("isPortAvailableOnLoopback reports false for an occupied port", () =>
       Effect.acquireUseRelease(
         openServer("127.0.0.1"),

diff --git a/packages/shared/src/Net.ts b/packages/shared/src/Net.ts
--- a/packages/shared/src/Net.ts
+++ b/packages/shared/src/Net.ts
@@ -6,15 +6,52 @@
 import * as Predicate from "effect/Predicate";
 import * as Schema from "effect/Schema";
 
-export class NetError extends Schema.TaggedErrorClass<NetError>()("NetError", {
-  host: Schema.String,
-  cause: Schema.optional(Schema.Defect()),
-}) {
+export class LoopbackPortListenError extends Schema.TaggedErrorClass<LoopbackPortListenError>()(
+  "LoopbackPortListenError",
+  {
+    host: Schema.String,
+    cause: Schema.Defect(),
+  },
+) {
   override get message(): string {
-    return "Failed to reserve loopback port";
+    return `Failed to listen for an ephemeral loopback port on ${this.host}.`;
   }
 }
 
+export class LoopbackPortAddressUnavailableError extends Schema.TaggedErrorClass<LoopbackPortAddressUnavailableError>()(
+  "LoopbackPortAddressUnavailableError",
+  {
+    host: Schema.String,
+    address: Schema.NullOr(Schema.String),
+    family: Schema.NullOr(Schema.String),
+    port: Schema.NullOr(Schema.Number),
+  },
+) {
+  override get message(): string {
+    return `Failed to read a usable ephemeral loopback port for ${this.host} (address ${this.address ?? "unavailable"}, family ${this.family ?? "unavailable"}, port ${this.port ?? "unavailable"}).`;
+  }
+}
+
+export class LoopbackPortReleaseError extends Schema.TaggedErrorClass<LoopbackPortReleaseError>()(
+  "LoopbackPortReleaseError",
+  {
+    host: Schema.String,
+    port: Schema.Number,
+    cause: Schema.Defect(),
+  },
+) {
+  override get message(): string {
+    return `Failed to release ephemeral loopback port ${this.port} on ${this.host}.`;
+  }
+}
+
+export const NetError = Schema.Union([
+  LoopbackPortListenError,
+  LoopbackPortAddressUnavailableError,
+  LoopbackPortReleaseError,
+]);
+export type NetError = typeof NetError.Type;
+
 const isErrnoExceptionWithCode = (
   cause: unknown,
 ): cause is {
@@ -162,21 +199,60 @@
         resume(effect);
       };
 
-      probe.once("error", (cause) => {
-        settle(Effect.fail(new NetError({ host, cause })));
-      });
+      const onListenError = (cause: Error) => {
+        settle(Effect.fail(new LoopbackPortListenError({ host, cause })));
+      };
 
-      probe.listen(0, host, () => {
-        const address = probe.address();
-        const port = typeof address === "object" && address !== null ? address.port : 0;
-        probe.close(() => {
-          if (port > 0) {
-            settle(Effect.succeed(port));
-            return;
-          }
-          settle(Effect.fail(new NetError({ host })));
+      probe.once("error", onListenError);
+
+      try {
+        probe.listen(0, host, () => {
+          probe.removeListener("error", onListenError);
+
+          const address = probe.address();
+          const addressDetails =
+            typeof address === "object" && address !== null
+              ? {
+                  address: address.address,
+                  family: address.family,
+                  port: address.port,
+                }
+              : {
+                  address,
+                  family: null,
+                  port: null,
+                };
+
+          probe.once("error", (cause) => {
+            settle(
+              Effect.fail(
+                new LoopbackPortReleaseError({
+                  host,
+                  port: addressDetails.port ?? 0,
+                  cause,
+                }),
+              ),
+            );
+          });
+
+          probe.close(() => {
+            if (addressDetails.port !== null && addressDetails.port > 0) {
+              settle(Effect.succeed(addressDetails.port));
+              return;
+            }
+            settle(
+              Effect.fail(
+                new LoopbackPortAddressUnavailableError({
+                  host,
+                  ...addressDetails,
+                }),
+              ),
+            );
+          });
         });
-      });
+      } catch (cause) {
+        settle(Effect.fail(new LoopbackPortListenError({ host, cause })));
+      }
 
       return Effect.sync(() => {
         closeServer(probe);

_{You can send follow-ups to the cloud agent here.}

Dismissing prior approval to re-evaluate e234889

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

• ✅ Fixed: Null port misclassified as release
  • Changed port assignment to only set releasePort when addressDetails.port is non-null and positive, keeping it undefined otherwise so the error handler correctly falls through to LoopbackPortListenError and the close callback consistently produces LoopbackPortAddressUnavailableError.

Or push these changes by commenting:

@cursor push 8c32fe77c3

Preview (8c32fe77c3)

diff --git a/packages/shared/src/Net.ts b/packages/shared/src/Net.ts
--- a/packages/shared/src/Net.ts
+++ b/packages/shared/src/Net.ts
@@ -231,26 +231,21 @@
                   family: null,
                   port: null,
                 };
-          const port = addressDetails.port ?? 0;
+          const port =
+            addressDetails.port !== null && addressDetails.port > 0
+              ? addressDetails.port
+              : undefined;
           releasePort = port;
 
           probe.close((cause) => {
-            if (cause) {
-              settle(
-                Effect.fail(
-                  new LoopbackPortReleaseError({
-                    host,
-                    port,
-                    cause,
-                  }),
-                ),
-              );
+            if (port !== undefined) {
+              if (cause) {
+                settle(Effect.fail(new LoopbackPortReleaseError({ host, port, cause })));
+                return;
+              }
+              settle(Effect.succeed(port));
               return;
             }
-            if (addressDetails.port !== null && addressDetails.port > 0) {
-              settle(Effect.succeed(addressDetails.port));
-              return;
-            }
             settle(
               Effect.fail(
                 new LoopbackPortAddressUnavailableError({

_{You can send follow-ups to the cloud agent here.}

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

• ✅ Fixed: Invalid port succeeds on close
  • Changed the close callback guard from === null || <= 0 to == null || !(port > 0) so that undefined and NaN port values are correctly rejected, matching the error handler's positive-check pattern.

Or push these changes by commenting:

@cursor push 39488f3161

Preview (39488f3161)

diff --git a/packages/shared/src/Net.ts b/packages/shared/src/Net.ts
--- a/packages/shared/src/Net.ts
+++ b/packages/shared/src/Net.ts
@@ -247,7 +247,10 @@
           addressDetails = resolvedAddressDetails;
 
           probe.close((cause) => {
-            if (resolvedAddressDetails.port === null || resolvedAddressDetails.port <= 0) {
+            if (
+              resolvedAddressDetails.port == null ||
+              !(resolvedAddressDetails.port > 0)
+            ) {
               settle(
                 Effect.fail(
                   new LoopbackPortAddressUnavailableError({

_{You can send follow-ups to the cloud agent here.}

Dismissing prior approval to re-evaluate bf8ca6a

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Bugbot Autofix prepared a fix for the issue found in the latest run.

• ✅ Fixed: Stale error path port check
  • Aligned the error handler's port validation to use the same strict checks (Number.isInteger, > 0, <= 65_535) as the close callback, ensuring consistent error types regardless of execution path.

Or push these changes by commenting:

@cursor push 63d8693aa8

Preview (63d8693aa8)

diff --git a/packages/shared/src/Net.ts b/packages/shared/src/Net.ts
--- a/packages/shared/src/Net.ts
+++ b/packages/shared/src/Net.ts
@@ -218,7 +218,10 @@
           Effect.fail(
             addressDetails === undefined
               ? new LoopbackPortListenError({ host, cause })
-              : addressDetails.port !== null && addressDetails.port > 0
+              : addressDetails.port !== null &&
+                  Number.isInteger(addressDetails.port) &&
+                  addressDetails.port > 0 &&
+                  addressDetails.port <= 65_535
                 ? new LoopbackPortReleaseError({ host, port: addressDetails.port, cause })
                 : new LoopbackPortAddressUnavailableError({
                     host,

_{You can send follow-ups to the cloud agent here.}

^{Reviewed by Cursor Bugbot for commit bf8ca6a. Configure here.}