Skip to content

orb(ingest): gate_verdict and gate_reasoncode_bucket are stored unvalidated #9642

Description

@JSONbored

⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.

Context

ingestOrbSignals (src/orb/ingest.ts) normalizes untrusted fleet telemetry before storing it. Its own
comment at line 186 states the policy: "Untrusted-input normalization: whitelist reversal_flag, clamp cycle
time, coerce the rest to null."
Two of the four enum-valued columns are whitelisted:

  • outcome — rejected outright unless in VALID_OUTCOMES (line 181);
  • reversal_flag — coerced to "none" unless in VALID_REVERSALS (line 187).

The other two are stored with only a length check:

typeof event.gate_verdict === "string" && event.gate_verdict.length <= MAX_VERDICT_CHARS ? event.gate_verdict : null,   // line 203
...
typeof event.gate_reasoncode_bucket === "string" && event.gate_reasoncode_bucket.length <= MAX_BUCKET_CHARS ? event.gate_reasoncode_bucket : null,  // line 206

Both are read downstream as CLOSED enums, by exact string equality:

  • foldInstance (src/orb/analytics.ts:223, 227) branches on c.verdict === "merge" / c.verdict === "close".
    Anything else — including a case variant like "Merge" — matches neither branch, so the row contributes to
    decided but to neither wouldMerge nor wouldClose, and lands in
    holds: decided - verdicts - policyActions (line 244). It is then published as a hold in
    pooled.coverage = verdicts / (verdicts + holds) (src/orb/analytics.ts:378) and rendered as
    fleetAccuracy.coveragePct on /v1/public/stats (src/review/public-stats.ts:588). A real decision is
    silently reclassified as a deferral to a human, understating the published coverage — the exact figure metrics: publish per-arm precision, coverage, and Wilson intervals — retire the bare accuracy scalar #8829
    added so a bare accuracy scalar could not be gamed by raising the hold rate.
  • gate_reasoncode_bucket is compared against the literal "policy_action" (src/orb/analytics.ts:219). Any
    other value is scored as a quality verdict. The writer's vocabulary is fixed and small
    (bucketReasonCode, src/selfhost/orb-collector.ts:170-185, returns one of nine literals), but the reader
    accepts anything.

The honest writer sends GateAction (src/review/parity.ts:191: "merge" | "close" | "hold") and
bucketReasonCode's literals, so this is latent rather than currently firing — but the ingest endpoint is open
by design (src/orb/analytics.ts:12-19: "open ingest stores everyone's signals"), the storage is
INSERT OR REPLACE keyed on (instance_id, repo_hash, pr_hash), and the module doc says outright that the
registration gate is what stops a stranger moving calibration. Registration does not stop a registered
instance running an older or buggier build from poisoning its own published coverage figure, and the
whitelist that would stop it exists three lines above for the sibling column.

Requirements

  • Add VALID_VERDICTS = new Set(["merge", "close", "hold"]) and
    VALID_REASONCODE_BUCKETS containing exactly the literals bucketReasonCode
    (src/selfhost/orb-collector.ts:170-185) can return: none, policy_action, issue_policy,
    duplicate_risk, slop_advisory, ai_quality, author_policy, ci_readiness, other.
  • gate_verdict outside the whitelist is stored as null (the existing not-a-string behaviour), not preserved
    verbatim. gate_reasoncode_bucket outside the whitelist is stored as null, which
    foldInstance already treats as "a normal quality verdict, matching prior behavior"
    (src/orb/analytics.ts:63-66) — no downstream change is needed for that path.
  • The two new sets must be defined next to VALID_OUTCOMES/VALID_REVERSALS at the top of
    src/orb/ingest.ts, and the existing length checks must be kept in addition to (not replaced by) the
    membership checks.
  • An invariant test must pin VALID_REASONCODE_BUCKETS against bucketReasonCode's actual return set, so the
    two cannot drift: the test must call bucketReasonCode with an input reaching each branch and assert every
    result is a member.
  • An invariant test must pin VALID_VERDICTS against the GateAction union in src/review/parity.ts:191.

⚠️ Required pattern: mirror VALID_REVERSALS and its use at src/orb/ingest.ts:13, 187 exactly — a
module-level Set, a typeof === "string" && SET.has(...) guard at the bind site, and coercion to a safe
default rather than rejecting the whole row. What does NOT satisfy this issue: rejecting the entire event when
a verdict is unrecognized (that would drop the outcome data too, unlike the reversal-flag precedent);
normalizing case or trimming before the membership check; or adding the whitelist without the two invariant
tests that pin it to the writers.

Deliverables

  • src/orb/ingest.ts whitelists gate_verdict against a VALID_VERDICTS set and
    gate_reasoncode_bucket against a VALID_REASONCODE_BUCKETS set, storing null for non-members.
  • An invariant test asserting every value bucketReasonCode can return is a member of
    VALID_REASONCODE_BUCKETS, exercising each of its branches.
  • An invariant test asserting VALID_VERDICTS equals the GateAction union.
  • A named regression test asserting an ingest payload carrying gate_verdict: "Merge" stores null and
    that computeFleetAnalytics's pooled.coverage is therefore unaffected by it — where today that row is
    counted as a hold and drags coveragePct down.
  • A test covering the accepted path for each of the three valid verdicts and at least two valid buckets.

All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example adding
the whitelists without the two invariant tests — does not resolve this issue.

Test Coverage Requirements

This repo enforces 99%+ Codecov patch coverage, branch-counted. src/orb/ingest.ts is inside src/**, which
is inside coverage.include, so it is measured and gated. Both arms of each new membership check need a test
(a member and a non-member for each of the two columns), as does the existing typeof !== "string" arm which
must keep behaving identically. The regression test in Deliverable 4 is mandatory.

Expected Outcome

Every enum-valued column on the fleet-signal ingest path is normalized against a whitelist pinned to its
writer, so a build-skewed or hostile registered instance cannot silently reclassify its own decisions as holds
and move the publicly-published fleetAccuracy.coveragePct.

Links & Resources

src/orb/ingest.ts:13-14, 176-215, src/orb/analytics.ts:57-68, 208-246, 368-382,
src/selfhost/orb-collector.ts:170-185, src/review/parity.ts:191, src/review/public-stats.ts:588.

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions