From 0c6bf1417279832a0c025e3bba9becb52f7f7d6d Mon Sep 17 00:00:00 2001 From: bitfathers94 <237535319+bitfathers94@users.noreply.github.com> Date: Mon, 27 Jul 2026 14:47:10 +0000 Subject: [PATCH] fix(engine): give fleet-run-manifest a dedicated per-repo worktree default The bare-string repo branch in normalizeRepoList defaulted maxConcurrentWorktrees from DEFAULT_FLEET_RUN_MANIFEST.totalConcurrentWorktrees (a fleet-wide total), while the object-form branch defaulted from a hardcoded literal 1. The two agree only because the fleet-wide total default is also 1 today; changing it would silently move the per-repo default for bare-string entries. Introduce DEFAULT_FLEET_RUN_MANIFEST_REPO_MAX_CONCURRENT_WORKTREES as the single source of truth for the per-repo default and use it in both branches. --- .../loopover-engine/src/fleet-run-manifest.ts | 15 +++++++++-- packages/loopover-engine/src/index.ts | 1 + test/unit/fleet-run-manifest-parser.test.ts | 25 +++++++++++++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/packages/loopover-engine/src/fleet-run-manifest.ts b/packages/loopover-engine/src/fleet-run-manifest.ts index ba7ba65b35..928913cd5c 100644 --- a/packages/loopover-engine/src/fleet-run-manifest.ts +++ b/packages/loopover-engine/src/fleet-run-manifest.ts @@ -45,6 +45,12 @@ export const DEFAULT_FLEET_RUN_MANIFEST: FleetRunManifest = Object.freeze({ totalConcurrentWorktrees: 1, }); +/** Default per-repo worktree budget for a repo entry that doesn't specify its own `maxConcurrentWorktrees`. Its own + * dedicated source of truth: a *per-repo* cap, deliberately distinct from the *fleet-wide* + * {@link DEFAULT_FLEET_RUN_MANIFEST.totalConcurrentWorktrees} even though both currently equal `1`, so a future + * change to the fleet-wide total default can never silently move the per-repo default. */ +export const DEFAULT_FLEET_RUN_MANIFEST_REPO_MAX_CONCURRENT_WORKTREES = 1; + const MAX_FLEET_RUN_MANIFEST_BYTES = 65_536; const MAX_MANIFEST_REPOS = 500; @@ -93,13 +99,18 @@ function normalizeRepoList(value: unknown, warnings: string[]): FleetRunManifest break; } let repoFullName: string | null; - let maxConcurrentWorktrees = DEFAULT_FLEET_RUN_MANIFEST.totalConcurrentWorktrees; + let maxConcurrentWorktrees = DEFAULT_FLEET_RUN_MANIFEST_REPO_MAX_CONCURRENT_WORKTREES; if (typeof entry === "string") { repoFullName = normalizeRepoFullName(entry); } else if (entry && typeof entry === "object" && !Array.isArray(entry)) { const record = entry as Record; repoFullName = normalizeRepoFullName(record.repoFullName); - maxConcurrentWorktrees = normalizePositiveInteger(record.maxConcurrentWorktrees, "maxConcurrentWorktrees", 1, warnings); + maxConcurrentWorktrees = normalizePositiveInteger( + record.maxConcurrentWorktrees, + "maxConcurrentWorktrees", + DEFAULT_FLEET_RUN_MANIFEST_REPO_MAX_CONCURRENT_WORKTREES, + warnings, + ); } else { warnings.push(`FleetRunManifest "repos" skipped a non-string, non-mapping entry.`); continue; diff --git a/packages/loopover-engine/src/index.ts b/packages/loopover-engine/src/index.ts index 9751101fdb..75133dd67a 100644 --- a/packages/loopover-engine/src/index.ts +++ b/packages/loopover-engine/src/index.ts @@ -574,6 +574,7 @@ export { } from "./ams-policy-spec.js"; export { DEFAULT_FLEET_RUN_MANIFEST, + DEFAULT_FLEET_RUN_MANIFEST_REPO_MAX_CONCURRENT_WORKTREES, parseFleetRunManifest, parseFleetRunManifestContent, type FleetRunManifest, diff --git a/test/unit/fleet-run-manifest-parser.test.ts b/test/unit/fleet-run-manifest-parser.test.ts index 8cd65f3e7f..eb1fbe757e 100644 --- a/test/unit/fleet-run-manifest-parser.test.ts +++ b/test/unit/fleet-run-manifest-parser.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import { DEFAULT_FLEET_RUN_MANIFEST, + DEFAULT_FLEET_RUN_MANIFEST_REPO_MAX_CONCURRENT_WORKTREES, parseFleetRunManifest, parseFleetRunManifestContent, } from "../../packages/loopover-engine/src/index"; @@ -61,6 +62,30 @@ describe("FleetRunManifest parser (#4299)", () => { expect(w).toMatch(/"maxConcurrentWorktrees" must be a positive whole number/); }); + it("derives both bare-string and object-form per-repo defaults from the dedicated per-repo constant, decoupled from the fleet-wide total default", () => { + // The per-repo default is its own source of truth, deliberately distinct from the fleet-wide total default. + // Both equal 1 today, so this pins the value once and lets the assertions below reference the constant rather + // than a bare `1` — a future change to the fleet-wide total default cannot silently move the per-repo default. + expect(DEFAULT_FLEET_RUN_MANIFEST_REPO_MAX_CONCURRENT_WORKTREES).toBe(1); + + // The frozen DEFAULT_FLEET_RUN_MANIFEST.totalConcurrentWorktrees can't be mutated in place, so drive a + // non-default fleet-wide total through the manifest input instead: a bare-string entry (no budget of its own) + // and an object-form entry (budget omitted) must BOTH still fall back to the dedicated per-repo constant, not + // the manifest's fleet-wide total. + const parsed = parseFleetRunManifest({ + repos: ["owner/a", { repoFullName: "owner/b" }], + totalConcurrentWorktrees: 7, + }); + expect(parsed.present).toBe(true); + expect(parsed.manifest.totalConcurrentWorktrees).toBe(7); + expect(parsed.manifest.repos).toEqual([ + { repoFullName: "owner/a", maxConcurrentWorktrees: DEFAULT_FLEET_RUN_MANIFEST_REPO_MAX_CONCURRENT_WORKTREES }, + { repoFullName: "owner/b", maxConcurrentWorktrees: DEFAULT_FLEET_RUN_MANIFEST_REPO_MAX_CONCURRENT_WORKTREES }, + ]); + // The per-repo default did not inherit the non-default fleet-wide total. + expect(parsed.manifest.repos.every((r) => r.maxConcurrentWorktrees !== parsed.manifest.totalConcurrentWorktrees)).toBe(true); + }); + it("falls a non-list repos field and a sub-1 total budget back to defaults with warnings", () => { const parsed = parseFleetRunManifest({ repos: "owner/a", totalConcurrentWorktrees: 0 }); expect(parsed.manifest.repos).toEqual([]);