Skip to content

engine(governor): governor-ledger's v8-ignore block hides real untested validation branches #10310

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

packages/loopover-engine/src/governor-ledger.ts validates and normalizes a governor ledger event
before an append-only SQLite insert. Its four private helpers — normalizeRequiredString,
normalizeOptionalRepoFullName, jsonRoundTripEqual, and serializePayload (lines 33-90) — are
wrapped in a single block comment:

/* v8 ignore start -- Normalization helpers are covered through normalizeGovernorLedgerEvent export tests. */

That claim is false for most of the file's real error branches. test/governor-ledger.test.ts (the
only file anywhere in the repo that calls normalizeGovernorLedgerEvent) contains exactly 3 tests,
and all three exercise only the repoFullName path-safety/character-set branches inside
normalizeOptionalRepoFullName. None of the following genuinely reachable throw branches in
normalizeGovernorLedgerEvent (lines 97-110) or its helpers are exercised by any test in the repo:

  • invalid_event — thrown when input is not an object (line 98)
  • invalid_event_type — thrown when eventType is missing/non-string/empty, or not one of
    GOVERNOR_LEDGER_EVENT_TYPES (lines 100-101, via normalizeRequiredString)
  • invalid_action_class — thrown when actionClass is missing/non-string/empty (line 105)
  • invalid_decision — thrown when decision is missing/non-string/empty (line 106)
  • invalid_reason — thrown when reason is missing/non-string/empty (line 107)
  • invalid_payload — thrown by serializePayload (lines 74-89) for three distinct cases: a
    non-object/array/null payload, a payload that throws inside JSON.stringify (e.g. a circular
    reference), and a payload whose JSON.parse(JSON.stringify(...)) round-trip is not
    jsonRoundTripEqual to the original (e.g. an object with undefined-valued keys, which
    JSON.stringify silently drops)

Because these helpers are under a blanket v8 ignore block, Codecov's patch-coverage gate will
never force a future edit to this file to add coverage for these branches — the ignore comment
actively hides the gap it claims doesn't exist.

Requirements

  • Remove the /* v8 ignore start ... stop */ block wrapping lines 33-90 in
    packages/loopover-engine/src/governor-ledger.ts. Do not add narrower, per-line ignore comments
    in its place unless a specific line is genuinely unreachable — the whole point of this issue is
    that these branches ARE reachable and must be tested, not re-hidden more precisely.
  • Add regression tests to packages/loopover-engine/test/governor-ledger.test.ts that directly call
    normalizeGovernorLedgerEvent (via the same ../dist/index.js import the existing tests use) and
    assert each of the following throws with the exact error message shown:
    • non-object input (e.g. null, "x", 42) → invalid_event
    • missing eventTypeinvalid_event_type
    • empty-string eventType (e.g. " ") → invalid_event_type
    • eventType not in GOVERNOR_LEDGER_EVENT_TYPES (e.g. "bogus") → invalid_event_type
    • missing actionClassinvalid_action_class
    • missing decisioninvalid_decision
    • missing reasoninvalid_reason
    • payload that is an array (e.g. []) → invalid_payload
    • payload that is nullinvalid_payload
    • payload containing a circular reference → invalid_payload
    • payload containing an undefined-valued key (e.g. { a: undefined }) → invalid_payload
      (this is the case jsonRoundTripEqual exists specifically to catch — JSON.stringify drops the
      key silently, so a naive round-trip check without jsonRoundTripEqual would miss it)
  • A valid payload (e.g. { foo: "bar" }) must still round-trip to payloadJson: '{"foo":"bar"}'
    with no throw — add or confirm a passing-case assertion for this alongside the failure cases above.

Deliverables

  • The blanket v8 ignore block in governor-ledger.ts is removed.
  • Every throw branch listed under Requirements has a dedicated regression test in
    packages/loopover-engine/test/governor-ledger.test.ts asserting the exact thrown error
    message.
  • The valid-payload passing case is covered.

All three Deliverables are required in this one PR — there is no narrower scope for this issue.

Test Coverage Requirements

packages/loopover-engine/src/** is measured by Codecov via two separate uploads whose hits are
unioned
— root test/** AND packages/loopover-engine/test/**. The regression tests for this
issue must be added under packages/loopover-engine/test/governor-ledger.test.ts specifically (not
only a root-level test/** file), or the patch-coverage gate can fail even with a passing
root-level test. Target 100% branch coverage of every line un-ignored by this change; every throw
statement and every non-throw fallthrough in the affected functions needs at least one asserting
test.

Expected Outcome

normalizeGovernorLedgerEvent's full validation surface — not just the repoFullName path-safety
subset — is exercised by real tests, and the Codecov patch gate will catch a future regression in
any of these branches instead of silently passing because the code is v8 ignored.

Links & Resources

  • packages/loopover-engine/src/governor-ledger.ts (lines 33-110)
  • packages/loopover-engine/test/governor-ledger.test.ts
  • Milestone: Miner Wave 4.6 — AMS Hardening Round 3

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