From 3e741afdb3f0c044d9f6cfa0a5f019bdaa02ca54 Mon Sep 17 00:00:00 2001 From: reyanthony062001-ops Date: Wed, 15 Jul 2026 07:22:47 +0000 Subject: [PATCH] fix(engine): normalize worktree-pool maxConcurrency so a NaN cap can't disable it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit availableWorktreeSlots and acquireWorktree read WorktreePoolConfig.maxConcurrency directly in a comparison. A NaN cap (e.g. derived upstream from Number() of an unparsable config value) makes `length >= NaN` always false, so at_capacity never fires and the pool allocates worktrees without bound — contradicting the module's documented "non-positive cap allocates nothing" contract — while availableWorktreeSlots returns NaN. Normalize maxConcurrency through a finiteNonNegativeInt helper at both read sites (non-finite or negative floors to 0, fractional floors down), mirroring governor/rate-limit.ts, governor/budget-cap.ts, and tenant-quota.ts. Adds regression tests for NaN, negative, and fractional maxConcurrency. Closes #5828 --- .../src/miner/worktree-pool.ts | 13 +++++-- test/unit/miner-worktree-pool.test.ts | 34 +++++++++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/packages/loopover-engine/src/miner/worktree-pool.ts b/packages/loopover-engine/src/miner/worktree-pool.ts index 5520b6e44f..2302131c93 100644 --- a/packages/loopover-engine/src/miner/worktree-pool.ts +++ b/packages/loopover-engine/src/miner/worktree-pool.ts @@ -41,9 +41,18 @@ export function isWorktreeAllocated(state: WorktreePoolState, attemptId: string) return state.allocations.some((allocation) => allocation.attemptId === attemptId); } +/** Normalize a possibly-untrusted concurrency cap before it gates allocation: a non-finite (`NaN`/`Infinity`), + * negative, or fractional `maxConcurrency` can never disable the cap or yield a `NaN` slot count — it floors to + * a non-negative integer, so an invalid value behaves as the documented non-positive cap (allocates nothing). + * Mirrors the `finiteNonNegativeInt` discipline already applied in governor/rate-limit.ts, governor/budget-cap.ts, + * and tenant-quota.ts (#5828). */ +function finiteNonNegativeInt(value: number): number { + return Number.isFinite(value) ? Math.max(0, Math.floor(value)) : 0; +} + /** Slots still available before the concurrency cap (never negative). Pure. */ export function availableWorktreeSlots(state: WorktreePoolState, config: WorktreePoolConfig): number { - return Math.max(0, config.maxConcurrency - state.allocations.length); + return Math.max(0, finiteNonNegativeInt(config.maxConcurrency) - state.allocations.length); } /** @@ -60,7 +69,7 @@ export function acquireWorktree( if (isWorktreeAllocated(state, input.attemptId)) { return { ok: false, reason: "already_allocated", state }; } - if (state.allocations.length >= config.maxConcurrency) { + if (state.allocations.length >= finiteNonNegativeInt(config.maxConcurrency)) { return { ok: false, reason: "at_capacity", state }; } const allocation: WorktreeAllocation = { diff --git a/test/unit/miner-worktree-pool.test.ts b/test/unit/miner-worktree-pool.test.ts index d67829d18b..b3c3abb051 100644 --- a/test/unit/miner-worktree-pool.test.ts +++ b/test/unit/miner-worktree-pool.test.ts @@ -78,4 +78,38 @@ describe("worktree pool allocator (#4297)", () => { const s = acquired(acquired(EMPTY_WORKTREE_POOL, "a"), "b"); expect(availableWorktreeSlots(s, { maxConcurrency: 1 })).toBe(0); }); + + it("treats a NaN maxConcurrency as a zero cap instead of disabling the limit", () => { + const cfg = { maxConcurrency: NaN }; + expect(availableWorktreeSlots(EMPTY_WORKTREE_POOL, cfg)).toBe(0); + const r = acquireWorktree(EMPTY_WORKTREE_POOL, cfg, { attemptId: "a", repoPath: REPO }); + expect(r.ok).toBe(false); + if (r.ok) return; + expect(r.reason).toBe("at_capacity"); + }); + + it("treats a negative maxConcurrency as a zero cap", () => { + const cfg = { maxConcurrency: -1 }; + expect(availableWorktreeSlots(EMPTY_WORKTREE_POOL, cfg)).toBe(0); + const r = acquireWorktree(EMPTY_WORKTREE_POOL, cfg, { attemptId: "a", repoPath: REPO }); + expect(r.ok).toBe(false); + if (r.ok) return; + expect(r.reason).toBe("at_capacity"); + }); + + it("floors a fractional maxConcurrency to the nearest integer", () => { + const cfg = { maxConcurrency: 2.5 }; + expect(availableWorktreeSlots(EMPTY_WORKTREE_POOL, cfg)).toBe(2); + const one = acquireWorktree(EMPTY_WORKTREE_POOL, cfg, { attemptId: "a", repoPath: REPO }); + expect(one.ok).toBe(true); + if (!one.ok) return; + const two = acquireWorktree(one.state, cfg, { attemptId: "b", repoPath: REPO }); + expect(two.ok).toBe(true); + if (!two.ok) return; + expect(availableWorktreeSlots(two.state, cfg)).toBe(0); + const three = acquireWorktree(two.state, cfg, { attemptId: "c", repoPath: REPO }); + expect(three.ok).toBe(false); + if (three.ok) return; + expect(three.reason).toBe("at_capacity"); + }); });