[codex] add diagnostics resource history#2685
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository 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)
Comment |
- Group sampled processes by PID and command to keep histories stable across elapsed-time drift - Show all process summaries in the window and refine the diagnostics table and chart UI - Add coverage for grouping drift and expanded summaries
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Redundant fetch logic diverges between refresh and interval
- Replaced the duplicated fetch logic in the refresh callback with a refreshKey state counter that triggers the useEffect to re-run, ensuring a single fetch path with isMounted guards and proper interval reset.
Or push these changes by commenting:
@cursor push 842ce7848e
Preview (842ce7848e)
diff --git a/apps/web/src/lib/processDiagnosticsState.ts b/apps/web/src/lib/processDiagnosticsState.ts
--- a/apps/web/src/lib/processDiagnosticsState.ts
+++ b/apps/web/src/lib/processDiagnosticsState.ts
@@ -83,22 +83,11 @@
const [data, setData] = useState<ServerProcessResourceHistoryResult | null>(null);
const [error, setError] = useState<string | null>(null);
const [isPending, setIsPending] = useState(true);
+ const [refreshKey, setRefreshKey] = useState(0);
const refresh = useCallback(() => {
- setIsPending(true);
- void ensureLocalApi()
- .server.getProcessResourceHistory({ bucketMs, windowMs })
- .then((result) => {
- setData(result);
- setError(null);
- })
- .catch((cause: unknown) => {
- setError(formatProcessDiagnosticsError(cause));
- })
- .finally(() => {
- setIsPending(false);
- });
- }, [bucketMs, windowMs]);
+ setRefreshKey((k) => k + 1);
+ }, []);
useEffect(() => {
let isMounted = true;
@@ -127,7 +116,7 @@
isMounted = false;
window.clearInterval(interval);
};
- }, [bucketMs, windowMs]);
+ }, [bucketMs, windowMs, refreshKey]);
return { data, error, isPending, refresh };
}You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 20a2a1c. Configure here.
| .finally(() => { | ||
| setIsPending(false); | ||
| }); | ||
| }, [bucketMs, windowMs]); |
There was a problem hiding this comment.
Redundant fetch logic diverges between refresh and interval
Low Severity
The refresh callback duplicates the fetch logic from the useEffect's read function but omits the isMounted guard and doesn't reset the interval timer. This means a manual refresh near an interval tick produces two concurrent in-flight requests that race on setData/setIsPending, and refresh can set state after unmount. Extracting a shared fetch function or having refresh simply restart the effect would avoid the divergence.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 20a2a1c. Configure here.
ApprovabilityVerdict: Needs human review This PR introduces a new process resource history monitoring feature with a backend sampling service, new RPC endpoint, and substantial new UI components. New features introducing user-facing behavior warrant human review regardless of how well-scoped they are. You can customize Macroscope's approvability policy. Learn more. |



Summary
Adds an in-memory process resource monitor for Diagnostics that samples the T3 server root process and live descendants over time. The new history API rolls samples up into CPU-time summaries and chart buckets so the UI can show cumulative CPU, current/average/peak CPU, and memory peaks across selectable windows.
Details
ProcessResourceMonitorservice with a bounded 60-minute in-memory ring buffer.server.getProcessResourceHistorycontracts/RPC/client wiring.Validation
bun fmtbun typecheckbun lint(passes with existing warnings)bun run test src/diagnostics/ProcessDiagnostics.test.ts src/diagnostics/ProcessResourceMonitor.test.tsfromapps/serverNote
Add process resource history monitoring and diagnostics UI
ProcessResourceMonitorservice (ProcessResourceMonitor.ts) that samples server process and descendants every 5s, retains samples within a fixed window, and exposes areadHistorymethod returning bucketed CPU/memory aggregates.server.getProcessResourceHistoryWebSocket RPC endpoint with full contract definitions in server.ts and rpc.ts.useProcessResourceHistory.Macroscope summarized 20a2a1c.
Note
Medium Risk
Adds a new background sampler and WebSocket RPC for process CPU/RSS history; risk is moderate due to continuous sampling/polling and new aggregation logic that could affect performance or memory if mis-sized.
Overview
Adds a new
ProcessResourceMonitorservice that samples the server root PID plus live descendants every 5s, retains a bounded in-memory history, and aggregates it into per-process CPU-time summaries and time-bucketed CPU/RSS stats.Exposes this via new contracts and
server.getProcessResourceHistoryRPC wiring (serverws.tshandler + clientlocalApi/wsRpcClient), and disables tracing for the new method to avoid span noise.Extends the web Diagnostics settings with a Resource History section (window selector, CPU timeline chart, and per-process table), and exports a few
ProcessDiagnosticshelpers (ProcessRow,buildDescendantEntries,isDiagnosticsQueryProcess,readProcessRows) for reuse; updates server tests to mock the new monitor and adds unit tests for the monitor’s sampling/aggregation behavior.Reviewed by Cursor Bugbot for commit 20a2a1c. Bugbot is set up for automated code reviews on this repo. Configure here.