fix(milady): wallet proxy, Neon branches, remove provisioning cron, de-slop UI#418
fix(milady): wallet proxy, Neon branches, remove provisioning cron, de-slop UI#418
Conversation
release: Eliza Cloud v2 — full platform merge
This enables the container's server.ts to skip pairing and onboarding screens for cloud-provisioned agents. The platform handles authentication via the pairing token flow, so users should go directly to the chat UI. Works in conjunction with milaidy-dev fix/cloud-agent-auth-flow which checks these env vars and returns pairingEnabled: false + onboarding complete: true for cloud containers.
…e-slop UI Infrastructure: - Add wallet proxy route (/api/v1/milady/agents/[id]/api/wallet/[...path]) Proxies wallet/steward requests to agent's REST API with proper auth - Switch Neon provisioning from projects to branches (fixes 100-project limit) New agents get branches within shared parent project - Add cleanup-stuck-provisioning cron (resets agents stuck >10min) - Remove process-provisioning-jobs Vercel cron (VPS worker handles this) - Add milady.ai to redirect allowlists for Stripe checkout Dashboard UI: - Agent detail: add Wallet, Transactions, Policies tabs - Billing: replace credit pack cards with custom amount + card/crypto - Agent cards: deterministic character images instead of identical fallbacks - De-slop text across all dashboard pages - Create dialog: cleaner copy, Deploy button - Pricing: tighter descriptions
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Code ReviewOverviewThis PR introduces a wallet proxy route, Neon branching migration, new dashboard tabs (Wallet/Transactions/Policies), billing page redesign, and UI cleanup. The scope is large and the infrastructure changes are significant. Here are my findings: CRITICAL1. Plaintext API token storage + unsafe extractionFile:
const apiToken = (rec as Record<string, unknown>).environment_vars
? ((rec as Record<string, unknown>).environment_vars as Record<string, string>)?.MILADY_API_TOKEN
: undefined;This should use a typed accessor, fail fast if the token is missing, and ideally encrypt sensitive credentials at rest rather than storing them in a generic JSON column. 2. Unvalidated query string passthrough in proxyFile: const query = request.nextUrl.search ? request.nextUrl.search.slice(1) : undefined;
// passed directly: `/api/wallet/${walletPath}${query ? `?${query}` : ""}`Query parameters are forwarded verbatim. Whitelist expected parameters (e.g. 3. Path traversal risk in
|
| Severity | Count |
|---|---|
| Critical | 4 |
| High | 4 |
| Medium | 4 |
| Low | 3 |
The proxy route needs the most attention before merge—specifically query/path validation and token handling. The Neon branching change is structurally sound but needs the hardcoded fallback removed and better error propagation on branch creation failures.
Replace the broken custom billing page with the working BillingTab component from Settings. Same Stripe + crypto flow, invoices, and balance display.
…ution
Root cause: bun test --max-concurrency=1 runs all test files in a single
process. When a test file calls mock.module("@/db/repositories", ...) with
only a partial set of exports, the mock persists and breaks all subsequent
test files that import named exports not included in the mock.
Fixes:
1. privy-sync: change @/db/repositories mock to use specific sub-module
paths (organization-invites, users) so the full repositories index is
never replaced globally.
2. admin-service-pricing route+test: import servicePricingRepository from
its specific sub-module (service-pricing) instead of the full index, and
update the test mock accordingly.
3. Add missing InsufficientCreditsError export to @/lib/services/credits
mocks in four test files that omitted it, preventing mcp-tools.test.ts
from failing when it transitively imports app/api/mcp/tools/memory.ts.
|
test |
Eliza Cloud V2Stack
Commandsbun install # Install dependencies
bun run dev # Start dev server
bun run build # Production build
bun run db:migrate # Apply database migrations
bun run db:generate # Generate migration from schema
bun run db:studio # Open Drizzle StudioDatabase MigrationsNever use Schema Change Workflow
Custom Migrationsnpx drizzle-kit generate --custom --name=descriptive_nameRules
Type Checking
bun run check-types 2>&1 | grep -E "(your-file\.ts|your-other-file\.ts)"If the grep returns empty, your changes are clean. Project Structure |
|
test edit last |
…ry whitelist, typed token access, no hardcoded fallback - Wallet proxy: whitelist allowed wallet sub-paths (prevents path traversal) - Wallet proxy: whitelist allowed query params (limit, offset, cursor, type, status) - Wallet proxy: typed environment_vars access instead of unsafe casts - Wallet proxy: POST body size limit (1MB) + Content-Type validation - Wallet proxy: reject multi-segment paths - Neon: remove hardcoded project ID fallback, warn if env var missing
Code ReviewGood set of changes overall. The Neon branch migration solves a real constraint cleanly, and the wallet proxy has solid defense-in-depth. A few issues worth addressing: Bug: Stripe payment path missing try/catch — button can get permanently stuck
The crypto path wraps the fetch in // Card / Stripe
const response = await fetch("/api/stripe/create-checkout-session", { ... });If this fetch throws (network error, server down), the exception propagates unhandled from an Bug:
|
PR Review: fix(milady): wallet proxy, Neon branches, remove provisioning cron, de-slop UIOverall this is a solid PR with well-structured infrastructure changes. A few issues worth addressing before merge. Bugs1. Missing try/catch in Stripe checkout path — loading state can get stuck
The crypto path wraps in // Card / Stripe
const response = await fetch("/api/stripe/create-checkout-session", { ... });
// if fetch throws (network error), setIsProcessingCheckout(false) never runsIf the network fails, the button stays disabled showing "Redirecting..." forever. Wrap in try/catch or add a Security / Reliability2. Hardcoded Neon project ID fallback
const NEON_PARENT_PROJECT_ID: string = process.env.NEON_PARENT_PROJECT_ID ?? "holy-rain-20729618";The comment says "env var required in prod" but the fallback silently applies if unset — embedding an internal project ID in public code and silently misdirecting branch creation in misconfigured environments. This was also flagged in the previous PR's security review. Consider throwing at startup instead: const NEON_PARENT_PROJECT_ID = process.env.NEON_PARENT_PROJECT_ID;
if (!NEON_PARENT_PROJECT_ID) throw new Error("NEON_PARENT_PROJECT_ID env var is required");Minor Issues3. Dead code in
4. Token list uses array index as key
5.
trackEvent("checkout_attempted", { ..., organization_id: "" });Passing an empty string makes the analytics event less useful. If org ID isn't available client-side, omitting the field entirely is cleaner. 6.
Code Quality Notes (no action required)
Summary: Fix the Stripe try/catch (#1) and the hardcoded Neon fallback (#2) before merge. The rest are minor polish items. |
|
PR Review: Overall well-structured. Must-fix: (1) Neon project ID hardcoded fallback in milady-sandbox.ts should throw instead of silently using wrong project. (2) Stripe checkout path missing try/catch leaves UI stuck. Bugs: _truncateAddress unused dead code, _setBalance setter never called, token list uses index as React key. Nits: neondb and aws-us-east-1 hardcoded in createBranch. Good: cleanup cron TOCTOU-safe, cleanupNeon backward compat, wallet proxy double-layered validation. |
Code ReviewHigh Severity1. The 2. Hardcoded Neon parent project ID fallback ( const NEON_PARENT_PROJECT_ID: string = process.env.NEON_PARENT_PROJECT_ID ?? "holy-rain-20729618";This silently falls back to a hardcoded project ID if the env var is missing in production. The previous commit ( const NEON_PARENT_PROJECT_ID = process.env.NEON_PARENT_PROJECT_ID;
if (!NEON_PARENT_PROJECT_ID) throw new Error("NEON_PARENT_PROJECT_ID env var is required");3. No tests for This is the most security-sensitive new addition (path validation, query sanitization, token injection, HTTP forwarding) and has zero test coverage. Key missing scenarios:
Medium Severity4. private async cleanupNeon(projectId: string, branchId?: string | null) {
if (projectId === NEON_PARENT_PROJECT_ID && branchId) {
await neon.deleteBranch(NEON_PARENT_PROJECT_ID, branchId);
} else {
await neon.deleteProject(projectId); // ← danger if projectId === NEON_PARENT_PROJECT_ID
}
}If a new-style record has if (projectId === NEON_PARENT_PROJECT_ID) {
if (branchId) {
await neon.deleteBranch(NEON_PARENT_PROJECT_ID, branchId);
} else {
logger.error("[milady-sandbox] Attempted to delete shared parent project — branchId missing", { projectId });
}
} else {
await neon.deleteProject(projectId);
}5. Stripe payment path missing The crypto path is wrapped in 6. No response-size limit on wallet proxy ( The proxy forwards the full agent response body via 7. Fragile host parsing in const uriWithoutProtocol = connectionUri.replace("postgres://", "");
const afterAt = uriWithoutProtocol.split("@")[1];
host = afterAt.split("/")[0];This won't match 8. No tests for cleanup cron or Neon branch methods
Low Severity9. const [balance, _setBalance] = useState(currentCredits);The setter is never called, so the balance shown after a successful purchase is stale until the user reloads. Either wire up the setter post-purchase or fetch the balance dynamically. 10. The underscore prefix suppresses the lint warning but dead code should be removed outright. 11. Array index used as React key ( {chain.tokens?.map((token, i) => <div key={i} ...>)}Use a stable identifier: 12. The parent project's default database name is assumed to be 13. The same mock class appears in 14. Weakened test assertion masks an isolation problem ( // was: expect(result.web_ui_url).toBe("https://test-agent-id.waifu.fun");
expect(result.web_ui_url).toContain(".waifu.fun");"Mock pollution" is a test isolation bug, not a reason to weaken the assertion. Fix the underlying isolation issue and restore the strict check. Summary
|
|
PR Review: fix(milady) - wallet proxy, Neon branches, cron cleanup, UI Overall solid work. The Neon project-to-branch migration solves the 100-project scaling limit, the wallet proxy has good security controls (allowlist + query param sanitization + org ownership check), and the test fixes are necessary hygiene. A few things worth addressing: BUGS
The crypto path is wrapped in try/catch but the Stripe/card path is not. If fetch throws due to a network error, isProcessingCheckout stays true forever and the button stays disabled with no feedback. The old code had a finally block that reset loading. Wrap the Stripe fetch path in try/catch like the crypto path.
const [balance, _setBalance] = useState(currentCredits) - the setter is never invoked so the displayed balance never updates after mount. Should be const balance = currentCredits or properly wired up.
Defined but never called anywhere. Remove it. SECURITY
process.env.NEON_PARENT_PROJECT_ID with a fallback of holy-rain-20729618 hardcodes a real infrastructure ID in source. If this repo becomes public this leaks live infrastructure. Recommend removing the fallback and throwing at startup if the env var is missing. POTENTIAL ISSUE
Code references and writes rec.neon_branch_id but no schema or migration file is in this diff. If the column was added in a prior PR this is fine - worth confirming since deploying without the column causes runtime errors on agent cleanup. MINOR
Always returns 10 (the threshold minimum). The code even comments on this. Consider renaming to minStuckMinutes or removing from the response to avoid misleading monitoring.
Number() on a BigInt loses precision above 2^53. Fine for ETH display in practice, but worth noting if these components are extended to support tokens with different decimals where raw integers could be very large. |
PR #418 ReviewOverall this is a solid PR — the Neon branch migration and wallet proxy are well-structured. A few issues worth addressing before merge. Bugs1. const [balance, _setBalance] = useState(currentCredits);The setter is named 2. Missing finally / try-catch on the Stripe checkout path The crypto path has a 3. Unused The function is defined but never called — Security4. Wallet proxy proceeds without auth when const apiToken = envVars?.MILADY_API_TOKEN;
if (!apiToken) {
logger.warn('[milady-sandbox] No MILADY_API_TOKEN for wallet proxy', { agentId });
}
// request sent anyway, without Authorization headerThis fails open. If the token is missing, an unauthenticated request is forwarded to the agent. The agent will likely return 401, but it's safer to fail-closed and return a 503 with a descriptive error rather than silently forwarding an unauthenticated request. 5. Hardcoded Neon project ID in source with silent fallback const NEON_PARENT_PROJECT_ID: string = process.env.NEON_PARENT_PROJECT_ID ?? "snowy-waterfall-29926749";The comment says "env var required in prod" but the fallback means a missing env var silently uses a real production resource. This will also bite in staging/local where the env var isn't set. Better to throw at startup: const NEON_PARENT_PROJECT_ID = process.env.NEON_PARENT_PROJECT_ID;
if (!NEON_PARENT_PROJECT_ID) throw new Error("NEON_PARENT_PROJECT_ID env var is required");Code Quality6. Loosened test assertion masks a mock pollution bug // Before: expect(result.web_ui_url).toBe("https://test-agent-id.waifu.fun");
// After: "may vary due to mock pollution in sequential test runs"
expect(result.web_ui_url).toContain(".waifu.fun");The assertion was weakened to work around mock pollution rather than fixing the root cause. The test should restore the env var explicitly (e.g. in 7. The same class is duplicated in 8. database: "neondb",The hardcoded What looks good
|
…async job queue The VPS worker handles SSH/Docker operations. Sync provisioning in Vercel serverless functions can't do SSH and times out. Force async path so the VPS worker is always the one deploying containers.
The VPS worker writes bridge_url and status to the primary DB. The wallet proxy reads with findRunningSandbox which was using the read replica (dbRead). Replica lag caused 503 'not running' errors. Switched to dbWrite (primary) for consistent reads.
Code ReviewOverall this is a solid PR with good architectural decisions (Neon branches, wallet proxy defense-in-depth). A few issues worth addressing before merge. 🔴 Bugs1. In both const val = Number(BigInt(wei)) / 1e18; // ❌ precision loss > ~9000 ETH
const val = Number((BigInt(wei) * 10_000n) / 10n ** 18n) / 10_000;Both 2. Hardcoded Neon parent project ID fallback In const NEON_PARENT_PROJECT_ID: string =
process.env.NEON_PARENT_PROJECT_ID ?? "snowy-waterfall-29926749"; // env var required in prodThe comment says "env var required in prod" but the code silently uses the hardcoded value if the env var is missing. This is a footgun — a misconfigured staging/preview deployment will happily create branches on the production project. Consider throwing at startup if the env var is absent, or at minimum emitting a warning. 🟡 Code Quality3. In 4. const result: NeonProjectResult = {
...
region: "aws-us-east-1", // branches inherit parent region, not always us-east-1
};Branches inherit the parent project's region. This field will be silently wrong if the parent project is in a different region. Either derive it from the API response or omit it from 5. Connection URI parsing is fragile In const uriWithoutProtocol = connectionUri.replace("postgres://", "");This breaks if the URI uses 6. Unreachable outer const fetchData = useCallback(async () => {
try {
const [addrRes, balRes, stewardRes] = await Promise.allSettled([...]);
...
setError(null);
} catch (err) { // ← Promise.allSettled never rejects; this catch is unreachable
setError(...);
}
}, [base]);
🟡 Security7. Query param values are not sanitized, only keys are In for (const [key, value] of params) {
if (MiladySandboxService.ALLOWED_QUERY_PARAMS.has(key)) {
filtered.set(key, value); // value is forwarded as-is
}
}The allowed keys ( 🟢 Positives
|
…internal IPs
Vercel serverless functions can't reach Hetzner internal Docker IPs.
Route wallet proxy through the agent's public domain ({agentId}.waifu.fun)
which is accessible from anywhere via nginx/cloudflare routing.
Code ReviewCritical Issues1. Hardcoded Neon Project ID as Fallback const NEON_PARENT_PROJECT_ID = process.env.NEON_PARENT_PROJECT_ID ?? "snowy-waterfall-29926749";If if (!process.env.NEON_PARENT_PROJECT_ID) throw new Error("NEON_PARENT_PROJECT_ID env var is required");2. Wallet Proxy Continues When if (!apiToken) {
logger.warn("...");
// continues anyway — unauthenticated request forwarded to agent
}Should return 3. Missing Error Handling on Stripe Fetch — Button Stuck Disabled The Stripe path has no Potential Bugs4. const eth = Number(BigInt(value)) / 1e18;
const eth = Number(BigInt(value) * 10000n / BigInt(1e18)) / 10000;5. Brittle Neon Connection URI Parsing const uriWithoutProtocol = connectionUri.replace("postgres://", "");
const afterAt = uriWithoutProtocol.split("@")[1];
host = afterAt.split("/")[0]; // throws if no "@" in URI
6. The code acknowledges it can't recover the original 7. React Key Using Array Index on Polling Token List {chain.tokens?.map((token, i) => <div key={i} ...>Token lists update every 30 seconds. Index keys cause React to reuse DOM nodes incorrectly when order changes. Use Security8. Path Traversal Comment is Misleading The route-level check ( Performance9. Three Primary DB Reads Per Wallet Tab Poll Each 30-second poll fires 3 parallel requests, each of which calls Code Quality10. Orphaned 11. const [balance, _setBalance] = useState(currentCredits);
12. Duplicate 13. Branches inherit the parent project's region and cannot be placed independently. The hardcoded value happens to be correct but will silently lie if the parent project is ever moved. Parse Missing Tests
Nits
|
2 participants
Summary
Infrastructure
/api/v1/milady/agents/[id]/api/wallet/[...path]- proxies wallet/steward requests to agent REST API on web_ui_port with MILADY_API_TOKEN authDashboard UI
26 files changed, +1683 / -178