diff --git a/packages/loopover-engine/src/fleet-run-manifest.ts b/packages/loopover-engine/src/fleet-run-manifest.ts index ba7ba65b35..39263d2f90 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,13 @@ 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/packages/loopover-engine/test/fleet-run-manifest-parser.test.ts b/packages/loopover-engine/test/fleet-run-manifest-parser.test.ts new file mode 100644 index 0000000000..8e99a1e352 --- /dev/null +++ b/packages/loopover-engine/test/fleet-run-manifest-parser.test.ts @@ -0,0 +1,43 @@ +import assert from "node:assert/strict"; +import { test } from "node:test"; + +import { + DEFAULT_FLEET_RUN_MANIFEST, + DEFAULT_FLEET_RUN_MANIFEST_REPO_MAX_CONCURRENT_WORKTREES, + parseFleetRunManifest, +} from "../dist/index.js"; + +test("barrel: the public entrypoint re-exports the fleet run-manifest parser API (#9324)", () => { + assert.equal(typeof parseFleetRunManifest, "function"); + assert.equal(DEFAULT_FLEET_RUN_MANIFEST_REPO_MAX_CONCURRENT_WORKTREES, 1); +}); + +test("normalizeRepoList: a string entry and a fieldless mapping both take the dedicated per-repo default", () => { + const parsed = parseFleetRunManifest({ repos: ["owner/a", { repoFullName: "owner/b" }] }); + assert.deepEqual(parsed.manifest.repos, [ + { repoFullName: "owner/a", maxConcurrentWorktrees: DEFAULT_FLEET_RUN_MANIFEST_REPO_MAX_CONCURRENT_WORKTREES }, + { repoFullName: "owner/b", maxConcurrentWorktrees: DEFAULT_FLEET_RUN_MANIFEST_REPO_MAX_CONCURRENT_WORKTREES }, + ]); + assert.deepEqual(parsed.warnings, []); +}); + +test("normalizeRepoList: a mapping's own maxConcurrentWorktrees overrides the per-repo default", () => { + const parsed = parseFleetRunManifest({ + repos: [ + { repoFullName: "owner/a", maxConcurrentWorktrees: 4 }, // explicit budget honoured + { repoFullName: "owner/b", maxConcurrentWorktrees: "x" }, // non-numeric → default + warning + ], + }); + assert.deepEqual(parsed.manifest.repos, [ + { repoFullName: "owner/a", maxConcurrentWorktrees: 4 }, + { repoFullName: "owner/b", maxConcurrentWorktrees: DEFAULT_FLEET_RUN_MANIFEST_REPO_MAX_CONCURRENT_WORKTREES }, + ]); + assert.match(parsed.warnings.join(" "), /"maxConcurrentWorktrees" must be a positive whole number/); +}); + +test("the per-repo default is a distinct source of truth from the fleet-wide total default", () => { + // Both currently equal 1, but they are deliberately separate constants (#9324). + const parsed = parseFleetRunManifest({ repos: ["owner/a"] }); + assert.equal(parsed.manifest.repos[0]?.maxConcurrentWorktrees, DEFAULT_FLEET_RUN_MANIFEST_REPO_MAX_CONCURRENT_WORKTREES); + assert.equal(DEFAULT_FLEET_RUN_MANIFEST_REPO_MAX_CONCURRENT_WORKTREES, DEFAULT_FLEET_RUN_MANIFEST.totalConcurrentWorktrees); +}); 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([]);