Skip to content

fix(engine): worktree-pool.ts doesn't normalize maxConcurrency, NaN disables the cap #5828

Description

@JSONbored

Context

packages/loopover-engine/src/miner/worktree-pool.ts implements the pure, in-memory scheduling logic for the pool of per-attempt git worktrees, capping concurrent fleet attempts at WorktreePoolConfig.maxConcurrency. The type's own doc comment states "A non-positive cap allocates nothing," and the cap is enforced at two call sites with no runtime normalization of the maxConcurrency value itself:

export function availableWorktreeSlots(state: WorktreePoolState, config: WorktreePoolConfig): number {
  return Math.max(0, config.maxConcurrency - state.allocations.length);
}
...
export function acquireWorktree(state, config, input) {
  if (isWorktreeAllocated(state, input.attemptId)) { return { ok: false, reason: "already_allocated", state }; }
  if (state.allocations.length >= config.maxConcurrency) { return { ok: false, reason: "at_capacity", state }; }
  ...
}

If config.maxConcurrency is ever NaN — e.g. a caller derives it from Number(someUnparsableConfigValue) upstream and doesn't itself guard the result — every comparison against it evaluates to false (length >= NaN is always false), so at_capacity never fires and the pool allocates worktrees without bound, directly contradicting the module's own documented cap-safety contract. availableWorktreeSlots also returns NaN in that case, silently breaking any caller (e.g. a dashboard or scheduler) that treats its return value as a real remaining-slots count.

This exact class of problem — a config-ceiling number that could arrive NaN/negative/fractional from an untrusted or loosely-typed source — is already solved three other places in the same package with an identical helper: governor/rate-limit.ts, governor/budget-cap.ts, and tenant-quota.ts all normalize their limit/cap inputs via a finiteNonNegativeInt-style function before using them in a comparison. worktree-pool.ts is the one module in this family that skips it.

test/unit/miner-worktree-pool.test.ts only exercises a normal positive cap and a shrunk (still finite, still positive) cap — there is no test for a NaN, negative, or fractional maxConcurrency.

Requirements

  • Add a finiteNonNegativeInt-equivalent normalization of config.maxConcurrency inside worktree-pool.ts, applied consistently at both availableWorktreeSlots and acquireWorktree (and anywhere else maxConcurrency is read), so a non-finite, negative, or fractional cap can never disable the concurrency limit or produce a NaN slot count.
  • A non-finite or negative maxConcurrency must behave as 0 (allocates nothing), matching the module's existing documented contract for a non-positive cap — do not introduce new behavior for the invalid-input case beyond what "non-positive cap" already means.
  • A fractional maxConcurrency (e.g. 2.5) should floor to the nearest integer, consistent with how rate-limit.ts's finiteNonNegativeInt treats fractional limits.

Deliverables

  • A normalization helper (reusing or mirroring the existing finiteNonNegativeInt pattern from governor/rate-limit.ts) applied to config.maxConcurrency in worktree-pool.ts.
  • availableWorktreeSlots and acquireWorktree updated to use the normalized value.
  • Regression tests in test/unit/miner-worktree-pool.test.ts covering maxConcurrency: NaN, maxConcurrency: -1, and a fractional maxConcurrency, asserting the pool correctly refuses allocation (or floors correctly) in each case.

Test Coverage Requirements

packages/loopover-engine/src/** is measured by Codecov; target 99%+ patch coverage on every changed line and branch (both the "value already valid" and "value needed normalizing" arms), plus the three regression tests listed above.

Expected Outcome

A NaN, negative, or fractional maxConcurrency can no longer silently disable the worktree pool's concurrency cap or return a NaN slot count — worktree-pool.ts matches the same defensive-normalization discipline already applied to rate-limit.ts, budget-cap.ts, and tenant-quota.ts in the same package.

Links & Resources

  • packages/loopover-engine/src/miner/worktree-pool.ts (WorktreePoolConfig, availableWorktreeSlots, acquireWorktree)
  • packages/loopover-engine/src/governor/rate-limit.ts (finiteNonNegativeInt — the precedent pattern to reuse)
  • packages/loopover-engine/src/governor/budget-cap.ts, packages/loopover-engine/src/tenant-quota.ts (the other two modules already applying the same normalization to a config-ceiling input)
  • packages/loopover-engine/test/unit/miner-worktree-pool.test.ts (existing coverage to extend)

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions