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
2 changes: 2 additions & 0 deletions packages/gittensory-miner/lib/claim-ledger-cli.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export type ParsedClaimClaimArgs =
note: string | undefined;
dryRun: boolean;
json: boolean;
apiBaseUrl: string | undefined;
}
| { error: string };

Expand All @@ -16,6 +17,7 @@ export type ParsedClaimReleaseArgs =
issueNumber: number;
dryRun: boolean;
json: boolean;
apiBaseUrl: string | undefined;
}
| { error: string };

Expand Down
37 changes: 31 additions & 6 deletions packages/gittensory-miner/lib/claim-ledger-cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ import { CLAIM_STATUSES, openClaimLedger } from "./claim-ledger.js";
import { argsWantJson, describeCliError, reportCliFailure } from "./cli-error.js";

const CLAIM_CLAIM_USAGE =
"Usage: gittensory-miner claim claim <owner/repo> <issue#> [--note <text>] [--dry-run] [--json]";
const CLAIM_RELEASE_USAGE = "Usage: gittensory-miner claim release <owner/repo> <issue#> [--dry-run] [--json]";
"Usage: gittensory-miner claim claim <owner/repo> <issue#> [--note <text>] [--api-base-url <url>] [--dry-run] [--json]";
const CLAIM_RELEASE_USAGE =
"Usage: gittensory-miner claim release <owner/repo> <issue#> [--api-base-url <url>] [--dry-run] [--json]";
const CLAIM_LIST_USAGE =
"Usage: gittensory-miner claim list [--repo <owner/repo>] [--status active|released|expired] [--json]";

Expand All @@ -27,7 +28,7 @@ function parseIssueNumberArg(value, usage) {
}

export function parseClaimClaimArgs(args) {
const options = { json: false, note: undefined, dryRun: false };
const options = { json: false, note: undefined, dryRun: false, apiBaseUrl: undefined };
const positional = [];

for (let index = 0; index < args.length; index += 1) {
Expand All @@ -50,6 +51,17 @@ export function parseClaimClaimArgs(args) {
index += 1;
continue;
}
// #5563: scope the claim 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: CLAIM_CLAIM_USAGE };
}
options.apiBaseUrl = value;
index += 1;
continue;
}
if (token.startsWith("-")) {
return { error: `Unknown option: ${token}` };
}
Expand All @@ -71,14 +83,16 @@ export function parseClaimClaimArgs(args) {
note: options.note,
dryRun: options.dryRun,
json: options.json,
apiBaseUrl: options.apiBaseUrl,
};
}

export function parseClaimReleaseArgs(args) {
const options = { json: false, dryRun: false };
const options = { json: false, dryRun: false, apiBaseUrl: undefined };
const positional = [];

for (const token of args) {
for (let index = 0; index < args.length; index += 1) {
const token = args[index];
if (token === "--json") {
options.json = true;
continue;
Expand All @@ -87,6 +101,15 @@ export function parseClaimReleaseArgs(args) {
options.dryRun = true;
continue;
}
if (token === "--api-base-url") {
const value = args[index + 1];
if (!value || value.startsWith("-")) {
return { error: CLAIM_RELEASE_USAGE };
}
options.apiBaseUrl = value;
index += 1;
continue;
}
if (token.startsWith("-")) {
return { error: `Unknown option: ${token}` };
}
Expand All @@ -107,6 +130,7 @@ export function parseClaimReleaseArgs(args) {
issueNumber: issue.issueNumber,
dryRun: options.dryRun,
json: options.json,
apiBaseUrl: options.apiBaseUrl,
};
}

Expand Down Expand Up @@ -216,6 +240,7 @@ export function runClaimClaim(args, options = {}) {
parsed.repoFullName,
parsed.issueNumber,
parsed.note,
parsed.apiBaseUrl,
);
if (parsed.json) {
console.log(JSON.stringify({ claim }, null, 2));
Expand Down Expand Up @@ -247,7 +272,7 @@ export function runClaimRelease(args, options = {}) {

try {
return withClaimLedger(options, (claimLedger) => {
const claim = claimLedger.releaseClaim(parsed.repoFullName, parsed.issueNumber);
const claim = claimLedger.releaseClaim(parsed.repoFullName, parsed.issueNumber, parsed.apiBaseUrl);
if (!claim) {
return reportCliFailure(parsed.json, "claim_not_found");
}
Expand Down
4 changes: 3 additions & 1 deletion packages/gittensory-miner/lib/claim-ledger-expiry.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ export function sweepExpiredClaims(store, nowMs, maxAgeMs = DEFAULT_MAX_CLAIM_AG
const expired = findExpiredClaims(activeClaims, nowMs, maxAgeMs);
const transitioned = [];
for (const claim of expired) {
const updated = store.expireClaim(claim.repoFullName, claim.issueNumber);
// Echo the row's OWN apiBaseUrl back (#5563) rather than defaulting: two forge hosts can each have an
// active claim on the same owner/repo#issue, and defaulting here would expire the wrong host's row.
const updated = store.expireClaim(claim.repoFullName, claim.issueNumber, claim.apiBaseUrl);
if (updated) transitioned.push(updated);
}
return transitioned;
Expand Down
14 changes: 8 additions & 6 deletions packages/gittensory-miner/lib/claim-ledger.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ export type ClaimStatus = "active" | "released" | "expired";

export type ClaimEntry = {
id: number;
apiBaseUrl: string;
repoFullName: string;
issueNumber: number;
claimedAt: string;
Expand All @@ -13,6 +14,7 @@ export type RecordClaimInput = {
repoFullName: string;
issueNumber: number;
note?: string;
apiBaseUrl?: string;
};

export type ListClaimsFilter = {
Expand All @@ -23,9 +25,9 @@ export type ListClaimsFilter = {
export type ClaimLedger = {
dbPath: string;
recordClaim(claim: RecordClaimInput): ClaimEntry;
claimIssue(repoFullName: string, issueNumber: number, note?: string): ClaimEntry;
releaseClaim(repoFullName: string, issueNumber: number): ClaimEntry | null;
expireClaim(repoFullName: string, issueNumber: number): ClaimEntry | null;
claimIssue(repoFullName: string, issueNumber: number, note?: string, apiBaseUrl?: string): ClaimEntry;
releaseClaim(repoFullName: string, issueNumber: number, apiBaseUrl?: string): ClaimEntry | null;
expireClaim(repoFullName: string, issueNumber: number, apiBaseUrl?: string): ClaimEntry | null;
listClaims(filter?: ListClaimsFilter): ClaimEntry[];
listActiveClaims(repoFullName?: string): ClaimEntry[];
purgeByRepo(repoFullName: string): number;
Expand All @@ -48,13 +50,13 @@ export function openClaimLedgerReadOnly(dbPath: string): ReadOnlyClaimLedger;

export function recordClaim(claim: RecordClaimInput): ClaimEntry;

export function releaseClaim(repoFullName: string, issueNumber: number): ClaimEntry | null;
export function releaseClaim(repoFullName: string, issueNumber: number, apiBaseUrl?: string): ClaimEntry | null;

export function expireClaim(repoFullName: string, issueNumber: number): ClaimEntry | null;
export function expireClaim(repoFullName: string, issueNumber: number, apiBaseUrl?: string): ClaimEntry | null;

export function listClaims(filter?: ListClaimsFilter): ClaimEntry[];

export function claimIssue(repoFullName: string, issueNumber: number, note?: string): ClaimEntry;
export function claimIssue(repoFullName: string, issueNumber: number, note?: string, apiBaseUrl?: string): ClaimEntry;

export function listActiveClaims(repoFullName?: string): ClaimEntry[];

Expand Down
100 changes: 70 additions & 30 deletions packages/gittensory-miner/lib/claim-ledger.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { DatabaseSync } from "node:sqlite";
import { DEFAULT_FORGE_CONFIG } from "./forge-config.js";
import { normalizeLocalStoreDbPath, openLocalStoreDb, resolveLocalStoreDbPath } from "./local-store.js";
import { applySchemaMigrations } from "./schema-version.js";
import { CLAIM_LEDGER_PURGE_SPEC, purgeStoreByRepo } from "./store-maintenance.js";
Expand Down Expand Up @@ -34,6 +35,14 @@ function normalizeIssueNumber(issueNumber) {
return issueNumber;
}

/** 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();
}

/** Optional free-text note: omitted/nullish → null; a string is kept as-is; anything else is rejected. */
function normalizeNote(note) {
if (note === undefined || note === null) return null;
Expand All @@ -44,6 +53,7 @@ function normalizeNote(note) {
function rowToClaim(row) {
return {
id: row.id,
apiBaseUrl: row.api_base_url,
repoFullName: row.repo_full_name,
issueNumber: row.issue_number,
claimedAt: row.claimed_at,
Expand All @@ -52,10 +62,37 @@ function rowToClaim(row) {
};
}

// v1 -> v2 (#5563): scope the UNIQUE constraint by (api_base_url, repo_full_name, issue_number) instead of bare
// (repo_full_name, issue_number) -- two different forge hosts serving a same-named repo/issue must not collide
// in this ledger. SQLite cannot ALTER a UNIQUE constraint 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. Runs inside applySchemaMigrations' own transaction, so a mid-rebuild failure
// leaves the file at v1 and retries cleanly on next open.
function addApiBaseUrlScope(db) {
db.exec(`
CREATE TABLE miner_claims_v2 (
id INTEGER PRIMARY KEY AUTOINCREMENT,
api_base_url TEXT NOT NULL,
repo_full_name TEXT NOT NULL,
issue_number INTEGER NOT NULL,
claimed_at TEXT NOT NULL,
status TEXT NOT NULL DEFAULT 'active' CHECK (status IN ('active', 'released', 'expired')),
note TEXT,
UNIQUE (api_base_url, repo_full_name, issue_number)
)
`);
db.prepare(
`INSERT 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");
db.exec("ALTER TABLE miner_claims_v2 RENAME TO miner_claims");
}

/**
* Opens the local claim ledger, creating the table on first use. `UNIQUE(repo_full_name, issue_number)` keeps ONE
* row per claimed issue, and `recordClaim` is a single atomic INSERT…ON CONFLICT statement (no read-then-write), so
* concurrent claims cannot duplicate a row. (#2314)
* Opens the local claim ledger, creating the table on first use. `UNIQUE(api_base_url, repo_full_name,
* issue_number)` keeps ONE row per claimed issue per forge host, and `recordClaim` is a single atomic
* INSERT…ON CONFLICT statement (no read-then-write), so concurrent claims cannot duplicate a row. (#2314, #5563)
*/
export function openClaimLedger(dbPath = resolveClaimLedgerDbPath()) {
const resolvedPath = normalizeDbPath(dbPath);
Expand All @@ -74,29 +111,33 @@ export function openClaimLedger(dbPath = resolveClaimLedgerDbPath()) {
UNIQUE (repo_full_name, issue_number)
)
`);
// Schema-version convention (#4832): stamp the baseline and run any post-baseline migrations (none yet).
applySchemaMigrations(db, []);
// Schema-version convention (#4832): stamp the baseline and run any post-baseline migrations.
applySchemaMigrations(db, [addApiBaseUrlScope]);

// Idempotent claim in ONE atomic statement: insert a new active claim, or — only if the existing row is NOT
// already active — re-activate it (a released/expired claim can be re-claimed). The `WHERE status <> 'active'`
// guard makes re-claiming an already-active issue a true no-op (no row churn), never a duplicate row.
const recordStatement = db.prepare(`
INSERT INTO miner_claims (repo_full_name, issue_number, claimed_at, status, note)
VALUES (?, ?, ?, 'active', ?)
ON CONFLICT(repo_full_name, issue_number) DO UPDATE SET
INSERT INTO miner_claims (api_base_url, repo_full_name, issue_number, claimed_at, status, note)
VALUES (?, ?, ?, ?, 'active', ?)
ON CONFLICT(api_base_url, repo_full_name, issue_number) DO UPDATE SET
claimed_at = excluded.claimed_at,
note = excluded.note,
status = 'active'
WHERE miner_claims.status <> 'active'
`);
const getStatement = db.prepare(
"SELECT * FROM miner_claims WHERE repo_full_name = ? AND issue_number = ?",
"SELECT * FROM miner_claims WHERE api_base_url = ? AND repo_full_name = ? AND issue_number = ?",
);
// RETURNING (matching portfolio-queue.js's own claim/release statements) makes the "nothing to release/expire"
// case observable directly from ONE atomic statement, rather than a separate post-UPDATE SELECT whose "row
// went missing" branch would be structurally unreachable (nothing else runs between the UPDATE and a SELECT
// on the same key within one synchronous call).
const releaseStatement = db.prepare(
"UPDATE miner_claims SET status = 'released' WHERE repo_full_name = ? AND issue_number = ? AND status = 'active'",
"UPDATE miner_claims SET status = 'released' WHERE api_base_url = ? AND repo_full_name = ? AND issue_number = ? AND status = 'active' RETURNING *",
);
const expireStatement = db.prepare(
"UPDATE miner_claims SET status = 'expired' WHERE repo_full_name = ? AND issue_number = ? AND status = 'active'",
"UPDATE miner_claims SET status = 'expired' WHERE api_base_url = ? AND repo_full_name = ? AND issue_number = ? AND status = 'active' RETURNING *",
);
const listAllStatement = db.prepare("SELECT * FROM miner_claims ORDER BY id ASC");
const listRepoStatement = db.prepare(
Expand All @@ -123,27 +164,26 @@ export function openClaimLedger(dbPath = resolveClaimLedgerDbPath()) {
const ledger = {
dbPath: resolvedPath,
recordClaim(claim) {
const apiBaseUrl = normalizeApiBaseUrl(claim?.apiBaseUrl);
const repoFullName = normalizeRepoFullName(claim?.repoFullName);
const issueNumber = normalizeIssueNumber(claim?.issueNumber);
const note = normalizeNote(claim?.note);
const claimedAt = new Date().toISOString();
recordStatement.run(repoFullName, issueNumber, claimedAt, note);
return rowToClaim(getStatement.get(repoFullName, issueNumber));
recordStatement.run(apiBaseUrl, repoFullName, issueNumber, claimedAt, note);
return rowToClaim(getStatement.get(apiBaseUrl, repoFullName, issueNumber));
},
releaseClaim(repoFullName, issueNumber) {
releaseClaim(repoFullName, issueNumber, apiBaseUrl) {
const normalizedForge = normalizeApiBaseUrl(apiBaseUrl);
const normalizedRepo = normalizeRepoFullName(repoFullName);
const normalizedIssue = normalizeIssueNumber(issueNumber);
const result = releaseStatement.run(normalizedRepo, normalizedIssue);
if (result.changes === 0) return null;
const row = getStatement.get(normalizedRepo, normalizedIssue);
const row = releaseStatement.get(normalizedForge, normalizedRepo, normalizedIssue);
return row ? rowToClaim(row) : null;
},
expireClaim(repoFullName, issueNumber) {
expireClaim(repoFullName, issueNumber, apiBaseUrl) {
const normalizedForge = normalizeApiBaseUrl(apiBaseUrl);
const normalizedRepo = normalizeRepoFullName(repoFullName);
const normalizedIssue = normalizeIssueNumber(issueNumber);
const result = expireStatement.run(normalizedRepo, normalizedIssue);
if (result.changes === 0) return null;
const row = getStatement.get(normalizedRepo, normalizedIssue);
const row = expireStatement.get(normalizedForge, normalizedRepo, normalizedIssue);
return row ? rowToClaim(row) : null;
},
listClaims(filter = {}) {
Expand All @@ -162,8 +202,8 @@ export function openClaimLedger(dbPath = resolveClaimLedgerDbPath()) {
}
return rows.map(rowToClaim);
},
claimIssue(repoFullName, issueNumber, note) {
return ledger.recordClaim({ repoFullName, issueNumber, note });
claimIssue(repoFullName, issueNumber, note, apiBaseUrl) {
return ledger.recordClaim({ repoFullName, issueNumber, note, apiBaseUrl });
},
listActiveClaims(repoFullName) {
const filter = { status: "active" };
Expand Down Expand Up @@ -230,21 +270,21 @@ export function recordClaim(claim) {
return getDefaultClaimLedger().recordClaim(claim);
}

export function releaseClaim(repoFullName, issueNumber) {
return getDefaultClaimLedger().releaseClaim(repoFullName, issueNumber);
export function releaseClaim(repoFullName, issueNumber, apiBaseUrl) {
return getDefaultClaimLedger().releaseClaim(repoFullName, issueNumber, apiBaseUrl);
}

export function expireClaim(repoFullName, issueNumber) {
return getDefaultClaimLedger().expireClaim(repoFullName, issueNumber);
export function expireClaim(repoFullName, issueNumber, apiBaseUrl) {
return getDefaultClaimLedger().expireClaim(repoFullName, issueNumber, apiBaseUrl);
}

export function listClaims(filter) {
return getDefaultClaimLedger().listClaims(filter);
}

/** Foundation-phase alias for `recordClaim({ repoFullName, issueNumber, note })`. (#3351) */
export function claimIssue(repoFullName, issueNumber, note) {
return getDefaultClaimLedger().claimIssue(repoFullName, issueNumber, note);
/** Foundation-phase alias for `recordClaim({ repoFullName, issueNumber, note, apiBaseUrl })`. (#3351) */
export function claimIssue(repoFullName, issueNumber, note, apiBaseUrl) {
return getDefaultClaimLedger().claimIssue(repoFullName, issueNumber, note, apiBaseUrl);
}

/** List only `active` claims, optionally scoped to one repo. (#3351) */
Expand Down
Loading