feat: dashboard improvements — admin infra, table reactivity, page separation#393
feat: dashboard improvements — admin infra, table reactivity, page separation#393
Conversation
- New useSandboxStatusPoll hook: polls individual agent status every 5s during pending/provisioning, auto-stops on terminal states - New useSandboxListPoll hook: polls agent list every 10s while any sandbox is active, fires toast on running transition - Create dialog stays open during provisioning with 4-step progress stepper (created → database → container → running), elapsed timer, retry button on error, and Open Web UI button on completion - Status cell in table now shows pulse animation for active states, scale-in animation for running transitions, shake for errors - Added shake and scaleIn keyframe animations to globals.css
- New admin infrastructure API endpoint (app/api/v1/admin/infrastructure/route.ts) - Admin infrastructure service with SSH node inspection and container health classification - Unit tests for container health classification (5 tests, all passing) - Classifies containers as healthy, failed, missing, warming, or stale based on runtime state, control plane records, and heartbeat freshness
- Major rewrite of milady-sandboxes-table: local state management, optimistic updates, no more stale data after start/stop/destroy actions - Added onDataRefresh callback to useSandboxListPoll hook for live table updates - Create dialog improvements with onCreated callback to trigger table refresh - Eliminates the stale-data-after-action problem that required manual page reload
- Containers page now shows only custom Docker containers - Milady page is now a dedicated managed agents view - New milady-page-wrapper component for consistent page header styling - Clear separation of concerns between managed agents and raw containers
- Redesigned agent detail page layout and UX - Cleaner information hierarchy and visual presentation
|
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)
📝 Coding Plan
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 |
0xSolace
force-pushed
the
sol/dashboard-improvements-20260316
branch
from
8af6928 to
2432739
Compare
Code ReviewOverall this is a solid, well-structured PR. The optimistic update pattern, page separation, and agent detail redesign are all improvements. Here are the issues worth addressing before merge. 🔴 High — Auth errors return 500 instead of 401/403
export async function GET(request: NextRequest) {
// requireAdmin can throw — not wrapped in try/catch
const { role } = await requireAdmin(request);
if (role !== "super_admin") { ... 403 ... }
try {
// only THIS block is protected
const snapshot = await getAdminInfrastructureSnapshot();
...
} catch (error) { ... 500 ... }
}Fix: wrap 🔴 High — Unbounded parallel SSH connections; function timeout risk
Recommend adding a concurrency limiter (e.g. 🟠 Medium —
|
PR Review — #393: Dashboard improvementsOverall this is a well-structured PR with good commit separation and test coverage for the happy path. Below are issues found across security, correctness, performance, and code quality. 🔴 HIGH — Auth error outside try/catch
const { role } = await requireAdmin(request); // outside try/catch
if (role !== "super_admin") { ... }
try {
const snapshot = await getAdminInfrastructureSnapshot();
...
} catch (error) { ... }If 🔴 HIGH — No overall timeout on infrastructure snapshot → Vercel 504
🟡 MEDIUM — SSH coordinates in API response
The full node shape including 🟡 MEDIUM —
|
| Priority | Issue |
|---|---|
| 🔴 HIGH | Auth error outside try/catch in new route |
| 🔴 HIGH | No overall timeout → Vercel 504 on SSH hang |
| 🟡 MEDIUM | SSH coordinates (host/port/user) exposed in API response |
| 🟡 MEDIUM | useSandboxListPoll missing initial poll |
| 🟡 MEDIUM | mergeApiData drops agents absent from API response |
| 🟡 MEDIUM | previousStatusesRef overwritten after poll update |
| 🟡 MEDIUM | Polling never stops on 404/401 responses |
| 🟡 MEDIUM | No caching on infrastructure snapshot |
| 🟡 MEDIUM | Raw SSH errors in API response |
| 🟠 LOW | Optimistic revert not synchronous |
| 🟠 LOW | buildResourceAlert loses severity |
| 🟠 LOW | docker ps prefix match instead of exact |
| 🟠 LOW | Double router.refresh() on dialog close |
| 🟠 LOW | Duplicate status color maps |
| 📝 | Test coverage for parsing, ghost detection, polling |
The two HIGH items (auth error escaping the try/catch and the missing overall SSH timeout) should be addressed before merge.
Reviewed by Claude Sonnet 4.6
Collapse multi-line <p> tag to single line per biome rules.
Two issues fixed: 1. Fire-and-forget calls in AgentBudgetService.deductBudget() for sendLowBudgetAlert() and triggerAutoRefill() used bare 'void' without .catch(), causing unhandled promise rejections that crashed the test runner. 2. Email template loadTemplate() used process.cwd()-relative path (lib/email/templates/) but templates live under packages/lib/. Switched to import.meta.dir-relative resolution for reliability. These bugs are pre-existing on dev branch.
- Fix unused variable warnings by prefixing with underscore - Fix import ordering and formatting in route.ts
Code ReviewOverall this is a well-structured PR with a clear scope. The optimistic updates and local state management are a real improvement, and the provisioning progress dialog is a nice UX addition. A few issues worth addressing before merge: 🔴 Bugs1. Double refresh on dialog close (
function handleClose() {
setOpen(false);
setTimeout(resetForm, 300);
void onCreated?.(); // triggers refreshData() in parent
router.refresh(); // also refreshes
}
2. The hook only starts the interval; it never runs a poll immediately when it mounts. A user who creates an agent and closes the dialog waits 10 seconds before seeing any live data. Add an initial // immediately after setting isPolling(true):
void poll();
intervalRef.current = setInterval(() => void poll(), intervalMs);3. The old // new — will throw in environments without the milady_sandboxes table
const sandboxes = await miladySandboxService.listAgents(user.organization_id);Restore the try/catch (or the 🟡 Performance / Security4. No caching on the admin infrastructure endpoint Every Three unused constants already hint at the intended solution that wasn't shipped: const _SNAPSHOT_CACHE_TTL_MS = 30_000; // unused
const _MAX_CONCURRENT_SSH_SESSIONS = 5; // unused
const _NODE_INSPECTION_TIMEOUT_MS = 25_000; // unusedAt minimum, wrap 5. const newIds = initialSandboxes.map((sb) => sb.id).join(",");If the server returns the same set of IDs in a different order (e.g. after a sort change), this won't detect the equality and will reset local state unnecessarily. Use a Set comparison or sort before joining. 🔵 Code Quality6. Status color maps duplicated across files
7.
onCreated?.()?.catch((err) => logger.error("onCreated callback failed", { error: String(err) }));8. Using ✅ Good Catches
|
- admin-infrastructure: add concurrency limiter (max 5 parallel SSH sessions), per-node inspection timeout (25s), and 30s in-memory snapshot cache - admin-infrastructure: differentiate warning vs critical in buildResourceAlert - admin-infrastructure: make parseDockerHealth more robust (strip prefix, exact match) - admin-infrastructure: remove unused 'unknown' ContainerLiveHealthStatus variant - use-sandbox-status-poll: stop polling on persistent 4xx errors or 5 consecutive failures - use-sandbox-status-poll: fire initial poll immediately in useSandboxListPoll - use-sandbox-status-poll: fix previousStatusesRef to preserve poll-derived statuses - milady-sandboxes-table: union merge in mergeApiData to preserve optimistic additions - create-milady-sandbox-dialog: deduplicate triple data refresh to single onCreated path
|
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 |
Code ReviewOverall this is a solid PR: the optimistic-update table, provisioning progress dialog, and page separation are all clear improvements. The admin infrastructure service is well-structured. A few issues worth addressing before merge. Bugs 1. Underscore-prefixed constants used without underscore in admin-infrastructure.ts Three constants are declared with a backtick _ backtick prefix but referenced without it inside getAdminInfrastructureSnapshot, which will throw ReferenceError at runtime. Declared: _NODE_INSPECTION_TIMEOUT_MS, _MAX_CONCURRENT_SSH_SESSIONS, _SNAPSHOT_CACHE_TTL_MS Referenced as: NODE_INSPECTION_TIMEOUT_MS, MAX_CONCURRENT_SSH_SESSIONS, SNAPSHOT_CACHE_TTL_MS The unit tests only cover classifyContainerHealth and do not call getAdminInfrastructureSnapshot, so they do not catch this. Drop the _ prefix from the declarations. Reliability 2. Module-level snapshotCache is unreliable on Vercel let snapshotCache: { data: AdminInfrastructureSnapshot; expiresAt: number } | null = null; Vercel serverless functions do not guarantee module reuse across invocations. The cache may never be hit in practice, and concurrent cold starts will each independently fire all SSH connections. Consider moving caching to Redis/KV, or treating the endpoint as always-fresh and documenting the expected latency (~10-25 s per node). 3. onCreated errors are silently swallowed in the dialog In handleClose, void onCreated?.() silently loses any rejection. If onCreated rejects, the table will not refresh and there is no user feedback. At minimum attach a .catch to log the error. Code Quality 4. Fragile camelCase / snake_case conversion in mergeApiData The merge function manually maps agentName -> agent_name, lastHeartbeatAt -> last_heartbeat_at, etc. SandboxListAgent even exposes both spellings (agentName and agent_name), indicating the API shape is inconsistent. A single canonical casing on the API response would eliminate this brittle mapping layer. 5. refreshData fetches an unbounded list fetch('/api/v1/milady/agents') — as orgs accumulate agents this returns everything on every poll tick and on every action. Worth confirming a default page limit is enforced server-side, or passing one in the query string. 6. Inline pLimit re-implements a maintained package The 18-line implementation is correct. For a file already at 959 lines, the p-limit package or a shared utility would be easier to audit. Minor note. Test Coverage The 5 tests for classifyContainerHealth are well-written. Untested code paths: inspectNodeRuntime and its parsing helpers (pure functions, easy to unit test), getAdminInfrastructureSnapshot, the admin infrastructure API route (auth enforcement, super_admin check), and the two polling hooks. What's Good
Summary: Item 1 (underscore constant mismatch) is a runtime crash on the first admin infrastructure page load — needs to be fixed before merge. Item 2 (serverless cache) is worth at least a code comment. Everything else is a minor quality note. |
- Add MILADY_PRICING constants (/bin/bash.02/hr running, /bin/bash.0025/hr idle, minimum deposit) - Add billing columns to milady_sandboxes schema (billing_status, last_billed_at, hourly_rate, total_billed, shutdown/warning timestamps) - Create SQL migration 0052_add_milady_billing_columns - Add credit gate (checkMiladyCreditGate) enforcing minimum balance before agent create/provision/resume - Create hourly milady-billing cron mirroring container-billing pattern: - Bills running agents at /bin/bash.02/hr - Bills stopped agents with snapshots at /bin/bash.0025/hr - Sends 48h shutdown warnings on insufficient credits - Auto-suspends agents after grace period - Add PostHog event types for milady billing analytics - Register cron in vercel.json (hourly at minute 0)
Previous CI fix incorrectly prefixed NODE_INSPECTION_TIMEOUT_MS, MAX_CONCURRENT_SSH_SESSIONS, and SNAPSHOT_CACHE_TTL_MS with underscores but the code still references the unprefixed names.
import.meta.dir is not available in this TypeScript configuration. Use fileURLToPath(import.meta.url) + path.dirname() instead.
- Remove duplicate local formatRelative function in milady-sandboxes-table - Fix PostHog EventProperties type to accept Record<string, unknown> - Extract sandbox status constants to shared module
- Add ambient declaration for @elizaos/plugin-sql/node (createDatabaseAdapter) - Add ambient declaration for bs58 module - Add ambient declaration for monaco-editor with IStandaloneCodeEditor - Fix webkitAudioContext type assertion in audio.ts
- Add ./packages/types/**/*.d.ts to include paths so ambient declarations are picked up by the split type-checker - Exclude **/*.test.ts and **/*.test.tsx to avoid vitest import errors in colocated test files
The v1 milady agent create and provision routes now call
checkMiladyCreditGate before proceeding. Tests that exercise these
routes need to mock the billing gate to return { allowed: true }.
Code ReviewBugs / Issues🔴 Critical — Billing cron: stale org balance for multi-sandbox orgs The cron fetches each org's 🟡 Medium — The billing query includes sandboxes with 🟡 Medium — total_billed: String(Number(sandbox.total_billed) + hourlyCost),
🟡 Low — If the user opens the dialog and immediately dismisses it (before submitting), Security🟡 Medium — Admin infrastructure endpoint serializes sensitive topology The API response includes 🟡 Low — return (
providedBuffer.length === secretBuffer.length && timingSafeEqual(providedBuffer, secretBuffer)
);The early-exit on length comparison is a minor timing oracle — it reveals whether the submitted secret has the correct length. Pad both buffers to the same length before calling 🟡 Low — Adding Performance🟡 Medium — Serial SSH inspection doesn't scale
🟡 Low — Unbounded sandbox query in admin service The query fetches all sandbox rows with no 🟡 Low — In-memory cache is ineffective in Vercel serverless let snapshotCache: { data: AdminInfrastructureSnapshot; expiresAt: number } | null = null;Module-level cache has a fresh context on every cold start in Vercel serverless. The 30-second TTL will never be hit in production — every request will be a cache miss. Either use a shared cache (Redis/KV) or remove the misleading cache mechanism. Code Quality
export interface SandboxListAgent {
agentName?: string;
agent_name?: string;
// ...
}Supporting both camelCase and snake_case for the same field suggests an inconsistent API response format. Standardize the API response and simplify the interface. Custom A 15-line custom concurrency limiter was written inline.
On retry after provisioning error, only Positives
SummaryThe overall structure is solid and follows established patterns. Two issues should be resolved before merge:
The sensitive SSH data exposure in the admin API is worth a follow-up discussion on what the dashboard actually renders vs. what it exposes in the response body. |
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 |
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
Project Structure |
This comment was marked as spam.
This comment was marked as spam.
|
PLACEHOLDER_REVIEW_CONTENT |
This comment was marked as spam.
This comment was marked as spam.
PR Review — feat: dashboard improvements (admin infra, table reactivity, page separation)Overall this is a solid PR with good test coverage for the new infrastructure service. The billing cron, credit gate, and table reactivity work are all well-structured. A few issues worth addressing before merge: 🔴 Critical — Billing double-charge riskFile: The // No last_billed_at guard here
.where(
and(
eq(miladySandboxes.status, "running"),
inArray(miladySandboxes.billing_status, ["active", "warning", "shutdown_pending"]),
),
)Fix: Add an idempotency check to the query: or(
isNull(miladySandboxes.last_billed_at),
lt(miladySandboxes.last_billed_at, sql`now() - interval '55 minutes'`),
)55 minutes gives a safe margin for a cron scheduled at 🟡 Medium — Stale org balance allows negative credit balancesFile: // Update org balance in memory for next sandbox in same org
org.credit_balance = String(result.newBalance);The This is hard to fully eliminate without per-org advisory locks, but at minimum the atomic 🟡 Medium — EventProperties type safety regressionFile: | Record<string, unknown>; // "escape hatch for ad-hoc properties"Adding 🟡 Medium — Internal Headscale URL exposed in API responseFile: serverUrl: HEADSCALE_API_URL,This exposes the internal VPN management server URL in admin API responses. Even though it's super-admin only, it unnecessarily leaks infrastructure topology. If this is needed for the dashboard display, it's fine — just confirm it's intentional. 🟢 Low — HMAC key for cron auth is unnecessarily complexFile: const hmacKey = Buffer.from("milady-billing-cron"); // hardcoded key
const a = createHmac("sha256", hmacKey).update(providedSecret).digest();
const b = createHmac("sha256", hmacKey).update(cronSecret).digest();
return timingSafeEqual(a, b);This is functionally secure (timing-safe, no length leak), but the HMAC here is being used as a fixed-key hash just to get constant-length outputs for 🟢 Low —
|
PR Review test via stdin heredoc |
|
Test single line - no special chars |
|
PR Review Part 1 of 4 - CRITICAL ISSUES: (1) Billing cron has no idempotency guard - double-billing risk. File: app/api/cron/milady-billing/route.ts. The query selecting billable sandboxes has no last_billed_at filter. Vercel retries cron jobs on failure, and the endpoint can be triggered manually. If it fires twice within the same hour, every billable org gets charged twice. Fix: add a time guard to WHERE clause using isNull(last_billed_at) OR lt(last_billed_at, now() minus 59 minutes). (2) Credit balance can go negative for multi-sandbox orgs. File: processSandboxBilling(). The pre-check currentBalance < hourlyCost happens outside the transaction with no floor guard. For an org with N sandboxes, if balance covers only one charge but they have two running sandboxes, both pass the pre-check with the same stale in-memory balance, then both transactions execute pushing balance negative. Fix: add gte(organizations.credit_balance, String(hourlyCost)) to the WHERE clause and treat updatedOrg === undefined as a failed pre-check. Items 1 and 2 are real money bugs - fix before shipping to production. |
|
PR Review Part 2 of 4 - MODERATE ISSUES: (3) warning billing status is dead code. MiladyBillingStatus includes warning and the billing query filters for it, but the billing code never transitions a sandbox to warning. The path is active to shutdown_pending directly. Either remove warning from the type and query, or document when a sandbox enters that state. (4) Non-deterministic warning email recipient. getOrgUserEmail() uses users[0].email but listByOrganization returns users in unspecified order. For multi-member orgs the shutdown warning goes to a random member. This fallback should order by created_at ASC to consistently reach the org owner. (5) No type-check verification mentioned. The PR description notes biome check passes but not bun run check-types. Worth confirming before merge given the new numeric column types and satisfies assertions. |
|
PR Review Part 3 of 4 - MINOR OBSERVATIONS: (6) hourly_rate DB column diverges from runtime computation. The schema adds an hourly_rate column but processSandboxBilling always computes the rate from constants and never reads sandbox.hourly_rate. If pricing constants change, the stored column value becomes stale. Either use the stored rate or remove the column. (7) In-memory snapshot cache is per-process. The snapshotCache in the admin infrastructure service is module-level. On Vercel each function invocation may be a fresh process, so the 30s TTL may not deduplicate as intended. Low impact but worth a comment. (8) SSH key provenance not visible in the diff. The admin infrastructure service opens SSH connections to Docker nodes but the diff does not show where the private key is sourced. A comment pointing to the required env var would help operators. (9) results.slice(0, 100) silently truncates billing response. Installations with more than 100 billable sandboxes will see a truncated cron response. Log output is untruncated so this is a monitoring UX issue only. |
|
PR Review Part 4 of 4 - WHAT IS SOLID: Migration uses IF NOT EXISTS throughout - safe to re-run, follows project migration rules, targeted and under 15 lines. HMAC + timingSafeEqual in verifyCronSecret is correct and well-commented. milady-billing-gate failing closed on DB error is the right default. pLimit + withTimeout in the SSH inspection service prevents runaway connections. Optimistic updates with proper rollback in milady-sandboxes-table is a clean pattern. useSandboxListPoll using useRef for callbacks avoids stale closure bugs. Summary: Good overall structure - the table reactivity rewrite is a clear improvement, the admin infra service is well-organized, and the migration follows project rules. Items 1 and 2 (double-billing and negative credit balance) are the critical ones to fix before this ships to production - both are real money bugs. |
|
PR Review: feat: dashboard improvements Overall this is a well-structured PR. The billing cron, admin infra service, and UI reactivity work are solid. A few issues worth addressing before merge. BUGS
SECURITY
PERFORMANCE
MINOR
WHAT IS GOOD
|
|
Code Review for PR 393 - Overall this is solid work. The billing infrastructure is largely done right. A few issues worth addressing. BUGS:
PERFORMANCE:
SECURITY:
SCHEMA:
WHAT IS GOOD:
MINOR:
|
Code ReviewOverall this is a solid, well-structured PR. The billing infrastructure is modelled closely after the existing containers billing pattern, the migration follows CLAUDE.md rules, and the optimistic-update table rewrite fixes a real UX bug. A few issues worth addressing before merging: 🐛 Potential Bugs1.
2.
3. In agent_name: agent.agentName ?? agent.agent_name ?? existing?.agent_name ?? null,
4. Billing cron doesn't reset If an org tops up credits but their agent is in
|
PR #393 Review — Dashboard Improvements: Admin Infra, Table Reactivity, Page SeparationOverall this is a solid, well-structured PR. The billing infrastructure is particularly well thought-out with proper transaction isolation and a rebill guard. A few issues worth addressing before merge: 🐛 Potential Bugs1.
Please verify this column exists, and if not, add it to the migration. 2. Duplicate
3.
|
lalalune
left a comment
lalalune
left a comment
There was a problem hiding this comment.
Reviewed the follow-up fixes and verified the latest CI run is green.
2 participants
Summary
Dashboard improvements covering admin infrastructure, table reactivity fixes, page separation, and agent detail redesign.
Changes
🆕 Admin Infrastructure Dashboard (
+1004 lines)/api/v1/admin/infrastructure) for SSH node inspection🐛 Table Reactivity Fix (
+470 / -239)milady-sandboxes-table: local state management + optimistic updatesonDataRefreshcallback touseSandboxListPollfor live table updatesonCreatedcallback triggers table refresh✨ Page Separation (
+48 / -63)milady-page-wrappercomponent for consistent header styling✨ Agent Detail Page (
+218 / -366)📝 Documentation
MILADY_CLOUD_ROADMAP.md— living roadmap for infrastructure, provisioning, dashboard, and platform goalsTesting
bunx @biomejs/biome check— ✅ all 10 files cleanbun test packages/tests/unit/admin-infrastructure.test.ts— ✅ 5/5 passingCommits (5 logical commits)
feat: admin infrastructure dashboard API + servicefix: table reactivity — optimistic updates, client-side data refreshfeat: separate milady instances and containers pagesfeat: agent detail page improvementsdocs: add milady cloud roadmap