fix(control-plane): remember reversed payout events so a redelivery never re-debits and a reversal credits the recorded amount

[rsnetworkinginc]

Closes #9612

Summary

fix(control-plane): remember reversed payout events so a redelivery never re-debits and a reversal credits the recorded amount

The in-memory FakeSettlementBackendDriver tracked settled payout events with a single Set<string> keyed by payoutEventKey. That representation broke both idempotency clauses its own interface documents:

1. A reversal re-armed the double-pay guard. reversePayout deleted the key from the set — the only record the event was ever processed — so a redelivered pull_request webhook after a refund/dispute hit !settledEventKeys.has(key) and decremented the pool a second time (fundPool 500 → record → 400 → reverse → 500 → record again → 400 instead of staying 500).
2. A reversal credited whatever amount the caller passed. amount is deliberately not part of the event key, so reversePayout({ ...event, amount: 1_000_000 }, "refund") after recording amount: 100 credited 1,000,000 back — the fake had no memory of what it actually took.

Root cause

Presence/absence of a key in one Set conflated "currently settled" with "ever recorded", and stored nothing about the decremented amount.

Fix

• Replaced settledEventKeys: Set<string> internally with a module-local Map<string, SettledEventRecord> ({ amount, reversed }), closed over by createFakeSettlementBackendDriver and exposed through get accessors — same fake shape as createFakeTenantProvisioningDriver. Entries are never deleted, so an ever-recorded event stays known.
• recordPayoutEligibleEvent is now a no-op (no balance/state change) for any key it has ever recorded, including one since reversed; it still pushes onto calls on every invocation.
• reversePayout credits back the recorded amount (record.amount), ignoring the reversal call's event.amount; still a no-op for unrecorded and already-reversed keys; still pushes onto calls on every invocation.
• The public settledEventKeys: ReadonlySet<string> property remains with unchanged semantics — keys currently recorded-and-not-reversed — derived from the new internal state.
• Balance arithmetic for record/reverse goes through one small closure helper (addToPoolBalance) so the ?? 0 default has a single, fully branch-covered home. fundPool, getPoolBalance, payoutEventKey, the calls log shape (amount: event.amount as passed), and both exported type surfaces are unchanged.
• Updated the JSDoc on recordPayoutEligibleEvent (ever-recorded rule) and reversePayout (recorded-amount rule), which the code now honours.

Diff: control-plane/src/settlement-backend-driver.ts and control-plane/test/settlement-backend-driver.test.ts only. Every pre-existing test passes unmodified. No settlement policy, no payoutEventKey change, no ledger abstraction.

Tests (RED → GREEN)

New named regression tests, run against the unfixed source first:

not ok 204 - record -> reverse -> record: a redelivery after a reversal is a no-op, never a second debit
not ok 205 - reversePayout credits back the recorded amount, not the amount on the reversal call
not ok 206 - recordPayoutEligibleEvent on a reversed event does not re-add its key to settledEventKeys
# tests 230
# pass 227
# fail 3

After the fix (plus a fourth test covering record on a never-funded pool):

# tests 231
# pass 231
# fail 0

• record → reverse → record asserts getPoolBalance("pool-1") === 500 and driver.calls.map(c => c.step) is exactly ["fundPool", "recordPayoutEligibleEvent", "reversePayout", "recordPayoutEligibleEvent"].
• reversePayout({ ...event, amount: 1_000_000 }, "refund") after recording amount: 100 restores the balance to exactly its pre-record value.
• record on a reversed event does not re-add its key to settledEventKeys.
• The pre-existing settledEventKeys presence/absence tests pass unmodified.

Coverage

Both arms of every changed conditional are exercised: record on a never-seen key, a currently-settled key, and a previously-reversed key; reverse on an unrecorded key, a settled key, and an already-reversed key; addToPoolBalance on an absent and a present pool entry. CI's own coverage recipe (node --experimental-strip-types scripts/control-plane-coverage.ts, c8 over the built dist remapped to control-plane/src/**):

Statements   : 100% ( 2278/2278 )
Branches     : 100% ( 470/470 )
Functions    : 100% ( 109/109 )
Lines        : 100% ( 2278/2278 )

control-plane/src/settlement-backend-driver.ts: LF 156 / LH 156, BRF 18 / BRH 18 — zero uncovered lines or branches in the file.

Validation commands (repo root)

git diff --check                                             # clean
npm run control-plane:test                                   # 231 tests, 231 pass, 0 fail
node --experimental-strip-types scripts/control-plane-coverage.ts   # 100% statements/branches/functions/lines
npm --prefix control-plane run cf:typecheck                  # clean

[loopover-orb]

Warning

⏸️ LoopOver review result - manual review recommended

_{Review updated: 2026-07-29 01:18:49 UTC}

2 files · 1 AI reviewer · no blockers · CI green · clean

⏸️ Suggested Action - Manual Review

Review summary
The AI review returned non-blocking notes for this change but did not include a separate narrative summary. Review the nits below before deciding this PR.

Nits — 3 non-blocking

• control-plane/src/settlement-backend-driver.ts: the settledEventKeys getter now allocates a new Set on every access by iterating the full map — fine for a test fake but worth a comment noting it's O(n) per read if this pattern is copied into a non-fake driver.
• control-plane/test/settlement-backend-driver.test.ts: the new tests are thorough but slightly duplicate assertions already covered by 'record -> reverse -> record' and 'on a reversed event does not re-add its key' — consider consolidating if patch coverage becomes a concern later.
• Consider exporting SettledEventRecord's shape or adding a JSDoc example in the interface for how a real backend should implement lifetime idempotency, since this fake sets the pattern other drivers (e.g. a future real settlement service) will need to mirror.

Decision drivers

• ✅ Code review — No blockers (1 reviewer)
• ✅ Gate result — Passing (No configured blocker found.)

Context & advisory signals — never blocks the verdict

Signal	Result	Evidence
Linked issue	✅ Linked	#9612
Related work	✅ No active overlap found	No same-issue or scoped active PR overlap found.
Change scope	✅ 20/20	Low review scope from cached public metadata (1 linked issue).
Validation posture	❌ 5/25	Preflight is holding this PR: the review lane is unavailable, so it is not ready for automated review.
Contributor workload	✅ 10/10	Author activity: 52 registered-repo PR(s), 27 merged, 3 issue(s).
Contributor context	✅ Confirmed Gittensor contributor	rsnetworkinginc; Gittensor profile; 52 PR(s), 3 issue(s).
Improvement	✅ Minor	risk: clean · value: minor · LLM: moderate

Review context

• Author: rsnetworkinginc
• Role context: outside_contributor
• Public audience mode: oss maintainer
• Lane context: Repository is registered but has no active allocation in the current snapshot.
• Public profile languages: TypeScript
• Official Gittensor activity: 52 PR(s), 3 issue(s).
• PR-specific overlap: none found.

Contributor next steps

• Start here: Await review-lane availability.
• Then work through the remaining 2 steps in the Signals table above.

Signal definitions

• Related work = same linked issue, overlapping active PRs, or title/path similarity.
• Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
• Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
• Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
• Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🧪 Chat with LoopOver

Ask 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.

• @loopover ask <question> answers contribution-quality Q&A with source citations and freshness.
• @loopover chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
• A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

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.

• Re-run LoopOver review

[superagent-security]

Superagent didn't find any vulnerabilities or security issues in this PR.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.20%. Comparing base (bd139a5) to head (6808da9).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9624      +/-   ##
==========================================
- Coverage   90.02%   89.20%   -0.83%     
==========================================
  Files         888      888              
  Lines      111983   112006      +23     
  Branches    26570    26571       +1     
==========================================
- Hits       100810    99912     -898     
- Misses       9843    11006    +1163     
+ Partials     1330     1088     -242     

Flag	Coverage Δ
backend	94.07% <ø> (-1.50%)	⬇️
control-plane	100.00% <100.00%> (+0.13%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
control-plane/src/settlement-backend-driver.ts	100.00% <100.00%> (+2.25%)	⬆️

... and 3 files with indirect coverage changes

[loopover-orb]

LoopOver approves — the gate is satisfied and CI is green.