Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion packages/loopover-miner/lib/portfolio-queue-cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,14 @@ export function selectNextEligibleTarget(entries, caps) {
}
const globalActiveCount = entries.filter((entry) => entry.status === "in_progress").length;
if (globalActiveCount >= caps.globalWipCap) return [];
// Host-scope the per-repo active count (#7224): a same-named repo on a DIFFERENT forge host is a distinct backlog
// (the store keys rows by apiBaseUrl too, #5563), so an in-progress item on host A must not consume host B's
// per-repo WIP budget. Single-host is unchanged: every entry shares one apiBaseUrl, so the added match is always true.
const repoActiveCount = entries.filter(
(entry) => entry.status === "in_progress" && entry.repoFullName === topQueued.repoFullName,
(entry) =>
entry.status === "in_progress" &&
entry.repoFullName === topQueued.repoFullName &&
entry.apiBaseUrl === topQueued.apiBaseUrl,
).length;
if (repoActiveCount >= caps.perRepoWipCap) return [];
return [{ repoFullName: topQueued.repoFullName, identifier: topQueued.identifier, apiBaseUrl: topQueued.apiBaseUrl }];
Expand Down
26 changes: 18 additions & 8 deletions packages/loopover-miner/lib/portfolio-queue-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,22 +56,32 @@ export function entriesToPortfolioQueue(entries) {
// Falls back to the github.com default (matching every store's own normalizeApiBaseUrl) so a row from
// before #5563 threaded apiBaseUrl through this fold still gets a valid, host-scoped id.
const apiBaseUrl = typeof entry.apiBaseUrl === "string" && entry.apiBaseUrl.trim() ? entry.apiBaseUrl.trim() : DEFAULT_FORGE_CONFIG.apiBaseUrl;
const repoKey = repoFullName.toLowerCase();
// Host-qualify the engine's per-repo WIP grouping key (#7224). nextEligibleItems groups its per-repo cap by each
// item's `repoFullName`, which it treats as an OPAQUE string -- the engine has no apiBaseUrl concept, the host is
// smuggled through the opaque `id` (queueItemId, #5563). The store keys rows by apiBaseUrl too, so two forge
// hosts' same-named repos are distinct backlogs; without qualifying the grouping key by host here, a per-repo cap
// was shared across them (e.g. perRepoWipCap: 1 let only ONE host's backlog advance). The `id` still carries the
// TRUE repoFullName and selectEligibleBatch maps results back via parseQueueItemId(id), so the real repo/host
// survive to the caller. Single-host behavior is unchanged: one apiBaseUrl means one grouping key per repo.
const repoLower = repoFullName.toLowerCase();
const repoKey = `${apiBaseUrl}\n${repoLower}`;
if (!bucketsByRepo.has(repoKey)) {
bucketsByRepo.set(repoKey, []);
// The bucket's own repoFullName stays the plain repo (display/diversification), while each ITEM carries the
// host-qualified key the engine groups on -- so the returned bucket shape is unchanged for single-host.
bucketsByRepo.set(repoKey, { repoFullName: repoLower, items: [] });
bucketOrder.push(repoKey);
}
bucketsByRepo.get(repoKey).push({
bucketsByRepo.get(repoKey).items.push({
id: queueItemId(apiBaseUrl, repoFullName, identifier),
repoFullName,
repoFullName: repoKey,
state: entry.status === "in_progress" ? "in_progress" : "queued",
});
}
return {
buckets: bucketOrder.map((repoFullName) => ({
repoFullName,
items: bucketsByRepo.get(repoFullName),
})),
buckets: bucketOrder.map((repoKey) => {
const bucket = bucketsByRepo.get(repoKey);
return { repoFullName: bucket.repoFullName, items: bucket.items };
}),
};
}

Expand Down
12 changes: 12 additions & 0 deletions test/unit/miner-portfolio-queue-cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,18 @@ describe("loopover-miner portfolio queue CLI (#2292)", () => {
{ apiBaseUrl: "https://api.github.com", repoFullName: "acme/other", identifier: "b1" },
]);
});

it("REGRESSION: an in-progress item on one host does not consume another host's per-repo WIP budget (#7224)", () => {
const entries = [
entry({ status: "in_progress", identifier: "host-a", apiBaseUrl: "https://api.github.com" }),
entry({ status: "queued", identifier: "host-b", apiBaseUrl: "https://ghe.example.com/api/v3" }),
];
// Same repo name, cap 1 — but the in-progress item is on a DIFFERENT forge host, so host B's queued row is
// eligible. Before #7224 the host-A item was mis-counted against host B's cap and this returned [].
expect(selectNextEligibleTarget(entries, { globalWipCap: 5, perRepoWipCap: 1 })).toEqual([
{ apiBaseUrl: "https://ghe.example.com/api/v3", repoFullName: "acme/widgets", identifier: "host-b" },
]);
});
});

it("runQueueNext with --global-wip/--per-repo-wip claims only within the configured caps (#4850)", () => {
Expand Down
15 changes: 15 additions & 0 deletions test/unit/miner-portfolio-queue-manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,21 @@ describe("initPortfolioQueueManager().claimNextBatch() (#4285)", () => {
expect(rows.every((row) => row.status === "in_progress")).toBe(true);
});

it("REGRESSION: a binding per-repo WIP cap of 1 still lets two hosts' same-named repos each advance (#7224)", () => {
const manager = memoryManager({ globalWipCap: 4, perRepoWipCap: 1 });
manager.enqueue({ repoFullName: "acme/widgets", identifier: "issue:1", priority: 1, apiBaseUrl: "https://api.github.com" });
manager.enqueue({ repoFullName: "acme/widgets", identifier: "issue:1", priority: 1, apiBaseUrl: "https://ghe.example.com/api/v3" });

// The cap binds PER HOST: each host's independent backlog gets its one claim. Before #7224, the two hosts were
// bucketed together under the bare repoFullName, so a cap of 1 let only ONE of them advance.
const claimed = manager.claimNextBatch();
expect(claimed).toHaveLength(2);
expect(claimed.map((entry) => entry.apiBaseUrl).sort()).toEqual([
"https://api.github.com",
"https://ghe.example.com/api/v3",
]);
});

it("does not claim rows another writer already took inside the same transaction window", () => {
const store = initPortfolioQueueStore(":memory:");
stores.push(store);
Expand Down