Skip to content

Commit fbf4712

Browse files
fix(engine): normalize worktree-pool maxConcurrency so a NaN cap can't disable it (#6041)
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 Co-authored-by: reyanthony062001-ops <reyanthony062001-ops@users.noreply.github.com>
1 parent f938c75 commit fbf4712

2 files changed

Lines changed: 45 additions & 2 deletions

File tree

packages/loopover-engine/src/miner/worktree-pool.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,18 @@ export function isWorktreeAllocated(state: WorktreePoolState, attemptId: string)
4141
return state.allocations.some((allocation) => allocation.attemptId === attemptId);
4242
}
4343

44+
/** Normalize a possibly-untrusted concurrency cap before it gates allocation: a non-finite (`NaN`/`Infinity`),
45+
* negative, or fractional `maxConcurrency` can never disable the cap or yield a `NaN` slot count — it floors to
46+
* a non-negative integer, so an invalid value behaves as the documented non-positive cap (allocates nothing).
47+
* Mirrors the `finiteNonNegativeInt` discipline already applied in governor/rate-limit.ts, governor/budget-cap.ts,
48+
* and tenant-quota.ts (#5828). */
49+
function finiteNonNegativeInt(value: number): number {
50+
return Number.isFinite(value) ? Math.max(0, Math.floor(value)) : 0;
51+
}
52+
4453
/** Slots still available before the concurrency cap (never negative). Pure. */
4554
export function availableWorktreeSlots(state: WorktreePoolState, config: WorktreePoolConfig): number {
46-
return Math.max(0, config.maxConcurrency - state.allocations.length);
55+
return Math.max(0, finiteNonNegativeInt(config.maxConcurrency) - state.allocations.length);
4756
}
4857

4958
/**
@@ -60,7 +69,7 @@ export function acquireWorktree(
6069
if (isWorktreeAllocated(state, input.attemptId)) {
6170
return { ok: false, reason: "already_allocated", state };
6271
}
63-
if (state.allocations.length >= config.maxConcurrency) {
72+
if (state.allocations.length >= finiteNonNegativeInt(config.maxConcurrency)) {
6473
return { ok: false, reason: "at_capacity", state };
6574
}
6675
const allocation: WorktreeAllocation = {

test/unit/miner-worktree-pool.test.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,4 +78,38 @@ describe("worktree pool allocator (#4297)", () => {
7878
const s = acquired(acquired(EMPTY_WORKTREE_POOL, "a"), "b");
7979
expect(availableWorktreeSlots(s, { maxConcurrency: 1 })).toBe(0);
8080
});
81+
82+
it("treats a NaN maxConcurrency as a zero cap instead of disabling the limit", () => {
83+
const cfg = { maxConcurrency: NaN };
84+
expect(availableWorktreeSlots(EMPTY_WORKTREE_POOL, cfg)).toBe(0);
85+
const r = acquireWorktree(EMPTY_WORKTREE_POOL, cfg, { attemptId: "a", repoPath: REPO });
86+
expect(r.ok).toBe(false);
87+
if (r.ok) return;
88+
expect(r.reason).toBe("at_capacity");
89+
});
90+
91+
it("treats a negative maxConcurrency as a zero cap", () => {
92+
const cfg = { maxConcurrency: -1 };
93+
expect(availableWorktreeSlots(EMPTY_WORKTREE_POOL, cfg)).toBe(0);
94+
const r = acquireWorktree(EMPTY_WORKTREE_POOL, cfg, { attemptId: "a", repoPath: REPO });
95+
expect(r.ok).toBe(false);
96+
if (r.ok) return;
97+
expect(r.reason).toBe("at_capacity");
98+
});
99+
100+
it("floors a fractional maxConcurrency to the nearest integer", () => {
101+
const cfg = { maxConcurrency: 2.5 };
102+
expect(availableWorktreeSlots(EMPTY_WORKTREE_POOL, cfg)).toBe(2);
103+
const one = acquireWorktree(EMPTY_WORKTREE_POOL, cfg, { attemptId: "a", repoPath: REPO });
104+
expect(one.ok).toBe(true);
105+
if (!one.ok) return;
106+
const two = acquireWorktree(one.state, cfg, { attemptId: "b", repoPath: REPO });
107+
expect(two.ok).toBe(true);
108+
if (!two.ok) return;
109+
expect(availableWorktreeSlots(two.state, cfg)).toBe(0);
110+
const three = acquireWorktree(two.state, cfg, { attemptId: "c", repoPath: REPO });
111+
expect(three.ok).toBe(false);
112+
if (three.ok) return;
113+
expect(three.reason).toBe("at_capacity");
114+
});
81115
});

0 commit comments

Comments
 (0)