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
6 changes: 5 additions & 1 deletion packages/gittensory-miner/lib/claim-ledger.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,12 @@ function addApiBaseUrlScope(db) {
UNIQUE (api_base_url, repo_full_name, issue_number)
)
`);
// OR IGNORE: a row this store's own read path already treats as unusable garbage (an unrecognized `status`,
// e.g. from a hand-edited or otherwise corrupted file) would violate the CHECK constraint above and abort the
// whole migration. Skipping it here is consistent with that same fail-closed posture, rather than turning one
// bad row into a permanently unmigratable file.
db.prepare(
`INSERT INTO miner_claims_v2 (id, api_base_url, repo_full_name, issue_number, claimed_at, status, note)
`INSERT OR IGNORE INTO miner_claims_v2 (id, api_base_url, repo_full_name, issue_number, claimed_at, status, note)
SELECT id, ?, repo_full_name, issue_number, claimed_at, status, note FROM miner_claims`,
).run(DEFAULT_FORGE_CONFIG.apiBaseUrl);
db.exec("DROP TABLE miner_claims");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@ import type { PortfolioCaps } from "@jsonbored/gittensory-engine";
import type { EnqueueItem, PortfolioQueueStore, QueueEntry } from "./portfolio-queue.js";

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

export function queueItemId(repoFullName: string, identifier: string): string;
export function queueItemId(apiBaseUrl: string, repoFullName: string, identifier: string): string;

export function parseQueueItemId(id: string): PortfolioQueueClaimTarget;

Expand Down
40 changes: 26 additions & 14 deletions packages/gittensory-miner/lib/portfolio-queue-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,37 @@
// claiming respects global/per-repo WIP caps and cross-repo diversification instead of a naive priority-only
// single-row dequeue. Caps are plain constructor arguments — not wired to .gittensory-miner.yml here.
import { nextEligibleItems } from "@jsonbored/gittensory-engine";
import { DEFAULT_FORGE_CONFIG } from "./forge-config.js";
import { initPortfolioQueueStore } from "./portfolio-queue.js";
import { DEFAULT_MAX_LEASE_MS, sweepStuckItems } from "./portfolio-queue-expiry.js";

const ITEM_ID_SEPARATOR = "::";

/** Stable composite id for projecting SQLite rows into the engine's PortfolioQueueItem shape. */
export function queueItemId(repoFullName, identifier) {
return `${repoFullName}${ITEM_ID_SEPARATOR}${identifier}`;
/**
* Stable composite id for projecting SQLite rows into the engine's PortfolioQueueItem shape. Encodes apiBaseUrl
* too (#5563) — the engine's own selection logic has no forge dimension, but two hosts can now enqueue an item
* under the same repoFullName+identifier (post-#5563 scoping), and the id is the ONLY thing selectEligibleBatch's
* output threads back to batchClaim; without the host baked in here, a selected item's host would be lost and
* batchClaim would default to github.com, potentially claiming a DIFFERENT row than the one the engine selected.
*/
export function queueItemId(apiBaseUrl, repoFullName, identifier) {
return `${apiBaseUrl}${ITEM_ID_SEPARATOR}${repoFullName}${ITEM_ID_SEPARATOR}${identifier}`;
}

/** Reverse {@link queueItemId} after engine selection so claims can target SQLite primary keys. */
export function parseQueueItemId(id) {
if (typeof id !== "string") throw new Error("invalid_queue_item_id");
const separatorIndex = id.indexOf(ITEM_ID_SEPARATOR);
if (separatorIndex <= 0 || separatorIndex === id.length - ITEM_ID_SEPARATOR.length) {
const firstSeparatorIndex = id.indexOf(ITEM_ID_SEPARATOR);
if (firstSeparatorIndex <= 0) throw new Error("invalid_queue_item_id");
const rest = id.slice(firstSeparatorIndex + ITEM_ID_SEPARATOR.length);
const secondSeparatorIndex = rest.indexOf(ITEM_ID_SEPARATOR);
if (secondSeparatorIndex <= 0 || secondSeparatorIndex === rest.length - ITEM_ID_SEPARATOR.length) {
throw new Error("invalid_queue_item_id");
}
return {
repoFullName: id.slice(0, separatorIndex),
identifier: id.slice(separatorIndex + ITEM_ID_SEPARATOR.length),
apiBaseUrl: id.slice(0, firstSeparatorIndex),
repoFullName: rest.slice(0, secondSeparatorIndex),
identifier: rest.slice(secondSeparatorIndex + ITEM_ID_SEPARATOR.length),
};
}

Expand All @@ -42,13 +53,16 @@ export function entriesToPortfolioQueue(entries) {
const repoFullName = typeof entry.repoFullName === "string" ? entry.repoFullName.trim() : "";
const identifier = typeof entry.identifier === "string" ? entry.identifier.trim() : "";
if (!repoFullName || !identifier) continue;
// 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();
if (!bucketsByRepo.has(repoKey)) {
bucketsByRepo.set(repoKey, []);
bucketOrder.push(repoKey);
}
bucketsByRepo.get(repoKey).push({
id: queueItemId(repoFullName, identifier),
id: queueItemId(apiBaseUrl, repoFullName, identifier),
repoFullName,
state: entry.status === "in_progress" ? "in_progress" : "queued",
});
Expand Down Expand Up @@ -99,12 +113,10 @@ export function initPortfolioQueueManager(options = {}) {
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.
// The engine primitive itself (@jsonbored/gittensory-engine's nextEligibleItems) has no apiBaseUrl concept --
// it only ever sees the opaque `id` string. queueItemId/parseQueueItemId (#5563) smuggle the host through
// that id round-trip, so selectFn's output below correctly carries each selected item's OWN apiBaseUrl into
// batchClaim, instead of every claim defaulting to github.com regardless of which host's row was selected.
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.
Expand Down
6 changes: 5 additions & 1 deletion packages/gittensory-miner/lib/portfolio-queue.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,13 @@ export function initPortfolioQueueStore(dbPath = resolvePortfolioQueueDbPath())
`);
// 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.
// OR IGNORE: a row this store's own read path already treats as unusable garbage (an unrecognized
// `status`, e.g. from a hand-edited or otherwise corrupted file) would violate the CHECK constraint above
// and abort the whole migration. Skipping it here is consistent with that same fail-closed posture, rather
// than turning one bad row into a permanently unmigratable file.
migrationDb
.prepare(
`INSERT INTO miner_portfolio_queue_v3
`INSERT OR IGNORE 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`,
Expand Down
35 changes: 35 additions & 0 deletions test/unit/miner-claim-ledger.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,41 @@ describe("gittensory-miner claim ledger (#2314)", () => {
expect(ledger.listClaims({ repoFullName: "acme/widgets" })).toHaveLength(2);
expect(geClaim.apiBaseUrl).toBe("https://ghe.example.com/api/v3");
});

it("REGRESSION: a legacy row violating the rebuilt table's status CHECK constraint is dropped, not a migration-aborting crash", () => {
const root = tempRoot();
const dbPath = join(root, "legacy-corrupt.sqlite3");
const legacy = new DatabaseSync(dbPath);
// No CHECK on status here, simulating a hand-edited or otherwise corrupted legacy file -- the real
// baseline schema always enforces the CHECK, so this can only arise from external tampering.
legacy.exec(`
CREATE TABLE miner_claims (
id INTEGER PRIMARY KEY AUTOINCREMENT,
repo_full_name TEXT NOT NULL,
issue_number INTEGER NOT NULL,
claimed_at TEXT NOT NULL,
status TEXT NOT NULL DEFAULT 'active',
note TEXT,
UNIQUE (repo_full_name, issue_number)
)
`);
legacy.exec(
"INSERT INTO miner_claims (repo_full_name, issue_number, claimed_at, status, note) VALUES ('acme/corrupt', 1, '2026-01-01T00:00:00.000Z', 'bogus', NULL)",
);
legacy.exec(
"INSERT INTO miner_claims (repo_full_name, issue_number, claimed_at, status, note) VALUES ('acme/widgets', 5, '2026-01-01T00:00:00.000Z', 'active', 'ok')",
);
legacy.close();

let opened: ReturnType<typeof openClaimLedger> | undefined;
expect(() => {
opened = openClaimLedger(dbPath);
}).not.toThrow();
const ledger = opened!;
ledgers.push(ledger);
// The corrupt row was dropped, not migrated -- only the valid row survived the rebuild.
expect(ledger.listClaims().map((claim) => claim.repoFullName)).toEqual(["acme/widgets"]);
});
});

describe("purgeByRepo (#5564)", () => {
Expand Down
51 changes: 50 additions & 1 deletion test/unit/miner-portfolio-queue-manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,43 @@ describe("entriesToPortfolioQueue() / selectEligibleBatch() (#4285)", () => {
"acme/beta",
"acme/gamma",
]);
expect(parseQueueItemId(queueItemId("acme/beta", "b-queued-1"))).toEqual({
expect(parseQueueItemId(queueItemId("https://api.github.com", "acme/beta", "b-queued-1"))).toEqual({
apiBaseUrl: "https://api.github.com",
repoFullName: "acme/beta",
identifier: "b-queued-1",
});
});

it("queueItemId/parseQueueItemId round-trip a non-default apiBaseUrl (#5563)", () => {
const id = queueItemId("https://ghe.example.com/api/v3", "acme/widgets", "issue:7");
expect(parseQueueItemId(id)).toEqual({
apiBaseUrl: "https://ghe.example.com/api/v3",
repoFullName: "acme/widgets",
identifier: "issue:7",
});
});

it("parseQueueItemId rejects a malformed id", () => {
expect(() => parseQueueItemId(42 as never)).toThrow("invalid_queue_item_id");
expect(() => parseQueueItemId("no-separators-at-all")).toThrow("invalid_queue_item_id");
expect(() => parseQueueItemId("https://api.github.com::acme/widgets")).toThrow("invalid_queue_item_id");
expect(() => parseQueueItemId("::acme/widgets::issue:7")).toThrow("invalid_queue_item_id");
expect(() => parseQueueItemId("https://api.github.com::acme/widgets::")).toThrow("invalid_queue_item_id");
});

it("entriesToPortfolioQueue falls back to the github.com default when a row's apiBaseUrl is missing (#5563)", () => {
const entries = [
{ repoFullName: "acme/alpha", identifier: "x", priority: 0, status: "queued", enqueuedAt: "t1" },
] as QueueEntry[];
const id = entriesToPortfolioQueue(entries).buckets[0]?.items[0]?.id;
expect(id).toBeDefined();
expect(parseQueueItemId(id!)).toEqual({
apiBaseUrl: "https://api.github.com",
repoFullName: "acme/alpha",
identifier: "x",
});
});

it("returns nothing when either cap is zero", () => {
const entries: QueueEntry[] = [
{ apiBaseUrl: "https://api.github.com", repoFullName: "acme/alpha", identifier: "x", priority: 0, status: "queued", enqueuedAt: "t1" },
Expand Down Expand Up @@ -110,6 +141,24 @@ describe("initPortfolioQueueManager().claimNextBatch() (#4285)", () => {
expect(manager.listQueue().find((entry) => entry.identifier === "a-queued-2")?.status).toBe("queued");
});

it("REGRESSION: claimNextBatch claims the correct host's row when two hosts share a repoFullName+identifier (#5563)", () => {
const manager = memoryManager({ globalWipCap: 4, perRepoWipCap: 2 });
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" });

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",
]);
expect(claimed.every((entry) => entry.status === "in_progress")).toBe(true);
// Every row is genuinely claimed at the store level -- not one host's row claimed twice under two ids.
const rows = manager.listQueue("acme/widgets");
expect(rows).toHaveLength(2);
expect(rows.every((row) => row.status === "in_progress")).toBe(true);
});

it("does not claim rows another writer already took inside the same transaction window", () => {
const store = initPortfolioQueueStore(":memory:");
stores.push(store);
Expand Down
37 changes: 37 additions & 0 deletions test/unit/miner-portfolio-queue.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -320,5 +320,42 @@ describe("gittensory-miner portfolio/queue store (#2292)", () => {
expect(store.listQueue("acme/widgets")).toHaveLength(2);
expect(geEntry.apiBaseUrl).toBe("https://ghe.example.com/api/v3");
});

it("REGRESSION: a legacy row violating the rebuilt table's status CHECK constraint is dropped, not a migration-aborting crash", () => {
const root = mkdtempSync(join(tmpdir(), "gittensory-miner-portfolio-legacy-corrupt-"));
roots.push(root);
const dbPath = join(root, "legacy-corrupt.sqlite3");
const legacy = new DatabaseSync(dbPath);
// No CHECK on status here, simulating a hand-edited or otherwise corrupted legacy file -- the real
// baseline schema always enforces the CHECK, so this can only arise from external tampering.
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',
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/corrupt', 'issue:1', 1, 'bogus', '2026-01-01T00:00:00.000Z', NULL)",
);
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();

let opened: ReturnType<typeof initPortfolioQueueStore> | undefined;
expect(() => {
opened = initPortfolioQueueStore(dbPath);
}).not.toThrow();
const store = opened!;
stores.push(store);
// The corrupt row was dropped, not migrated -- only the valid row survived the rebuild.
expect(store.listQueue().map((entry) => entry.repoFullName)).toEqual(["acme/widgets"]);
});
});
});