mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-22 16:25:58 +00:00
* feat(attribution): conflict-free contributor mappings via contributors/emails/ directory
The AUTHOR_MAP dict in scripts/release.py was a merge-conflict magnet:
every concurrent salvage PR appended entries to the same lines of the
same file, so parallel PRs re-conflicted on every merge to main.
New system: one file per email under contributors/emails/ — filename is
the commit-author email, first non-comment line is the GitHub login.
File additions never conflict, so any number of PRs can add mappings
concurrently.
- scripts/release.py: AUTHOR_MAP is now LEGACY_AUTHOR_MAP (frozen)
merged with the directory at import time (directory wins). All
existing consumers (resolve_author, contributor_audit.py) unchanged.
- scripts/add_contributor.py: idempotent CLI to add a mapping; refuses
conflicting reassignments (incl. against the legacy map), validates
email/login shapes.
- contributor-check.yml: attribution gate now accepts a mapping file OR
a legacy entry; failure message prints the exact add_contributor
command. Also auto-resolves bare <login>@users.noreply.github.com
emails is intentionally NOT added (kept id+login form only, matching
previous behavior).
- contributor_audit.py: guidance now points at add_contributor.py.
- tests/scripts/test_contributor_map.py: 12 tests covering loader,
merge precedence, CLI idempotency/conflict/validation, subprocess E2E.
* feat(ci): one-shot per-file flake retry in the parallel test runner
A failing test FILE is re-run once in a fresh subprocess. Pass-on-retry
counts as green but is loudly reported in a '⚠ FLAKY' summary section
(with both attempts' output preserved) so the flake gets fixed instead
of eating a full-run rerun. Deterministic failures fail both attempts —
regressions cannot be laundered green.
- --file-retries N / HERMES_TEST_FILE_RETRIES (default 1, 0 disables)
- E2E verified: simulated first-run-fail flake goes green with banner;
deterministic failure still exits 1; retries=0 restores old behavior.
This converts the dominant CI failure mode (one timing-sensitive test
flaking a 4600-test shard, requiring a manual 10-minute rerun and an
agent triage loop) into a self-healing retry that costs one file's
runtime.
* test(approval): loosen wall-clock perf bounds 0.15s -> 2.0s
These guard against catastrophic regex backtracking (seconds-to-minutes
class), but 0.15s is within scheduler-stall noise on loaded shared CI
runners — test_max_accepted_separator_free_input_is_fast failed a CI
shard this week on runner load alone. 2.0s still catches the regression
class with zero flake surface.
* fix(ci): job timeouts everywhere + retries on all network installs
Reliability pass over every workflow:
- timeout-minutes on all 21 jobs that lacked one (a hung job previously
burned the 6-hour default runner budget)
- ./.github/actions/retry wrapped around every network-fetching install
that lacked it: pip installs (deploy-site, skills-index), npm ci
(deploy-site website, upload_to_pypi web + ui-tui), uv sync (docker
test deps). Deterministic build steps (npm run build) deliberately
NOT retried — split into separate steps so a real build failure fails
fast instead of retrying 3x.
* docs(agents): document the file-retry flake policy
* fix(ci): curl retries on deploy hook + skills-index probe
* fix(ci): kill the remaining transient-failure classes in workflows + Dockerfile
From the workflow reliability audit:
- tests.yml: duration-cache restore had NO restore-keys while saves use
run_id-suffixed keys — the cache never matched once, so LPT slicing
always ran blind and unbalanced slices pushed heavy files toward the
per-file timeout. One-line restore-keys fixes slice balancing.
- Label gates (lint ci-reviewed, supply-chain mcp-catalog-reviewed):
'gh pr view || true' turned an API blip into 'label absent' → false
BLOCKING failure. Now 3x retry, and API failure is reported as an API
failure instead of a missing label.
- detect-changes action: compare API retried before failing open (was
silently running all lanes on any blip).
- uv-lockfile-check: 'uv lock --check' resolves against PyPI — retried
so registry blips don't read as 'lockfile stale'.
- docker.yml merge job: imagetools create retried (Docker Hub eventual
consistency on just-pushed digests).
- Dockerfile: apt-get Acquire::Retries=3; s6-overlay ADDs converted to
curl --retry 3 (ADD cannot retry; checksums still enforced); npm
--fetch-retries=5; playwright chromium fetch retried 3x.
- Advisory artifact uploads (per-slice durations, ci-timings report)
get continue-on-error so an artifact-service blip can't fail a green
test slice.
* fix(tests): kill the two root-cause flakes — leaking pre-warm timer + env-dependent provider list
- test_tui_gateway_server.py: session.create / non-eager session.resume
arm a 50ms threading.Timer (_schedule_agent_build) that outlives its
test and fires into the NEXT test's _make_agent mock, racily
corrupting captured state (the recurring session_resume shard
failures). Replaced the per-test whack-a-mole stub with a module-wide
autouse fixture; the 3 worker-lifecycle tests that genuinely need the
deferred build opt back in via @pytest.mark.real_agent_prewarm (new
marker in pyproject).
- test_api_key_providers.py: PROVIDER_ENV_VARS is now derived from the
live PROVIDER_REGISTRY instead of a hand-list that had drifted
(missing HF_TOKEN / DEEPINFRA_API_KEY) — resolve_provider('auto')
tests failed on any machine with HF_TOKEN exported. E2E-verified with
HF_TOKEN/DEEPINFRA_API_KEY set: 42/42 pass.
* test: de-flake 30 timing-sensitive test files for loaded CI runners
Root-cause fixes from the flake audit (session-DB mining + repo sweep):
Event-based sync instead of sleep-sync:
- title_generator: mock sets threading.Event, wait(10) replaces
sleep(0.3) hoping the daemon thread got scheduled
- docker zombie_reaping / profile_gateway: poll-for-state helpers
replace fixed 1-3s sleeps (s6 transitions + SIGCHLD reaping are async)
- process_registry tree test: select()-bounded readline replaces an
unbounded blocking read (parent wedge now fails THIS test with a clear
message instead of an opaque rc=124 file kill); SIGTERM grace 1s->2s
(the 1s partition window mid-interpreter-startup is how a child PID
escaped the live-system guard in CI)
Timeout raises (loaded 8-way-sliced runners see ~5s scheduling floors;
all of these complete in ms-to-1s when healthy so the raises cost
nothing on green runs):
- subprocess/thread waits <= 2s raised to 10-15s across mcp_tool,
mcp_circuit_breaker, mcp_reconnect_retry_reset, mcp_parked_self_probe,
mcp_cancelled_error_propagation, registry, clarify_gateway, interrupt,
voice_cli_integration, docker_environment, session_store_lock_io,
planned_stop_watcher, cli_interrupt_subagent, thread_scoped_output
(joins now also assert not is_alive() so stragglers fail loudly)
- wall-clock discrimination ceilings loosened where the guarded hang is
10x larger: local_background_child_hang 4s->10s, interrupt_cleanup
setup 5s->20s + pgid-exit 30s->60s, mcp_stability grandchild spinup
5s->15s, protocol/gil-starvation fast-handler 0.5s->2s,
iso_certify_seam 1.5s->5s, wait_for_mcp_discovery 0.1s->1s
- narrow assertion windows widened: honcho first-turn wait 0.4..0.65 ->
0.25..2.0 (property is bounded-not-hung, not an exact wall-clock);
compression fork-lock TTL 1s->3s (12 refresh chances per lease);
compression-lock expiry margins symmetric (ttl 0.05->0.5, sleep 1.0)
- telegram hung-DNS bound 1.0->1.4 (fake hang is 1.5s — must stay under)
* fix(tests): repair indentation from de-flake batch edit
* fix(tests): harden env isolation and replace remaining sleep-sync races
The full 42k-test run and complete npm check surfaced three more classes:
- Environment isolation: local ~/.honcho defaultHost and SSH_* variables
leaked into Python/TUI tests. Pin the default Honcho host in the
hermetic fixture, isolate the one fallback test from ~/.honcho, and
blank SSH_* around terminalSetup tests. This flipped 20 false failures
back to deterministic behavior on developer machines.
- Background-thread sleep-sync: Honcho async writer tests patched
time.sleep globally, then busy-polled with that same mocked sleep. Under
full-suite load the poller could starve the writer. Each test now waits
on an Event emitted by the exact flush/retry transition; 30/30 passed
under 15-way contention.
- Desktop streaming: the test slept 80ms and assumed a 500ms timer could
not fire before its assertion. A loaded runner descheduled the test for
>500ms and both chunks arrived. Producer controls now gate second-chunk
and completion transitions explicitly.
Also make file-retry observability complete: a self-healed flaky file now
prints BOTH attempts' full output in the FLAKY summary. Two behavioral
runner tests prove pass-on-retry is green+loud+traceback-preserving, while
a deterministic failure remains red.
* refactor(ci): use gh bot pat, better retries
refactor(ci): use retry action for PR label fetch
the retry action now captures stdout as a step output, so it can serve
double duty: retry + output capture for commands like 'gh pr view' whose
result must be consumed by later steps.
Retry action gains:
- 'stdout' output (heredoc-delimited to preserve newlines)
- tee to temp file so stdout still streams to the job log
- step id 'retry' for output reference
Both lint.yml and supply-chain-audit.yml now use the retry action
directly with 'command: gh pr view ...' and read
steps.<id>.outputs.stdout.
ci: use AUTOFIX_BOT_PAT for all gh CLI / GitHub API auth
Replace secrets.GITHUB_TOKEN and github.token with
secrets.AUTOFIX_BOT_PAT across all workflows and composite actions
that use the gh CLI or GitHub API. The PAT has consistent permissions
across fork PRs (where GITHUB_TOKEN is read-only), avoids API rate
limit sharing with the default token, and is already used by
js-autofix.yml for the same reasons.
19 sites swapped across 9 files:
- lint.yml (3): label fetch, comment post/edit, comment update
- supply-chain-audit.yml (5): scan, critical comment, unbounded dep
comment, label fetch, mcp-catalog comment
- lockfile-diff.yml (1): PR comment post/update
- skills-index-freshness.yml (1): issue creation on degraded probe
- skills-index.yml (2): index build, trigger deploy workflow
- upload_to_pypi.yml (2): release view poll, release upload
- ci.yml (1): timings report
- deploy-site.yml (2): skills index crawl
- detect-changes/action.yml (1): compare API call
---------
Co-authored-by: ethernet <arilotter@gmail.com>
228 lines
8.9 KiB
Python
228 lines
8.9 KiB
Python
"""Regression tests for _wait_for_process subprocess cleanup on exception exit.
|
|
|
|
When the poll loop exits via KeyboardInterrupt or SystemExit (SIGTERM via
|
|
cli.py signal handler, SIGINT on the main thread in non-interactive -q mode,
|
|
or explicit sys.exit from some caller), the child subprocess must be killed
|
|
before the exception propagates — otherwise the local backend's use of
|
|
os.setsid leaves an orphan with PPID=1.
|
|
|
|
The live repro that motivated this: hermes chat -q ... 'sleep 300', SIGTERM
|
|
to the python process, sleep 300 survived with PPID=1 for the full 300 s
|
|
because _wait_for_process never got to call _kill_process before python
|
|
died. See commit message for full context.
|
|
"""
|
|
import os
|
|
import signal
|
|
import subprocess
|
|
import threading
|
|
import time
|
|
from types import SimpleNamespace
|
|
|
|
import pytest
|
|
|
|
from tools.environments import local as local_mod
|
|
from tools.environments.local import LocalEnvironment
|
|
|
|
|
|
@pytest.fixture(autouse=True)
|
|
def _isolate_hermes_home(tmp_path, monkeypatch):
|
|
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
|
(tmp_path / "logs").mkdir(exist_ok=True)
|
|
|
|
|
|
def _pgid_still_alive(pgid: int) -> bool:
|
|
"""Return True if any process in the given process group is still alive."""
|
|
try:
|
|
os.killpg(pgid, 0) # signal 0 = existence check
|
|
return True
|
|
except ProcessLookupError:
|
|
return False
|
|
|
|
|
|
def _process_group_snapshot(pgid: int) -> str:
|
|
"""Return a process-table snapshot for diagnostics."""
|
|
return subprocess.run(
|
|
["ps", "-o", "pid,ppid,pgid,stat,cmd", "-g", str(pgid)],
|
|
capture_output=True,
|
|
text=True,
|
|
check=False,
|
|
).stdout.strip()
|
|
|
|
|
|
def _wait_for_pgid_exit(pgid: int, timeout: float = 60.0) -> bool:
|
|
"""Wait for a process group to disappear under loaded xdist hosts.
|
|
|
|
The cleanup chain is: SIGTERM → 3s TimeoutStopSec → SIGKILL → reap.
|
|
Under heavy xdist load (40 parallel workers, 6-shard CI), the full
|
|
sequence can exceed 10s. Default timeout is generous to avoid CI
|
|
flakes; in practice the wait returns in <1s on quiet hosts.
|
|
"""
|
|
deadline = time.monotonic() + timeout
|
|
while time.monotonic() < deadline:
|
|
if not _pgid_still_alive(pgid):
|
|
return True
|
|
time.sleep(0.1)
|
|
return not _pgid_still_alive(pgid)
|
|
|
|
|
|
def test_kill_process_uses_cached_pgid_if_wrapper_already_exited(monkeypatch):
|
|
"""If the shell wrapper exits before cleanup, still kill its process group.
|
|
|
|
Without the cached pgid fallback, ``os.getpgid(proc.pid)`` raises for the
|
|
dead wrapper and cleanup falls back to ``proc.kill()``, which cannot reach
|
|
orphaned grandchildren still running in the original process group.
|
|
"""
|
|
env = object.__new__(LocalEnvironment)
|
|
proc = SimpleNamespace(
|
|
pid=12345,
|
|
_hermes_pgid=67890,
|
|
poll=lambda: 0,
|
|
kill=lambda: None,
|
|
)
|
|
killpg_calls = []
|
|
|
|
def fake_getpgid(_pid):
|
|
raise ProcessLookupError
|
|
|
|
def fake_killpg(pgid, sig):
|
|
killpg_calls.append((pgid, sig))
|
|
if sig == 0:
|
|
raise ProcessLookupError
|
|
|
|
monkeypatch.setattr(os, "getpgid", fake_getpgid)
|
|
monkeypatch.setattr(os, "killpg", fake_killpg)
|
|
|
|
env._kill_process(proc)
|
|
|
|
assert killpg_calls == [(67890, signal.SIGTERM), (67890, 0)]
|
|
|
|
|
|
def test_kill_process_uses_windows_tree_kill(monkeypatch):
|
|
"""Windows must kill the whole Bash process tree, not just the wrapper."""
|
|
env = object.__new__(LocalEnvironment)
|
|
terminate_calls = []
|
|
waits = []
|
|
killed = []
|
|
|
|
def fake_terminate(pid, *, force=False):
|
|
terminate_calls.append((pid, force))
|
|
|
|
proc = SimpleNamespace(
|
|
pid=12345,
|
|
kill=lambda: killed.append(True),
|
|
wait=lambda timeout=None: waits.append(timeout),
|
|
)
|
|
|
|
monkeypatch.setattr(local_mod, "_IS_WINDOWS", True)
|
|
monkeypatch.setattr("gateway.status.terminate_pid", fake_terminate)
|
|
|
|
env._kill_process(proc)
|
|
|
|
assert terminate_calls == [(12345, True)]
|
|
assert waits == [2.0]
|
|
assert killed == []
|
|
|
|
|
|
def test_wait_for_process_kills_subprocess_on_keyboardinterrupt():
|
|
"""When KeyboardInterrupt arrives mid-poll, the subprocess group must be
|
|
killed before the exception is re-raised."""
|
|
env = LocalEnvironment(cwd="/tmp")
|
|
try:
|
|
result_holder = {}
|
|
proc_holder = {}
|
|
started = threading.Event()
|
|
raise_at = [None] # set by the main thread to tell worker when
|
|
|
|
# Drive execute() on a separate thread so we can SIGNAL-interrupt it
|
|
# via a thread-targeted exception without killing our test process.
|
|
def worker():
|
|
# Spawn a subprocess that will definitely be alive long enough
|
|
# to observe the cleanup, via env.execute(...) — the normal path
|
|
# that goes through _wait_for_process.
|
|
try:
|
|
result_holder["result"] = env.execute("sleep 30", timeout=60)
|
|
except BaseException as e: # noqa: BLE001 — we want to observe it
|
|
result_holder["exception"] = type(e).__name__
|
|
|
|
t = threading.Thread(target=worker, daemon=True)
|
|
t.start()
|
|
# Wait until the subprocess actually exists. LocalEnvironment.execute
|
|
# does init_session() (one spawn) before the real command, so we need
|
|
# to wait until a sleep 30 is visible. Use pgrep-style lookup via
|
|
# /proc to find the bash process running our sleep.
|
|
deadline = time.monotonic() + 20.0 # generous: init_session + spawn dilate under CI load
|
|
target_pid = None
|
|
while time.monotonic() < deadline:
|
|
# Walk our children and grand-children to find one running 'sleep 30'
|
|
try:
|
|
import psutil # optional — fall back if absent
|
|
for p in psutil.Process(os.getpid()).children(recursive=True):
|
|
try:
|
|
if "sleep 30" in " ".join(p.cmdline()):
|
|
target_pid = p.pid
|
|
break
|
|
except (psutil.NoSuchProcess, psutil.AccessDenied):
|
|
continue
|
|
except ImportError:
|
|
# Fall back to ps
|
|
ps = subprocess.run(
|
|
["ps", "-eo", "pid,ppid,pgid,cmd"], capture_output=True, text=True,
|
|
)
|
|
for line in ps.stdout.splitlines():
|
|
if "sleep 30" in line and "grep" not in line:
|
|
parts = line.split()
|
|
if parts and parts[0].isdigit():
|
|
target_pid = int(parts[0])
|
|
break
|
|
if target_pid:
|
|
break
|
|
time.sleep(0.1)
|
|
|
|
assert target_pid is not None, (
|
|
"test setup: couldn't find 'sleep 30' subprocess after 5 s"
|
|
)
|
|
pgid = os.getpgid(target_pid)
|
|
assert _pgid_still_alive(pgid), "sanity: subprocess should be alive"
|
|
|
|
# Now inject a KeyboardInterrupt into the worker thread the same
|
|
# way CPython's signal machinery would. We use ctypes.PyThreadState_SetAsyncExc
|
|
# which is how signal delivery to non-main threads is simulated.
|
|
import ctypes
|
|
# py-thread-state exception targets need the ident, not the Thread
|
|
tid = t.ident
|
|
assert tid is not None
|
|
# Fire KeyboardInterrupt into the worker thread
|
|
ret = ctypes.pythonapi.PyThreadState_SetAsyncExc(
|
|
ctypes.c_ulong(tid), ctypes.py_object(KeyboardInterrupt),
|
|
)
|
|
assert ret == 1, f"SetAsyncExc returned {ret}, expected 1"
|
|
|
|
# Give the worker a moment to: hit the exception at the next poll,
|
|
# run the except-block cleanup (_kill_process), and exit. Under
|
|
# xdist load the SIGTERM → 3s wait → SIGKILL chain can take longer
|
|
# than 5s before the worker's join() returns; bumped to 15s.
|
|
t.join(timeout=30.0)
|
|
assert not t.is_alive(), "worker didn't exit within 30 s of the interrupt"
|
|
|
|
# The critical assertion: the subprocess GROUP must be dead. Not
|
|
# just the bash wrapper — the 'sleep 30' child too. Under xdist load,
|
|
# process-group disappearance can lag briefly after the worker exits,
|
|
# especially if the process is already dying or waiting to be reaped.
|
|
assert _wait_for_pgid_exit(pgid), (
|
|
f"subprocess group {pgid} is STILL ALIVE after worker received "
|
|
f"KeyboardInterrupt — orphan bug regressed. This is the "
|
|
f"sleep-300-survives-SIGTERM scenario from Physikal's Apr 2026 "
|
|
f"report. See tools/environments/base.py _wait_for_process "
|
|
f"except-block.\n{_process_group_snapshot(pgid)}"
|
|
)
|
|
# And the worker should have observed the KeyboardInterrupt (i.e.
|
|
# it re-raised cleanly, not silently swallowed).
|
|
assert result_holder.get("exception") == "KeyboardInterrupt", (
|
|
f"worker result: {result_holder!r} — expected KeyboardInterrupt "
|
|
f"propagation after cleanup"
|
|
)
|
|
finally:
|
|
try:
|
|
env.cleanup()
|
|
except Exception:
|
|
pass
|