fix(test): cap default worker count to the fd budget (#65219)

[kimdzhekhon]

What does this PR do?

scripts/run_tests_parallel.py defaulted the per-file parallel test runner's worker count to cpu_count*2 with no awareness of the process's file-descriptor budget (RLIMIT_NOFILE). On a many-core macOS box with the platform's default soft limit (256), that can pick 20 workers — each running a full pytest subprocess whose fixtures (tempdirs, asyncio event loops, aiohttp test sockets) hold several dozen fds — and exhaust descriptors mid-suite. The resulting OSError: [Errno 24] Too many open files then surfaces as unrelated test/collection failures instead of a clear infrastructure signal.

_default_worker_count() now caps cpu_count*2 against the live RLIMIT_NOFILE soft limit using a conservative 64-fd-per-worker allowance, falling back to the old cpu-based default on platforms without RLIMIT_NOFILE (e.g. Windows) or when the soft limit is generous (most Linux CI runners are unaffected).

This doesn't fix descriptor leaks within a single file's test run (a separate, per-file concern) — it only keeps the default worker count from making exhaustion more likely than the box can actually sustain. HERMES_TEST_WORKERS / -j still override the default explicitly, unaffected by this change.

Related Issue

Fixes #65219

Type of Change

• 🐛 Bug fix

Changes Made

• scripts/run_tests_parallel.py: add _FD_BUDGET_PER_WORKER and _default_worker_count(), wire it into the -j/--jobs argparse default, update the module docstring's HERMES_TEST_WORKERS description.
• tests/test_run_tests_parallel.py: two regression tests — one asserting a low soft limit (256) caps the default to 4 workers on a 10-core box, one asserting a generous soft limit (65536) leaves cpu_count*2 unaffected on a 4-core box.

How to Test

scripts/run_tests.sh tests/test_run_tests_parallel.py -v

Manual repro of the capped path:

python3 -c "
import resource, sys
resource.setrlimit(resource.RLIMIT_NOFILE, (256, resource.getrlimit(resource.RLIMIT_NOFILE)[1]))
sys.path.insert(0, 'scripts')
import run_tests_parallel as r
print(r._default_worker_count())  # 4, not cpu_count*2
"

What platforms you tested on

macOS (Darwin 25.5.0, arm64). The fd-budget path is POSIX-only by design (gated on resource.RLIMIT_NOFILE availability); Windows and any platform without it fall back to the pre-existing cpu_count*2 default unchanged.

Checklist

• Conventional Commits (fix(test): ...)
• Focused, single-purpose change (worker-count default only)
• Tested locally — tests/test_run_tests_parallel.py passes (7/7)
• ruff check clean on changed files

[tonydwb]

Code Review Summary

Verdict: Approved (LGTM)

Overview

Caps default worker count to the fd budget in test infrastructure. +92/0.

Security

• No hardcoded secrets or credentials

Code Quality

• Clean resource limit handling

Looks Good

• Appropriate test infrastructure fix

---

Reviewed by Hermes Agent

[teknium1]

Thanks for the focused reproduction and tests. The current runner still defaults to cpu_count*2 at scripts/run_tests_parallel.py:657, so the reported concurrency behavior remains on main.

Problems

• scripts/run_tests_parallel.py:301-324 starts a separate pytest subprocess for each file, while ThreadPoolExecutor only controls concurrent launches at scripts/run_tests_parallel.py:941-948. RLIMIT_NOFILE is a per-process limit, so the proposed soft_limit // 64 cap does not establish a descriptor pool shared by those workers and does not demonstrate that it prevents the reported EMFILE failure.
• The added tests mock resource.getrlimit() and assert the formula, but do not exercise the subprocess runner or the reported failure mode.

Suggested changes

• Identify the actual shared macOS resource limit or per-child descriptor leak first, then test the mitigation through the runner's subprocess path.

Automated hermes-sweeper review.

[teknium1]

RLIMIT_NOFILE applies to each process, but every worker here is a separate pytest subprocess. This arithmetic treats it as one budget shared by all workers, so it cannot establish that reducing -j prevents a child from hitting EMFILE. Please diagnose the actual shared resource or per-child leak before capping concurrency this way.

[GottZ]

This was generated by AI during triage.

Summary

Thirteen PRs are in this test-runner complex: #54008 resolves separatorless pytest flags; #57152/#57261 and #51896 address Windows path-list or encoding failures; #60304/#61238 address interrupted-run reporting; #30643/#63873/#66253 change worker sizing; and #40648, #51900, and part of #42204 cover separate runner-adjacent concerns. For #65219 specifically, #30643 and #63873 reduce concurrency, while #66253 proposes an RLIMIT_NOFILE formula that the contributor review says does not establish or test prevention of the reported subprocess EMFILE failure.

Related pull requests

• fix(test): cap default runner workers #30643 [closed] partial — (+27/-7) — n/a: Caps the automatic pool at four, which reduces resource pressure but neither derives an FD-safe limit nor reproduces EMFILE; it remains relevant as an earlier mitigation, but the contributor rejected the fixed cap and salvaged only its hermeticity changes in test: keep tirith checks hermetic #30844.
• fix(tests): neutralize inherited voice TTS state #40648 [closed] related — (+230/-4) — n/a: Neutralizes inherited voice/TTS state rather than worker concurrency or descriptor exhaustion; despite its keep_open review, it is currently closed and its separate salvage path requires moving generated probes out of the source checkout.
• fix(tests): forward pytest flags without requiring -- separator #42195 [closed] fixes — (+97/-3) — n/a: Implements separatorless pytest-flag forwarding rather than resource control; it remains relevant as the earlier [Bug]: scripts/run_tests.sh rejects pytest flags passed without -- #42189 fix but is superseded by fix(tests): bare pytest flags pass through run_tests.sh without a '--' separator #54008, whose merged implementation is identified at commit 2523917.
• fix(testing): accept pytest flags without explicit separator #42204 fixes — (+156/-29) — n/a: Its runner portion duplicates fix(tests): bare pytest flags pass through run_tests.sh without a '--' separator #54008, while the unrelated auxiliary 5xx-fallback diff changes sync and async paths with only sync regression coverage; consistent with the keep_open verdict, salvage requires splitting and rebasing that auxiliary work around refactor(auxiliary): separate _is_server_error and _is_unavailable_error from _is_payment_error #42088 with both paths tested.
• fix(testing): tolerate legacy console encodings #51896 partial — (+51/-0) — n/a: Makes progress output replacement-safe under legacy encodings, fixing only the encoding half of run_tests_parallel.py on Windows: ':' path-list split breaks drive letters; per-file progress lines lost to UnicodeEncodeError on piped stdout #57149 and not worker or FD behavior; its keep_open evidence is subsumed for that half by the broader fix(tests): Windows-aware path-list split and UTF-8 progress output in parallel runner #57152 diff.
• fix(testing): keep duration cache repo-relative #51900 related — (+65/-3) — n/a: Keeps duration-cache keys repository-relative and prunes absolute legacy keys, without changing concurrency or descriptor handling; it remains independently salvageable under its keep_open review.
• fix(tests): bare pytest flags pass through run_tests.sh without a '--' separator #54008 [merged] best fix — (+164/-13) — n/a: Merged reference implementation for [Bug]: scripts/run_tests.sh rejects pytest flags passed without -- #42189 that routes bare and value-taking pytest flags while preserving explicit -- passthrough; it remains relevant because it establishes the current runner argument-routing baseline, not because it addresses EMFILE.
• fix(tests): Windows-aware path-list split and UTF-8 progress output in parallel runner #57152 best fix — (+112/-9) — n/a; recorded best fix for run_tests_parallel.py on Windows: ':' path-list split breaks drive letters; per-file progress lines lost to UnicodeEncodeError on piped stdout #57149: Fixes both drive-letter-safe path-list splitting and Windows progress encoding, without affecting macOS FD exhaustion; its keep_open review requires direct end-to-end coverage for --files and HERMES_TEST_PATHS.
• fix(test-runner): handle Windows path lists safely #57261 [closed] fixes — (+116/-6) — n/a: Implements the same two Windows fixes as fix(tests): Windows-aware path-list split and UTF-8 progress output in parallel runner #57152 and was explicitly closed as its duplicate; it remains relevant as corroborating implementation and regression design.
• fix(test): handle SIGTERM gracefully in parallel test runner #60304 [closed] partial — (+54/-3) — n/a: Adds SIGTERM/SIGINT handling and waits for in-flight futures for Parallel verify harness can obscure completed full-suite results as rc=143 #60189, but exits from the handler before the normal reporting path; it remains relevant as the closed signal-handling predecessor to Preserve parallel runner failure summaries #61238.
• Preserve parallel runner failure summaries #61238 [closed] best fix — (+283/-43) — n/a; recorded best fix for Parallel verify harness can obscure completed full-suite results as rc=143 #60189: Retains child return codes, classifies completed summaries separately from infrastructure failures, and adds signal cleanup and a subprocess probe, but still returns supervisor-derived rc=143; although now closed, this directly reflects the contributor keep_open review's unresolved status-contract concern.
• fix(tests): avoid oversubscribing runner workers #63873 best fix — (+46/-8) — n/a; recorded best fix for test: full macOS suite exhausts file descriptors under per-file parallel runner #65219: Reduces the active default from cpu_count*2 to cpu_count with focused policy tests, mitigating oversubscription without claiming an FD-safe bound or EMFILE detection; its keep_open review finds the current-main policy and salvage path concrete.
• fix(test): cap default worker count to the fd budget (#65219) #66253 partial — (+92/-3) — n/a: Caps workers using soft RLIMIT_NOFILE divided by 64, but the blocking contributor keep_open review notes that pytest files run in separate processes and the mocked formula tests do not exercise or prevent the reported EMFILE path; salvage requires identifying the actual shared macOS constraint or per-child leak and testing it through the subprocess runner.

Duplicates

#42195 and the runner portion of #42204 are superseded by merged #54008. #57261 duplicates #57152, while #51896 overlaps only with their legacy-encoding half; #60304 and #61238 overlap on signal handling but are not full duplicates because #61238 additionally retains statuses, classifies reports, cleans up child trees, and exercises a signal lifecycle.

Suggested consolidation

Keep #63873 open with a salvage path as the recorded best existing #65219 mitigation: preserve its focused cpu_count*2-to-cpu_count correction and policy tests while separately adding reproduction-backed EMFILE detection or resource accounting. Author action on #66253: identify the actual shared macOS resource constraint or per-child descriptor leak and add an end-to-end subprocess reproduction before retaining an RLIMIT-based cap; this explicitly follows, rather than overrides, the blocking contributor keep_open review. Keep #60304, #61238, #42195, and #57261 closed for the reasons above; treat #42204's runner portion as superseded by #54008, and consolidate #51896 only into #57152's overlapping encoding work after #57152 adds the requested --files and HERMES_TEST_PATHS coverage.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I65219(["issue #65219 (open)"])
    P66253["PR #66253 (open)"]
    P66253 -.->|partial| I65219
    class I65219 open
    class P66253 open
    class P66253 target
    click I65219 "https://github.com/NousResearch/hermes-agent/issues/65219"
    click P66253 "https://github.com/NousResearch/hermes-agent/pull/66253"

Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 13 pull requests and 4 issues in this complex. Each diff was read against this issue; Assessment working set: 100 kB of PR diffs, 38 kB of issue/PR text, 9 kB of discussion (18 comments), 17 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.