Skip to content

test: round-2 stability closeout — log-leak sandbox, order-dependence fix, write-guard hardening - #74553

Merged
teknium1 merged 4 commits into
mainfrom
salvage/test-stability-round2
Jul 30, 2026
Merged

test: round-2 stability closeout — log-leak sandbox, order-dependence fix, write-guard hardening#74553
teknium1 merged 4 commits into
mainfrom
salvage/test-stability-round2

Conversation

@teknium1

Copy link
Copy Markdown
Contributor

Summary

Closes out the test-stability backlog: the last 2 real fixes salvaged (log-file leak at collection time, order-dependent CLI flake), plus a hardening of the kanban write-guard merged in #74517 whose sys.modules probe raced lazy imports.

Changes

Also closed as covered by #74517 (verified empirically): #50699/#50681 (call-time db path), #43344 + #58612/#58609 (suite-wide Keychain guard).

Validation

Gate Result
Full suite (CI-parity runner) 2,460 files, 22,601 passed, 0 failed, 0 flaky
tests/cli both orders + full dir 27 + 723 green
Log-isolation + guard + kanban suites green ×3
Kanban deny-list under sandbox points at real ~/.hermes (probe)

Infographic

round 2 infographic

mehmetkr-31 and others added 4 commits July 29, 2026 19:26
…source

test_session_not_found_goes_to_stdout_in_full_mode passes in isolation but
fails in a full tests/cli run. Two independent leaks from the same neighbor
test conspire:

1. test_cli_init.py's _make_cli() reloads cli.py while prompt_toolkit is
   stubbed with MagicMocks and never reloads it back, so sys.modules['cli']
   is left with a mock _pt_print/_PT_ANSI and cli._cprint silently no-ops
   for every later test. Fixed by reloading cli once more with the real
   modules visible (try/finally).

2. prompt_toolkit's print_formatted_text caches its Output on the
   process-global default AppSession the first time it renders without an
   explicit output=. Under capsys (which swaps sys.stdout per test), the
   first CLI test to emit through _cprint locks that cache onto its own
   captured stdout, so later capsys tests read an empty buffer. Fixed with
   an autouse fixture in a new tests/cli/conftest.py that resets the cached
   output around each test.

Neither change touches production code or the flaky test's own assertion.

Related to #59358 (which addresses the same flaky test by mocking _cprint in
the assertion instead; this fixes the two underlying leaks at the source and
does not modify test_resume_quiet_stderr.py).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
hermes_cli/main.py calls setup_logging() at module scope. That resolves
get_hermes_home() and attaches rotating file handlers to the ROOT logger via
a QueueHandler. So merely importing it - which many test modules do, directly
or transitively - points the whole pytest session's logging at
<HERMES_HOME>/logs/agent.log and errors.log.

The _isolate_env fixture already sandboxes HERMES_HOME, but fixtures run
after collection has imported the test modules, and by then the handler holds
an absolute path to the real file. Verified by importing hermes_cli.main in a
clean interpreter and walking the queue listener: both handlers pointed at the
developer's own ~/.hermes/logs/.

Measured on a live install: 126 warnings in a personal agent.log came from
test runs rather than the running gateway - phantom 'FakeTree' Discord
registration failures and 'rejected invalid API key' entries whose paths only
exist in tests/gateway/test_api_server_runs.py. That noise makes genuine
warnings hard to find exactly when someone is debugging.

conftest is imported before any test module, so sandboxing HERMES_HOME there
closes the window. The per-test fixture still applies afterwards.

Also fixes 4 pre-existing failures: tests/gateway/test_channel_directory.py
TestBuildFromSessions was reading the operator's real sessions data for the
same reason.

Full gateway+tools suites: 66 failures on clean origin/main, 62 with this
change, 0 new. The regression guard asserts the value captured AT conftest
import - reading os.environ inside a test passes even with the fix removed,
because the per-test fixture has sandboxed it by then.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…les probe

The guard's sys.modules.get() can observe hermes_cli.kanban_db while a
lazy import is still executing on a fixture boundary — the partially
initialized module has no .connect yet (AttributeError flake in the
full-suite verification run, 1/2460 files). A half-imported module has
no callers to guard; skip this round and let the next fixture patch the
completed module.
@teknium1 teknium1 added the ci-reviewed applied to manually approve dangerous changes label Jul 30, 2026
@github-actions

github-actions Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

૮ >ﻌ< ა ci review

ran on 141bcf7

ℹ️ Info

Desktop E2E visual evidence · View test artifacts · View job

1 visual diff.

inline evidence upload failed.

Failed to upload diff-665a0833239e-onboarding-overlay-diff.png with gh image (exit code 1): Error uploading /home/runner/work/_temp/e2e-evidence/diff-665a0833239e-onboarding-overlay-diff.png: step 0 (get upload token): uploadToken not found on repo page — do you have write access to NousResearch/hermes-agent? (or, if NousResearch enforces SAML SSO, authorize at https://github.com/orgs/NousResearch/sso)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-reviewed applied to manually approve dangerous changes comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have type/test Test coverage or test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants