diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e2f952b..4d7d421f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,15 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. ## Unreleased +### A fractional or out-of-range computer setting takes the fallback instead of breaking the boot + +`numberFromEnv` accepted anything `Number` called finite and positive, so `PORT=80.5` bound +nothing usable, `PORT=99999` misbound at boot, a fractional timeout fired before any action could +finish, and `COMPUTER_MAX_BROWSERS=2.5` reached eviction math as a fraction — each reading as a +broken computer rather than a mistaken variable. Every reader is a port, a timeout, or a count, +so only whole numbers on sight are values now and anything else takes the documented fallback; +the port additionally keeps its 1–65535 range, the way the supervisor's own port parser already +does. Zero semantics are unchanged: `COMPUTER_BROWSER_IDLE_MS=0` still keeps browsers resident. ### A malformed page size is refused instead of silently coerced `GET /channels` and `GET /api/admin/people` read `?limit=` with `Number.parseInt`, which diff --git a/agent-computer/src/env.ts b/agent-computer/src/env.ts index 96eaacf1..fbcbd82c 100644 --- a/agent-computer/src/env.ts +++ b/agent-computer/src/env.ts @@ -1,10 +1,21 @@ /** - * A positive number from the environment, or the fallback. + * A positive whole number from the environment, or the fallback. * * `Number.parseInt(process.env.X ?? "default")` is not enough: an unset variable declared in a * compose file arrives as an empty string rather than as absent, so `??` never fires and the parse * yields `NaN`. Empty, absent, non-numeric and non-positive all mean "not set" and take the fallback. * + * Whole numbers only: every reader is a port, a timeout in milliseconds, or a count of browsers, + * and none of them has a fractional answer. `Number("80.5")` is finite and greater than zero, so + * without this `PORT=80.5` bound nothing usable, `ACTION_TIMEOUT_MS=0.5` fired before any action + * could finish, and `COMPUTER_MAX_BROWSERS=2.5` reached eviction math as a fraction. Scientific + * notation and hex are not whole numbers on sight either, for the same reason `listenPort` on the + * supervisor refuses them rather than reading part of one. + * + * `min`/`max` bound the value where the setting has a range, which is the port: `PORT=99999` + * parsed fine and then misbound at boot, a deployment failure instead of the documented fallback. + * Timeouts and counts carry no range and stay unbounded above, like before. + * * `zeroSwitchesItOff` is for the one setting where zero is an answer rather than a mistake. * `COMPUTER_BROWSER_IDLE_MS=0` is documented as "keeps them resident", and `chooseIdle` reads a * timeout of zero as the sweep being switched off — but the value never reached it, because zero is @@ -23,12 +34,18 @@ export function numberFromEnv( name: string, fallback: number, - { zeroSwitchesItOff = false }: { zeroSwitchesItOff?: boolean } = {}, + { + zeroSwitchesItOff = false, + min, + max, + }: { zeroSwitchesItOff?: boolean; min?: number; max?: number } = {}, ): number { const raw = process.env[name]?.trim(); if (!raw) return fallback; - const value = Number(raw); - if (!Number.isFinite(value)) return fallback; + if (!/^\d+$/.test(raw)) return fallback; + const value = Number.parseInt(raw, 10); + if (min !== undefined && value < min) return fallback; + if (max !== undefined && value > max) return fallback; return (zeroSwitchesItOff ? value >= 0 : value > 0) ? value : fallback; } diff --git a/agent-computer/src/index.ts b/agent-computer/src/index.ts index 16868122..16b66ddb 100644 --- a/agent-computer/src/index.ts +++ b/agent-computer/src/index.ts @@ -91,7 +91,11 @@ console.info( }), ); -const PORT = numberFromEnv("PORT", 4100); +/* + * A whole port in range, or the default: a fraction never binds and an out-of-range one + * misbinds at boot, which is a deployment failure instead of the documented fallback. + */ +const PORT = numberFromEnv("PORT", 4100, { min: 1, max: 65535 }); const NAVIGATION_TIMEOUT_MS = numberFromEnv("NAVIGATION_TIMEOUT_MS", 30000); /** diff --git a/agent-computer/tests/number-from-env.test.ts b/agent-computer/tests/number-from-env.test.ts index 00aa36da..f3109bff 100644 --- a/agent-computer/tests/number-from-env.test.ts +++ b/agent-computer/tests/number-from-env.test.ts @@ -35,6 +35,27 @@ describe("numberFromEnv", () => { process.env[NAME] = "-5"; expect(numberFromEnv(NAME, 10000)).toBe(10000); }); + + /** + * Whole numbers only. + * + * Every reader is a port, a timeout in milliseconds, or a count of browsers, and none of them + * has a fractional answer. `Number("80.5")` is finite and greater than zero, so a fraction used + * to be returned verbatim: a fractional port misbound at boot and a fractional timeout fired + * before any action could finish, reading as a broken computer rather than a caller error. + */ + test.each([["80.5"], ["0.5"], ["2.5"], ["30000.0"], ["1e3"], ["0x10"]])( + "falls back on %p, which is not a whole number on sight", + (raw) => { + process.env[NAME] = raw; + expect(numberFromEnv(NAME, 10000)).toBe(10000); + }, + ); + + test("still takes a large whole number where the setting has no range", () => { + process.env[NAME] = "1800000"; + expect(numberFromEnv(NAME, 10000)).toBe(1800000); + }); }); /** @@ -80,4 +101,39 @@ describe("numberFromEnv where zero switches the setting off", () => { process.env[NAME] = "5000"; expect(numberFromEnv(NAME, 10000, { zeroSwitchesItOff: true })).toBe(5000); }); + + test("still falls back on a fraction where zero switches the setting off", () => { + process.env[NAME] = "0.5"; + expect(numberFromEnv(NAME, 10000, { zeroSwitchesItOff: true })).toBe(10000); + }); +}); + +/** + * Settings with a range. + * + * A port parsed fine and then misbound at boot: `PORT=99999` was returned verbatim and the + * deployment failed instead of taking the documented fallback. The range rides on the same + * function so every integer setting keeps one rule, rather than the port growing its own parser + * that the next setting copies slightly wrong. + */ +describe("numberFromEnv with a range", () => { + const RANGE = { min: 1, max: 65535 }; + + test.each([ + ["4300", 4300], + ["1", 1], + ["65535", 65535], + [" 4100 ", 4100], + ])("takes a whole port in range: %p", (raw, port) => { + process.env[NAME] = raw; + expect(numberFromEnv(NAME, 4100, RANGE)).toBe(port); + }); + + test.each([["0"], ["99999"], ["65536"], ["80.5"], ["-1"], ["soon"]])( + "falls back on %p, which is out of range or not a whole number", + (raw) => { + process.env[NAME] = raw; + expect(numberFromEnv(NAME, 4100, RANGE)).toBe(4100); + }, + ); });