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
3 changes: 2 additions & 1 deletion packages/gittensory-miner/lib/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ export function printHelp(input) {
" gittensory-miner loop --search <query> --miner-login <login> [--max-cycles <n>] [--cycle-delay-ms <ms>] [--dry-run] [--json]",
" Autonomous discover->claim->attempt->reenter loop",
" gittensory-miner queue list [--repo <owner/repo>] [--json] List portfolio backlog rows",
" gittensory-miner queue next [--dry-run] [--json] Claim the highest-priority queued item",
" gittensory-miner queue next [--global-wip <n>] [--per-repo-wip <n>] [--dry-run] [--json]",
" Claim the highest-priority queued item, optionally WIP-cap-aware",
" gittensory-miner queue claim-batch [--global-wip <n>] [--per-repo-wip <n>] [--dry-run] [--json]",
" gittensory-miner queue done <owner/repo> <identifier> [--dry-run] [--json]",
" gittensory-miner queue release <owner/repo> <identifier> [--dry-run] [--json] Return a claimed item to the queue",
Expand Down
11 changes: 10 additions & 1 deletion packages/gittensory-miner/lib/portfolio-queue-cli.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,16 @@ export type ParsedQueueListArgs =
}
| { error: string };

export type ParsedQueueNextArgs = { json: boolean; dryRun: boolean } | { error: string };
export type ParsedQueueNextArgs =
| { json: boolean; dryRun: boolean; globalWipCap: number | undefined; perRepoWipCap: number | undefined }
| { error: string };

export type QueueClaimTarget = { repoFullName: string; identifier: string; apiBaseUrl: string };

export function selectNextEligibleTarget(
entries: Array<{ repoFullName: string; identifier: string; apiBaseUrl: string; status: string }>,
caps: { globalWipCap: number; perRepoWipCap: number } | null,
): QueueClaimTarget[];

export type ParsedQueueDoneArgs =
| {
Expand Down
90 changes: 83 additions & 7 deletions packages/gittensory-miner/lib/portfolio-queue-cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import { runPortfolioDashboard } from "./portfolio-dashboard.js";
import { argsWantJson, describeCliError, reportCliFailure } from "./cli-error.js";

const QUEUE_LIST_USAGE = "Usage: gittensory-miner queue list [--repo <owner/repo>] [--json]";
const QUEUE_NEXT_USAGE = "Usage: gittensory-miner queue next [--dry-run] [--json]";
const QUEUE_NEXT_USAGE =
"Usage: gittensory-miner queue next [--global-wip <n>] [--per-repo-wip <n>] [--dry-run] [--json]";
const QUEUE_DONE_USAGE =
"Usage: gittensory-miner queue done <owner/repo> <identifier> [--api-base-url <url>] [--dry-run] [--json]";
const QUEUE_RELEASE_USAGE =
Expand Down Expand Up @@ -81,13 +82,70 @@ export function parseQueueListArgs(args) {
return options;
}

// #4850: --global-wip/--per-repo-wip are OMITTED (undefined) by default -- queue next stays uncapped, byte-
// identical to its pre-#4850 behavior, unless an operator explicitly opts in. Mirrors queue claim-batch's own
// flag names (portfolio-queue-manager.js's WIP-cap-aware claimer), but claim-batch's OWN default of 1/1 is not
// reused here: claim-batch's whole purpose is cap enforcement, while queue next has always been a plain
// highest-priority dequeue and must not silently start capping existing callers that never asked for it.
export function parseQueueNextArgs(args) {
const parsed = parseJsonFlag(args);
if ("error" in parsed) return parsed;
if (parsed.positional.length > 0) {
const options = { json: false, dryRun: false, globalWipCap: undefined, perRepoWipCap: undefined };
const positional = [];

for (let index = 0; index < args.length; index += 1) {
const token = args[index];
if (token === "--json") {
options.json = true;
continue;
}
if (token === "--dry-run") {
options.dryRun = true;
continue;
}
if (token === "--global-wip" || token === "--per-repo-wip") {
const value = Number(args[index + 1]);
if (args[index + 1] === undefined || !Number.isFinite(value) || value < 0) {
return { error: QUEUE_NEXT_USAGE };
}
if (token === "--global-wip") options.globalWipCap = value;
else options.perRepoWipCap = value;
index += 1;
continue;
}
if (token.startsWith("-")) {
return { error: `Unknown option: ${token}` };
}
positional.push(token);
}

if (positional.length > 0) {
return { error: QUEUE_NEXT_USAGE };
}
return { json: parsed.json, dryRun: parsed.dryRun };
return options;
}

/**
* Pick at most one atomically-claimable target from the store's already-priority-ordered active rows (queued
* AND in_progress interleaved, exactly `batchClaim`'s own `entries` shape). `caps` of `null` replicates the
* pre-#4850 behavior: the single highest-priority queued row, unconditionally. When caps are set, refuses to
* select anything once the global or the target row's own per-repo in-progress count has reached its cap --
* "stops claiming once the cap is reached" (#4850), not a diversifying batch selection (that remains
* claim-batch's job via the engine's own `nextEligibleItems`).
* @param {Array<{ repoFullName: string, identifier: string, apiBaseUrl: string, status: string }>} entries
* @param {{ globalWipCap: number, perRepoWipCap: number } | null} caps
*/
export function selectNextEligibleTarget(entries, caps) {
const topQueued = entries.find((entry) => entry.status === "queued");
if (!topQueued) return [];
if (!caps) {
return [{ repoFullName: topQueued.repoFullName, identifier: topQueued.identifier, apiBaseUrl: topQueued.apiBaseUrl }];
}
const globalActiveCount = entries.filter((entry) => entry.status === "in_progress").length;
if (globalActiveCount >= caps.globalWipCap) return [];
const repoActiveCount = entries.filter(
(entry) => entry.status === "in_progress" && entry.repoFullName === topQueued.repoFullName,
).length;
if (repoActiveCount >= caps.perRepoWipCap) return [];
return [{ repoFullName: topQueued.repoFullName, identifier: topQueued.identifier, apiBaseUrl: topQueued.apiBaseUrl }];
}

/** Shared `<owner/repo> <identifier> [--api-base-url <url>] [--json]` parse for the item-targeting subcommands
Expand Down Expand Up @@ -220,10 +278,17 @@ export function runQueueNext(args, options = {}) {
return reportCliFailure(argsWantJson(args), parsed.error);
}

const capsRequested = parsed.globalWipCap !== undefined || parsed.perRepoWipCap !== undefined;
if (parsed.dryRun) {
const dryRunResult = { outcome: "dry_run" };
const dryRunResult = capsRequested
? { outcome: "dry_run", globalWipCap: parsed.globalWipCap, perRepoWipCap: parsed.perRepoWipCap }
: { outcome: "dry_run" };
if (parsed.json) {
console.log(JSON.stringify(dryRunResult, null, 2));
} else if (capsRequested) {
console.log(
`DRY RUN: would dequeue the highest-priority queued item within WIP caps (global-wip: ${parsed.globalWipCap ?? "unset"}, per-repo-wip: ${parsed.perRepoWipCap ?? "unset"}). No portfolio-queue write was made.`,
);
} else {
console.log("DRY RUN: would dequeue the highest-priority queued item. No portfolio-queue write was made.");
}
Expand All @@ -232,7 +297,18 @@ export function runQueueNext(args, options = {}) {

try {
return withPortfolioQueue(options, (portfolioQueue) => {
const entry = portfolioQueue.dequeueNext();
let entry;
if (capsRequested) {
// Unset dimensions stay genuinely uncapped (Infinity), not silently defaulted to 1 like claim-batch.
const caps = {
globalWipCap: parsed.globalWipCap ?? Number.POSITIVE_INFINITY,
perRepoWipCap: parsed.perRepoWipCap ?? Number.POSITIVE_INFINITY,
};
const claimed = portfolioQueue.batchClaim((entries) => selectNextEligibleTarget(entries, caps));
entry = claimed[0] ?? null;
} else {
entry = portfolioQueue.dequeueNext();
}
if (parsed.json) {
console.log(JSON.stringify({ entry }, null, 2));
} else {
Expand Down
159 changes: 159 additions & 0 deletions test/unit/miner-portfolio-queue-cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
runQueueNext,
runQueueRelease,
runQueueRequeue,
selectNextEligibleTarget,
} from "../../packages/gittensory-miner/lib/portfolio-queue-cli.js";
import type { QueueEntry } from "../../packages/gittensory-miner/lib/portfolio-queue.d.ts";

Expand Down Expand Up @@ -131,6 +132,164 @@ describe("gittensory-miner portfolio queue CLI (#2292)", () => {
expect(log).toHaveBeenCalledWith("none");
});

describe("selectNextEligibleTarget() (#4850)", () => {
function entry(overrides: Record<string, unknown> = {}) {
return {
apiBaseUrl: "https://github.com/ghapi",
repoFullName: "acme/widgets",
identifier: "issue:1",
status: "queued",
...overrides,
};
}

it("null caps replicates the pre-#4850 unconditional highest-priority selection", () => {
const entries = [entry({ status: "in_progress", identifier: "in-flight" }), entry()];
expect(selectNextEligibleTarget(entries, null)).toEqual([
{ apiBaseUrl: "https://github.com/ghapi", repoFullName: "acme/widgets", identifier: "issue:1" },
]);
});

it("returns nothing when there is no queued row, regardless of caps", () => {
expect(selectNextEligibleTarget([], null)).toEqual([]);
expect(selectNextEligibleTarget([entry({ status: "in_progress" })], { globalWipCap: 5, perRepoWipCap: 5 })).toEqual([]);
});

it("refuses to select once the global cap is already reached", () => {
const entries = [
entry({ status: "in_progress", identifier: "a", repoFullName: "acme/a" }),
entry({ status: "in_progress", identifier: "b", repoFullName: "acme/b" }),
entry({ status: "queued", identifier: "c", repoFullName: "acme/c" }),
];
expect(selectNextEligibleTarget(entries, { globalWipCap: 2, perRepoWipCap: 5 })).toEqual([]);
expect(selectNextEligibleTarget(entries, { globalWipCap: 3, perRepoWipCap: 5 })).toEqual([
{ apiBaseUrl: "https://github.com/ghapi", repoFullName: "acme/c", identifier: "c" },
]);
});

it("refuses to select once the top queued row's own repo has reached its per-repo cap", () => {
const entries = [
entry({ status: "in_progress", identifier: "a1" }),
entry({ status: "queued", identifier: "a2" }),
];
expect(selectNextEligibleTarget(entries, { globalWipCap: 5, perRepoWipCap: 1 })).toEqual([]);
// A different repo's own cap isn't saturated, so a queued row there is still eligible.
const otherRepo = [
entry({ status: "in_progress", identifier: "a1" }),
entry({ status: "queued", identifier: "b1", repoFullName: "acme/other" }),
];
expect(selectNextEligibleTarget(otherRepo, { globalWipCap: 5, perRepoWipCap: 1 })).toEqual([
{ apiBaseUrl: "https://github.com/ghapi", repoFullName: "acme/other", identifier: "b1" },
]);
});
});

it("runQueueNext with --global-wip/--per-repo-wip claims only within the configured caps (#4850)", () => {
const portfolioQueue = tempQueueStore();
portfolioQueue.enqueue({ repoFullName: "acme/widgets", identifier: "issue:1", priority: 1 });
portfolioQueue.dequeueNext(); // one already in-flight, uncapped

const log = vi.spyOn(console, "log").mockImplementation(() => undefined);
portfolioQueue.enqueue({ repoFullName: "acme/widgets", identifier: "issue:2", priority: 1 });

// global-wip 1 with one already in_progress -- refuses to claim a second.
expect(
runQueueNext(["--global-wip", "1", "--json"], { initPortfolioQueue: () => portfolioQueue }),
).toBe(0);
expect(JSON.parse(String(log.mock.calls[0]?.[0]))).toEqual({ entry: null });
expect(portfolioQueue.listQueue("acme/widgets").find((e) => e.identifier === "issue:2")?.status).toBe("queued");

// Raising the cap lets it claim.
log.mockClear();
expect(
runQueueNext(["--global-wip", "2", "--json"], { initPortfolioQueue: () => portfolioQueue }),
).toBe(0);
expect(JSON.parse(String(log.mock.calls[0]?.[0]))).toEqual({
entry: expect.objectContaining({ identifier: "issue:2", status: "in_progress" }),
});
});

it("runQueueNext with only --per-repo-wip set leaves the global dimension genuinely uncapped (#4850)", () => {
const portfolioQueue = tempQueueStore();
portfolioQueue.enqueue({ repoFullName: "acme/alpha", identifier: "issue:1", priority: 1 });
portfolioQueue.dequeueNext(); // acme/alpha already at 1 in-flight
portfolioQueue.enqueue({ repoFullName: "acme/beta", identifier: "issue:2", priority: 1 });

const log = vi.spyOn(console, "log").mockImplementation(() => undefined);
// Global left unset (uncapped): a different repo (acme/beta) is still claimable even though acme/alpha
// already has an in-flight item -- only the per-repo dimension is enforced here.
expect(
runQueueNext(["--per-repo-wip", "1", "--json"], { initPortfolioQueue: () => portfolioQueue }),
).toBe(0);
expect(JSON.parse(String(log.mock.calls[0]?.[0]))).toEqual({
entry: expect.objectContaining({ identifier: "issue:2", repoFullName: "acme/beta", status: "in_progress" }),
});
});

it("runQueueNext without --global-wip/--per-repo-wip is unaffected by an already-saturated repo (#4850 backward compat)", () => {
const portfolioQueue = tempQueueStore();
portfolioQueue.enqueue({ repoFullName: "acme/widgets", identifier: "issue:1", priority: 1 });
portfolioQueue.dequeueNext();
portfolioQueue.enqueue({ repoFullName: "acme/widgets", identifier: "issue:2", priority: 1 });

const log = vi.spyOn(console, "log").mockImplementation(() => undefined);
expect(runQueueNext(["--json"], { initPortfolioQueue: () => portfolioQueue })).toBe(0);
expect(JSON.parse(String(log.mock.calls[0]?.[0]))).toEqual({
entry: expect.objectContaining({ identifier: "issue:2", status: "in_progress" }),
});
});

it("parseQueueNextArgs accepts --global-wip/--per-repo-wip and rejects a malformed value (#4850)", () => {
expect(parseQueueNextArgs([])).toEqual({
json: false,
dryRun: false,
globalWipCap: undefined,
perRepoWipCap: undefined,
});
expect(parseQueueNextArgs(["--global-wip", "3", "--per-repo-wip", "2"])).toEqual({
json: false,
dryRun: false,
globalWipCap: 3,
perRepoWipCap: 2,
});
expect(parseQueueNextArgs(["--global-wip", "not-a-number"])).toEqual({
error: expect.stringContaining("Usage: gittensory-miner queue next"),
});
expect(parseQueueNextArgs(["--global-wip"])).toEqual({
error: expect.stringContaining("Usage: gittensory-miner queue next"),
});
expect(parseQueueNextArgs(["extra-positional"])).toEqual({
error: expect.stringContaining("Usage: gittensory-miner queue next"),
});
expect(parseQueueNextArgs(["--bogus"])).toEqual({ error: "Unknown option: --bogus" });
});

it("runQueueNext --dry-run reports the requested caps when set (#4850)", () => {
const log = vi.spyOn(console, "log").mockImplementation(() => undefined);
const initPortfolioQueueSpy = vi.fn();

expect(
runQueueNext(["--dry-run", "--global-wip", "2", "--json"], { initPortfolioQueue: initPortfolioQueueSpy }),
).toBe(0);
expect(initPortfolioQueueSpy).not.toHaveBeenCalled();
expect(JSON.parse(String(log.mock.calls[0]?.[0]))).toEqual({
outcome: "dry_run",
globalWipCap: 2,
perRepoWipCap: undefined,
});

log.mockClear();
expect(runQueueNext(["--dry-run", "--per-repo-wip", "1"], { initPortfolioQueue: initPortfolioQueueSpy })).toBe(0);
expect(String(log.mock.calls[0]?.[0])).toContain("DRY RUN: would dequeue the highest-priority queued item within WIP caps");
expect(String(log.mock.calls[0]?.[0])).toContain("global-wip: unset");
expect(String(log.mock.calls[0]?.[0])).toContain("per-repo-wip: 1");

log.mockClear();
expect(runQueueNext(["--dry-run", "--global-wip", "2"], { initPortfolioQueue: initPortfolioQueueSpy })).toBe(0);
expect(String(log.mock.calls[0]?.[0])).toContain("global-wip: 2");
expect(String(log.mock.calls[0]?.[0])).toContain("per-repo-wip: unset");
});

it("#4847: --dry-run reports what next/done would do and returns 0 without opening the portfolio queue", () => {
const log = vi.spyOn(console, "log").mockImplementation(() => undefined);
const initPortfolioQueueSpy = vi.fn();
Expand Down