Feat: Sandbox details events table #300
Open
Claude / Claude Code Review
completed
Apr 21, 2026 in 8m 29s
Code review found 3 important issues
Found 5 candidates, confirmed 3. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 3 |
| 🟡 Nit | 0 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🔴 Important | src/features/dashboard/sandbox/events/view.tsx:62-70 |
No error state in SandboxEventsView: API failures show misleading empty table |
| 🔴 Important | src/core/server/api/routers/sandbox.ts:60-68 |
hasNextPage false positive on exact page boundary |
| 🔴 Important | src/core/modules/sandboxes/repository.server.ts:344-346 |
listSandboxLifecycleEvents returns all event types, not just lifecycle events |
Annotations
Check failure on line 70 in src/features/dashboard/sandbox/events/view.tsx
claude / Claude Code Review
No error state in SandboxEventsView: API failures show misleading empty table
The new `SandboxEventsView` has no error state: when the TRPC query fails (network error, 404, 500), `isPending` is false and `data` is undefined, so the component falls through and renders an empty "No events found" table indistinguishable from a sandbox that legitimately has no events. With `refetchOnWindowFocus: false`, users have no automatic recovery path and no indication that an error occurred.
Check failure on line 68 in src/core/server/api/routers/sandbox.ts
claude / Claude Code Review
hasNextPage false positive on exact page boundary
The `hasNextPage` flag in the `sandbox.events` tRPC query is determined by `eventsResult.data.length === limit`, which produces a false positive when the final page contains exactly `limit` (default 20) events. In that case the Next button remains enabled, and clicking it fetches an empty page that shows "No events found" with no indication the user has reached the end of the list.
Check failure on line 346 in src/core/modules/sandboxes/repository.server.ts
claude / Claude Code Review
listSandboxLifecycleEvents returns all event types, not just lifecycle events
The new `listSandboxLifecycleEvents` method passes `options` directly to the API with no lifecycle prefix filter, so selecting "All" in the event type filter fetches every sandbox event type from the backend—including non-lifecycle events like `sandbox.process.oom`. Fix by defaulting `types` to `SANDBOX_LIFECYCLE_EVENT_TYPE_VALUES` when `options.types` is undefined, or by filtering results client-side with `event.type.startsWith(SANDBOX_LIFECYCLE_EVENT_PREFIX)`.
Loading