fix(orb): economic-integrity and miner-identity fixes (#9077, #9078, #9079, #9080)#9238
Conversation
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
❌ 16 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
loopover-ui | c5d38aa | Commit Preview URL Branch Preview URL |
Jul 27 2026, 09:15 AM |
|
Caution 🛑 LoopOver review result - fixes requiredReview updated: 2026-07-27 08:56:08 UTC
Review summary Nits — 3 non-blocking
CI checks failing
Decision drivers
Context & advisory signals — never blocks the verdict
Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver 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://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. Decision record
🟩 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 LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
2502400 to
2b31b38
Compare
…9078) verifyDecisionLedger walked decision_ledger's hash chain but never joined decision_records to recompute contentDigest(JSON.parse(record_json)) against the digest each row committed to -- the reconciliation its own doc comment promised. A rewrite of record_json (or a hand-deletion of the row) passed verification clean as long as the chain itself stayed self-consistent. Adds two distinct break kinds: content_mismatch (record_json no longer hashes to what the chain committed to) and missing_record (the chain references a decision_records id that no longer exists). Both batch-fetch against the ledger window already being verified, so this stays a single extra query rather than N+1. Also stops persistDecisionRecord from conflating "the record failed to persist at all" with "the record persisted but its ledger append failed" -- the latter is now its own dedicated console.error alarm (forwarded to Sentry like other operator-facing anomaly signals) instead of falling through to the generic swallowed warn.
…ogin (#9079) fetchOfficialGittensorMiner matched a PR/issue author against the upstream /miners list purely by lowercased githubUsername. A GitHub login is renameable and a freed one is reclaimable by anyone, so if the upstream list still carries a miner's old username (its refresh cadence is upstream's, not ours), whoever claims that freed login inherits confirmed_miner -- a genuinely elevated role (command self-retrigger, the miner-scoped open-PR cap, the unlinked-issue-guardrail velocity exception). fetchOfficialGittensorMiner/fetchGittensorContributorSnapshot now accept an optional githubId and match on it FIRST when the caller has one, never falling back to a username match once an id was supplied. Threaded through getCachedOfficialMinerDetection's callers via PullRequestRecord/ IssueRecord.authorGithubId (captured from the webhook's user.id since #9125) and unlinked-issue-guardrail's/miner-detection-cache's own isConfirmedOfficialMiner. Callers with no stored id yet keep the prior login-only behavior unchanged (a strict narrowing, never a widening, of the old always-username match). Left the bulk fetchOfficialGittensorMinerLogins cohort-classification read and a couple of advisory-only lookups (reputation-wire's install-wide reputation widening, the MCP/decision-pack snapshot tools that only ever receive a login as their input) on login-only matching -- none of them grant an elevated role, and re-scoping every caller's own input contract was out of scope for this fix.
…9080) The bounty mirror import route collected every lifecycle event into one array and persisted them all only after the whole loop finished, so a mid-loop failure left every bounty upserted so far already advanced with zero lifecycle rows recorded for any of them -- unrecoverable once the next import's diff runs against the now-updated rows. Each item is now upserted and its own lifecycle event written in the same step, wrapped in its own try/catch, so one bad row can no longer take the rest of the batch down with it. upsertBounty's insert only handled a conflict on the id primary key, but the table also has a UNIQUE(repo_full_name, issue_number) index. Upstream re-issuing a bounty on the same issue under a new id hit that second, unhandled unique index and threw, 500ing the import and wedging every subsequent import on the same row permanently. A second chained onConflictDoUpdate (SQLite's native multi-target upsert syntax) resolves both conflicts to an update; the surviving row always keeps whichever id it already has, so lifecycle events already keyed to it are never orphaned by a later re-issue. classifyBountyLifecycle's status regexes were unanchored substring matches, so a base word buried mid-token false-positived: "unclaimed" and "not_completed" both classified as completed, "avoid" classified as cancelled, "renewed" classified as active. Anchoring each alternative on a leading word boundary keeps every genuine prefix match (cancelled, completed, rewarded, ...) while no longer matching those buried substrings. fundingStatus compared the amount against exactly one hardcoded zero-string literal ("0.0000"), so any other zero-ish spelling upstream might send ("0", "0.0", "0.00") reported as funded. Parsing to a number and requiring it to be finite and positive treats every zero-ish spelling the same, string or numeric, and degrades a malformed amount to target-only/unknown instead of reporting funded. Closes #9080
…atisfaction verdict (#9077) isLinkedIssueTrustworthy trusts every OPEN linked issue unconditionally, and trustMaintainerAuthoredIssueForReward's entire unlock was "the linked issue's author independently checks out as a repo maintainer" -- nothing verified the PR actually addressed the issue it cited. A contributor could get a reward label (e.g. gittensor:priority) applied by citing any open, maintainer- authored, reward-labeled issue, whether or not their PR touched it. A label reached only via that relaxed maintainer-authored path is now additionally required to have a PUBLISHED "addressed" linked-issue- satisfaction verdict for the exact (repo, PR, issue) triple, read via getLatestPublishedLinkedIssueSatisfaction rather than spending a fresh model call. This doesn't touch the direct author/assignee path (already strict) or a non-reward trustMaintainerAuthoredIssue mapping (bug/feature mirroring, lower stakes). A repo that hasn't opted into linkedIssueSatisfactionGateMode never persists a verdict at all, so the reward relaxation simply never fires for it -- a fail-safe degrade, not a silent bypass. Closes #9077
…text once propagation is enabled (#9077) resolvePrTypeLabel fell through to deriveKindFromTitle (pure PR-title regex matching, entirely author-controlled) whenever linked-issue label propagation was enabled but produced no exclusive/additive match for a pass -- silently reintroducing the exact author-controlled-free-text classification a repo opted OUT of by turning propagation on. A contributor could pick their own gittensor:bug/gittensor:feature multiplier by title wording alone, on any repo running propagation, whenever the linked issue simply hadn't been triaged yet. resolvePrTypeLabel now has a distinct "propagation_unmatched" outcome for that case: no label is applied, and removeLabels still clears every configured category (so a stale prior label doesn't look like a confirmed classification). This only changes behavior for a repo that has explicitly enabled linkedIssueLabelPropagation -- the shipped default (propagation disabled, the overwhelming majority of installs) is completely unchanged, still title-derived exactly as before. This does NOT eliminate title-derived classification for the shipped default itself -- doing so would silently disable type labels for every repo that hasn't configured propagation with mappings covering bug/feature, which is a larger design decision (a real diff/changed-path classifier, or a default-behavior migration) better suited to its own follow-up given this issue's own "Long Term Features" milestone. See the PR description for the full disposition.
2b31b38 to
c5d38aa
Compare
Summary
Four ORB economic-integrity / miner-identity issues, one PR per the usual bundling convention.
#9079 — miner identity matched on renameable login instead of stable githubId
fetchOfficialGittensorMinernow matches on the upstream/minerslist's immutablegithubIdfirst whenever the caller has one (threaded through viaPullRequestRecord/IssueRecord.authorGithubId, captured since #9125), falling back to username-only matching only when no id is known yet — a strict narrowing of the old always-username behavior, never a widening.#9078 — decision-ledger verify endpoint never reconciled record_json against record_digest
verifyDecisionLedgernow joinsdecision_recordsfor the window under verification and recomputescontentDigest(JSON.parse(record_json))against what the chain committed to, reporting two new break kinds:content_mismatchandmissing_record. A failed ledger append is now its own dedicatedconsole.erroralarm instead of falling into the same swallowed warn as every other persist failure.#9080 — bounty mirror import: audit gap + permanent-wedge bug
upsertBountynow chains a secondonConflictDoUpdateon(repo_full_name, issue_number)(SQLite's native multi-target upsert), so upstream re-issuing a bounty on the same issue under a new id reconciles instead of 500ing and permanently wedging every subsequent import.classifyBountyLifecycle's status regexes are now anchored on a leading word boundary (unclaimed/not_completed/avoid/renewedno longer false-positive as completed/cancelled/active via unanchored substring matches).fundingStatusnow parses the amount to a number and requires it finite and positive, instead of comparing against exactly one hardcoded zero-string literal ("0"/"0.0"/"0.00"no longer read as funded).#9077 — reward multiplier chosen by contributor-authored PR title text
Two independent, judgment-scoped fixes:
Satisfaction-verdict gate on reward-label maintainer trust (full fix for the SUSPECTED vector). A
trustMaintainerAuthoredIssueForReward-flagged label unlocked via the maintainer-authored-issue relaxation now additionally requires a published"addressed"linked-issue-satisfaction verdict for the exact (repo, PR, issue) triple, read via the existinglinked_issue_satisfaction_cacherather than spending a fresh model call. Before this, "the linked issue's author happens to be a maintainer" was the entire evidence bar — a contributor could getgittensor:priorityapplied by citing any open, maintainer-authored, reward-labeled issue whether or not their PR touched it. This is a deliberate coupling: a repo that hasn't opted intolinkedIssueSatisfactionGateModewill never see this reward relaxation fire (fail-safe degrade, not a bypass).Partial mitigation for the CONFIRMED title-derivation vector.
resolvePrTypeLabelno longer falls back toderiveKindFromTitlewhen a repo haslinkedIssueLabelPropagation.enabledbut the linked issue's labels matched none of the configured mappings — a new"propagation_unmatched"outcome applies no label at all instead (clearing any stale prior label too). This only changes behavior for a repo that has explicitly opted into propagation.Judgment call, flagged explicitly: this does not remove title-derived bug/feature classification for the shipped default (propagation disabled), which is what the issue's own concrete "feat: add support for X" example is about. Doing so would silently stop applying any type label at all for the overwhelming majority of self-hosted installs that haven't configured propagation mappings covering bug/feature — a bigger default-behavior change than seemed safe to ship unilaterally in this PR without a broader rollout/design discussion (the issue's own milestone is "ORB - Long Term Features & Improvements II"). The other suggested alternative — classifying from diff/changed-file evidence — isn't cheaply available at the type-label decision's call site today (no file-change data is fetched there); wiring that in is a larger, separate change. Flagging this so it can be tracked as an explicit follow-up rather than silently left half-done.
Test plan
npx tsc --noEmit -p tsconfig.json --incremental false(app) — cleannpm run buildinpackages/loopover-engine— cleanpackages/loopover-engineown test suite (npm test) — 727/734 pass; the 7 failures are in an unrelatediterate-loop/self-review module and reproduce identically onorigin/mainbefore this branch's changes (confirmed viagit stash)pr-type-label{,-engine}.test.ts,linked-issue-label-propagation-fetch.test.ts,linked-issue-label-propagation{,-engine}.test.ts,queue.test.ts,queue-5.test.ts,decision-record.test.ts,gittensor-api.test.ts,signals{,-edge-cases,-v2}.test.ts,db-persistence.test.ts,integration/routes-errors.test.ts,integration/api.test.ts— 850/850 passorigin/main, rebuilt the engine package, re-ran the full targeted suite post-rebaseCloses #9077
Closes #9078
Closes #9079
Closes #9080