Skip to content

fix(procs): show process-tree daemon metrics#435

Merged
jdx merged 8 commits into
jdx:mainfrom
sargunv-headway:cursor/sv/process-tree-metrics-35b3
May 28, 2026
Merged

fix(procs): show process-tree daemon metrics#435
jdx merged 8 commits into
jdx:mainfrom
sargunv-headway:cursor/sv/process-tree-metrics-35b3

Conversation

@sargunv-headway

@sargunv-headway sargunv-headway commented May 14, 2026

Copy link
Copy Markdown
Contributor

Note

I did this with Cursor and GPT-5.5. The PR description was written by the agent, edited by me, and I'll respond to review comments myself, not via the agent.

The individual commits show the workflow: wrote failing test, wrote the fix, applied findings from a parallel agent review

Summary

BUG: Web/TUI daemon metrics were using stats from only the directly supervised PID, so wrapper-style daemons could under-report CPU and RSS when the actual work happens in child processes. We use daemons launched as mise tasks, so in all cases the metrics were just reporting stats for the mise process itself, not the underlying service.

This fix reuses the existing descendant process-tree aggregation that resource-limit enforcement already relies on.

Verification

  • added a regression test that spawns a parent/child process tree and verifies reported RSS includes descendants
  • cargo +stable nextest run get_stats_includes_descendant_rss

cursoragent and others added 3 commits May 13, 2026 23:27
Co-authored-by: sargunv-headway <sargunv-headway@users.noreply.github.com>
Co-authored-by: sargunv-headway <sargunv-headway@users.noreply.github.com>
Co-authored-by: sargunv-headway <sargunv-headway@users.noreply.github.com>
@greptile-apps

greptile-apps Bot commented May 14, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a metrics under-reporting bug where CPU and RSS figures for daemons launched via mise (or other wrapper processes) only reflected the supervisor PID, ignoring all child processes. The fix threads the existing descendant BFS aggregation — previously used only for resource-limit enforcement — into every stats collection path: single-daemon get_stats, batch list views, TUI polling, and the detail show route.

  • get_stats now delegates to get_batch_group_stats, which builds a parent→children map once and BFS-traverses each daemon's tree, giving O(N + ΣD_i) cost instead of O(1) per PID.
  • All list/index/show routes now call PROCS.refresh_processes() (full snapshot) instead of the former targeted refresh_pids(), ensuring newly spawned child processes are visible to sysinfo before the BFS runs.
  • A regression test spawns a sh -c "sleep 30 & wait" tree and asserts that get_stats reports direct_rss + child_rss, directly verifying the fix.

Confidence Score: 5/5

Safe to merge — the change is a targeted, well-tested correctness fix with no behavioural regressions.

All three routes (list, index, show) and the TUI poller now call refresh_processes() before BFS aggregation, and the regression test directly asserts the previously-broken descendant-RSS accounting. The two consistency issues flagged in earlier review rounds — split-lock in get_extended_stats and root-only refresh in list views — are both resolved here. No new defects were introduced; the unrelated StartOptions simplification in the start route is a no-op because build_run_options never reads the quiet field.

No files require special attention.

Important Files Changed

Filename Overview
src/procs.rs Core fix: get_stats now delegates to get_batch_group_stats (BFS tree aggregation); get_extended_stats inlines its own single-lock BFS; Procs::new pre-warms the sysinfo snapshot; refresh_daemon_pids removed as all callers now use refresh_processes(). Regression test added.
src/tui/app.rs TUI polling now calls refresh_processes() (full scan) and uses get_batch_tree_stats_map for a single-pass batch collection instead of per-daemon get_stats calls.
src/web/helpers.rs Added daemon_row_with_stats accepting precomputed ProcessStats so list routes can pass the batch-aggregated map; daemon_row kept as a convenience wrapper for single-use call sites.
src/web/routes/daemons.rs List and partial-list routes refresh with refresh_processes() and use get_batch_tree_stats_map; show route now also calls refresh_processes(); unrelated simplification of StartOptions construction in start route (harmless — quiet is not propagated by build_run_options).
src/web/routes/index.rs Index page now collects PIDs from user daemons, conditionally calls refresh_processes(), then uses get_batch_tree_stats_map for a single-pass stats build — matches the pattern used in the daemons list route.
Cargo.toml Trivial: removed a trailing blank line at end of file.

Reviews (5): Last reviewed commit: "fix(procs): refresh full process snapsho..." | Re-trigger Greptile

Comment thread src/procs.rs
Comment thread src/procs.rs
Co-authored-by: sargunv-headway <sargunv-headway@users.noreply.github.com>
cursoragent and others added 3 commits May 20, 2026 23:39
Co-authored-by: sargunv-headway <sargunv-headway@users.noreply.github.com>
Co-authored-by: sargunv-headway <sargunv-headway@users.noreply.github.com>
Comment thread src/web/routes/daemons.rs
Co-authored-by: Sargun Vohra <sargunv-headway@users.noreply.github.com>
@jdx jdx merged commit c1d64c6 into jdx:main May 28, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants