fix(miner): scope claim-ledger by forge host, not bare repoFullName - #5576
Conversation
UNIQUE(repo_full_name, issue_number) let two different forge hosts (github.com vs. a GitHub Enterprise host, #4784) serving a same-named owner/repo collide in the claim ledger. Rebuild the constraint to UNIQUE(api_base_url, repo_full_name, issue_number), migrate existing rows via a real table rebuild backfilling the pre-#4784 implicit default, and thread an optional apiBaseUrl through the store's API, the CLI (--api-base-url), and the expiry sweep (echoes each claim's own host instead of defaulting). Advances #5563 (claim-ledger.js of 5 affected stores).
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
loopover-ui | 9a974ae | Commit Preview URL Branch Preview URL |
Jul 13 2026, 06:46 AM |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5576 +/- ##
==========================================
+ Coverage 94.98% 94.99% +0.01%
==========================================
Files 573 573
Lines 45469 45490 +21
Branches 14680 14680
==========================================
+ Hits 43187 43212 +25
+ Misses 1531 1527 -4
Partials 751 751
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
Warning 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏸️ Gittensory review result - manual review recommendedReview updated: 2026-07-13 06:50:44 UTC
⏸️ Suggested Action - Manual Review Review summary Nits — 5 non-blocking
Concerns raised — review before merging
📋 Copy for AI agents — paste into your coding agent
Review context
Contributor next steps
Signal definitions
[BETA] Chat with GittensoryAsk Gittensory a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://gittensory.aethereal.dev/docs/gittensory-commands 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.
|
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).
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).
repo_full_name TEXT PRIMARY KEY let two forge hosts (github.com vs. a GitHub Enterprise host, #4784) serving a same-named owner/repo share one "current discover/plan/prepare state" row. Rebuild the constraint to PRIMARY KEY (api_base_url, repo_full_name), backfilling existing rows with the pre-#4784 implicit default. The migration uses INSERT OR IGNORE so a pre-existing row with an already-invalid state (this store's read path already fails closed on those) can't abort the whole rebuild. Thread an optional apiBaseUrl through the store's API (getRunState/ setRunState) and the admin CLI (--api-base-url on state get/set). Every existing caller is unaffected: apiBaseUrl defaults to https://api.github.com when omitted. manage-status.js's collectRunPortfolio folds run state into one row per repo NAME (not per host) for its dashboard view -- documented in-code as a known, safe (no data loss, read-only) display limitation for the narrow case of the same repo name existing on two hosts, rather than silently patched over; broadening that fold to be host-aware is a separate, larger dashboard-shape change. Advances #5563 (run-state.js of 5 affected stores; claim-ledger.js landed in #5576, portfolio-queue.js in #5583).
#5583) 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).
repo_full_name TEXT PRIMARY KEY let two forge hosts (github.com vs. a GitHub Enterprise host, #4784) serving a same-named owner/repo share one "current discover/plan/prepare state" row. Rebuild the constraint to PRIMARY KEY (api_base_url, repo_full_name), backfilling existing rows with the pre-#4784 implicit default. The migration uses INSERT OR IGNORE so a pre-existing row with an already-invalid state (this store's read path already fails closed on those) can't abort the whole rebuild. Thread an optional apiBaseUrl through the store's API (getRunState/ setRunState) and the admin CLI (--api-base-url on state get/set). Every existing caller is unaffected: apiBaseUrl defaults to https://api.github.com when omitted. manage-status.js's collectRunPortfolio folds run state into one row per repo NAME (not per host) for its dashboard view -- documented in-code as a known, safe (no data loss, read-only) display limitation for the narrow case of the same repo name existing on two hosts, rather than silently patched over; broadening that fold to be host-aware is a separate, larger dashboard-shape change. Advances #5563 (run-state.js of 5 affected stores; claim-ledger.js landed in #5576, portfolio-queue.js in #5583).
… repoFullName (#5591) repo_full_name TEXT PRIMARY KEY let two forge hosts (github.com vs. a GitHub Enterprise host, #4784) serving a same-named owner/repo share one reputation-history row. Rebuild the constraint to PRIMARY KEY (api_base_url, repo_full_name), following governor-state.js's existing idempotent-ALTER convention (ensurePauseColumns) rather than introducing the applySchemaMigrations framework into this file for the first time -- a full PRIMARY KEY rebuild still needs the create-new/copy/ drop/rename dance, just gated by a column-presence check instead of a version stamp. Uses INSERT OR IGNORE for the copy step (a corrupted legacy row violating the rebuilt table's NOT NULL columns is dropped, not a migration-aborting crash), regression-tested the same way as run-state.js's #5563 fix. Threads an optional apiBaseUrl through loadReputationHistory/ saveReputationHistory. loadReputationHistory/saveReputationHistory have no real callers yet (attempt-runner.js's own comments note this governor input isn't wired into the attempt pipeline yet), so there is no CLI surface or call site to thread through -- this is purely the storage-layer fix, mirroring openClaimLedgerReadOnly's scaffold-only precedent. Advances #5563 (governor_reputation_history of 5 affected stores; claim-ledger.js landed in #5576, portfolio-queue.js in #5583, run-state.js in #5585; deny-hook-synthesis.js remains).
…re repoFullName (#5595) deny_rule_proposals' PRIMARY KEY (repo_full_name, id) let two forge hosts (github.com vs. a GitHub Enterprise host, #4784) serving a same-named owner/repo share one proposal row (and its maintainer approval decision). Rebuild the constraint to PRIMARY KEY (api_base_url, repo_full_name, id). This file has no schema-version framework of its own (unlike the package's other local stores) -- it uses a raw DatabaseSync connection, no applySchemaMigrations. Follows governor-state.js's idempotent column-presence-gated rebuild convention instead of introducing a new framework dependency here for the first time. Uses INSERT OR IGNORE for the copy step, matching the fix already applied to every other #5563 migration in this epic: a legacy row with an already-invalid status value (this store's own CHECK-constrained schema already rejects those) is dropped, not a migration-aborting crash. Threads an optional apiBaseUrl through refreshProposals/listProposals/ setProposalStatus/resolveEffectiveRules. initDenyHookSynthesisStore has no real callers yet (feeds the consumption surface #2343 will eventually wire into evaluateDenyHooks; this store owns derivation + audit, not live hook interception), so there is no CLI surface or call site to thread through -- purely the storage-layer fix, mirroring governor_reputation_history's scaffold-only precedent. Closes #5563 (5th and final store: claim-ledger.js in #5576, portfolio-queue.js in #5583, run-state.js in #5585, governor-state.js in #5591, plus the claimNextBatch/migration hardening follow-up in #5594).
Summary
claim-ledger.js'sUNIQUE(repo_full_name, issue_number)let two forge hosts (github.com vs. a GitHub Enterprise host, De-hardcode discovery from gittensory's own conventions #4784) serving a same-namedowner/repocollide in the claim ledger.UNIQUE(api_base_url, repo_full_name, issue_number)via a real SQLite table-rebuild migration (schema-version.js'sapplySchemaMigrations), backfilling existing rows with the pre-De-hardcode discovery from gittensory's own conventions #4784 implicit defaulthttps://api.github.com.apiBaseUrlthrough the store's API (recordClaim/releaseClaim/expireClaim/claimIssue), the admin CLI (--api-base-urlonclaim/release), and the expiry sweep (echoes each claim's own host back toexpireClaiminstead of defaulting — otherwise expiry could act on the wrong host's row).releaseClaim/expireClaimnow useUPDATE ... RETURNING *(matchingportfolio-queue.js's existing pattern) instead of a separate post-UPDATESELECT, removing a structurally-unreachable defensive branch.apiBaseUrldefaults tohttps://api.github.comwhen omitted.First of 5 stores from #5563 (
portfolio-queue.js,governor-state.js's reputation table,run-state.js,deny-hook-synthesis.jsfollow in separate PRs, given file-count size). Full end-to-end forge-threading through the attempt/loop pipeline (attempt-cli.js,loop-cli.js) remains a distinct, pre-existing gap — none of that pipeline resolvesapiBaseUrlfor these particular stores today, so it is out of this issue's boundary.Advances #5563
Test plan
npx vitest run test/unit/miner-claim-ledger*.test.ts— 49/49 passing, including a migration test (pre-existing on-disk file upgrades and preserves rows) and cross-host collision regression testsnpx tsc --noEmitcleannpm run build:minercleannpm run test:coverage(full, unsharded) — 15803/15815 passing (12 pre-existing skips), 100% patch coverage on touched lines/branchesnpm audit --audit-level=moderate— 0 vulnerabilities