fix: harden quota probe subprocess handling

[Michaelyklam]

Thinking Path

• Issue Subprocess pool refactor for profile-isolated quota probes (follow-up to #1873) #1912 identified three operational risks in profile-isolated provider quota probes: uncapped subprocess fan-out, stdin inheritance, and orphaned children after parent death.
• A full warm worker-pool is larger than a safe maintenance slice, so this PR takes the low-risk first slice: harden the existing subprocess path without changing the quota-provider API contract.
• The implementation keeps the existing isolated child-process boundary, adds a concrete concurrency cap, and wires parent-death handling where POSIX supports it.

What Changed

• Added a module-level bounded semaphore around account-usage quota probes to cap concurrent profile-isolated subprocesses.
• Added stdin=subprocess.DEVNULL for the quota child process.
• Added POSIX preexec_fn and child bootstrap wiring for prctl(PR_SET_PDEATHSIG, SIGTERM) so probe children receive SIGTERM when the WebUI parent dies.
• Documented that persistent warm worker reuse remains the next follow-up if this first slice is not enough under load.
• Added regression coverage for DEVNULL stdin, preexec wiring, semaphore bound, and semaphore call-path behavior.

Why It Matters

• Prevents quota-panel bursts from spawning unbounded provider-probe subprocesses.
• Reduces the risk of probe children outliving the WebUI process during restarts or hard kills.
• Keeps the implementation narrow and compatible with the existing profile-isolation model.

Verification

• python3 -m py_compile api/providers.py
• git diff --check
• env -u HERMES_CONFIG_PATH -u HERMES_WEBUI_HOST /home/michael/.hermes/hermes-agent/venv/bin/python -m pytest tests/test_provider_quota_status.py -q → 17 passed in 6.52s
• env -u HERMES_CONFIG_PATH -u HERMES_WEBUI_HOST /home/michael/.hermes/hermes-agent/venv/bin/python -m pytest tests/ -k provider -x -q → 486 passed, 4614 deselected, 1 warning in 185.04s

Risks / Follow-ups

• This is intentionally not the full warm worker-pool refactor from Subprocess pool refactor for profile-isolated quota probes (follow-up to #1873) #1912; it is a safe first slice for concurrency/orphan hardening. Persistent worker reuse can still be implemented in a follow-up if spawn latency remains a problem.
• preexec_fn is only wired on POSIX. Non-POSIX platforms keep the prior subprocess behavior plus DEVNULL/concurrency cap.

Refs #1912

Model Used

• Freebuff/Codebuff free mode with minimax/minimax-m2.7 for implementation and first-pass test execution.
• OpenAI Codex / GPT-5.5 via Hermes Agent for orchestration, review, final verification, and PR publication guardrails.
• Tools: terminal, git/GitHub CLI, file edits, Hermes Kanban.

[nesquena-hermes]

Summary

Read the diff at api/providers.py (origin/master vs PR HEAD) and the new
regression cases in tests/test_provider_quota_status.py:416-567. The intent
is right — this is the right first slice for #1912, scope is narrow, tests
exist for each invariant, and CI is green. One concrete safety concern with
the preexec_fn path, plus a small nit on a private-attribute assertion in
the new tests.

The preexec_fn path is unsafe in the multithreaded WebUI

The WebUI HTTP server is ThreadingHTTPServer (server.py:11, server.py:30),
so every /api/provider/quota call runs on a request thread
(api/routes.py:2928-2931). The CPython docs are explicit that
preexec_fn is unsafe in the presence of threads — the child can deadlock
between fork() and execve() because the in-child interpreter may try to
acquire a lock held by another thread in the parent at fork time. Quoting
the Python docs:

The preexec_fn parameter is NOT SAFE to use in the presence of threads in
your application. The child process could deadlock before exec is called.

The PR wires preexec_fn=_account_usage_preexec_fn in
api/providers.py:617-621:

if hasattr(os, "fork"):  # POSIX
    kwargs["preexec_fn"] = _account_usage_preexec_fn

…where the preexec body itself does:

def _account_usage_preexec_fn() -> None:
    try:
        import ctypes
        libc = ctypes.CDLL(None)
        libc.prctl(1, signal.SIGTERM)  # PR_SET_PDEATHSIG=1, SIGTERM=15
    except Exception:
        pass

In practice the ctypes.CDLL(None) + prctl here is brief, but it still
holds the import lock and runs arbitrary Python in a forked-without-exec
window inside a multithreaded process — exactly the pattern the docs warn
against. Under load (multiple provider polls × multiple profiles) this is a
real, if intermittent, hazard.

The bootstrap path already covers the same invariant — safely

The PR also installs _ACCOUNT_USAGE_PARENT_DEATHSIG_BOOTSTRAP as a
prefix on the python -c argument
(api/providers.py:622-627). That code runs after execve, in the fresh
child interpreter, before any provider work — and it does the same
prctl(PR_SET_PDEATHSIG, SIGTERM) call. PDEATHSIG is per-thread state set
on the calling thread, so setting it as the very first thing in the child
interpreter is functionally equivalent to setting it in preexec for the
quota probe's purposes: the child's parent at that point is still the WebUI
process (no intervening exec or reparent), so a parent SIGKILL after the
bootstrap line will still deliver SIGTERM to the child.

There is one tiny window the preexec covered that the bootstrap doesn't:
the few ms between execve() and the first line of the bootstrap. If the
WebUI dies in that window, the child is briefly un-protected. For a 35 s
probe this is negligible vs. the thread-deadlock risk you're taking on by
keeping preexec_fn.

Recommendation

Drop the preexec_fn wiring and keep the bootstrap only:

kwargs: dict[str, Any] = {
    "stdin": subprocess.DEVNULL,
    "stdout": subprocess.PIPE,
    "stderr": subprocess.PIPE,
    "text": True,
    "timeout": _ACCOUNT_USAGE_SUBPROCESS_TIMEOUT_SECONDS,
    "check": False,
}
# Parent-death signal is installed by _ACCOUNT_USAGE_PARENT_DEATHSIG_BOOTSTRAP
# inside the child interpreter (after exec) — safer than preexec_fn in a
# multithreaded server (CPython docs warn about fork-without-exec hazards).

That removes the _account_usage_preexec_fn helper and the hasattr(os, "fork") branch. The test_account_usage_preexec_fn_is_wired_on_posix case
becomes test_account_usage_bootstrap_includes_pdeathsig — assert that the
bootstrap string contains prctl(1, signal.SIGTERM) and is prepended to the
-c argument. That's actually a stronger invariant: it catches the case
where someone accidentally drops the prefix concatenation.

Test-only nit (non-blocking)

test_account_usage_probe_semaphore_has_correct_bound reads sem._value
directly (test file:464). _value is a private impl detail that mutates on
acquire()/release() — fine at init, but brittle. BoundedSemaphore also
exposes _initial_value which is the bound itself, or you can just keep
the cap in a module constant _MAX_CONCURRENT_ACCOUNT_USAGE_PROBES (already
done) and assert against that without poking the semaphore internals.

Otherwise

• stdin=subprocess.DEVNULL is a clean win — api/providers.py:611.
• The BoundedSemaphore(2) cap is conservative but correct for the
  35 s probe timeout; Subprocess pool refactor for profile-isolated quota probes (follow-up to #1873) #1912 also called this out as defensive.
• Scope is right: explicitly not the warm worker-pool, leaving that for a
  follow-up. CI passing on 3.11/3.12/3.13.

LGTM modulo the preexec_fn removal.