Read an empty PORT as unset, so SERVER_PORT still moves the server and NaN never reaches Bun.serve (#340)

#312 read the port as `process.env.PORT ?? process.env.SERVER_PORT ?? "3001"`. `??` fires only on
undefined, and a variable a compose file declares but the host never set, or `PORT=` left in a
`.env`, arrives as an empty string: SERVER_PORT was never consulted, the disagreement check
(`PORT && SERVER_PORT && …`) never fired, and `Number.parseInt("")` handed `Bun.serve` a NaN, which
it answers by binding an ephemeral port. The server came up somewhere nobody had asked for, and
the script polling SERVER_PORT reported one that never started — the failure #312 set out to
remove, back through the other name. `parseInt` also parses a prefix, so `PORT=30o1` started the
server on port 30 without a word.

The port is now read by `loadConfig` through the same `optional` every other setting uses, where
empty means unset, refusing anything that is not a whole number in 1–65535 the way the handoff
caps and AGENT_STALL_TIMEOUT_MS already refuse a typo. Tests cover the empty-string case that is
the whole point, the disagreement, and the refusals under both names.

Co-authored-by: kevin9327 <kevin9327@users.noreply.github.com>

Author: kevin9327

.env.example

@@ -9,8 +9,9 @@ KEY_ENCRYPTION_KEY=AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=
 # AUDIT_RETENTION_DAYS=365
 # Two names for one number, and they must agree when both are set.
 #
-# The server accepts either PORT or SERVER_PORT (server/src/index.ts), preferring PORT when both
-# are present and refusing to start if they disagree, so a single edit is enough. scripts/start.sh
+# The server accepts either PORT or SERVER_PORT (server/src/config.ts), preferring PORT when both
+# are present and refusing to start if they disagree, so a single edit is enough. An empty value
+# is an unset one, so `PORT=` with SERVER_PORT set moves the server too. scripts/start.sh
 # and the app's Vite proxy read SERVER_PORT/APP_PORT, and docs/configuration.md documents
 # SERVER_PORT as the setting. Only PORT shipped here historically, so moving the server by
 # editing one line left the script still looking at 3001: it found whatever else was there,

CHANGELOG.md

@@ -8,6 +8,16 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged.
 
 ## Unreleased
 
+### An empty `PORT` no longer starts the server on a port nobody asked for
+
+`PORT` and `SERVER_PORT` name one number, and either is meant to move the server. A `PORT` that was
+declared but empty — a compose file passing a variable the host never set, or `PORT=` left in a
+`.env` next to a `SERVER_PORT` that was set — was read as "set to nothing": `SERVER_PORT` was
+ignored, the number parsed to `NaN`, and the server came up on an ephemeral port while everything
+that polls `SERVER_PORT` reported it had never started. An empty value now counts as unset, the way
+every other setting already treats it, and a value that is not a whole port number (`30o1` used to
+start the server on port 30) refuses to start instead.
+
 ### Coworkers are made in a wizard and managed in a dialog
 
 Creating a coworker is now a three-step wizard — who it is, who may see it, then where it runs,

server/src/config.ts

@@ -132,6 +132,8 @@ export type HandoffCaps = {
 };
 
 export type DeploymentConfig = {
+  /** The port the API listens on. Named `PORT` or `SERVER_PORT`; see `serverPort`. */
+  port: number;
   databaseUrl: string;
   keyEncryptionKey: string;
   /**
@@ -854,6 +856,44 @@ function agentStallTimeoutMs(environment: Environment): number {
   return milliseconds;
 }
 
+/** Where the API listens when nothing says otherwise: what `.env.example` and the image ship. */
+const DEFAULT_PORT = 3001;
+
+/**
+ * The port the API listens on, from either of its two names.
+ *
+ * `PORT` and `SERVER_PORT` name one number: either moves the server, and two that disagree are
+ * refused at boot rather than half-applied. Read through `optional` like every other setting here,
+ * and that is the point. An unset variable declared in a compose file, or left as `PORT=` in a
+ * `.env`, arrives as an empty string rather than as absent, so `process.env.PORT ??
+ * process.env.SERVER_PORT` never fell through to the second name, and `Number.parseInt("")` is
+ * `NaN`. Given `NaN`, `Bun.serve` binds an ephemeral port: the server came up somewhere nobody had
+ * asked for, `SERVER_PORT` ignored, and the script polling it reported a server that never
+ * started — the failure #312 set out to remove, back through the other name.
+ *
+ * A value that is not a whole port number is refused for the reason the caps above are: `30o1`
+ * used to start the server on port 30, and a typo has to fail where somebody is looking.
+ */
+function serverPort(environment: Environment): number {
+  const read = (name: string): number | undefined => {
+    const raw = optional(environment, name);
+    if (raw === undefined) return undefined;
+    const value = Number(raw);
+    if (!Number.isInteger(value) || value < 1 || value > 65535) {
+      throw new Error(`${name} must be a whole number between 1 and 65535`);
+    }
+    return value;
+  };
+  const port = read("PORT");
+  const serverPort = read("SERVER_PORT");
+  if (port !== undefined && serverPort !== undefined && port !== serverPort) {
+    throw new Error(
+      `PORT (${port}) and SERVER_PORT (${serverPort}) disagree: set one or set both to the same value`,
+    );
+  }
+  return port ?? serverPort ?? DEFAULT_PORT;
+}
+
 export function loadConfig(
   environment: Environment = process.env,
 ): DeploymentConfig {
@@ -863,6 +903,7 @@ export function loadConfig(
   const workerSharedSecret = optional(environment, "WORKER_SHARED_SECRET");
 
   return {
+    port: serverPort(environment),
     databaseUrl: required(environment, "DATABASE_URL"),
     keyEncryptionKey: keyEncryptionKey(environment),
     ...(managedAgent ? { managedAgent } : {}),

server/src/index.ts

@@ -143,17 +143,9 @@ const identifyActor: IdentifyActor = async (request) => {
 };
 
 const config = loadConfig();
-const rawPort = process.env.PORT ?? process.env.SERVER_PORT ?? "3001";
-if (
-  process.env.PORT &&
-  process.env.SERVER_PORT &&
-  process.env.PORT !== process.env.SERVER_PORT
-) {
-  throw new Error(
-    `PORT (${process.env.PORT}) and SERVER_PORT (${process.env.SERVER_PORT}) disagree: set one or set both to the same value`,
-  );
-}
-const port = Number.parseInt(rawPort, 10);
+// Read with the rest of the configuration, where an empty variable is an absent one. See
+// `serverPort` in config.ts for what `process.env.PORT ?? …` did with `PORT=` instead.
+const port = config.port;
 const database = createDatabase(config.databaseUrl);
 await initializeDevActorUser(database, config.singleUser);
 // The vault, built before the agent store because a customer's agent may sit behind a key and that

server/tests/config.test.ts

@@ -394,6 +394,59 @@ describe("deployment configuration", () => {
     },
   );
 
+  test("listens on 3001 when neither PORT nor SERVER_PORT is set", () => {
+    expect(loadConfig(baseEnvironment).port).toBe(3001);
+  });
+
+  test("moves the server by either name", () => {
+    expect(loadConfig({ ...baseEnvironment, PORT: "3005" }).port).toBe(3005);
+    expect(loadConfig({ ...baseEnvironment, SERVER_PORT: "3005" }).port).toBe(
+      3005,
+    );
+    expect(
+      loadConfig({ ...baseEnvironment, PORT: " 3005 ", SERVER_PORT: "3005" })
+        .port,
+    ).toBe(3005);
+  });
+
+  /*
+   * An unset variable declared in a compose file, or left as `PORT=` in a `.env`, arrives as an
+   * empty string rather than as absent. `process.env.PORT ?? process.env.SERVER_PORT` saw the empty
+   * string and never reached the second name, and `Number.parseInt("")` handed `Bun.serve` a NaN,
+   * which it answers by binding an ephemeral port nobody asked for.
+   */
+  test("reads SERVER_PORT when PORT is declared but empty, and the other way round", () => {
+    expect(
+      loadConfig({ ...baseEnvironment, PORT: "", SERVER_PORT: "3005" }).port,
+    ).toBe(3005);
+    expect(
+      loadConfig({ ...baseEnvironment, PORT: "3005", SERVER_PORT: "" }).port,
+    ).toBe(3005);
+    expect(
+      loadConfig({ ...baseEnvironment, PORT: "", SERVER_PORT: "" }).port,
+    ).toBe(3001);
+  });
+
+  test("refuses to start when PORT and SERVER_PORT disagree", () => {
+    expect(() =>
+      loadConfig({ ...baseEnvironment, PORT: "3001", SERVER_PORT: "3005" }),
+    ).toThrow("PORT (3001) and SERVER_PORT (3005) disagree");
+  });
+
+  // `Number.parseInt("30o1")` is 30, and the server used to come up there. Refused instead, the way
+  // a mistyped cap is: a port has to fail at start-up, where somebody is looking.
+  test.each(["30o1", "three", "0", "65536", "1.5", "-1"])(
+    "refuses to start on PORT=%p",
+    (value) => {
+      expect(() => loadConfig({ ...baseEnvironment, PORT: value })).toThrow(
+        "PORT must be a whole number between 1 and 65535",
+      );
+      expect(() =>
+        loadConfig({ ...baseEnvironment, SERVER_PORT: value }),
+      ).toThrow("SERVER_PORT must be a whole number between 1 and 65535");
+    },
+  );
+
   test("configures Docker as the per-Bot computer provider", () => {
     const config = loadConfig({
       ...baseEnvironment,