From 9c2ffaf4e38bb69ee3fe1fbea1e0f26b0d215071 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Mon, 13 Jul 2026 00:07:46 -0700 Subject: [PATCH] fix(miner): scope portfolio-queue by forge host, not bare repoFullName PRIMARY KEY (repo_full_name, identifier) let two different forge hosts (github.com vs. a GitHub Enterprise host, #4784) serving a same-named owner/repo collide in the portfolio queue. Rebuild the constraint to PRIMARY KEY (api_base_url, repo_full_name, identifier) as the store's next schema migration (v2 -> v3, after the existing leased_at migration), backfilling existing rows with the pre-#4784 implicit default. Thread an optional apiBaseUrl through the store's API (enqueue/markDone/ markFailed/reclaimStuckItem/requeueItem/batchClaim), the admin CLI (--api-base-url on done/release/requeue), and the two places that already had a resolved apiBaseUrl in scope but weren't passing it through: enqueueRankedDiscovery (discover-cli.js's real discovery pipeline) and the stuck-lease/malformed-identifier sweeps in portfolio-queue-expiry.js and loop-cli.js, which now echo each item's own apiBaseUrl back instead of defaulting -- a defaulted echo would touch the wrong host's row whenever two hosts share an owner/repo+identifier. markDone now uses UPDATE ... RETURNING (matching this store's own existing statements), removing a structurally-unreachable defensive branch the old separate-SELECT pattern left behind. Every existing caller is unaffected: apiBaseUrl defaults to https://api.github.com when omitted. claimNextBatch's engine-driven selection (portfolio-queue-manager.js) has no forge dimension in @jsonbored/gittensory-engine's PortfolioQueueItem shape -- documented as a known, safe (no-collision) limitation rather than silently patched over. Advances #5563 (portfolio-queue.js of 5 affected stores; claim-ledger.js landed in #5576). --- packages/gittensory-miner/lib/discover-cli.js | 2 +- packages/gittensory-miner/lib/loop-cli.js | 12 +- .../lib/portfolio-discovery.d.ts | 1 + .../lib/portfolio-discovery.js | 5 + .../lib/portfolio-queue-cli.d.ts | 1 + .../lib/portfolio-queue-cli.js | 63 ++++++--- .../lib/portfolio-queue-expiry.js | 4 +- .../lib/portfolio-queue-manager.d.ts | 4 +- .../lib/portfolio-queue-manager.js | 14 +- .../gittensory-miner/lib/portfolio-queue.d.ts | 19 ++- .../gittensory-miner/lib/portfolio-queue.js | 116 ++++++++++++----- test/unit/miner-discover-cli.test.ts | 5 + test/unit/miner-loop-cli.test.ts | 33 +++++ test/unit/miner-migrate-cli.test.ts | 11 +- test/unit/miner-portfolio-discovery.test.ts | 13 ++ test/unit/miner-portfolio-queue-cli.test.ts | 66 ++++++++++ ...ner-portfolio-queue-crash-recovery.test.ts | 8 +- .../unit/miner-portfolio-queue-expiry.test.ts | 31 ++++- .../miner-portfolio-queue-manager.test.ts | 22 +++- test/unit/miner-portfolio-queue.test.ts | 122 ++++++++++++++++++ 20 files changed, 473 insertions(+), 79 deletions(-) diff --git a/packages/gittensory-miner/lib/discover-cli.js b/packages/gittensory-miner/lib/discover-cli.js index 0d6364d203..c92a4a1ac2 100644 --- a/packages/gittensory-miner/lib/discover-cli.js +++ b/packages/gittensory-miner/lib/discover-cli.js @@ -256,7 +256,7 @@ export async function runDiscover(args, options = {}) { goalSpecsByRepo: options.goalSpecsByRepo, goalSpecContentByRepo: options.goalSpecContentByRepo, }); - const enqueueSummary = enqueue(rankedSummary.issues, { queueStore: portfolioQueue }); + const enqueueSummary = enqueue(rankedSummary.issues, { queueStore: portfolioQueue, apiBaseUrl }); const result = { fanOutCount: fanOut.issues.length, diff --git a/packages/gittensory-miner/lib/loop-cli.js b/packages/gittensory-miner/lib/loop-cli.js index 4b3a353fdb..76662fb768 100644 --- a/packages/gittensory-miner/lib/loop-cli.js +++ b/packages/gittensory-miner/lib/loop-cli.js @@ -346,7 +346,7 @@ export async function runLoop(args, options = {}) { if (issueNumber === null) { // Never produced by enqueueRankedDiscovery in practice (always "issue:N") -- fail soft rather than // crash the whole run: this exact item can never be attempted, so it will never resolve on retry. - portfolioQueue.markDone(claimed.repoFullName, claimed.identifier); + portfolioQueue.markDone(claimed.repoFullName, claimed.identifier, claimed.apiBaseUrl); cycles.push({ cycle: cycleIndex, outcome: "skipped_malformed_identifier", identifier: claimed.identifier }); claimed = portfolioQueue.dequeueNext(); continue; @@ -364,7 +364,9 @@ export async function runLoop(args, options = {}) { convergence: convergenceInput, convergenceThresholds: amsPolicy.spec.convergenceThresholds ?? DEFAULT_AMS_POLICY_SPEC.convergenceThresholds, inFlightItem: { repoFullName: claimed.repoFullName, identifier: claimed.identifier }, - markFailed: (repoFullName, identifier) => portfolioQueue.markFailed(repoFullName, identifier), + // Echoes claimed.apiBaseUrl (#5563), NOT the callback's own repoFullName/identifier alone -- two forge + // hosts can share an in-flight item with the same repo name+identifier. + markFailed: (repoFullName, identifier) => portfolioQueue.markFailed(repoFullName, identifier, claimed.apiBaseUrl), }, { append: (event) => governorLedger.appendGovernorEvent(event) }, ); @@ -419,14 +421,14 @@ export async function runLoop(args, options = {}) { const permanentBlock = attemptOutcome === "blocked_rejection_signaled"; if (submitted) { - portfolioQueue.markDone(claimed.repoFullName, claimed.identifier); + portfolioQueue.markDone(claimed.repoFullName, claimed.identifier, claimed.apiBaseUrl); convergenceInput.reachedDone = true; convergenceInput.consecutiveFailures = 0; } else if (permanentBlock) { - portfolioQueue.markDone(claimed.repoFullName, claimed.identifier); + portfolioQueue.markDone(claimed.repoFullName, claimed.identifier, claimed.apiBaseUrl); convergenceInput.consecutiveFailures += 1; } else { - portfolioQueue.markFailed(claimed.repoFullName, claimed.identifier); + portfolioQueue.markFailed(claimed.repoFullName, claimed.identifier, claimed.apiBaseUrl); convergenceInput.consecutiveFailures += 1; convergenceInput.reenqueues += 1; } diff --git a/packages/gittensory-miner/lib/portfolio-discovery.d.ts b/packages/gittensory-miner/lib/portfolio-discovery.d.ts index bed57cb072..fa1052facf 100644 --- a/packages/gittensory-miner/lib/portfolio-discovery.d.ts +++ b/packages/gittensory-miner/lib/portfolio-discovery.d.ts @@ -13,6 +13,7 @@ export type EnqueueRankedDiscoveryOptions = { queueStore: PortfolioQueueStore; eventLedger?: EventLedger; minRankScore?: number | null; + apiBaseUrl?: string; }; export type EnqueueRankedDiscoverySummary = { diff --git a/packages/gittensory-miner/lib/portfolio-discovery.js b/packages/gittensory-miner/lib/portfolio-discovery.js index 89abca883a..821955120e 100644 --- a/packages/gittensory-miner/lib/portfolio-discovery.js +++ b/packages/gittensory-miner/lib/portfolio-discovery.js @@ -50,6 +50,10 @@ export function enqueueRankedDiscovery(rankedIssues, options = {}) { } const minRankScore = normalizeMinRankScore(options.minRankScore); + // #5563: threaded through from the caller's already-resolved forge host, so a non-default (GitHub Enterprise) + // tenant's ranked issues land in the queue scoped to their own host instead of colliding with a same-named + // owner/repo on github.com. Omitted/nullish falls through to the queue store's own github.com default. + const apiBaseUrl = options.apiBaseUrl; const summary = { enqueued: 0, @@ -73,6 +77,7 @@ export function enqueueRankedDiscovery(rankedIssues, options = {}) { repoFullName: normalized.repoFullName, identifier: `issue:${normalized.issueNumber}`, priority: normalized.rankScore, + apiBaseUrl, }); summary.enqueued += 1; diff --git a/packages/gittensory-miner/lib/portfolio-queue-cli.d.ts b/packages/gittensory-miner/lib/portfolio-queue-cli.d.ts index 276403a815..5fd7ae3ebc 100644 --- a/packages/gittensory-miner/lib/portfolio-queue-cli.d.ts +++ b/packages/gittensory-miner/lib/portfolio-queue-cli.d.ts @@ -16,6 +16,7 @@ export type ParsedQueueDoneArgs = identifier: string; dryRun: boolean; json: boolean; + apiBaseUrl: string | undefined; } | { error: string }; diff --git a/packages/gittensory-miner/lib/portfolio-queue-cli.js b/packages/gittensory-miner/lib/portfolio-queue-cli.js index ee7a44d932..3a76a1ee34 100644 --- a/packages/gittensory-miner/lib/portfolio-queue-cli.js +++ b/packages/gittensory-miner/lib/portfolio-queue-cli.js @@ -5,9 +5,12 @@ import { argsWantJson, describeCliError, reportCliFailure } from "./cli-error.js const QUEUE_LIST_USAGE = "Usage: gittensory-miner queue list [--repo ] [--json]"; const QUEUE_NEXT_USAGE = "Usage: gittensory-miner queue next [--dry-run] [--json]"; -const QUEUE_DONE_USAGE = "Usage: gittensory-miner queue done [--dry-run] [--json]"; -const QUEUE_RELEASE_USAGE = "Usage: gittensory-miner queue release [--dry-run] [--json]"; -const QUEUE_REQUEUE_USAGE = "Usage: gittensory-miner queue requeue [--dry-run] [--json]"; +const QUEUE_DONE_USAGE = + "Usage: gittensory-miner queue done [--api-base-url ] [--dry-run] [--json]"; +const QUEUE_RELEASE_USAGE = + "Usage: gittensory-miner queue release [--api-base-url ] [--dry-run] [--json]"; +const QUEUE_REQUEUE_USAGE = + "Usage: gittensory-miner queue requeue [--api-base-url ] [--dry-run] [--json]"; const QUEUE_CLAIM_BATCH_USAGE = "Usage: gittensory-miner queue claim-batch [--global-wip ] [--per-repo-wip ] [--dry-run] [--json]"; @@ -87,19 +90,48 @@ export function parseQueueNextArgs(args) { return { json: parsed.json, dryRun: parsed.dryRun }; } -/** Shared ` [--json]` parse for the item-targeting subcommands (done/release/requeue). - * `usage` is the command-specific message surfaced on a malformed argv. */ +/** Shared ` [--api-base-url ] [--json]` parse for the item-targeting subcommands + * (done/release/requeue). `usage` is the command-specific message surfaced on a malformed argv. */ function parseRepoIdentifierArgs(args, usage) { - const parsed = parseJsonFlag(args); - if ("error" in parsed) return parsed; - if (parsed.positional.length !== 2) { + const options = { json: false, dryRun: false, apiBaseUrl: undefined }; + const positional = []; + + for (let index = 0; index < args.length; index += 1) { + const token = args[index]; + if (token === "--json") { + options.json = true; + continue; + } + // #4847: reports what a real mutation would do and returns before opening the portfolio queue at all. + if (token === "--dry-run") { + options.dryRun = true; + continue; + } + // #5563: scope the target to a non-default forge host, so it doesn't collide with (or get confused for) a + // same-named repo on the default github.com host. + if (token === "--api-base-url") { + const value = args[index + 1]; + if (!value || value.startsWith("-")) { + return { error: usage }; + } + options.apiBaseUrl = value; + index += 1; + continue; + } + if (token.startsWith("-")) { + return { error: `Unknown option: ${token}` }; + } + positional.push(token); + } + + if (positional.length !== 2) { return { error: usage }; } - const repo = parseRepoArg(parsed.positional[0], usage); + const repo = parseRepoArg(positional[0], usage); if ("error" in repo) return repo; - const identifier = parsed.positional[1]?.trim(); + const identifier = positional[1]?.trim(); if (!identifier) { return { error: usage }; } @@ -107,8 +139,9 @@ function parseRepoIdentifierArgs(args, usage) { return { repoFullName: repo.repoFullName, identifier, - dryRun: parsed.dryRun, - json: parsed.json, + dryRun: options.dryRun, + json: options.json, + apiBaseUrl: options.apiBaseUrl, }; } @@ -230,7 +263,7 @@ export function runQueueDone(args, options = {}) { try { return withPortfolioQueue(options, (portfolioQueue) => { - const entry = portfolioQueue.markDone(parsed.repoFullName, parsed.identifier); + const entry = portfolioQueue.markDone(parsed.repoFullName, parsed.identifier, parsed.apiBaseUrl); if (!entry) { return reportCliFailure(parsed.json, "queue_entry_not_found"); } @@ -266,7 +299,7 @@ export function runQueueRelease(args, options = {}) { try { return withPortfolioQueue(options, (portfolioQueue) => { - const entry = portfolioQueue.reclaimStuckItem(parsed.repoFullName, parsed.identifier); + const entry = portfolioQueue.reclaimStuckItem(parsed.repoFullName, parsed.identifier, parsed.apiBaseUrl); if (!entry) { return reportCliFailure(parsed.json, "queue_entry_not_in_progress"); } @@ -303,7 +336,7 @@ export function runQueueRequeue(args, options = {}) { try { return withPortfolioQueue(options, (portfolioQueue) => { - const entry = portfolioQueue.requeueItem(parsed.repoFullName, parsed.identifier); + const entry = portfolioQueue.requeueItem(parsed.repoFullName, parsed.identifier, parsed.apiBaseUrl); if (!entry) { return reportCliFailure(parsed.json, "queue_entry_not_requeuable"); } diff --git a/packages/gittensory-miner/lib/portfolio-queue-expiry.js b/packages/gittensory-miner/lib/portfolio-queue-expiry.js index 209fae0e71..d88387897b 100644 --- a/packages/gittensory-miner/lib/portfolio-queue-expiry.js +++ b/packages/gittensory-miner/lib/portfolio-queue-expiry.js @@ -42,7 +42,9 @@ export function sweepStuckItems(store, nowMs, maxLeaseMs = DEFAULT_MAX_LEASE_MS) const stuck = findStuckItems(inProgress, nowMs, maxLeaseMs); const reclaimed = []; for (const item of stuck) { - const updated = store.reclaimStuckItem(item.repoFullName, item.identifier); + // Echo the item's OWN apiBaseUrl back (#5563) rather than defaulting: two forge hosts can each have an + // in-flight item with the same owner/repo+identifier, and defaulting here would reclaim the wrong host's row. + const updated = store.reclaimStuckItem(item.repoFullName, item.identifier, item.apiBaseUrl); if (updated) reclaimed.push(updated); } return reclaimed; diff --git a/packages/gittensory-miner/lib/portfolio-queue-manager.d.ts b/packages/gittensory-miner/lib/portfolio-queue-manager.d.ts index 49ce435943..9dd469ff07 100644 --- a/packages/gittensory-miner/lib/portfolio-queue-manager.d.ts +++ b/packages/gittensory-miner/lib/portfolio-queue-manager.d.ts @@ -30,8 +30,8 @@ export type PortfolioQueueManager = { dbPath: string; enqueue(item: EnqueueItem): QueueEntry; listQueue(repoFullName?: string | null): QueueEntry[]; - markDone(repoFullName: string, identifier: string): QueueEntry | null; - markFailed(repoFullName: string, identifier: string): QueueEntry | null; + markDone(repoFullName: string, identifier: string, apiBaseUrl?: string): QueueEntry | null; + markFailed(repoFullName: string, identifier: string, apiBaseUrl?: string): QueueEntry | null; reclaimStuckItems(maxLeaseMs?: number): QueueEntry[]; claimNextBatch(): QueueEntry[]; close(): void; diff --git a/packages/gittensory-miner/lib/portfolio-queue-manager.js b/packages/gittensory-miner/lib/portfolio-queue-manager.js index 06cd2fdec9..cc5d7bb25c 100644 --- a/packages/gittensory-miner/lib/portfolio-queue-manager.js +++ b/packages/gittensory-miner/lib/portfolio-queue-manager.js @@ -89,16 +89,22 @@ export function initPortfolioQueueManager(options = {}) { listQueue(repoFullName) { return store.listQueue(repoFullName); }, - markDone(repoFullName, identifier) { - return store.markDone(repoFullName, identifier); + markDone(repoFullName, identifier, apiBaseUrl) { + return store.markDone(repoFullName, identifier, apiBaseUrl); }, - markFailed(repoFullName, identifier) { - return store.markFailed(repoFullName, identifier); + markFailed(repoFullName, identifier, apiBaseUrl) { + return store.markFailed(repoFullName, identifier, apiBaseUrl); }, /** Sweep leases orphaned by a crashed/killed process back to 'queued', returning the reclaimed items (#4827). */ reclaimStuckItems(maxLeaseMs = staleLeaseMs) { return sweepStuckItems(store, Date.now(), maxLeaseMs); }, + // NOTE (#5563): claimNextBatch's engine-driven selection (queueItemId/parseQueueItemId, entriesToPortfolioQueue) + // has no apiBaseUrl dimension -- @jsonbored/gittensory-engine's PortfolioQueueItem shape predates multi-forge + // support. selectFn below therefore never supplies target.apiBaseUrl, so batchClaim falls back to the + // github.com default for every claim; a non-default-host item enqueued under a different apiBaseUrl safely + // fails to match (no row, no claim, no corruption) rather than being claimed under the wrong host. Retrofitting + // the engine primitive itself with a forge dimension is out of this store-level fix's scope. claimNextBatch() { // Reclaim orphaned leases first, so an item stranded 'in_progress' by a dead process becomes eligible again // instead of permanently consuming a WIP slot and starving the queue. diff --git a/packages/gittensory-miner/lib/portfolio-queue.d.ts b/packages/gittensory-miner/lib/portfolio-queue.d.ts index 7f623adf43..15c9ff9d57 100644 --- a/packages/gittensory-miner/lib/portfolio-queue.d.ts +++ b/packages/gittensory-miner/lib/portfolio-queue.d.ts @@ -1,6 +1,7 @@ export type QueueStatus = "queued" | "in_progress" | "done"; export type QueueEntry = { + apiBaseUrl: string; repoFullName: string; identifier: string; priority: number; @@ -12,10 +13,12 @@ export type EnqueueItem = { repoFullName: string; identifier: string; priority?: number | null; + apiBaseUrl?: string; }; /** Lease-annotated view of an in-flight row: when it was claimed, for the expiry sweep (#4827). */ export type QueueLeaseEntry = { + apiBaseUrl: string; repoFullName: string; identifier: string; status: QueueStatus; @@ -28,12 +31,14 @@ export type PortfolioQueueStore = { dequeueNext(): QueueEntry | null; listQueue(repoFullName?: string | null): QueueEntry[]; listInProgress(): QueueLeaseEntry[]; - markDone(repoFullName: string, identifier: string): QueueEntry | null; - markFailed(repoFullName: string, identifier: string): QueueEntry | null; - reclaimStuckItem(repoFullName: string, identifier: string): QueueEntry | null; - requeueItem(repoFullName: string, identifier: string): QueueEntry | null; + markDone(repoFullName: string, identifier: string, apiBaseUrl?: string): QueueEntry | null; + markFailed(repoFullName: string, identifier: string, apiBaseUrl?: string): QueueEntry | null; + reclaimStuckItem(repoFullName: string, identifier: string, apiBaseUrl?: string): QueueEntry | null; + requeueItem(repoFullName: string, identifier: string, apiBaseUrl?: string): QueueEntry | null; batchClaim( - selectFn: (entries: QueueEntry[]) => Array<{ repoFullName: string; identifier: string }>, + selectFn: ( + entries: QueueEntry[], + ) => Array<{ repoFullName: string; identifier: string; apiBaseUrl?: string }>, ): QueueEntry[]; close(): void; }; @@ -50,8 +55,8 @@ export function dequeueNext(): QueueEntry | null; export function listQueue(repoFullName?: string | null): QueueEntry[]; -export function markDone(repoFullName: string, identifier: string): QueueEntry | null; +export function markDone(repoFullName: string, identifier: string, apiBaseUrl?: string): QueueEntry | null; -export function markFailed(repoFullName: string, identifier: string): QueueEntry | null; +export function markFailed(repoFullName: string, identifier: string, apiBaseUrl?: string): QueueEntry | null; export function closeDefaultPortfolioQueueStore(): void; diff --git a/packages/gittensory-miner/lib/portfolio-queue.js b/packages/gittensory-miner/lib/portfolio-queue.js index 7521c51f89..ff1d2bb590 100644 --- a/packages/gittensory-miner/lib/portfolio-queue.js +++ b/packages/gittensory-miner/lib/portfolio-queue.js @@ -1,3 +1,4 @@ +import { DEFAULT_FORGE_CONFIG } from "./forge-config.js"; import { normalizeLocalStoreDbPath, openLocalStoreDb, resolveLocalStoreDbPath } from "./local-store.js"; import { applySchemaMigrations } from "./schema-version.js"; @@ -44,8 +45,17 @@ function normalizePriority(priority) { return priority; } +/** Optional forge host, scoping rows so two hosts serving the same owner/repo name never collide (#5563). + * Omitted/nullish → the github.com default, so every pre-existing single-forge caller is unaffected. */ +function normalizeApiBaseUrl(apiBaseUrl) { + if (apiBaseUrl === undefined || apiBaseUrl === null) return DEFAULT_FORGE_CONFIG.apiBaseUrl; + if (typeof apiBaseUrl !== "string" || !apiBaseUrl.trim()) throw new Error("invalid_api_base_url"); + return apiBaseUrl.trim(); +} + function rowToEntry(row) { return { + apiBaseUrl: row.api_base_url, repoFullName: row.repo_full_name, identifier: row.identifier, priority: row.priority, @@ -58,6 +68,7 @@ function rowToEntry(row) { * from `rowToEntry` so the base entry shape every existing caller relies on is unchanged. */ function rowToLeaseEntry(row) { return { + apiBaseUrl: row.api_base_url, repoFullName: row.repo_full_name, identifier: row.identifier, status: row.status, @@ -92,6 +103,12 @@ export function initPortfolioQueueStore(dbPath = resolvePortfolioQueueDbPath()) // table, so add it idempotently. Expressed as the store's first schema migration (#4832): the baseline table is // version 1; migration 1→2 adds `leased_at`. The migration stays defensive (checks table_info) so a version-0 // file that already ran the pre-convention ad-hoc ALTER is not re-altered into a duplicate-column error. + // + // v2 -> v3 (#5563): rebuild PRIMARY KEY (repo_full_name, identifier) into PRIMARY KEY (api_base_url, + // repo_full_name, identifier) -- two forge hosts serving a same-named owner/repo must not collide in this + // queue. SQLite cannot ALTER a PRIMARY KEY in place, so this rebuilds the table: create the new shape, copy + // every existing row with the pre-#4784 implicit single-forge default backfilled, drop the old table, rename + // the new one in. applySchemaMigrations(db, [ (migrationDb) => { const hasLeasedAtColumn = migrationDb @@ -100,6 +117,32 @@ export function initPortfolioQueueStore(dbPath = resolvePortfolioQueueDbPath()) .some((column) => column.name === "leased_at"); if (!hasLeasedAtColumn) migrationDb.exec("ALTER TABLE miner_portfolio_queue ADD COLUMN leased_at TEXT"); }, + (migrationDb) => { + migrationDb.exec(` + CREATE TABLE miner_portfolio_queue_v3 ( + api_base_url TEXT NOT NULL, + repo_full_name TEXT NOT NULL, + identifier TEXT NOT NULL, + priority REAL NOT NULL DEFAULT 0, + status TEXT NOT NULL DEFAULT 'queued' CHECK (status IN ('queued', 'in_progress', 'done')), + enqueued_at TEXT NOT NULL, + leased_at TEXT, + PRIMARY KEY (api_base_url, repo_full_name, identifier) + ) + `); + // ORDER BY rowid preserves the old table's FIFO insertion order in the new table's freshly-assigned rowids + // (the composite PRIMARY KEY above is not itself the rowid), so this rebuild doesn't reshuffle queue order. + migrationDb + .prepare( + `INSERT INTO miner_portfolio_queue_v3 + (api_base_url, repo_full_name, identifier, priority, status, enqueued_at, leased_at) + SELECT ?, repo_full_name, identifier, priority, status, enqueued_at, leased_at + FROM miner_portfolio_queue ORDER BY rowid`, + ) + .run(DEFAULT_FORGE_CONFIG.apiBaseUrl); + migrationDb.exec("DROP TABLE miner_portfolio_queue"); + migrationDb.exec("ALTER TABLE miner_portfolio_queue_v3 RENAME TO miner_portfolio_queue"); + }, ]); // `rowid` is a stable, unique key assigned once at first insert (re-enqueue updates in place, never re-inserts), @@ -111,19 +154,20 @@ export function initPortfolioQueueStore(dbPath = resolvePortfolioQueueDbPath()) // jumping the queue. (Restamping `enqueued_at` would be inconsistent — the fixed `rowid` still pins the old // position whenever timestamps collide — so position is deliberately preserved instead.) const enqueueStatement = db.prepare(` - INSERT INTO miner_portfolio_queue (repo_full_name, identifier, priority, status, enqueued_at) - VALUES (?, ?, ?, 'queued', ?) - ON CONFLICT(repo_full_name, identifier) DO UPDATE SET + INSERT INTO miner_portfolio_queue (api_base_url, repo_full_name, identifier, priority, status, enqueued_at) + VALUES (?, ?, ?, ?, 'queued', ?) + ON CONFLICT(api_base_url, repo_full_name, identifier) DO UPDATE SET priority = excluded.priority, status = 'queued' WHERE miner_portfolio_queue.status <> 'in_progress' `); const getStatement = db.prepare( - "SELECT * FROM miner_portfolio_queue WHERE repo_full_name = ? AND identifier = ?", + "SELECT * FROM miner_portfolio_queue WHERE api_base_url = ? AND repo_full_name = ? AND identifier = ?", ); // Claim the highest-priority queued item ATOMICALLY: one UPDATE selects the ordered top row in a subquery and // flips it to 'in_progress', RETURNING it — so two processes sharing the file can't both claim the same row (a - // separate SELECT-then-UPDATE would race). + // separate SELECT-then-UPDATE would race). Deliberately global (no api_base_url filter): the queue is a single + // cross-host priority ordering, not a per-host one. // Claiming stamps `leased_at` with the caller-supplied claim time; leaving 'in_progress' (done/failed/reclaim) // clears it back to NULL so only genuinely in-flight rows carry a lease. const dequeueStatement = db.prepare(` @@ -133,12 +177,16 @@ export function initPortfolioQueueStore(dbPath = resolvePortfolioQueueDbPath()) ) RETURNING * `); - const markDoneStatement = db.prepare( - "UPDATE miner_portfolio_queue SET status = 'done', leased_at = NULL WHERE repo_full_name = ? AND identifier = ? AND status <> 'done'", - ); + // RETURNING (rather than a separate post-UPDATE SELECT) makes the "nothing to mark done" case observable + // directly from one atomic statement. + const markDoneStatement = db.prepare(` + UPDATE miner_portfolio_queue SET status = 'done', leased_at = NULL + WHERE api_base_url = ? AND repo_full_name = ? AND identifier = ? AND status <> 'done' + RETURNING * + `); const markFailedStatement = db.prepare(` UPDATE miner_portfolio_queue SET status = 'queued', leased_at = NULL - WHERE repo_full_name = ? AND identifier = ? AND status = 'in_progress' + WHERE api_base_url = ? AND repo_full_name = ? AND identifier = ? AND status = 'in_progress' RETURNING * `); const listAllStatement = db.prepare(`SELECT * FROM miner_portfolio_queue ${ORDER}`); @@ -153,7 +201,7 @@ export function initPortfolioQueueStore(dbPath = resolvePortfolioQueueDbPath()) ); const reclaimStatement = db.prepare(` UPDATE miner_portfolio_queue SET status = 'queued', leased_at = NULL - WHERE repo_full_name = ? AND identifier = ? AND status = 'in_progress' + WHERE api_base_url = ? AND repo_full_name = ? AND identifier = ? AND status = 'in_progress' RETURNING * `); // Requeue only ever targets a COMPLETED ('done') row — an in-flight item is released via reclaimStatement, and @@ -161,24 +209,25 @@ export function initPortfolioQueueStore(dbPath = resolvePortfolioQueueDbPath()) // row keeps its rowid/enqueued_at, so it re-enters the queue at its original FIFO position, not the back. const requeueStatement = db.prepare(` UPDATE miner_portfolio_queue SET status = 'queued', leased_at = NULL - WHERE repo_full_name = ? AND identifier = ? AND status = 'done' + WHERE api_base_url = ? AND repo_full_name = ? AND identifier = ? AND status = 'done' RETURNING * `); const claimTargetStatement = db.prepare(` UPDATE miner_portfolio_queue SET status = 'in_progress', leased_at = ? - WHERE repo_full_name = ? AND identifier = ? AND status = 'queued' + WHERE api_base_url = ? AND repo_full_name = ? AND identifier = ? AND status = 'queued' RETURNING * `); return { dbPath: resolvedPath, enqueue(item) { + const apiBaseUrl = normalizeApiBaseUrl(item?.apiBaseUrl); const repoFullName = normalizeRepoFullName(item?.repoFullName); const identifier = normalizeIdentifier(item?.identifier); const priority = normalizePriority(item?.priority); const enqueuedAt = new Date().toISOString(); - enqueueStatement.run(repoFullName, identifier, priority, enqueuedAt); - return rowToEntry(getStatement.get(repoFullName, identifier)); + enqueueStatement.run(apiBaseUrl, repoFullName, identifier, priority, enqueuedAt); + return rowToEntry(getStatement.get(apiBaseUrl, repoFullName, identifier)); }, dequeueNext() { const row = dequeueStatement.get(new Date().toISOString()); @@ -190,8 +239,9 @@ export function initPortfolioQueueStore(dbPath = resolvePortfolioQueueDbPath()) }, /** Reclaim a single stuck in-flight item back to 'queued' (clearing its lease), returning it — or null if it is * no longer 'in_progress' (already finished/reclaimed by another sweep). The sweep target of #4827. */ - reclaimStuckItem(repoFullName, identifier) { + reclaimStuckItem(repoFullName, identifier, apiBaseUrl) { const row = reclaimStatement.get( + normalizeApiBaseUrl(apiBaseUrl), normalizeRepoFullName(repoFullName), normalizeIdentifier(identifier), ); @@ -201,8 +251,9 @@ export function initPortfolioQueueStore(dbPath = resolvePortfolioQueueDbPath()) * (rowid/enqueued_at unchanged). Returns the entry, or null when there is no 'done' item to requeue — i.e. * it is already 'queued', is currently 'in_progress' (release it via {@link reclaimStuckItem} instead), or * does not exist. The manual counterpart to {@link reclaimStuckItem} for the queue CLI's escape hatch (#4828). */ - requeueItem(repoFullName, identifier) { + requeueItem(repoFullName, identifier, apiBaseUrl) { const row = requeueStatement.get( + normalizeApiBaseUrl(apiBaseUrl), normalizeRepoFullName(repoFullName), normalizeIdentifier(identifier), ); @@ -214,19 +265,21 @@ export function initPortfolioQueueStore(dbPath = resolvePortfolioQueueDbPath()) : listRepoStatement.all(normalizeRepoFullName(repoFullName)); return rows.map(rowToEntry); }, - markDone(repoFullName, identifier) { - const normalizedRepo = normalizeRepoFullName(repoFullName); - const normalizedIdentifier = normalizeIdentifier(identifier); - const result = markDoneStatement.run(normalizedRepo, normalizedIdentifier); - if (result.changes === 0) return null; - const row = getStatement.get(normalizedRepo, normalizedIdentifier); + markDone(repoFullName, identifier, apiBaseUrl) { + const row = markDoneStatement.get( + normalizeApiBaseUrl(apiBaseUrl), + normalizeRepoFullName(repoFullName), + normalizeIdentifier(identifier), + ); return row ? rowToEntry(row) : null; }, /** Release an in-flight item back to `queued` when a run halts (#2347). */ - markFailed(repoFullName, identifier) { - const normalizedRepo = normalizeRepoFullName(repoFullName); - const normalizedIdentifier = normalizeIdentifier(identifier); - const row = markFailedStatement.get(normalizedRepo, normalizedIdentifier); + markFailed(repoFullName, identifier, apiBaseUrl) { + const row = markFailedStatement.get( + normalizeApiBaseUrl(apiBaseUrl), + normalizeRepoFullName(repoFullName), + normalizeIdentifier(identifier), + ); return row ? rowToEntry(row) : null; }, /** @@ -243,9 +296,10 @@ export function initPortfolioQueueStore(dbPath = resolvePortfolioQueueDbPath()) const leasedAt = new Date().toISOString(); const claimed = []; for (const target of targets) { + const apiBaseUrl = normalizeApiBaseUrl(target?.apiBaseUrl); const repoFullName = normalizeRepoFullName(target?.repoFullName); const identifier = normalizeIdentifier(target?.identifier); - const row = claimTargetStatement.get(leasedAt, repoFullName, identifier); + const row = claimTargetStatement.get(leasedAt, apiBaseUrl, repoFullName, identifier); if (row) claimed.push(rowToEntry(row)); } db.exec("COMMIT"); @@ -278,12 +332,12 @@ export function listQueue(repoFullName) { return getDefaultPortfolioQueueStore().listQueue(repoFullName); } -export function markDone(repoFullName, identifier) { - return getDefaultPortfolioQueueStore().markDone(repoFullName, identifier); +export function markDone(repoFullName, identifier, apiBaseUrl) { + return getDefaultPortfolioQueueStore().markDone(repoFullName, identifier, apiBaseUrl); } -export function markFailed(repoFullName, identifier) { - return getDefaultPortfolioQueueStore().markFailed(repoFullName, identifier); +export function markFailed(repoFullName, identifier, apiBaseUrl) { + return getDefaultPortfolioQueueStore().markFailed(repoFullName, identifier, apiBaseUrl); } export function closeDefaultPortfolioQueueStore() { diff --git a/test/unit/miner-discover-cli.test.ts b/test/unit/miner-discover-cli.test.ts index 3b24c23d66..e947167022 100644 --- a/test/unit/miner-discover-cli.test.ts +++ b/test/unit/miner-discover-cli.test.ts @@ -615,6 +615,11 @@ describe("runDiscover (#4247)", () => { "tenant-secret", expect.objectContaining({ apiBaseUrl: "https://ghe.example.com/api/v3" }), ); + // REGRESSION (#5563): the enqueued portfolio-queue row itself carries the resolved forge host, not just + // the fan-out call — otherwise a same-named repo on github.com would collide with this GHE tenant's row. + expect(portfolioQueue.listQueue()).toEqual([ + expect.objectContaining({ apiBaseUrl: "https://ghe.example.com/api/v3" }), + ]); } finally { if (previous === undefined) delete process.env.FORGE_PAT; else process.env.FORGE_PAT = previous; diff --git a/test/unit/miner-loop-cli.test.ts b/test/unit/miner-loop-cli.test.ts index ae2c9e4b47..77a4b8b961 100644 --- a/test/unit/miner-loop-cli.test.ts +++ b/test/unit/miner-loop-cli.test.ts @@ -452,6 +452,39 @@ describe("runLoop (#5135)", () => { }); }); + it("REGRESSION: an item whose identifier isn't 'issue:N' is marked done (skipped) rather than crashing the loop, scoped by its own apiBaseUrl (#5563)", async () => { + const { eventLedger, governorLedger, portfolioQueue, runState, governorState, paths } = tempStores(); + const log = vi.spyOn(console, "log").mockImplementation(() => undefined); + // Never produced by enqueueRankedDiscovery in practice, but the loop must fail soft rather than crash if + // some other writer ever enqueues a malformed identifier -- and it must echo the row's OWN apiBaseUrl back + // to markDone, not the github.com default, so a non-default-host row isn't silently left untouched. + portfolioQueue.enqueue({ repoFullName: "acme/widgets", identifier: "not-an-issue", apiBaseUrl: "https://ghe.example.com/api/v3" }); + const runDiscoverSpy = vi.fn(async () => 0); + const runAttemptSpy = vi.fn(); + + const exitCode = await runLoop(["acme/widgets", "--miner-login", "alice", "--max-cycles", "1", "--json"], { + env: { GITHUB_TOKEN: "ghp_loop_test" }, + openGovernorState: () => governorState, + initEventLedger: () => eventLedger, + initGovernorLedger: () => governorLedger, + initPortfolioQueue: () => portfolioQueue, + initRunStateStore: () => runState, + runDiscover: runDiscoverSpy, + runAttempt: runAttemptSpy, + ...readyLoopOptions(), + }); + + expect(exitCode).toBe(0); + expect(runAttemptSpy).not.toHaveBeenCalled(); + const printed = JSON.parse(String(log.mock.calls[0]?.[0])); + expect(printed.cycles[0]).toMatchObject({ outcome: "skipped_malformed_identifier", identifier: "not-an-issue" }); + + const after = reopenAfterRun(paths); + expect(after.portfolioQueue.listQueue()).toEqual([ + expect.objectContaining({ apiBaseUrl: "https://ghe.example.com/api/v3", identifier: "not-an-issue", status: "done" }), + ]); + }); + it("REGRESSION (#5394): polls CI status before PR disposition, on the same PR", async () => { const { eventLedger, governorLedger, portfolioQueue, runState, governorState } = tempStores(); vi.spyOn(console, "log").mockImplementation(() => undefined); diff --git a/test/unit/miner-migrate-cli.test.ts b/test/unit/miner-migrate-cli.test.ts index e252ec71a1..b7995dee8c 100644 --- a/test/unit/miner-migrate-cli.test.ts +++ b/test/unit/miner-migrate-cli.test.ts @@ -85,14 +85,15 @@ describe("gittensory-miner migrate (#4871)", () => { const results = runMigrateChecks(env); const portfolioQueue = results.find((result) => result.name === "portfolio-queue"); - expect(portfolioQueue).toMatchObject({ ok: true, status: "migrated", versionBefore: 1, versionAfter: 2 }); + // Runs BOTH post-baseline migrations in sequence: v1->v2 adds leased_at, v2->v3 adds api_base_url (#5563). + expect(portfolioQueue).toMatchObject({ ok: true, status: "migrated", versionBefore: 1, versionAfter: 3 }); const verifyDb = new DatabaseSync(dbPath, { readOnly: true }); try { - expect( - verifyDb.prepare("PRAGMA table_info(miner_portfolio_queue)").all().some((column) => column.name === "leased_at"), - ).toBe(true); - expect(verifyDb.prepare("PRAGMA user_version").get()?.user_version).toBe(2); + const columns = verifyDb.prepare("PRAGMA table_info(miner_portfolio_queue)").all().map((column) => column.name); + expect(columns).toContain("leased_at"); + expect(columns).toContain("api_base_url"); + expect(verifyDb.prepare("PRAGMA user_version").get()?.user_version).toBe(3); } finally { verifyDb.close(); } diff --git a/test/unit/miner-portfolio-discovery.test.ts b/test/unit/miner-portfolio-discovery.test.ts index 0bb2e7c2cc..96206b08c5 100644 --- a/test/unit/miner-portfolio-discovery.test.ts +++ b/test/unit/miner-portfolio-discovery.test.ts @@ -169,6 +169,19 @@ describe("gittensory-miner portfolio discovery (#2292)", () => { ]); }); + it("threads options.apiBaseUrl through to every enqueued row, so a non-default host doesn't collide with github.com (#5563)", () => { + const queueStore = tempQueueStore(); + queueStore.enqueue({ repoFullName: "acme/widgets", identifier: "issue:7", apiBaseUrl: "https://api.github.com" }); + enqueueRankedDiscovery([rankedIssue({ issueNumber: 7, rankScore: 10 })], { + queueStore, + apiBaseUrl: "https://ghe.example.com/api/v3", + }); + expect(queueStore.listQueue("acme/widgets")).toHaveLength(2); + expect( + queueStore.listQueue("acme/widgets").find((entry) => entry.apiBaseUrl === "https://ghe.example.com/api/v3"), + ).toMatchObject({ identifier: "issue:7", status: "queued" }); + }); + it("rejects invalid rankedIssues, queue store, event ledger, or minRankScore", () => { const queueStore = tempQueueStore(); expect(() => enqueueRankedDiscovery(null as never, { queueStore })).toThrow("invalid_ranked_issues"); diff --git a/test/unit/miner-portfolio-queue-cli.test.ts b/test/unit/miner-portfolio-queue-cli.test.ts index bc1ab2a906..b1b192bc5f 100644 --- a/test/unit/miner-portfolio-queue-cli.test.ts +++ b/test/unit/miner-portfolio-queue-cli.test.ts @@ -62,6 +62,7 @@ describe("gittensory-miner portfolio queue CLI (#2292)", () => { it("renderQueueTable formats numeric priority and empty output", () => { const entries: QueueEntry[] = [ { + apiBaseUrl: "https://api.github.com", repoFullName: "acme/widgets", identifier: "issue:7", status: "queued", @@ -239,6 +240,33 @@ describe("gittensory-miner portfolio queue CLI (#2292)", () => { expect(parseQueueRequeueArgs([])).toEqual({ error: expect.stringContaining("queue requeue") }); }); + it("parseQueueDoneArgs, parseQueueReleaseArgs, and parseQueueRequeueArgs accept --api-base-url (#5563)", () => { + expect(parseQueueDoneArgs(["acme/widgets", "issue:1", "--api-base-url", "https://ghe.example.com/api/v3"])).toEqual({ + repoFullName: "acme/widgets", + identifier: "issue:1", + dryRun: false, + json: false, + apiBaseUrl: "https://ghe.example.com/api/v3", + }); + expect(parseQueueDoneArgs(["acme/widgets", "issue:1", "--api-base-url"])).toEqual({ + error: expect.stringContaining("queue done"), + }); + expect(parseQueueReleaseArgs(["acme/widgets", "issue:1", "--api-base-url", "https://ghe.example.com/api/v3"])).toEqual({ + repoFullName: "acme/widgets", + identifier: "issue:1", + dryRun: false, + json: false, + apiBaseUrl: "https://ghe.example.com/api/v3", + }); + expect(parseQueueRequeueArgs(["acme/widgets", "issue:1", "--api-base-url", "https://ghe.example.com/api/v3"])).toEqual({ + repoFullName: "acme/widgets", + identifier: "issue:1", + dryRun: false, + json: false, + apiBaseUrl: "https://ghe.example.com/api/v3", + }); + }); + it("#4847: --dry-run reports what release/requeue would do and returns 0 without opening the portfolio queue", () => { const log = vi.spyOn(console, "log").mockImplementation(() => undefined); const initPortfolioQueueSpy = vi.fn(); @@ -297,6 +325,44 @@ describe("gittensory-miner portfolio queue CLI (#2292)", () => { expect(portfolioQueue.listQueue("acme/widgets")[0]?.status).toBe("queued"); }); + it("runQueueDone, runQueueRelease, and runQueueRequeue thread --api-base-url through, so two hosts don't collide (#5563)", () => { + const portfolioQueue = tempQueueStore(); + const options = { initPortfolioQueue: () => portfolioQueue }; + const log = vi.spyOn(console, "log").mockImplementation(() => undefined); + + portfolioQueue.enqueue({ repoFullName: "acme/widgets", identifier: "issue:1", apiBaseUrl: "https://api.github.com" }); + portfolioQueue.enqueue({ repoFullName: "acme/widgets", identifier: "issue:1", apiBaseUrl: "https://ghe.example.com/api/v3" }); + + // Marking the GHE host's row done must not touch the github.com row. + expect( + runQueueDone(["acme/widgets", "issue:1", "--api-base-url", "https://ghe.example.com/api/v3", "--json"], options), + ).toBe(0); + expect(JSON.parse(String(log.mock.calls[0]?.[0]))).toEqual({ + entry: expect.objectContaining({ apiBaseUrl: "https://ghe.example.com/api/v3", status: "done" }), + }); + const stillQueued = portfolioQueue.listQueue("acme/widgets").find((entry) => entry.status === "queued"); + expect(stillQueued?.apiBaseUrl).toBe("https://api.github.com"); + + // release: claim the github.com row, then release only it via --api-base-url. + portfolioQueue.dequeueNext(); + log.mockClear(); + expect( + runQueueRelease(["acme/widgets", "issue:1", "--api-base-url", "https://api.github.com", "--json"], options), + ).toBe(0); + expect(JSON.parse(String(log.mock.calls[0]?.[0]))).toEqual({ + entry: expect.objectContaining({ apiBaseUrl: "https://api.github.com", status: "queued" }), + }); + + // requeue: the GHE row is 'done' from above -- requeue only it via --api-base-url. + log.mockClear(); + expect( + runQueueRequeue(["acme/widgets", "issue:1", "--api-base-url", "https://ghe.example.com/api/v3", "--json"], options), + ).toBe(0); + expect(JSON.parse(String(log.mock.calls[0]?.[0]))).toEqual({ + entry: expect.objectContaining({ apiBaseUrl: "https://ghe.example.com/api/v3", status: "queued" }), + }); + }); + it("release emits the full entry as JSON under --json", () => { const portfolioQueue = tempQueueStore(); portfolioQueue.enqueue({ repoFullName: "acme/widgets", identifier: "issue:7b", priority: 2 }); diff --git a/test/unit/miner-portfolio-queue-crash-recovery.test.ts b/test/unit/miner-portfolio-queue-crash-recovery.test.ts index cbabebe564..68978bf660 100644 --- a/test/unit/miner-portfolio-queue-crash-recovery.test.ts +++ b/test/unit/miner-portfolio-queue-crash-recovery.test.ts @@ -89,7 +89,13 @@ describe("portfolio-queue crash recovery (#4868)", () => { // The crash alone does not un-stick the row -- it is still genuinely 'in_progress' on disk. expect(bootstrap.listInProgress()).toEqual([ - { repoFullName: "acme/widgets", identifier: "pr:1", status: "in_progress", leasedAt: claimed.leasedAt }, + { + apiBaseUrl: "https://api.github.com", + repoFullName: "acme/widgets", + identifier: "pr:1", + status: "in_progress", + leasedAt: claimed.leasedAt, + }, ]); // Sweeping before the lease bound elapses must NOT reclaim it (still within the grace window). diff --git a/test/unit/miner-portfolio-queue-expiry.test.ts b/test/unit/miner-portfolio-queue-expiry.test.ts index 3172d79ed9..e89537a45c 100644 --- a/test/unit/miner-portfolio-queue-expiry.test.ts +++ b/test/unit/miner-portfolio-queue-expiry.test.ts @@ -29,6 +29,7 @@ afterEach(() => { }); const leaseItem = (overrides: Partial = {}): QueueLeaseEntry => ({ + apiBaseUrl: "https://api.github.com", repoFullName: "o/a", identifier: "x", status: "in_progress", @@ -47,7 +48,13 @@ describe("portfolio-queue lease bookkeeping (#4827)", () => { const claimed = store.dequeueNext(); expect(claimed).toMatchObject({ identifier: "x", status: "in_progress" }); expect(store.listInProgress()).toEqual([ - { repoFullName: "o/a", identifier: "x", status: "in_progress", leasedAt: "2026-07-12T10:00:00.000Z" }, + { + apiBaseUrl: "https://api.github.com", + repoFullName: "o/a", + identifier: "x", + status: "in_progress", + leasedAt: "2026-07-12T10:00:00.000Z", + }, ]); }); @@ -144,6 +151,28 @@ describe("sweepStuckItems (#4827)", () => { expect(store.listQueue("o/a").find((e) => e.identifier === "old")?.status).toBe("queued"); }); + it("REGRESSION: echoes each item's own apiBaseUrl to reclaimStuckItem, so it can't reclaim the wrong host's row (#5563)", () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-06-01T00:00:00.000Z")); + const store = tempStore(); + const maxLeaseMs = 30 * 60 * 1000; + + // Two forge hosts each hold an in-flight lease on the SAME owner/repo+identifier -- only possible post-#5563's + // scoped uniqueness. + store.enqueue({ repoFullName: "acme/widgets", identifier: "issue:1", apiBaseUrl: "https://ghe.example.com/api/v3" }); + store.dequeueNext(); + vi.setSystemTime(new Date("2026-06-01T00:29:00.000Z")); // still within the lease bound + store.enqueue({ repoFullName: "acme/widgets", identifier: "issue:1", apiBaseUrl: "https://api.github.com" }); + store.dequeueNext(); + + const nowMs = Date.parse("2026-06-01T00:31:00.000Z"); // GHE lease (31min old) exceeds the bound; github.com (2min) doesn't + const reclaimed = sweepStuckItems(store, nowMs, maxLeaseMs); + expect(reclaimed).toEqual([expect.objectContaining({ apiBaseUrl: "https://ghe.example.com/api/v3", status: "queued" })]); + + const stillInProgress = store.listInProgress(); + expect(stillInProgress).toEqual([expect.objectContaining({ apiBaseUrl: "https://api.github.com" })]); + }); + it("defaults the bound to DEFAULT_MAX_LEASE_MS and reclaims nothing when all leases are fresh", () => { vi.useFakeTimers(); vi.setSystemTime(new Date("2026-07-12T10:00:00.000Z")); diff --git a/test/unit/miner-portfolio-queue-manager.test.ts b/test/unit/miner-portfolio-queue-manager.test.ts index 4e1b6471ce..cf4f8c50a6 100644 --- a/test/unit/miner-portfolio-queue-manager.test.ts +++ b/test/unit/miner-portfolio-queue-manager.test.ts @@ -33,12 +33,13 @@ describe("normalizePortfolioCaps() (#4285)", () => { describe("entriesToPortfolioQueue() / selectEligibleBatch() (#4285)", () => { it("mirrors the engine diversification scenario through persisted row shapes", () => { + const apiBaseUrl = "https://api.github.com"; const entries: QueueEntry[] = [ - { repoFullName: "acme/alpha", identifier: "a-running", priority: 0, status: "in_progress", enqueuedAt: "t1" }, - { repoFullName: "acme/alpha", identifier: "a-queued-1", priority: 0, status: "queued", enqueuedAt: "t2" }, - { repoFullName: "acme/alpha", identifier: "a-queued-2", priority: 0, status: "queued", enqueuedAt: "t3" }, - { repoFullName: "acme/beta", identifier: "b-queued-1", priority: 0, status: "queued", enqueuedAt: "t4" }, - { repoFullName: "acme/gamma", identifier: "c-queued-1", priority: 0, status: "queued", enqueuedAt: "t5" }, + { apiBaseUrl, repoFullName: "acme/alpha", identifier: "a-running", priority: 0, status: "in_progress", enqueuedAt: "t1" }, + { apiBaseUrl, repoFullName: "acme/alpha", identifier: "a-queued-1", priority: 0, status: "queued", enqueuedAt: "t2" }, + { apiBaseUrl, repoFullName: "acme/alpha", identifier: "a-queued-2", priority: 0, status: "queued", enqueuedAt: "t3" }, + { apiBaseUrl, repoFullName: "acme/beta", identifier: "b-queued-1", priority: 0, status: "queued", enqueuedAt: "t4" }, + { apiBaseUrl, repoFullName: "acme/gamma", identifier: "c-queued-1", priority: 0, status: "queued", enqueuedAt: "t5" }, ]; expect( @@ -57,7 +58,7 @@ describe("entriesToPortfolioQueue() / selectEligibleBatch() (#4285)", () => { it("returns nothing when either cap is zero", () => { const entries: QueueEntry[] = [ - { repoFullName: "acme/alpha", identifier: "x", priority: 0, status: "queued", enqueuedAt: "t1" }, + { apiBaseUrl: "https://api.github.com", repoFullName: "acme/alpha", identifier: "x", priority: 0, status: "queued", enqueuedAt: "t1" }, ]; expect(selectEligibleBatch(entries, { globalWipCap: 0, perRepoWipCap: 1 })).toEqual([]); expect(selectEligibleBatch(entries, { globalWipCap: 1, perRepoWipCap: 0 })).toEqual([]); @@ -70,6 +71,15 @@ describe("initPortfolioQueueManager().claimNextBatch() (#4285)", () => { expect(manager.claimNextBatch()).toEqual([]); }); + it("markDone and markFailed pass repoFullName/identifier/apiBaseUrl straight through to the store (#5563)", () => { + const manager = memoryManager({ globalWipCap: 2, perRepoWipCap: 2 }); + manager.enqueue({ repoFullName: "acme/alpha", identifier: "x", apiBaseUrl: "https://ghe.example.com/api/v3" }); + manager.store.dequeueNext(); + expect(manager.markFailed("acme/alpha", "x", "https://ghe.example.com/api/v3")?.status).toBe("queued"); + manager.store.dequeueNext(); + expect(manager.markDone("acme/alpha", "x", "https://ghe.example.com/api/v3")?.status).toBe("done"); + }); + it("respects a saturated per-repo cap", () => { const manager = memoryManager({ globalWipCap: 4, perRepoWipCap: 1 }); manager.enqueue({ repoFullName: "acme/alpha", identifier: "running", priority: 1 }); diff --git a/test/unit/miner-portfolio-queue.test.ts b/test/unit/miner-portfolio-queue.test.ts index e4545e426d..9f00f8f5ff 100644 --- a/test/unit/miner-portfolio-queue.test.ts +++ b/test/unit/miner-portfolio-queue.test.ts @@ -1,6 +1,7 @@ import { existsSync, mkdtempSync, rmSync, statSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; +import { DatabaseSync } from "node:sqlite"; import { afterEach, describe, expect, it, vi } from "vitest"; import { QUEUE_STATUSES, @@ -8,6 +9,7 @@ import { dequeueNext, enqueue, initPortfolioQueueStore, + markDone, markFailed, resolvePortfolioQueueDbPath, } from "../../packages/gittensory-miner/lib/portfolio-queue.js"; @@ -199,4 +201,124 @@ describe("gittensory-miner portfolio/queue store (#2292)", () => { expect(() => store.markFailed("no-slash", "1")).toThrow("invalid_repo_full_name"); expect(() => store.markFailed("o/a", " ")).toThrow("invalid_identifier"); }); + + it("module-level markDone delegates to the default portfolio-queue store", () => { + const root = mkdtempSync(join(tmpdir(), "gittensory-miner-portfolio-default-")); + roots.push(root); + vi.stubEnv("GITTENSORY_MINER_PORTFOLIO_QUEUE_DB", join(root, "portfolio-queue.sqlite3")); + enqueue({ repoFullName: "o/a", identifier: "work", priority: 1 }); + expect(markDone("o/a", "work")?.status).toBe("done"); + expect(markDone("o/a", "work")).toBeNull(); + }); + + describe("forge-scoping (#5563)", () => { + it("defaults apiBaseUrl to the github.com default when omitted", () => { + const entry = tempStore().enqueue({ repoFullName: "o/a", identifier: "x" }); + expect(entry.apiBaseUrl).toBe("https://api.github.com"); + }); + + it("two forge hosts can each hold an item with the same owner/repo+identifier without colliding", () => { + const store = tempStore(); + const gh = store.enqueue({ repoFullName: "acme/widgets", identifier: "issue:1", apiBaseUrl: "https://api.github.com" }); + const ghe = store.enqueue({ repoFullName: "acme/widgets", identifier: "issue:1", apiBaseUrl: "https://ghe.example.com/api/v3" }); + expect(gh.apiBaseUrl).not.toBe(ghe.apiBaseUrl); + expect(store.listQueue("acme/widgets")).toHaveLength(2); + + expect(store.markDone("acme/widgets", "issue:1", "https://api.github.com")?.apiBaseUrl).toBe("https://api.github.com"); + const stillQueued = store.listQueue("acme/widgets").find((entry) => entry.status === "queued"); + expect(stillQueued?.apiBaseUrl).toBe("https://ghe.example.com/api/v3"); + }); + + it("markFailed, reclaimStuckItem, and requeueItem are all scoped by apiBaseUrl too", () => { + const store = tempStore(); + store.enqueue({ repoFullName: "acme/widgets", identifier: "issue:1", apiBaseUrl: "https://api.github.com" }); + store.enqueue({ repoFullName: "acme/widgets", identifier: "issue:1", apiBaseUrl: "https://ghe.example.com/api/v3" }); + + // dequeueNext is global (no host filter): claims the github.com row first (enqueued first), then — since + // that row is no longer 'queued' — the GHE row on the next call. + const claimedGh = store.dequeueNext(); + expect(claimedGh?.apiBaseUrl).toBe("https://api.github.com"); + const claimedGhe = store.dequeueNext(); + expect(claimedGhe?.apiBaseUrl).toBe("https://ghe.example.com/api/v3"); + + // Releasing the github.com host's in-flight row must not touch the still-in-flight GHE row. + expect(store.markFailed("acme/widgets", "issue:1", "https://api.github.com")?.status).toBe("queued"); + const stillInProgress = store.listInProgress(); + expect(stillInProgress).toEqual([expect.objectContaining({ apiBaseUrl: "https://ghe.example.com/api/v3" })]); + + const released = store.reclaimStuckItem("acme/widgets", "issue:1", "https://ghe.example.com/api/v3"); + expect(released?.status).toBe("queued"); + expect(store.listInProgress()).toEqual([]); + + store.markDone("acme/widgets", "issue:1", "https://api.github.com"); + const requeued = store.requeueItem("acme/widgets", "issue:1", "https://api.github.com"); + expect(requeued?.status).toBe("queued"); + expect(requeued?.apiBaseUrl).toBe("https://api.github.com"); + // The GHE row (still 'queued' from its own reclaim above) is untouched by requeueItem's github.com scope. + expect(store.listQueue("acme/widgets")).toHaveLength(2); + }); + + it("batchClaim threads target.apiBaseUrl through claimTargetStatement, scoped per host", () => { + const store = tempStore(); + store.enqueue({ repoFullName: "acme/widgets", identifier: "issue:1", apiBaseUrl: "https://api.github.com" }); + store.enqueue({ repoFullName: "acme/widgets", identifier: "issue:1", apiBaseUrl: "https://ghe.example.com/api/v3" }); + const claimed = store.batchClaim((entries) => + entries.map((entry) => ({ repoFullName: entry.repoFullName, identifier: entry.identifier, apiBaseUrl: entry.apiBaseUrl })), + ); + expect(claimed.map((entry) => entry.apiBaseUrl).sort()).toEqual([ + "https://api.github.com", + "https://ghe.example.com/api/v3", + ]); + }); + + it("rejects a non-string or blank apiBaseUrl", () => { + const store = tempStore(); + expect(() => store.enqueue({ repoFullName: "o/a", identifier: "1", apiBaseUrl: " " })).toThrow( + "invalid_api_base_url", + ); + expect(() => store.enqueue({ repoFullName: "o/a", identifier: "1", apiBaseUrl: 42 as never })).toThrow( + "invalid_api_base_url", + ); + }); + + it("migrates an existing pre-#5563 file (already at the leased_at v2 shape), backfilling api_base_url and preserving every row", () => { + const root = mkdtempSync(join(tmpdir(), "gittensory-miner-portfolio-legacy-")); + roots.push(root); + const dbPath = join(root, "legacy.sqlite3"); + const legacy = new DatabaseSync(dbPath); + legacy.exec(` + CREATE TABLE miner_portfolio_queue ( + repo_full_name TEXT NOT NULL, + identifier TEXT NOT NULL, + priority REAL NOT NULL DEFAULT 0, + status TEXT NOT NULL DEFAULT 'queued' CHECK (status IN ('queued', 'in_progress', 'done')), + enqueued_at TEXT NOT NULL, + leased_at TEXT, + PRIMARY KEY (repo_full_name, identifier) + ) + `); + legacy.exec("PRAGMA user_version = 2"); + legacy.exec( + "INSERT INTO miner_portfolio_queue (repo_full_name, identifier, priority, status, enqueued_at, leased_at) VALUES ('acme/widgets', 'issue:5', 3, 'queued', '2026-01-01T00:00:00.000Z', NULL)", + ); + legacy.close(); + + const store = initPortfolioQueueStore(dbPath); + stores.push(store); + expect(store.listQueue("acme/widgets")).toEqual([ + { + apiBaseUrl: "https://api.github.com", + repoFullName: "acme/widgets", + identifier: "issue:5", + priority: 3, + status: "queued", + enqueuedAt: "2026-01-01T00:00:00.000Z", + }, + ]); + // The old bare (repo_full_name, identifier) collision is gone: a second host can now enqueue the same pair. + const geEntry = store.enqueue({ repoFullName: "acme/widgets", identifier: "issue:5", apiBaseUrl: "https://ghe.example.com/api/v3" }); + expect(store.listQueue("acme/widgets")).toHaveLength(2); + expect(geEntry.apiBaseUrl).toBe("https://ghe.example.com/api/v3"); + }); + }); });