fix(keeper): persist budget circuit-breaker halt across restarts

[Morenikeoa]

Problem

KeeperBudget's latched halt (_haltKind/_haltReason) is purely in-memory, with zero persistence — confirmed: nothing in budget.ts touches Redis, Supabase, or disk.

The class's own design intent (file header) is that day-cap breaches and operator halts "never auto-recover" and require an explicit POST /admin/budget/resume. But a crash, OOM, or orchestrator-driven restart re-initializes KeeperBudget with _haltKind = undefined, fully un-halting the breaker with no record that it was ever tripped and no re-validation that the underlying condition (a runaway spend bug, a stuck retry loop draining the wallet) is resolved.

Production Impact

An auto-restarting orchestrator (Railway/Docker/k8s) silently resumes spending into the same unresolved incident that caused the original breach, with operators seeing no alert that the breaker reset. A day-cap halt exists specifically to stop wallet drainage during an active incident — this gap defeats that purpose across any restart.

Fix

Added opt-in halt-state persistence via a new haltStatePath dependency on KeeperBudget:

• On _halt(), the kind/reason are written to a local JSON file.
• At construction, if the file exists, the halt is restored immediately (canSpend() returns false before any sends happen) and onHalt is re-fired so paging/metrics react exactly as if the halt had just occurred.
• On resume(), the file is removed.

haltStatePath is undefined by default — every existing KeeperBudget consumer and test is completely unaffected unless it opts in explicitly. The production singleton (sharedBudget in src/lib/keeper-send.ts) opts in via KEEPER_BUDGET_HALT_STATE_PATH (default /tmp/keeper-budget-halt.json).

Scope note: this covers process-level restarts that reuse the same filesystem — the common automatic-restart case (uncaught exception, orchestrator-driven OOM-kill-then-restart). A full container recreate on an ephemeral filesystem is not covered; operators wanting that level of durability can point the env var at a mounted persistent volume.

Proof of Fix

New tests in tests/lib/budget.test.ts (KeeperBudget — BUG-106: halt-state persistence):

• No filesystem activity when haltStatePath is unset (confirms zero behavior change for existing consumers)
• A halt persisted by one instance is restored by a fresh instance constructed with the same path, re-firing onHalt
• resume() clears the persisted file — a later restart starts clean
• An unwritten path starts clean
• A malformed/corrupt persisted file is handled gracefully (logs, does not throw, starts clean)

Test Output

 Test Files  1 passed (1)
      Tests  62 passed (62)

Also ran the dependent suites that construct KeeperBudget/sharedBudget (tests/lib/keeper-send.test.ts, tests/services/crank.test.ts, tests/services/liquidation.test.ts): 98 passed, 9 skipped, zero regressions.

pnpm build — clean, zero errors.

[coderabbitai]

Warning

Review limit reached

@Morenikeoa, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 47 minutes and 54 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 79b31b9f-8dcb-4451-9eed-e2d5ee9dc6ae

📥 Commits

Reviewing files that changed from the base of the PR and between 8ee810d and 5c1a751.

📒 Files selected for processing (3)

• src/lib/budget.ts
• src/lib/keeper-send.ts
• tests/lib/budget.test.ts

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[dcccrypto]

Independent verification — not an approval (QA/Security own that), just evidence for whoever reviews. Verdict: genuine, and the persistence loop is tested in both directions.

Method: ran the PR's own tests first (62 passed), then neutralised each half of the loop independently rather than both at once, so each is shown to bind on its own.

A — restore on construction removed:

× persists a halt to disk and restores it in a fresh instance constructed with the same path
1 failed | 61 passed

B — clear-on-resume removed (fs.unlinkSync):

× clears the persisted file on resume — a later restart starts clean
1 failed | 61 passed

Both halves bind, and each fails only its own test — no overlap, which is what you want from a two-directional feature.

Why B is the one I went looking for

The obvious failure mode of "persist the halt" isn't the persisting — it's forgetting to un-persist. A resume() that clears in-memory state but leaves the file behind gives you a keeper that comes back halted after every restart, forever, with an operator who has already "resumed" it and no idea why. That's a self-inflicted permanent outage dressed as a safety feature.

It's covered, and the test name says exactly what it protects. Good.

Details worth calling out

• _restoreHaltState fails open on a missing/corrupt file — catch { return; } and a !parsed.kind || !parsed.reason guard, so a truncated or hand-edited file starts clean rather than crashing the keeper at boot. Right call: an unreadable halt file shouldn't be a boot loop.
• _clearHaltStateFile swallows unlink errors with a comment explaining why it's safe (next halt overwrites; missing file reads as no-halt). Deliberate, and correct — worth having the reasoning in the code rather than a bare catch {}.
• The restore path logs at error level and tells the operator not to resume() before investigating the original cause. A restored halt is exactly the situation where someone reflexively clears the alarm; saying so in the log is a good touch.
• haltStatePath is opt-in (undefined → no filesystem access), and there's a test pinning that it doesn't touch disk when unset. That keeps existing deployments and tests unaffected.

No changes requested from me. For contrast with the other keeper PRs I've looked at recently — #363, #357 and #359 all changed security-relevant behaviour with no new tests — this one ships the coverage with the change, and it's the shape I'd point at as the standard.