From 797c7ea24eee0cd750fa7ae536b2485e4112eda0 Mon Sep 17 00:00:00 2001 From: Nikhil Sonti Date: Tue, 21 Apr 2026 14:49:52 -0700 Subject: [PATCH] fix: address PR review comments for podman-cpu-memory-env MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude review on #774 flagged: - readMachineResources/readPositiveInt had no direct test coverage (FakePodmanRuntime overrides initMachine entirely, so the env parsing path was dead code in the test suite). - ensureReady and initMachine both logged 'Initializing Podman machine...' one after the other — two lines for one operation. Fixes: - Export readMachineResources and add 4 unit tests covering defaults, valid parse, non-numeric fallback, and zero/negative fallback. - Reword the initMachine log from a duplicate headline to a concise resource-allocation detail ('Allocating 4 CPUs, 4096 MB RAM') that complements the 'Initializing Podman machine...' headline emitted by ensureReady. --- .../api/services/openclaw/podman-runtime.ts | 4 +- .../services/openclaw/podman-runtime.test.ts | 44 +++++++++++++++++++ 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/packages/browseros-agent/apps/server/src/api/services/openclaw/podman-runtime.ts b/packages/browseros-agent/apps/server/src/api/services/openclaw/podman-runtime.ts index 13529400b..96dd543f1 100644 --- a/packages/browseros-agent/apps/server/src/api/services/openclaw/podman-runtime.ts +++ b/packages/browseros-agent/apps/server/src/api/services/openclaw/podman-runtime.ts @@ -28,7 +28,7 @@ function readPositiveInt(value: string | undefined, fallback: number): number { return Number.isFinite(parsed) && parsed > 0 ? parsed : fallback } -function readMachineResources(): { cpus: number; memoryMb: number } { +export function readMachineResources(): { cpus: number; memoryMb: number } { return { cpus: readPositiveInt( process.env.BROWSEROS_PODMAN_CPUS, @@ -114,7 +114,7 @@ export class PodmanRuntime { if (isLinux) return const { cpus, memoryMb } = readMachineResources() - onLog?.(`Initializing Podman machine (${cpus} CPUs, ${memoryMb} MB RAM)`) + onLog?.(`Allocating ${cpus} CPUs, ${memoryMb} MB RAM`) const proc = Bun.spawn( [ diff --git a/packages/browseros-agent/apps/server/tests/api/services/openclaw/podman-runtime.test.ts b/packages/browseros-agent/apps/server/tests/api/services/openclaw/podman-runtime.test.ts index ef2c7382a..132ab2323 100644 --- a/packages/browseros-agent/apps/server/tests/api/services/openclaw/podman-runtime.test.ts +++ b/packages/browseros-agent/apps/server/tests/api/services/openclaw/podman-runtime.test.ts @@ -12,6 +12,7 @@ import { configurePodmanRuntime, getPodmanRuntime, PodmanRuntime, + readMachineResources, resolveBundledPodmanPath, } from '../../../../src/api/services/openclaw/podman-runtime' @@ -167,3 +168,46 @@ describe('podman runtime', () => { expect(runtime.startCalls).toBe(1) }) }) + +describe('readMachineResources', () => { + const ORIGINAL_CPUS = process.env.BROWSEROS_PODMAN_CPUS + const ORIGINAL_MEMORY = process.env.BROWSEROS_PODMAN_MEMORY_MB + + afterEach(() => { + restoreEnv('BROWSEROS_PODMAN_CPUS', ORIGINAL_CPUS) + restoreEnv('BROWSEROS_PODMAN_MEMORY_MB', ORIGINAL_MEMORY) + }) + + it('returns defaults when env vars are unset', () => { + delete process.env.BROWSEROS_PODMAN_CPUS + delete process.env.BROWSEROS_PODMAN_MEMORY_MB + + expect(readMachineResources()).toEqual({ cpus: 4, memoryMb: 4096 }) + }) + + it('parses valid env values', () => { + process.env.BROWSEROS_PODMAN_CPUS = '6' + process.env.BROWSEROS_PODMAN_MEMORY_MB = '8192' + + expect(readMachineResources()).toEqual({ cpus: 6, memoryMb: 8192 }) + }) + + it('falls back to defaults for non-numeric input', () => { + process.env.BROWSEROS_PODMAN_CPUS = 'abc' + process.env.BROWSEROS_PODMAN_MEMORY_MB = '' + + expect(readMachineResources()).toEqual({ cpus: 4, memoryMb: 4096 }) + }) + + it('falls back to defaults for zero and negative values', () => { + process.env.BROWSEROS_PODMAN_CPUS = '0' + process.env.BROWSEROS_PODMAN_MEMORY_MB = '-512' + + expect(readMachineResources()).toEqual({ cpus: 4, memoryMb: 4096 }) + }) +}) + +function restoreEnv(key: string, value: string | undefined): void { + if (value === undefined) delete process.env[key] + else process.env[key] = value +}