fix(tests): stop the suite writing into the operator's real agent.log (SCA-4600) - #78
Merged
Merged
Conversation
The LSP tests were writing language-server lifecycle records into `~/.hermes/logs/agent.log`. That log is the only durable record of LSP client lifecycle — gateway uptime is short, so a live `ps` snapshot usually cannot answer "is LSP accumulation bounded" and the log is the instrument. Replaying `log_active`/`log_reaped` pairs over the polluted log reported 20 concurrent clients climbing with no balancing reaps; filtering the synthetic `$TMPDIR` roots collapsed that to 7 with spawns and reaps balanced exactly, against a ground truth of 0 live processes. The pollution did not merely inflate a number, it inverted the verdict that a build decision (an LRU cap) rested on. Two independent causes, both closed here: 1. `tests/conftest.py` sandboxed HERMES_HOME only when it was unset. So the hole was open for exactly the people most likely to hit it — anyone running the suite from inside a Hermes agent or gateway process, where HERMES_HOME is exported and points at the real root. There the sandbox was skipped, import-time `setup_logging()` attached rotating handlers to the real agent.log, and every propagating logger in the suite wrote into it. The sandbox is now unconditional. With HERMES_HOME unset (CI and default local runs) behavior is unchanged: both paths create the same tempdir sandbox. `tests/test_log_isolation.py` already guarded this property and was genuinely failing under a preset HERMES_HOME; it now passes. 2. The `caplog_lsp` fixture called `caplog.set_level(DEBUG, ...)`, which attaches caplog's handler but leaves propagation on, so records still climbed to root — and at DEBUG, so even the steady-state events the eventlog design deliberately keeps below INFO were written out. The fixture now disables propagation for the duration and restores it. Fixing the writer rather than filtering `$TMPDIR` at read time: the latter would leave the log corrupt for every other consumer and every future reader who does not know to filter. `hermes.lint.lsp` production verbosity is untouched — its INFO `log_active`/`log_reaped` lines are exactly what makes accumulation auditable. Making the sandbox unconditional put the NousResearch#69385 kanban regression back in reach by a new route: `_capture_real_kanban_root()` resolves via `get_default_hermes_root()`, which reads HERMES_HOME from the environment, which is now always rewired. It restores the pre-sandbox value for the duration of that call so the deny-list keeps pointing at the operator's real root. Covered by a new guard test. Verified with a positive control on the identical probe (`pytest tests/agent/lsp/ tests/test_cli_skin_integration.py` under `HERMES_HOME=~/.hermes`): 4848 bytes appended to the real agent.log before the fix, 0 bytes after. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codex review on PR #61 flagged the `caplog_lsp` fixture as breaking capture: pytest's `LogCaptureHandler` lives on the root logger, so setting `hermes.lint.lsp.propagate = False` should cut these tests off from their own records. The predicted failure does not occur — the file passes 7/7 at 93c1991, and the claimed repro (`pytest -q tests/agent/lsp/test_eventlog.py`) reproduces green. Under the pinned pytest 9.1.1, `caplog.set_level(level, logger=...)` also binds the capture handler to the named logger, so records are captured with propagation off. The concern behind it is still fair: that binding is a pytest implementation detail, not a documented promise, and the fixture was silently depending on it. This makes the capture path explicit and self-contained. It is a provable no-op today (pytest has already bound the same handler object, so `attached_here` is False and neither `addHandler` nor `removeHandler` runs) and becomes load-bearing only if that behaviour changes. Teardown removes only a handler this fixture added, leaving pytest's own handler lifecycle untouched. Verified: - tests/agent/lsp/test_eventlog.py 7 passed - tests/test_log_isolation.py 4 passed - tests/agent/lsp/ 61 passed - anti-pollution control re-run under `HERMES_HOME=~/.hermes` (`pytest tests/agent/lsp/ tests/test_cli_skin_integration.py`): 69 passed, 0 bytes appended to the real agent.log Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
test_sandbox_overrides_a_preset_hermes_home can only distinguish the
conditional sandbox from the unconditional one when HERMES_HOME is already
set at conftest import. Nothing in CI sets it, so with the conditional form
restored the `if not os.environ.get("HERMES_HOME")` branch is taken,
_SESSION_HERMES_HOME is bound anyway, and the guard passes on the exact code
it exists to reject.
Measured on this tree, reverting tests/conftest.py to the conditional form:
HERMES_HOME preset -> 2 failed, 2 passed (guard fires)
HERMES_HOME unset -> 4 passed (guard inert -- CI's case)
So the regression could be reintroduced and ship green.
Re-run the assertion in a child pytest that supplies the distinguishing
condition. Same revert now fails under CI conditions. The spawn is bounded
(timeout=300) so a wedged child fails the file instead of hanging a slice,
and a sentinel env var makes the spawning test skip itself in the child so
it cannot recurse.
This was referenced Aug 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Recovered from an orphaned local worktree during the 2026-08-15 disk/worktree sweep — three test-hardening commits closing SCA-4600 (LSP test suite writes into the production event log and inverts the accumulation):
Branch content was already pushed; only the PR was never opened. No overlap with the open #63–#77 stack (verified by patch-equivalence against every open PR head).
🤖 Generated with Claude Code