fix(orb): whitelist gate_verdict and gate_reasoncode_bucket on ingest - #9896
Conversation
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-29 19:41:31 UTC
Review summary Nits — 3 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionPartially addressed 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. 🟩 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.
|
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
ingestOrbSignals whitelists outcome and reversal_flag against closed vocabularies but stored gate_verdict and gate_reasoncode_bucket with only a length check, though both are read downstream as closed enums by exact string equality. An off-vocabulary gate_verdict (a case variant like "Merge", or any foreign string) matches neither foldInstance branch, so a real decision is silently reclassified into `holds` and understates the published fleetAccuracy.coveragePct — the exact metric JSONbored#8829 added so coverage could not be gamed by raising the hold rate. The open ingest endpoint means a registered instance on an older/buggier build can poison its own published coverage. Add VALID_VERDICTS (merge|close|hold, the writer's GateAction vocabulary) and VALID_REASONCODE_BUCKETS (bucketReasonCode's nine literals) next to VALID_OUTCOMES/ VALID_REVERSALS, and AND the membership check onto the existing length check for each column, coercing an off-vocabulary value to null (which foldInstance already treats as a normal quality verdict). No downstream change needed. Closes JSONbored#9642
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9896 +/- ##
==========================================
+ Coverage 79.29% 79.33% +0.03%
==========================================
Files 281 282 +1
Lines 58566 58667 +101
Branches 6785 6822 +37
==========================================
+ Hits 46440 46541 +101
Misses 11843 11843
Partials 283 283
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Summary
ingestOrbSignals(src/orb/ingest.ts) normalizes untrusted fleet telemetry, whitelistingoutcome(VALID_OUTCOMES) andreversal_flag(VALID_REVERSALS) against closed vocabularies. The other two enum-valued columns were stored with only a length check:Both are read downstream as closed enums by exact equality:
foldInstance(src/orb/analytics.ts) branches onverdict === "merge"/=== "close". Any other value (e.g. the case variant"Merge") matches neither, so the row lands inholdsand is published as a deferral infleetAccuracy.coveragePcton/v1/public/stats— silently understating the coverage figure metrics: publish per-arm precision, coverage, and Wilson intervals — retire the bare accuracy scalar #8829 added specifically so it couldn't be gamed by raising the hold rate.gate_reasoncode_bucketis compared against the literal"policy_action"; the writer's vocabulary (bucketReasonCode,src/selfhost/orb-collector.ts) is nine fixed literals, but the reader accepted anything.The ingest endpoint is open by design, and storage is
INSERT OR REPLACEkeyed on(instance_id, repo_hash, pr_hash)— so a registered instance on an older/buggier build can poison its own published coverage, and the whitelist that would stop it already exists three lines above for the sibling column.Fix
Added
VALID_VERDICTS = new Set(["merge", "close", "hold"])(the honest writer'sGateActionvocabulary) andVALID_REASONCODE_BUCKETS(the ninebucketReasonCodeliterals) next toVALID_OUTCOMES/VALID_REVERSALS, and AND-ed the membership check onto the existing length check for each column (length check kept, not replaced). An off-vocabulary value is stored asnull, whichfoldInstancealready treats as a normal quality verdict — no downstream change.Tests
Added to
test/integration/orb-ingest.test.ts: a validgate_verdict(close/hold) is stored while an off-vocabulary but length-legal value ("Merge","banana") becomesnull; likewisegate_reasoncode_bucketkeepspolicy_action/otherand nullsmade_up_bucket. Both fail against the current length-only check. Full suite passes (50/50).Closes #9642