fix(lsp): guard a non-finite idle_timeout and report the bounds in force (SCA-4721)

[pai-scaffolde]

Ports the only two things #50 carries that the live LSP stack does not, so #50 can be closed as fully superseded and the hermes-agent queue stops being hostage to it (SCA-4721).

Based on the stack tip #67 (6a3d95b100), not opened as a competing branch on agent/lsp/manager.py.

1. A non-finite lsp.idle_timeout silently disarmed the reaper

resolve_max_clients() rejects zero, negatives, strings and the non-finite floats. idle_timeout was coerced inline with a bare float() and a 0 < x < MIN_IDLE_TIMEOUT clamp — and .nan / .inf are valid YAML that survive float() without raising and slip that clamp. Driving the real create_from_config at the tip:

idle_timeout=nan -> _idle_timeout=nan  starts=False cutoff= nan :: reaper never starts
idle_timeout=inf -> _idle_timeout=inf  starts=True  cutoff=-inf :: reaper runs, reaps nothing

The count cap still binds, so this was never unbounded accumulation. It is the reaper's own axis switched off: idle servers never drain and the fleet pins at max_clients indefinitely.

The fix is a named resolve_idle_timeout() beside resolve_max_clients(), not a second inline isfinite check — coercing the two knobs in two places is how they drifted apart to begin with. 0 is the one non-positive value kept as written (the user guide documents it as "hold every server's index warm"); a negative reaches that same disabled state by accident and derives the default instead.

2. get_status() now reports the bounds in force

hermes lsp status is how the cap gets audited without reading source — the surface that made SCA-4621 and SCA-4583 diagnosable. It now carries max_clients, idle_timeout, and per-client idle_seconds / inflight, in both the JSON and human-readable paths:

  idle_timeout:    123.0s
  max_clients:     7
  active clients:  1
    - pyright              state=ready      idle=412s inflight=1 root=/tmp/ws

3. Rejected bounds log at WARNING, not debug

An operator setting max_clients: 0 reads it as "no limit" and got no visible signal the host default was substituted. The warning fires only on an actual substitution ("4" and 4 compare numerically equal), so it stays a signal rather than startup noise.

Explicitly not in scope

The monotonic-clock fix. #62 already carries it and goes further (CLOCK_BOOTTIME, suspend and wall-clock-step coverage). Not re-ported.

⚠️ One thing the restack must not miss

#62 and this branch merge clean textually (git merge-tree --write-tree exits 0, no conflict) and are still semantically incompatible until one line changes. idle_seconds and _last_used are only meaningful relative to each other; #62 moves _last_used onto _idle_clock() while get_status here reads time.time().

Rather than leave that as a comment, it is pinned by a test. Probed by actually performing the merge locally:

$ git merge origin/fix/lsp-reaper-monotonic-clock     # clean, no conflict
$ pytest tests/agent/lsp/test_client_bounds.py
E   AssertionError: idle_seconds=1786616903.0171711 is not an elapsed time
E   — get_status and _last_used are reading different clocks
E   assert 1786616903.0171711 < 60.0
1 failed, 23 passed

test_status_idle_seconds_reads_the_same_clock_as_last_used bounds the age on both sides for exactly this reason — a bare >= 5 is satisfied by an epoch gap of 1e9 and would have shipped that number to the operator's status output.

The fix is one line in get_status, verified on the merged tree: now = time.time() → now = _idle_clock(), after which all 24 pass. Whichever of #62 / this lands second should make that change.

Verification

RED first, against tip 6a3d95b100 — max_clients already passed, idle_timeout failed, exactly as SCA-4719's eviction-coverage comment predicted:

>       assert svc._idle_timeout == DEFAULT_IDLE_TIMEOUT, bad
E       AssertionError: nan
E       assert nan == 600

Then green:

$ pytest tests/agent/lsp/test_client_bounds.py          -> 24 passed
$ pytest tests/agent/lsp/                               -> 154 passed, 5 failed
$ ruff check agent/lsp/manager.py agent/lsp/cli.py \
      tests/agent/lsp/test_client_bounds.py             -> All checks passed

The 5 failures are pre-existing and environmental, not from this change — git stash on the same commit reproduces the identical 5 (tests/conftest.py live-system guard blocking os.kill on a PID outside the test subtree, in test_client_e2e, test_diagnostics_field, test_stale_diagnostics).

Merge-order: this branch descends directly from origin/fix/sca-4688-type-aware-footprint (git merge-base --is-ancestor confirms), so it is clean onto the stack tip by construction; the sibling probe against #62 is above. scripts/pr_merge_order.py lives only on #75 and is not on this stack — the equivalent merge-tree probe was run by hand instead.

Ported from #50: test_non_finite_bounds_fall_back_instead_of_crashing_the_lsp_path, test_status_reports_the_bounds, test_human_readable_status_renders_the_bounds (driven through the real _cmd_status rather than a re-rendered copy of its format string), test_config_max_clients_zero_is_rejected_loudly, and test_user_guide_does_not_promise_a_disableable_cap as a pure regression guard — the guide was already correct.

Merge authority stays with the operator.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bd55c5a875

ℹ️ 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".

[pai-scaffolde]

Not merged — blocked on a real defect surfaced by landing #62 first.

Python tests / Run tests slice 5/8 fails:

AssertionError: idle_seconds=1787197454.6432443 is not an elapsed time
  — get_status and _last_used are reading different clocks
tests/agent/lsp/test_client_bounds.py::test_status_idle_seconds_reads_the_same_clock_as_last_used

Cause. #62 (fix(lsp): read a monotonic clock for idle bookkeeping) replaced
self._last_used[key] = time.time() with _idle_clock() (CLOCK_BOOTTIME) in all
three write sites. This PR adds get_status, which computes:

now = time.time()
...
"idle_seconds": max(0.0, now - self._last_used.get(k, now)),

Wall clock minus a monotonic stamp — hence the epoch-sized idle_seconds. In the
original stack #62 and #67 were siblings, so this branch forked before #62 and
never saw that change; the conflict only appears now that both are on main.

Fix. get_status should read _idle_clock() (the same clock _last_used is
stamped with) instead of time.time(). This PR's own new test then passes — it is
correctly catching the mismatch.

I did not apply that change: it is a behavioural fix to the live LSP reaper rather
than a rebase/lint, so it belongs to the author. Verified this is not an artifact
of the replay onto main — the replayed delta's file set matches this PR's file
set exactly (4 files), nothing dropped.

#77 stacks on this PR and is therefore also left open.

🤖 Generated with Claude Code

[pai-scaffolde]

Pushed a one-line fix to this branch while landing the LSP stack.

get_status() computed each client's age with time.time(), but
_last_used is stamped by _idle_clock() (CLOCK_BOOTTIME) since #62.
Once #62 landed on main, every reported idle_seconds became the gap
between the two epochs (~1.79e9) instead of an elapsed time — and this
PR's own guard test caught it:

FAILED tests/agent/lsp/test_client_bounds.py::test_status_idle_seconds_reads_the_same_clock_as_last_used
E  AssertionError: idle_seconds=1787197454.6432443 is not an elapsed time

The comment directly above the line already required both to read the
same clock; the change just makes the code match it (time.time() ->
_idle_clock()). No other behaviour touched.

This branch was also replayed onto current main (its stacked ancestors
landed as squashes). Verified the replay kept the full production delta —
the only file not carried over is contributors/emails/engineer@scaffolde.ai,
which main already has.