perf(resource-monitor): use two-stage process refresh on Windows

[mweinbach]

Problem

On Windows, t3-resource-monitor was excluded from the two-stage process refresh used on Linux. Every 2 seconds it ran ProcessRefreshKind::everything() across every process on the machine, reading PEB command lines, memory, and disk I/O system-wide instead of only for the PIDs T3 Code tracks.

Change

• native/resource-monitor/src/main.rs: enable two-stage refresh on Windows. The system-wide discovery pass now uses ProcessRefreshKind::nothing().with_cpu(), and the full detail refresh runs only for tracked PIDs (plus the monitor's own PID for the start-time offset check), the same as Linux.

Scope and approval

Split from #18508 per CONTRIBUTING.md's one-problem rule. This PR changes only the Windows process refresh in native/resource-monitor/src/main.rs. The unnecessary binary lookup changes were removed; ResourceMonitorBinary.ts is unchanged from main.

Verification

• Maintenance verification on macOS ARM64: cargo test --locked --manifest-path native/resource-monitor/Cargo.toml — 17 passed, 0 failed.
• Windows version/architecture and before/after sampling measurements are still pending Windows guest access. The macOS test result does not establish Windows performance; no new Windows timing result is claimed.

Model: Claude 5.5 Opus

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-13T00:23:29.924771Z	5ec2dae	New commits

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[juliusmarminge]

Note

This comment is posted by Julius' dot

Which binary layout currently fails, and why does the Windows refresh change need the added lookup paths? The existing resolver already checks target-triple directories and accepts an explicit binary path. Please clarify that relationship under the one-problem rule. Also add the Windows version/architecture, before/after sampling results, and a check of the failing binary lookup. The 17-test count alone doesn't establish those behaviors under the verification requirement.

[macroscopeapp]

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — The PR changes the existing Windows resource-monitor pipeline from full process-detail refreshes to selective refreshes for tracked processes, materially changing when substantial OS inspection work occurs. The added binary paths are low risk, but the production sampling gate warrants deliberate human review.

Notes:

• All code in this push has already been reviewed. Approvability was decided on eligibility alone.

^{You can add or adjust custom eligibility rules. Learn more.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b7fd6e8c35

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[coderabbitai]

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration

• Configuration used: Path: .coderabbit.config.ts
• Review profile: CHILL
• Plan: Advanced
• Run ID: 965af38c-27f3-4b1b-ab25-c24f930c2871

📥 Commits

Reviewing files that changed from the base of the PR and between 0306c30 and 5ec2dae.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Path: .coderabbit.config.ts
• Review profile: CHILL
• Plan: Advanced
• Run ID: 38d7a7f3-8ebb-415c-8706-411a1124e086

📥 Commits

Reviewing files that changed from the base of the PR and between 6f6197c and b7fd6e8.

📒 Files selected for processing (2)

• apps/server/src/resourceTelemetry/ResourceMonitorBinary.ts
• native/resource-monitor/src/main.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

📝 Walkthrough

Walkthrough

The pull request adds bundled resource-monitor binary search paths. It also changes process discovery and detail collection on Linux and Windows.

Changes

Bundled Binary Lookup

Layer / File(s)	Summary
Add bundled binary candidates apps/server/src/resourceTelemetry/ResourceMonitorBinary.ts	The search list adds a Rust-target-specific release path and generic release and debug paths at two repository-relative depths. The target-specific path is included when both the platform key and Rust target are defined.

Process Collection

Layer / File(s)	Summary
Separate process discovery and detail refreshes native/resource-monitor/src/main.rs	On Linux and Windows, discovery uses the CPU-only, task-free refresh. When tracked processes exist, detail collection uses a separate System instance for tracked process IDs and, if needed, the monitor process. The collector derives and carries a start-time epoch offset.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to b7fd6

The binary lookup and Windows process refresh changes have no identified issue requiring a fix before merge.

Security Architecture Review

Security architecture risk: 🔵 Low · up to b7fd6

The change reduces unnecessary process-detail collection and preserves existing telemetry access controls. Additional executable fallback locations deserve attention, but no new attacker-controlled execution path was demonstrated. Windows failure behavior and deployment filesystem permissions remain incompletely verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• observed — Discovery still enumerates host processes, but Windows command, memory, and disk-detail requests are narrowed to selected process trees and the monitor. This reduces system-wide sensitive-detail collection without establishing a new tenant or privilege boundary.

Security Findings and Attack Paths

• inferred — The added fallback directories become conditional executable trust roots: a matching binary can be selected and spawned when higher-priority candidates are unavailable. An attacker would need write access to such a location and an affected lookup layout. That write capability was not established; existing overrides and packaged candidates provide important counterevidence to a demonstrated new execution vulnerability.

Trust Boundaries and Controls

• observed — The server owns monitor configuration and supplies its own root PID. Telemetry history and subscription operations remain assigned diagnostics-read authorization scope. The inspected changes do not expose the monitor's stdin command stream or arbitrary root selection through a new public API.

Resilience and Maintainability Implications

• inferred — A failed detail refresh that produces no entries, or an unavailable monitor start-time offset, prevents detailed samples from being emitted. The next sampling attempt constructs fresh detail state. This supports failure containment, although Windows interruption and provider-failure behavior were not runtime-verified.

Pre-merge checks | 3 | 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check		The description includes the required sections and reports the verification limits clearly. However, it incorrectly states that ResourceMonitorBinary.ts is unchanged and that the scope is limited to W…	Update the Problem, Change, Scope and approval, and Verification sections to describe the ResourceMonitorBinary.ts changes. Explain why the binary lookup change belongs in this pull request, or remove those changes from the changeset. Ensur…

✅ Passed checks (3 passed)

Check name	Status	Explanation
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Title check		The title clearly and concisely identifies the main change: enabling two-stage process refresh on Windows.

Full details: Description check

Explanation

The description includes the required sections and reports the verification limits clearly. However, it incorrectly states that ResourceMonitorBinary.ts is unchanged and that the scope is limited to Windows process refresh, while the changeset also modifies binary lookup paths.

Resolution

Update the Problem, Change, Scope and approval, and Verification sections to describe the ResourceMonitorBinary.ts changes. Explain why the binary lookup change belongs in this pull request, or remove those changes from the changeset. Ensure the stated scope matches the actual files changed.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}