feat: PR 393 review fixes and stabilization

[lalalune]

Summary

• Fix Privy JWT algorithm mismatch with jose 6.x
• Headscale API compatibility + admin containers tab null safety
• Sanitize container data to prevent React render errors
• Stabilize unit test isolation
• Format fixes for biome

🤖 Generated with Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
eliza-cloud-v2	Ready	Preview, Comment	Mar 17, 2026 0:16am

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 33d1cd72-a3a1-4192-a3bb-ec0adfb884a4

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch shaw/pr393-review-fixes

📝 Coding Plan

• Generate coding plan for human review comments

---

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 and usage tips.}

[claude]

PR #394 Review — feat: PR 393 review fixes and stabilization

Overall assessment: This is a solid, well-structured PR. The billing infrastructure is production-ready with good atomicity guarantees, and the UI fixes address real crash paths. A few specific concerns are worth addressing before a future billing-critical deploy.

---

Milady Billing Cron (app/api/cron/milady-billing/route.ts)

Strengths:

• Correct use of timingSafeEqual for HMAC comparison — avoids timing side-channels.
• The two-step transaction (claim sandbox row via conditional UPDATE, then atomic credit deduction) is the right pattern for preventing duplicate billing across overlapping cron runs.
• REBILL_GUARD_MINUTES = 55 is a sensible guard for an hourly job.
• Error classes (AlreadyBilledRecentlyError, InsufficientCreditsDuringBillingError) make control flow clean and explicit.

Issues / Concerns:

1. Sequential processing at scale. Sandboxes are processed with a simple for...of loop. With a growing number of agents this will eventually breach the 5-minute maxDuration. The existing pLimit utility introduced in admin-infrastructure.ts in this same PR should be applied here too — a concurrency of ~20 parallel billing tasks would keep wall-clock time predictable.

2. In-memory balance update is stale between multi-sandbox orgs.

   org.credit_balance = String(result.newBalance);

   This updates the snapshot used for the next sandbox in the same org, but newBalance comes from the transaction's RETURNING clause — which is correct. However if two sandboxes in the same org both clear the rebill guard simultaneously, both could read the same snapshot before either transaction commits. The atomic SQL guard (gte(organizations.credit_balance, hourlyCost)) already prevents a negative balance, but the shutdown-warning path can fire for both sandboxes even if only one would actually exhaust the balance. This is a UX-only issue (spurious warning email), not a financial one, but worth noting.

3. billing_status field on stopped-with-backups sandboxes. The query for idle sandboxes filters on inArray(miladySandboxes.billing_status, ["active", "warning", "shutdown_pending"]) but a stopped agent that gets suspended has billing_status = "suspended". The suspend transition happens by setting status = "stopped". If a suspended agent still has last_backup_at set, it would not be re-billed (correct), but if the billing cron runs before the status update is visible on the read replica, a brief window exists where it could bill a just-suspended agent. The existing isNotNull(miladySandboxes.last_backup_at) guard helps, but explicit exclusion of billing_status = "suspended" in the query would be belt-and-suspenders.

4. total_billed precision mismatch. Schema: numeric(10,2) (2 decimal places). The cron accumulates IDLE_HOURLY_RATE = 0.0025, which has 4 decimal places. The SQL + will silently truncate the fractional cents in total_billed. This doesn't affect billing correctness (billing uses credit_balance, not total_billed), but total_billed will under-report idle charges. Either change the column to numeric(12,4) or document that total_billed is rounded to cents.

5. Missing exempt status handling. MiladyBillingStatus includes "exempt" but the cron's billing queries use inArray(..., ["active", "warning", "shutdown_pending"]). Exempt sandboxes are correctly excluded from billing — this is fine — but there's no comment explaining the intent. A brief inline comment would prevent future engineers from treating this as a bug.

6. getOrgUserEmail returns the first user regardless of role. If an org has multiple members, this may email the wrong person. The existing billing_email field on organization_billing is already the preferred path; getOrgUserEmail is a fallback that probably deserves a TODO or a filter on owner/admin role.

---

Database Migration (0052_add_milady_billing_columns.sql)

Follows the rules in CLAUDE.md correctly:

• Uses ADD COLUMN IF NOT EXISTS.
• CREATE INDEX IF NOT EXISTS (not CONCURRENTLY — correct for a transaction-wrapped migration).
• Targeted migration, under 100 lines.

No issues.

---

Auth Fix (packages/lib/auth/jwks.ts)

// Before
cachedPublicKey = await importSPKI(pem, ALGORITHM);
// After
cachedPublicKey = await importSPKI(pem, ALGORITHM, { extractable: true });

And the CryptoKey → KeyLike type alias change to match jose 6.x API. Both changes are correct. The { extractable: true } option is required for jose 6.x to use the key in jwtVerify. Good fix.

---

JWT Session Fix (packages/lib/services/eliza-app/session-service.ts)

// Before
.setExpirationTime(expiresAt)   // Date object — broken in jose 6.x
// After
.setExpirationTime(Math.floor(expiresAt.getTime() / 1000))  // Unix timestamp — correct

Correct. jose 6.x setExpirationTime accepts a Unix epoch number or a relative string like "2h", not a Date object.

The jwtVerify generic removal and manual cast to ElizaAppSessionPayload is a pragmatic workaround for the jose 6.x type narrowing changes — acceptable given the existing check-types situation.

---

Milady Credit Gate (packages/lib/services/milady-billing-gate.ts)

Strengths:

• Fails closed on DB errors (denies provisioning if balance can't be verified) — correct.
• Consistent 402 response on all three provisioning paths (create, provision, resume).

Minor concern: The gate reads organizationsRepository.findById() which uses the read replica (dbRead). If there's replication lag after a deposit (Stripe webhook → credit update), a user who just topped up might be blocked for a few seconds. This is an edge case but worth a comment in the code.

---

Headscale API Compatibility (app/api/v1/admin/headscale/route.ts)

The fallback from /api/v1/node (v0.23+) to /api/v1/machine (legacy) is correct. The error handling wrap for AuthenticationError / ForbiddenError is a needed fix from PR #393.

Minor: The nodes response shape in the type annotation has machineKey? and nodeKey? as optional, while the legacy machines shape has machineKey as required. This reflects real API differences but means downstream code that normalizes both shapes should handle the optionality explicitly. Current downstream code passes both through machines variable without further normalization, which is fine since the UI only uses name, ipAddresses, online, and user.

---

Admin Infrastructure Service (packages/lib/services/admin-infrastructure.ts)

Strengths:

• Custom pLimit implementation is clean and correct for this use case.
• withTimeout wrapper prevents SSH hang from blocking the entire snapshot.
• SNAPSHOT_CACHE_TTL_MS = 30_000 avoids hammering nodes on rapid admin page refreshes.

Concern: SNAPSHOT_CACHE_TTL_MS is a module-level in-memory cache. In Vercel Serverless, each function invocation gets a fresh process (cold start) or a recycled one (warm). The cache will be useful for warm instances but invisible across concurrent invocations or cold starts. This is acceptable behavior but means the cache provides less benefit than it appears — a Redis-backed cache would be more reliable if SSH load on nodes becomes a concern.

---

UI: create-milady-sandbox-dialog.tsx

The refactor to an inline provisioning progress view (replacing router.refresh() with a polling hook + step indicator) is a significant UX improvement. The implementation is clean.

One concern: onCreated?.()?.catch(() => {}) silently swallows any error from the parent refresh callback. If onCreated throws (e.g., a failed fetch), the user won't know. The comment "Best-effort refresh" is appropriate, but consider at least logging the error.

---

UI: user-menu.tsx

The error boundary + safe* helper refactor is the right approach for a component that accesses deeply nested Privy user objects. The safeGetInitials function has one subtle edge case:

const parts = name.trim().split(/\s+/).filter(Boolean);
if (parts.length > 0 && parts[0].length > 0) {
  return parts.map((n) => n[0]).join("").toUpperCase().slice(0, 2);
}

If name is a single character (e.g., a wallet prefix like "0"), this returns "0" — which is fine. No issue.

---

Test Coverage (packages/tests/unit/milady-billing-route.test.ts)

Four focused tests covering the main billing paths:

• Idempotency guard (already billed → skip)
• Insufficient credits → warning sent
• Successful billing → rate persisted
• Post-billing warning threshold

Good coverage of the happy path and the two main failure modes. Missing test cases worth adding in follow-up:

• Shutdown execution (when billing_status = "shutdown_pending" and scheduled_shutdown_at <= now)
• Idle sandbox billing (stopped + last_backup_at not null)
• No billable sandboxes (empty state)
• Unauthorized request (missing/wrong CRON_SECRET)

---

Minor/Nit

• formatRelative and formatRelativeShort in sandbox-status.ts are nearly identical (differ only in the "Never" vs "—" fallback and whether days use toLocaleDateString). These could be unified with a { short: boolean } option to avoid divergence.
• docs/BILLING_AUDIT.md and docs/ONBOARDING_INTEGRATION_PLAN.md are large doc additions (+~1400 lines combined). These are useful but aren't gated by any CI — consider adding a lint rule or at least a CODEOWNERS entry for docs/ so billing-critical documentation gets reviewed carefully.
• bun.lock is updated (Bun 1.3.9 bump) — this is expected and fine.

---

Summary of blocking concerns:

1. Sequential sandbox processing will hit the 5-minute timeout at scale — add concurrency.
2. total_billed column precision doesn't match IDLE_HOURLY_RATE — fix schema or document.

Everything else is non-blocking. The billing atomicity, migration hygiene, auth fixes, and test coverage are all solid.

🤖 Generated with Claude Code