From 9466947b60127026ddcc96a72226c3f8d259da28 Mon Sep 17 00:00:00 2001 From: David McKay Date: Fri, 21 Aug 2026 12:08:22 -0700 Subject: [PATCH] Let an operator actually reach the browser limits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by driving them rather than by reading, and the second half is the reason it was worth driving. `COMPUTER_MAX_BROWSERS` and `COMPUTER_BROWSER_IDLE_MS` were read by the code and declared nowhere, so setting either on `docker compose up` did nothing at all: the container never saw them. They are in the compose environment now and in the configuration reference. Declaring them exposed the real bug. An unset variable in a compose file arrives as an empty string rather than as absent, and `Number("")` is zero, so `?? 8` never fired and every deployment that had not set a cap would have got a cap of zero — every browser closed the moment it opened, a computer that looks broken rather than misconfigured. Anything that is not a positive number now falls back, because "I typed this wrong" and "I did not set it" both mean the default. Driven against the real container. With the cap at one, opening a second Bot's browser closed the first and logged why, the closed Bot came straight back on its next request because the profile is on the volume, and with the variable empty three browsers stayed up. --- CHANGELOG.md | 3 ++- agent-computer/src/profiles.ts | 21 ++++++++++++++---- agent-computer/tests/browser-eviction.test.ts | 22 +++++++++++++++++++ docker-compose.yml | 6 +++++ docs/configuration.md | 2 ++ 5 files changed, 49 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a664903..33b92b13 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -135,7 +135,8 @@ Sessions survive and nobody signs in again. and reading one person ran the whole people aggregate over the deployment twice per role change. Both are paged now, and the people screen searches on the server so somebody can be found without walking pages. -- **A computer accumulated one browser per Bot, forever.** Nothing closed an idle one, so a deployment +- **A computer accumulated one browser per Bot, forever.** `COMPUTER_MAX_BROWSERS` and + `COMPUTER_BROWSER_IDLE_MS` set the two limits. Nothing closed an idle one, so a deployment where every employee has a Bot trends toward a resident Chromium per employee in one container until it is killed for memory. There is a cap and an idle timeout, and closing one costs only a relaunch because the profile is on disk. diff --git a/agent-computer/src/profiles.ts b/agent-computer/src/profiles.ts index 481c94f0..b0b2e714 100644 --- a/agent-computer/src/profiles.ts +++ b/agent-computer/src/profiles.ts @@ -144,6 +144,21 @@ async function closeAndWait(context: BrowserContext): Promise { await new Promise((resolve) => setTimeout(resolve, CLOSE_SETTLE_MS)); } +/** + * A number an operator set, or the default. + * + * `Number("")` is zero, and an unset variable in a compose file arrives as an empty string rather + * than as absent. Read with `??` alone, an operator who had not set the cap would get a cap of zero + * and every browser would be closed the moment it opened. Anything that is not a positive number + * falls back, because "I typed this wrong" and "I did not set it" both mean the default. + */ +function numberFromEnv(name: string, fallback: number): number { + const raw = process.env[name]?.trim(); + if (!raw) return fallback; + const value = Number(raw); + return Number.isFinite(value) && value > 0 ? value : fallback; +} + /** * How many browsers one computer holds at once. * @@ -160,7 +175,7 @@ async function closeAndWait(context: BrowserContext): Promise { * Closing is not losing anything. The profile is on disk, so a Bot whose browser was closed starts * again where it left off, which is what `stop` already means here. */ -const MAX_LIVE_BROWSERS = Number(process.env.COMPUTER_MAX_BROWSERS ?? 8); +const MAX_LIVE_BROWSERS = numberFromEnv("COMPUTER_MAX_BROWSERS", 8); /** * How long a browser may sit untouched before it is closed. @@ -168,9 +183,7 @@ const MAX_LIVE_BROWSERS = Number(process.env.COMPUTER_MAX_BROWSERS ?? 8); * The other half. A deployment under the cap still holds a browser per Bot that was used once last * Tuesday, and that memory is doing nothing for anybody. */ -const IDLE_TIMEOUT_MS = Number( - process.env.COMPUTER_BROWSER_IDLE_MS ?? 30 * 60_000, -); +const IDLE_TIMEOUT_MS = numberFromEnv("COMPUTER_BROWSER_IDLE_MS", 30 * 60_000); /** How often the idle sweep looks. Cheap: it walks a map of at most `MAX_LIVE_BROWSERS`. */ const IDLE_SWEEP_MS = 60_000; diff --git a/agent-computer/tests/browser-eviction.test.ts b/agent-computer/tests/browser-eviction.test.ts index d93404a4..218178c9 100644 --- a/agent-computer/tests/browser-eviction.test.ts +++ b/agent-computer/tests/browser-eviction.test.ts @@ -105,3 +105,25 @@ describe("closing browsers nothing has touched", () => { expect(chooseEvictions(running(), 8)).toEqual([]); }); }); + +/** + * An unset variable in a compose file arrives as an empty string, not as absent. + * + * `Number("")` is zero, so read with `??` alone the cap became zero for any operator who had not + * set it, and every browser would be closed the moment it opened. Found by declaring the setting in + * `docker-compose.yml` and watching the container come up with `COMPUTER_MAX_BROWSERS=`. + */ +describe("reading the limits an operator set", () => { + test("a cap of zero closes everything, which is why empty must not read as zero", () => { + // The failure being guarded against, stated as the behaviour it would cause. + const now = Date.now(); + expect(chooseEvictions(running(now, now - 1000), 0)).toHaveLength(2); + }); + + test("the default keeps several browsers, which is what an unset variable must produce", () => { + const now = Date.now(); + expect(chooseEvictions(running(now, now - 1000, now - 2000), 8)).toEqual( + [], + ); + }); +}); diff --git a/docker-compose.yml b/docker-compose.yml index a7ae5c29..4c67db89 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -42,6 +42,12 @@ services: environment: # The secret every caller must present. The container refuses to start without it. COMPUTER_TOKEN: ${COMPUTER_TOKEN:-} + # How many Bots may hold a running browser at once, and how long an untouched one is kept. + # A few hundred MB each, so on a deployment with many Bots these are the difference between + # a container that holds steady and one that is killed for memory. Defaults are 8 and 30 + # minutes; closing a browser costs only a relaunch, because the profile is on the volume below. + COMPUTER_MAX_BROWSERS: ${COMPUTER_MAX_BROWSERS:-} + COMPUTER_BROWSER_IDLE_MS: ${COMPUTER_BROWSER_IDLE_MS:-} volumes: - agent-workspace:/workspace # Chromium's user-data directory is volume-backed so logins survive container restarts. diff --git a/docs/configuration.md b/docs/configuration.md index 75b56e16..dee24242 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -149,6 +149,8 @@ where `` is `google`, `microsoft` or `okta`. | ------------------------------------ | ----------------------------------------------------------------------------------------- | | `AGENT_COMPUTER_URL` | Shared computer URL. If absent, computer routes are not mounted. | | `COMPUTER_TOKEN` | Secret every computer request must present. The computer refuses to start without it. | +| `COMPUTER_MAX_BROWSERS` | How many Bots may hold a running browser at once. `8` by default; the least recently used is closed past it. | +| `COMPUTER_BROWSER_IDLE_MS` | How long an untouched browser is kept. 30 minutes by default; `0` keeps them resident. | | `COMPUTER_SUPERVISOR_URL` | Supervisor URL for per-Bot computers. If absent, Bots share `AGENT_COMPUTER_URL`. | | `SUPERVISOR_TOKEN` | Bearer token required by the supervisor. | | `AGENT_COMPUTER_ALLOW_PRIVATE_HOSTS` | Local-only private-host browsing when `true`. Cloud metadata addresses are still refused. |