diff --git a/packages/gittensory-miner/lib/governor-state.d.ts b/packages/gittensory-miner/lib/governor-state.d.ts index becabf54a0..d2efc4030e 100644 --- a/packages/gittensory-miner/lib/governor-state.d.ts +++ b/packages/gittensory-miner/lib/governor-state.d.ts @@ -29,8 +29,8 @@ export type GovernorState = { saveCapUsage(capUsage: GovernorCapUsage): void; loadPauseState(): GovernorPauseState; savePauseState(pauseState: GovernorPauseInput): GovernorPauseState; - loadReputationHistory(repoFullName: string): RepoOutcomeHistory; - saveReputationHistory(repoFullName: string, history: RepoOutcomeHistory): RepoOutcomeHistory; + loadReputationHistory(repoFullName: string, apiBaseUrl?: string): RepoOutcomeHistory; + saveReputationHistory(repoFullName: string, history: RepoOutcomeHistory, apiBaseUrl?: string): RepoOutcomeHistory; recordOwnSubmission(record: OwnSubmissionRecord): OwnSubmissionRecord; listRecentOwnSubmissions(filter?: ListRecentOwnSubmissionsFilter): OwnSubmissionRecord[]; close(): void; @@ -52,9 +52,9 @@ export function loadPauseState(): GovernorPauseState; export function savePauseState(pauseState: GovernorPauseInput): GovernorPauseState; -export function loadReputationHistory(repoFullName: string): RepoOutcomeHistory; +export function loadReputationHistory(repoFullName: string, apiBaseUrl?: string): RepoOutcomeHistory; -export function saveReputationHistory(repoFullName: string, history: RepoOutcomeHistory): RepoOutcomeHistory; +export function saveReputationHistory(repoFullName: string, history: RepoOutcomeHistory, apiBaseUrl?: string): RepoOutcomeHistory; export function recordOwnSubmission(record: OwnSubmissionRecord): OwnSubmissionRecord; diff --git a/packages/gittensory-miner/lib/governor-state.js b/packages/gittensory-miner/lib/governor-state.js index 5c3b0124c2..16cf46b26b 100644 --- a/packages/gittensory-miner/lib/governor-state.js +++ b/packages/gittensory-miner/lib/governor-state.js @@ -1,3 +1,4 @@ +import { DEFAULT_FORGE_CONFIG } from "./forge-config.js"; import { normalizeLocalStoreDbPath, openLocalStoreDb, resolveLocalStoreDbPath } from "./local-store.js"; // Governor cross-attempt state persistence (#5134, Wave 3.5). Every governor-*.js wrapper @@ -39,6 +40,14 @@ function normalizeRepoFullName(repoFullName) { return `${owner}/${repo}`; } +/** 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 parseJsonColumn(value, fallback) { if (typeof value !== "string") return fallback; try { @@ -72,6 +81,39 @@ function ensurePauseColumns(db) { } } +// Rebuild governor_reputation_history's bare `repo_full_name` PRIMARY KEY into a (api_base_url, repo_full_name) +// composite (#5563) -- two forge hosts serving a same-named owner/repo must not share one reputation row. +// 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. Guarded by a column-presence check (matching ensurePauseColumns' idempotence) so this only runs once +// per file, same technique as portfolio-queue.js's own post-creation migration. +function ensureReputationHistoryForgeScope(db) { + const hasApiBaseUrlColumn = db + .prepare("PRAGMA table_info(governor_reputation_history)") + .all() + .some((column) => column.name === "api_base_url"); + if (hasApiBaseUrlColumn) return; + db.exec(` + CREATE TABLE governor_reputation_history_v2 ( + api_base_url TEXT NOT NULL, + repo_full_name TEXT NOT NULL, + decided INTEGER NOT NULL, + unfavorable INTEGER NOT NULL, + updated_at TEXT NOT NULL, + PRIMARY KEY (api_base_url, repo_full_name) + ) + `); + // OR IGNORE: a source row that somehow violates the rebuilt table's NOT NULL columns (a hand-edited or + // otherwise corrupted file) is skipped rather than aborting the whole migration -- same fail-closed posture + // as run-state.js's own #5563 migration. + db.prepare( + `INSERT OR IGNORE INTO governor_reputation_history_v2 (api_base_url, repo_full_name, decided, unfavorable, updated_at) + SELECT ?, repo_full_name, decided, unfavorable, updated_at FROM governor_reputation_history`, + ).run(DEFAULT_FORGE_CONFIG.apiBaseUrl); + db.exec("DROP TABLE governor_reputation_history"); + db.exec("ALTER TABLE governor_reputation_history_v2 RENAME TO governor_reputation_history"); +} + /** Opens the local governor-state store, creating tables on first use. */ export function openGovernorState(dbPath = resolveGovernorStateDbPath()) { const resolvedPath = normalizeDbPath(dbPath); @@ -102,6 +144,7 @@ export function openGovernorState(dbPath = resolveGovernorStateDbPath()) { updated_at TEXT NOT NULL ) `); + ensureReputationHistoryForgeScope(db); db.exec(` CREATE TABLE IF NOT EXISTS governor_own_submissions ( id INTEGER PRIMARY KEY AUTOINCREMENT, @@ -128,11 +171,13 @@ export function openGovernorState(dbPath = resolveGovernorStateDbPath()) { paused_at = excluded.paused_at, updated_at = excluded.updated_at `); - const getReputationStatement = db.prepare("SELECT * FROM governor_reputation_history WHERE repo_full_name = ?"); + const getReputationStatement = db.prepare( + "SELECT * FROM governor_reputation_history WHERE api_base_url = ? AND repo_full_name = ?", + ); const upsertReputationStatement = db.prepare(` - INSERT INTO governor_reputation_history (repo_full_name, decided, unfavorable, updated_at) - VALUES (?, ?, ?, ?) - ON CONFLICT(repo_full_name) DO UPDATE SET + INSERT INTO governor_reputation_history (api_base_url, repo_full_name, decided, unfavorable, updated_at) + VALUES (?, ?, ?, ?, ?) + ON CONFLICT(api_base_url, repo_full_name) DO UPDATE SET decided = excluded.decided, unfavorable = excluded.unfavorable, updated_at = excluded.updated_at @@ -225,17 +270,19 @@ export function openGovernorState(dbPath = resolveGovernorStateDbPath()) { ); return { paused, reason, pausedAt }; }, - loadReputationHistory(repoFullName) { - const normalized = normalizeRepoFullName(repoFullName); - const row = getReputationStatement.get(normalized); + loadReputationHistory(repoFullName, apiBaseUrl) { + const normalizedForge = normalizeApiBaseUrl(apiBaseUrl); + const normalizedRepo = normalizeRepoFullName(repoFullName); + const row = getReputationStatement.get(normalizedForge, normalizedRepo); if (!row) return { ...DEFAULT_REPUTATION_HISTORY }; return { decided: row.decided, unfavorable: row.unfavorable }; }, - saveReputationHistory(repoFullName, history) { - const normalized = normalizeRepoFullName(repoFullName); + saveReputationHistory(repoFullName, history, apiBaseUrl) { + const normalizedForge = normalizeApiBaseUrl(apiBaseUrl); + const normalizedRepo = normalizeRepoFullName(repoFullName); const decided = Number.isInteger(history?.decided) ? history.decided : 0; const unfavorable = Number.isInteger(history?.unfavorable) ? history.unfavorable : 0; - upsertReputationStatement.run(normalized, decided, unfavorable, new Date().toISOString()); + upsertReputationStatement.run(normalizedForge, normalizedRepo, decided, unfavorable, new Date().toISOString()); return { decided, unfavorable }; }, recordOwnSubmission(record) { @@ -293,12 +340,12 @@ export function savePauseState(pauseState) { return getDefaultGovernorState().savePauseState(pauseState); } -export function loadReputationHistory(repoFullName) { - return getDefaultGovernorState().loadReputationHistory(repoFullName); +export function loadReputationHistory(repoFullName, apiBaseUrl) { + return getDefaultGovernorState().loadReputationHistory(repoFullName, apiBaseUrl); } -export function saveReputationHistory(repoFullName, history) { - return getDefaultGovernorState().saveReputationHistory(repoFullName, history); +export function saveReputationHistory(repoFullName, history, apiBaseUrl) { + return getDefaultGovernorState().saveReputationHistory(repoFullName, history, apiBaseUrl); } export function recordOwnSubmission(record) { diff --git a/test/unit/miner-governor-state.test.ts b/test/unit/miner-governor-state.test.ts index 884b29c252..65bf23d9d0 100644 --- a/test/unit/miner-governor-state.test.ts +++ b/test/unit/miner-governor-state.test.ts @@ -6,8 +6,10 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { closeDefaultGovernorState, loadPauseState, + loadReputationHistory, openGovernorState, savePauseState, + saveReputationHistory, } from "../../packages/gittensory-miner/lib/governor-state.js"; const roots: string[] = []; @@ -236,6 +238,128 @@ describe("governor-state reputation history (#5134)", () => { expect(() => state.loadReputationHistory("not-a-repo")).toThrow(/invalid_repo_full_name/); expect(() => state.saveReputationHistory("not-a-repo", { decided: 1, unfavorable: 0 })).toThrow(/invalid_repo_full_name/); }); + + describe("forge-scoping (#5563)", () => { + it("two forge hosts can each hold their own reputation history for the same owner/repo without colliding", () => { + const state = tempState(); + state.saveReputationHistory("acme/widgets", { decided: 10, unfavorable: 4 }, "https://api.github.com"); + state.saveReputationHistory("acme/widgets", { decided: 2, unfavorable: 1 }, "https://ghe.example.com/api/v3"); + expect(state.loadReputationHistory("acme/widgets", "https://api.github.com")).toEqual({ decided: 10, unfavorable: 4 }); + expect(state.loadReputationHistory("acme/widgets", "https://ghe.example.com/api/v3")).toEqual({ + decided: 2, + unfavorable: 1, + }); + }); + + it("defaults apiBaseUrl to the github.com default when omitted", () => { + const state = tempState(); + state.saveReputationHistory("acme/widgets", { decided: 3, unfavorable: 0 }); + expect(state.loadReputationHistory("acme/widgets", "https://api.github.com")).toEqual({ + decided: 3, + unfavorable: 0, + }); + }); + + it("rejects a non-string or blank apiBaseUrl", () => { + const state = tempState(); + expect(() => state.loadReputationHistory("acme/widgets", " ")).toThrow("invalid_api_base_url"); + expect(() => state.saveReputationHistory("acme/widgets", { decided: 1, unfavorable: 0 }, 42 as never)).toThrow( + "invalid_api_base_url", + ); + }); + + it("migrates an existing pre-#5563 file, backfilling api_base_url and preserving every row", () => { + const root = mkdtempSync(join(tmpdir(), "gittensory-miner-governor-state-legacy-")); + roots.push(root); + const dbPath = join(root, "legacy.sqlite3"); + const legacy = new DatabaseSync(dbPath); + legacy.exec(` + CREATE TABLE governor_reputation_history ( + repo_full_name TEXT PRIMARY KEY, + decided INTEGER NOT NULL, + unfavorable INTEGER NOT NULL, + updated_at TEXT NOT NULL + ) + `); + legacy.exec( + "INSERT INTO governor_reputation_history (repo_full_name, decided, unfavorable, updated_at) VALUES ('acme/widgets', 7, 2, '2026-01-01T00:00:00.000Z')", + ); + legacy.close(); + + const state = openGovernorState(dbPath); + states.push(state); + expect(state.loadReputationHistory("acme/widgets", "https://api.github.com")).toEqual({ + decided: 7, + unfavorable: 2, + }); + // The old bare repo_full_name PRIMARY KEY collision is gone: a second host can now hold its own history. + state.saveReputationHistory("acme/widgets", { decided: 1, unfavorable: 0 }, "https://ghe.example.com/api/v3"); + expect(state.loadReputationHistory("acme/widgets", "https://api.github.com")).toEqual({ + decided: 7, + unfavorable: 2, + }); + expect(state.loadReputationHistory("acme/widgets", "https://ghe.example.com/api/v3")).toEqual({ + decided: 1, + unfavorable: 0, + }); + }); + + it("REGRESSION: a legacy row violating the rebuilt table's NOT NULL columns is dropped, not a migration-aborting crash", () => { + const root = mkdtempSync(join(tmpdir(), "gittensory-miner-governor-state-legacy-corrupt-")); + roots.push(root); + const dbPath = join(root, "legacy-corrupt.sqlite3"); + const legacy = new DatabaseSync(dbPath); + // No NOT NULL on decided/unfavorable here, simulating a hand-edited or otherwise corrupted legacy file -- + // the real baseline schema always enforces NOT NULL, so this can only arise from external tampering. + legacy.exec(` + CREATE TABLE governor_reputation_history ( + repo_full_name TEXT PRIMARY KEY, + decided INTEGER, + unfavorable INTEGER, + updated_at TEXT NOT NULL + ) + `); + legacy + .prepare("INSERT INTO governor_reputation_history (repo_full_name, decided, unfavorable, updated_at) VALUES (?, NULL, NULL, ?)") + .run("acme/corrupt", "2026-01-01T00:00:00.000Z"); + legacy + .prepare("INSERT INTO governor_reputation_history (repo_full_name, decided, unfavorable, updated_at) VALUES (?, ?, ?, ?)") + .run("acme/widgets", 3, 1, "2026-01-01T00:00:00.000Z"); + legacy.close(); + + let opened: ReturnType | undefined; + expect(() => { + opened = openGovernorState(dbPath); + }).not.toThrow(); + const state = opened!; + states.push(state); + expect(state.loadReputationHistory("acme/widgets", "https://api.github.com")).toEqual({ + decided: 3, + unfavorable: 1, + }); + // The corrupt row was dropped, not migrated -- it reads back as the zero default, not a NULL leak. + expect(state.loadReputationHistory("acme/corrupt", "https://api.github.com")).toEqual({ + decided: 0, + unfavorable: 0, + }); + }); + + it("migration is idempotent: reopening an already-migrated file doesn't rebuild the table again", () => { + const root = mkdtempSync(join(tmpdir(), "gittensory-miner-governor-state-idempotent-")); + roots.push(root); + const dbPath = join(root, "state.sqlite3"); + const first = openGovernorState(dbPath); + first.saveReputationHistory("acme/widgets", { decided: 5, unfavorable: 2 }, "https://ghe.example.com/api/v3"); + first.close(); + + const second = openGovernorState(dbPath); + states.push(second); + expect(second.loadReputationHistory("acme/widgets", "https://ghe.example.com/api/v3")).toEqual({ + decided: 5, + unfavorable: 2, + }); + }); + }); }); describe("governor-state own-submission history (#5134)", () => { @@ -295,4 +419,19 @@ describe("governor-state module-level default singleton (#5134)", () => { expect(written).toMatchObject({ paused: true, reason: "singleton pause" }); expect(loadPauseState()).toEqual(written); }); + + it("loadReputationHistory/saveReputationHistory module-level wrappers round-trip through the default singleton, forwarding apiBaseUrl (#5563)", () => { + const root = mkdtempSync(join(tmpdir(), "gittensory-miner-governor-state-singleton-reputation-")); + roots.push(root); + vi.stubEnv("GITTENSORY_MINER_GOVERNOR_STATE_DB", join(root, "governor-state.sqlite3")); + expect(loadReputationHistory("acme/widgets", "https://ghe.example.com/api/v3")).toEqual({ + decided: 0, + unfavorable: 0, + }); + const written = saveReputationHistory("acme/widgets", { decided: 4, unfavorable: 1 }, "https://ghe.example.com/api/v3"); + expect(written).toEqual({ decided: 4, unfavorable: 1 }); + expect(loadReputationHistory("acme/widgets", "https://ghe.example.com/api/v3")).toEqual(written); + // The github.com default host is untouched. + expect(loadReputationHistory("acme/widgets")).toEqual({ decided: 0, unfavorable: 0 }); + }); });