fix(server): a directory no longer resolves as the resource monitor binary

[Furox-Art]

Problem

T3CODE_RESOURCE_MONITOR_PATH (and the desktop bootstrap's resourceMonitorPath) is accepted as soon as the path exists. A directory satisfies that, and on Windows it also clears the POSIX exec-bit check because Node reports permissive synthetic modes (0o666/0o777). Pointing the override at the folder that contains the binary — an easy mistake — therefore resolves successfully, and the failure only surfaces later as an opaque spawn error from the telemetry client instead of naming the configured path.

Expected behavior: an override that is not a file is rejected where it is validated, with the path in the error.

Change

resolve now rejects a directory candidate next to the existing non-executable check and reports it through the same ResourceMonitorBinaryNotExecutable error, which already carries path and mode. The directory check runs on every platform; the exec-bit test stays POSIX-only for the same reason it was before.

No startup policy changes. resolve is only called when the telemetry client starts an attempt, and its failures map to degraded/unavailable telemetry status, so the server keeps running. This PR only makes the bad path get named at resolution time instead of at spawn time.

Scope and approval

Small, focused fix of an obvious validation bug in an already-shipped optional override, so it qualifies without a prior issue. Rebuilt against orchestrator V2 as asked in #12612 (comment).

The old PR also added an envValidation.ts module, startup rejection for invalid values, and CLI plumbing for T3CODE_CODEX_LAUNCH_ARGS. V2 makes most of that unnecessary: tokenizeCliArgs never throws, launch args are filtered through a whitelist before spawn, and the resource-monitor candidate chain already falls back instead of aborting. This PR keeps only the one defect that survives the rewrite, and touches no product policy.

Verification

corepack pnpm exec vp test run src/resourceTelemetry/ResourceMonitorBinary.test.ts in apps/server:

• With the fix — Tests 5 passed | 2 skipped (7). The two skips are the existing skipIf(windowsHost) POSIX chmod cases; the host here is Windows.
• Reproduced first, then fixed: reverting only ResourceMonitorBinary.ts (new test kept) fails exactly the new case and nothing else —

  × rejects a directory override
  FAIL src/resourceTelemetry/ResourceMonitorBinary.test.ts > ResourceMonitorBinary > rejects a directory override
  Unknown Error: C:\Users\Furkan\AppData\Local\Temp\t3-resource-monitor-binary-Xk6hWg
       Tests  1 failed | 4 passed | 2 skipped (7)
  

  i.e. resolve returned the directory as the binary path.
• tsc --noEmit in apps/server: exit 0.
• vp fmt --check on both changed files: clean.

Agent harness: Qoder, driving the Furox-Art checkout.

[macroscopeapp]

Approvability

Verdict: Approved at dddc218

Macroscope's review found this PR approvable — The PR is a small, self-contained bug fix that rejects directory paths before native telemetry spawning and adds a regression test for Windows. Valid resource-monitor binaries and existing runtime paths retain their behavior.

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

[coderabbitai]

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: 9f1b546a-7694-4547-adb5-82637a03ed5c

---

📥 Commits

Reviewing files that changed from the base of the PR and between 0f2a80f and dddc218.

---

📒 Files selected for processing (2)

• apps/server/src/resourceTelemetry/ResourceMonitorBinary.test.ts
• apps/server/src/resourceTelemetry/ResourceMonitorBinary.ts

---

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

Binary resolution now rejects directory candidates on all platforms. A test verifies that a Windows/arm64 directory override returns ResourceMonitorBinaryNotExecutable with the directory path.

Changes

Binary candidate validation

Layer / File(s)	Summary
Directory candidate rejection apps/server/src/resourceTelemetry/ResourceMonitorBinary.ts, apps/server/src/resourceTelemetry/ResourceMonitorBinary.test.ts	Resolution rejects directory candidates on all platforms. On non-Windows platforms, candidates without execute bits remain rejected. The test checks the error and path for a directory override on Windows/arm64.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge

---

Merge Risk: ⚪ Minimal · up to dddc2

Directory overrides are rejected as monitor binaries, preventing them from reaching the later spawn path. No material merge risk is apparent in the reviewed change.

Architecture Summary

Architecture risk: 🔵 Low · up to dddc2

The change affects 1 system.

Changed systems: apps/server

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

• observed — apps/server (service) was modified; 2 changed files map to changed impact.

Before / after behavior

• observed — Modified behavior in apps/server/src/resourceTelemetry/ResourceMonitorBinary.test.ts: Adds a test for a directory override using Windows/ARM64 configuration; resolving the binary is expected to fail with ResourceMonitorBinaryNotExecutable containing the override path.
• observed — Modified behavior in apps/server/src/resourceTelemetry/ResourceMonitorBinary.ts: Existing candidates are now statted on Windows as well as other platforms. A candidate is rejected when it is a directory on any platform, or when it is not Windows and has no execute bits; previously, the stat and no-execute-bit check ran only on non-Windows platforms.

---

Pre-merge checks | 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check		The title clearly and concisely describes the primary fix: directories no longer resolve as the resource monitor binary. It uses a conventional commit format.
Description check		The description includes complete Problem, Change, Scope and approval, and Verification sections. It explains expected behavior, implementation scope, approval rationale, test results, limitations, an…
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.

---

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

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

---

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

[Furox-Art]

This one is ready and I cannot move it further from the outside — flagging for a maintainer:

• two approving reviews already: @macroscopeapp ("Approved at 0f2a80f") and @coderabbitai;
• Macroscope checks green: Approvability Check SUCCESS, Correctness Check SUCCESS (the SKIPPED entries are the size-config and conventions jobs that don't apply to a two-file diff);
• mergeable: MERGEABLE, but mergeStateStatus: BLOCKED — i.e. it is waiting on a human, not on CI or on me.

Scope stays as described: apps/server/src/resourceTelemetry/ResourceMonitorBinary.ts plus its test — a path that exists but is a directory no longer resolves as the resource-monitor binary. No behaviour change for a valid file path.

Happy to rebase onto main the moment it drifts, and I'll re-verify the test on the new base before pushing.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
gh pr view 16838 --repo pingdotgg/t3code --json mergeable,mergeStateStatus,reviewDecision,reviews,statusCheckRollup,headRefOid
ast-grep outline apps/server/src/resourceTelemetry/ResourceMonitorBinary.ts
ast-grep outline apps/server/src/resourceTelemetry/ResourceMonitorBinary.test.ts
rg -n -C 12 'Directory|ResourceMonitorBinaryNotExecutable|rejects a directory' apps/server/src/resourceTelemetry/ResourceMonitorBinary.ts apps/server/src/resourceTelemetry/ResourceMonitorBinary.test.ts

Length of output: 29797

---

@Furox-Art, confirmed for the current head, 0f2a80f:

• Macroscope and CodeRabbit both have approving reviews.
• All reported checks are successful or skipped.
• GitHub reports MERGEABLE, with merge state BLOCKED.
• The code rejects directory candidates on every platform. The new test checks the error type and directory path.

One qualification: BLOCKED does not identify the exact unmet merge requirement. A maintainer needs to inspect the merge requirements for #16838. The reported status shows no failing or pending checks.

You are interacting with an AI system.

[maria-rcks]

Note

Written by claude-opus-5-5 on behalf of Maria

Hi! We are cleaning up open PRs, and this one does not say which model or harness was used to create it. If this change is really important, we recommend rebuilding the PR with a newer model and noting the model and harness in the PR description.

[maria-rcks]

Note

Written by claude-opus-5-5 on behalf of Maria

Reopening, this was closed by mistake. Sorry for the noise!

Dismissing prior approval to re-evaluate 0f2a80f

Dismissing prior approval to re-evaluate dddc218