fix(miner): scope deny-hook synthesis proposals by forge host, not bare repoFullName - #5595
Conversation
…re repoFullName 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).
|
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 | 0ea1161 | Commit Preview URL Branch Preview URL |
Jul 13 2026, 07:50 AM |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5595 +/- ##
=======================================
Coverage 95.01% 95.01%
=======================================
Files 573 573
Lines 45546 45560 +14
Branches 14661 14661
=======================================
+ Hits 43277 43291 +14
Misses 1518 1518
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 07:55:05 UTC
⏸️ Suggested Action - Manual Review Review summary Nits — 6 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.
|
Summary
deny_rule_proposals'PRIMARY KEY (repo_full_name, id)let two forge hosts (github.com vs. a GitHub Enterprise host, De-hardcode discovery from gittensory's own conventions #4784) serving a same-namedowner/reposhare one proposal row (and its maintainer approval decision).PRIMARY KEY (api_base_url, repo_full_name, id). This file has no schema-version framework of its own (rawDatabaseSync, noapplySchemaMigrations) — followsgovernor-state.js's idempotent column-presence-gated rebuild convention instead of introducing a new framework dependency here for the first time.INSERT OR IGNOREfor the copy step, matching the fix already applied to every other Scope local ledger keys by forge host, not bare repoFullName #5563 migration in this epic (fix(miner): claimNextBatch can claim the wrong host's row across two forge hosts #5594): a legacy row with an already-invalidstatusvalue is dropped, not a migration-aborting crash.apiBaseUrlthroughrefreshProposals/listProposals/setProposalStatus/resolveEffectiveRules.initDenyHookSynthesisStorehas no real callers yet (feeds the consumption surface maintainer: PreToolUse-hook-enforced house rules (deny even under bypassPermissions) #2343 will eventually wire intoevaluateDenyHooks), so there's no CLI surface or call site to thread through — purely the storage-layer fix, mirroringgovernor_reputation_history's scaffold-only precedent (fix(miner): scope governor_reputation_history by forge host, not bare repoFullName #5591).Fifth and final store from #5563. Series:
claim-ledger.jsin #5576,portfolio-queue.jsin #5583,run-state.jsin #5585,governor-state.js's reputation table in #5591,claimNextBatch/migration-hardening follow-up in #5594.Scope note carried from every PR in this series: full end-to-end forge-threading through the attempt/loop pipeline (
attempt-cli.js,loop-cli.jsnever resolveapiBaseUrlfor these stores' write paths at all today) remains a separate, pre-existing gap outside #5563's boundary — the schema-level collision risk (the actual defect #5563 tracks) is now fully closed across all 5 stores.Closes #5563
Test plan
npx vitest run test/unit/miner-deny-hook-synthesis.test.ts— 13/13 passing, including a migration test, a cross-host coexistence test, aresolveEffectiveRulesapiBaseUrl-threading test, and a regression test provingINSERT OR IGNOREdrops a corrupted legacy row instead of crashing the migrationnpx tsc --noEmitcleannpm run build:minercleannpx vitest run test/unit/miner-*.test.ts— 1645/1645 passing