scoped_spawn_lost_user_bus() re-derived the user bus from an empty base env,
so it only ever looked under /run/user/<uid>. The worker itself is launched
with systemd_user_bus_env(worker_env), which honours a configured
XDG_RUNTIME_DIR. On a host whose bus lives outside the default runtime dir,
any unrelated systemd-run exit was therefore misread as "bus gone": the job
error named a missing bus that was still there, the cached scope verdict
flipped to False, and the next 60s of cron fires dispatched without cgroup
isolation.
The check now takes the spawn env, drops the bus address the spawn already
carried, re-derives from that, and decides on the DBUS_SESSION_BUS_ADDRESS
key rather than on dict truthiness (a non-empty env with no bus was never
"bus present").
Review finding: scoped_spawn_lost_user_bus used systemd_user_bus_env({}) instead of the worker's spawn env, so a configured XDG_RUNTIME_DIR made unrelated wrapper exits flip the scope cache to unscoped dispatch.
One TTL for both probe verdicts (the success-TTL constant collapses into
`_SYSTEMD_SCOPE_PROBE_TTL_SECONDS`), and the ack-wait loop consults
`scoped_spawn_lost_user_bus()` when a scoped dispatch exits before the
worker acknowledged: with `/run/user/<uid>/bus` gone the job error names
the missing bus and the enable-linger remedy instead of the wrapper's bare
`exit 1`, and the cached True flips so the next fire degrades to a direct
external subprocess rather than consuming another occurrence on a dead
wrapper. Contributor test trimmed to the revalidation invariant.
The early-EOF reaper made _finish_reader return without publishing when
wait() raises, so the session stays tracked for later reconciliation.
That is right for the pipe path (_reconcile_local_exit can still reap via
session.process), but PTY sessions have no session.process: when
ptyprocess.wait raises (waitpid ECHILD after isalive() already reaped the
child) the exitstatus is known, yet poll() reported "running" forever.
Only leave the session tracked when exit_code() is still None; otherwise
record the known status and finish as before.
Review finding: PTY session whose pty.wait raises stays in _running forever (fail-open regression vs main)
Drop the wait-timeout call-shape assertion; the invariants are that the
session records the real exit code and that a failed reap does not publish a
false completion.
Drop the no-op and still-running change-detectors (4 invariant tests remain:
pipe closed on finish, PTY closed on finish, poll still serves buffered
output, prune releases handles). The accretion-caps fake session now
carries process/_pty like the real dataclass, since prune reads them.
Widen #75162: _prune_if_needed() drops finished sessions (TTL expiry and
oldest-finished eviction at MAX_PROCESSES) — release their Popen/PTY
handles there too, covering sessions inserted into _finished without
passing through _move_to_finished(). The release helper is idempotent,
so double-close on the normal path is a no-op. Adds two tests: prune
releases handles of dropped sessions, and a still-running session's
pipe stays open.
Finished sessions retained their subprocess.Popen pipe objects (and PTY
masters) until the finished-process TTL (FINISHED_TTL_SECONDS, default 30
minutes) elapsed. Under heavy background churn — deployments, archivers,
watchers — finished-but-unpruned sessions accumulated one open pipe FD
each, exhausting the gateway process's file descriptor budget and
surfacing as a 'file descriptor limit' error on new background spawns.
The registry never rejects spawns (it prunes oldest-finished at
MAX_PROCESSES), so the real defect was the retained-handle leak, not a
registry-cap rejection. The fix closes each finished session's Popen
stdout/stderr/stdin streams and PTY master in _move_to_finished(), right
after the reader loop drains EOF. poll()/wait()/read_log() serve output
from the buffered output_buffer — never from the pipe — so the release is
lossless.
Tests: 4 new cases in TestFinishedHandleRelease — Popen pipes closed,
PTY closed, no-handle sessions safe, and poll() still serves buffered
output after the release. All 4 fail on main (reproduction) and pass
with the fix.
Execute the selected no-op rather than freeze its spelling, while rejecting
/bin/true to model the NixOS failure. Mark the regression Linux-only and
retain the current user-bus environment handling.
Consolidates the earlier NixOS scope-probe report and fix in #102587 with
the PATH-independent payload from #105436. The fallback resolver is not
needed when /bin/sh is used directly.
Co-authored-by: Scott Garrand <sgarrand@gmail.com>
A system-level gateway unit has no ordering against user@<uid>.service and
linger may be enabled after boot, so the bus can appear after the one-shot
adoption in run_gateway() ran. Derive XDG_RUNTIME_DIR/DBUS_SESSION_BUS_ADDRESS
fresh for the availability probe and every scoped spawn (cron worker, Kanban
worker, PTY/pipe terminal spawns, scope cleanup) so the 60s failure TTL can
actually recover. Refs #104893.
The spawn_local systemd test asserted the property was present; it now
asserts the invariant the fix establishes (no OOMPolicy= on a transient
scope), so a reintroduction fails here instead of on older systemd hosts.
Follow-up to #92164: the delta window now ends on a character boundary
(up to 3 trailing continuation bytes are held for the next poll), so
multibyte output no longer decodes to U+FFFD at the seam. Verified on
bash, dash and busybox sh; exhaustive-prefix regression test added.
The background-process poller for non-local backends ran `cat` on the
whole log file every two seconds, then threw away everything except the
part it had not seen yet. The offset it needed was already tracked one
line below, so the full read was pure waste.
Cost of one poll grew with the total output so far, which makes the cost
of a run grow with the square of its length. A job writing 10 MB over an
hour moved about 9 GB across the docker or SSH channel to deliver 10 MB
of output.
The poller now asks the shell for the file size and the bytes after the
offset in one command. Reading the size first and cutting the tail at
that same size keeps the two in step, so a file that grows mid-command
never sends a byte twice. A file that shrank was rotated or truncated,
so the offset drops back to 0 and the buffer is dropped.
The output buffer is now appended to rather than replaced, matching the
local reader loops, and the offset is counted in bytes because the shell
counts bytes.
Cross-platform hardening of @toprakeker's systemd cgroup isolation
(PR #71378, landed via #81264):
- Gate every scope-path branch on a new _IS_LINUX constant instead of
'not _IS_WINDOWS', so macOS (and any other POSIX platform) provably
never touches systemd code — no probe subprocess, no scope argv,
byte-identical legacy spawn.
- Unit tests: darwin no-op guarantee (no probe exec, no scope argv build,
legacy argv byte-identical, no unit recorded) and probe-returns-False
off Linux.
- New live Windows E2E (tests/tools/test_process_registry_windows_live.py,
wired into the on-demand windows-venv-e2e lane): real spawn_local on
windows-latest asserting jobs run exactly as before — spawned, output
captured, exit code correct, systemd path never reached even under
faked gateway identity.
Refs #70716, #71378.
When the configured Subagent Model is rejected by the provider (HTTP 400:
"<model> is not a valid model ID"), every subagent in a delegation batch dies
before doing any work, but the batch report only buried the cause inside each
per-task block. Detect the config-level case in the delegation batch renderer
(both the multi-task fan-out and single-task variants) and emit one actionable
notice at the top of the report naming the configured model + provider, and
pointing at Settings -> Advanced -> Subagent Model (hermes config get
delegation.model). The notice only fires when a result entry's error/summary
both matches a model_not_found phrase AND names the currently configured model,
so a stale task failing on a removed model isn't mis-attributed. Detection
loads the delegation config lazily and fails open (no notice) on any error.
When no fallback chain is configured, the notice calls out that no failover was
attempted. Renderer-only change: no changes to delegate_tool status derivation
or the result schema.
Closes#97654.
Factory Droid v0.175.0 made TaskOutput/TaskStop accept task-ID prefixes so
background tasks can be referenced without pasting the full ID. Hermes'
process tool had the same friction: every action required the exact
proc_<12-hex> session ID.
ProcessRegistry.get() now falls back to unique-prefix resolution when the
exact lookup misses: 'proc_4dae' or bare '4dae' resolves to
proc_4dae56ca81f6 when exactly one running/finished session matches.
Ambiguous or too-short (<4 suffix chars) prefixes still return None, so
callers keep their existing 'No process with ID ...' error and nothing is
ever picked arbitrarily. Exact IDs never pay the scan, and a full ID that
happens to prefix another always wins.
All process actions (poll/log/wait/kill/write/submit/close) route through
get(), so they all gain prefix support from the single change.
Add TestNotificationRedaction class with two tests:
1. test_completion_notification_redacts_secret — verifies _move_to_finished
redacts API keys in completion notifications before enqueueing
2. test_watch_match_notification_redacts_secret — verifies _check_watch_patterns
redacts secrets in watch_match notifications before enqueueing
These tests cover the gap identified in #43025 where the explicit process
tool path (poll/log/wait) was redacted but the automatic notification
delivery path was not.
* fix: warn agents off driving interactive console TUIs via pty on Windows
Driving 'gh auth login' (and other survey-style console TUIs) through a
pty background process on Windows silently hangs: these programs read
Win32 console key events via ReadConsoleInput, not the stdin byte
stream, so Enter keypresses submitted over process stdin never register.
The agent-visible symptom is a prompt frozen at 'Press Enter to open
browser...' while the user sees nothing, and a turn interrupt then kills
the process, invalidating any device code the user already entered on
github.com.
Two guidance fixes, both proven in a live session on Windows 10:
- agent/prompt_builder.py: extend _WINDOWS_BASH_SHELL_HINT to steer
agents toward non-interactive paths (flags, --with-token, config
files, curl-polled OAuth device flow) instead of answering console
prompts programmatically.
- skills/github/github-auth: document the pitfall and add the manual
OAuth device-flow procedure (curl against gh's public client_id,
poll for the token, finish with 'gh auth login --with-token'), which
succeeded first try after two interactive attempts hung.
* fix: send CRLF for Enter on Windows PTY submit; correct root cause in guidance
Review feedback (helix4u) was right on both counts:
1. Root cause correction. gh's 'Press Enter to open browser' prompt is
waitForEnter -> bufio.Scanner reading stdin, not a survey/console-API
prompt. The real bug is ours: submit_stdin appended a bare \n, and
through pywinpty/ConPTY a lone \n is not delivered as a line
terminator, so the child's blocking line read never returns. Verified
empirically against pywinpty 2.0.15 with a readline() child:
\n -> hang, \r -> line delivered, \r\n -> line delivered.
Fix: submit_stdin now appends \r\n for Windows PTY sessions (POSIX
PTYs and Popen pipes keep \n). Windows-only regression tests cover
the PTY and pipe branches.
2. Prompt hint rewritten: instead of claiming Windows console TUIs
cannot be driven, it now says to use process(submit) rather than raw
writes with bare \n, and to prefer non-interactive paths when a CLI
offers one.
3. Skill device flow rewritten as an executable script: parses the
device-code response, polls per the returned interval, handles
authorization_pending / slow_down (+5s per GitHub docs) /
expired_token / access_denied / unexpected responses, pipes the token
straight into gh without echoing it, and drops the undocumented
workflow scope (repo,read:org,gist is the documented minimum for
gh auth login --with-token). The pitfall note is narrowed to the
reproduced condition.
many tests patched sys.platform or a module's _IS_WINDOWS flag, then
ran on linux ci. the patch selects the branch under test, but the host
does not have the behavior the branch exists for. the test proves the
patch, not the platform. some gated assertions never ran on any host.
this commit adds three markers: linux_only, macos_only, windows_only.
a conftest hook skips a marked test on the other hosts, with a clear
reason. no test fakes a host now. two documented fakes remain
(android/termux, freebsd) because no ci runner exists for them.
each fake site got one of four treatments:
- gate it: the real host supplies the platform; mocks cover real
dependencies only, never host identity
- patch the module's own probe when the subject is the probe's consumer
- assert against the real host when the fake stood in for any non-x host
- delete the patch when it set the value the host already has
bare skipif(sys.platform != ...) guards became markers too. the lane
model skips these on linux and never imports them on windows, so they
ran on no host. platform parametrize tables are now one marked test
per os.
running on real hosts found real errors: a chrome-sandbox failure in
test_gui_command that main hides, and two windows failures fixed here.
the agents.md testing section now documents the policy.
- Remove dead use_systemd_scope = False assignment (leftover from
the old try/except pattern, immediately overwritten).
- Update stale log label supervisor= -> in_supervised_gateway=
to match the renamed variable.
- Convert autouse _mark_gateway_process fixture to opt-in
_gateway_identity so negative tests start from a clean slate
instead of undoing the fixture's env/PID mocks.
- Parametrize 4 near-duplicate negative tests (2 scenarios x
pipe/PTY) into 2 parametrized tests, reducing ~130 lines to ~80.
76 tests pass, ruff clean, net -32 LOC.
The #70716 regression fix changes popen_start_new_session from False to
True in the systemd-scope branch. Update the assertion in
test_wraps_in_systemd_scope_when_supervisor_and_available and the
docstring in test_systemd_post_spawn_failure_never_kills_gateway_process_group.
When Hermes runs as a systemd gateway with MemoryHigh/MemoryMax limits,
local background terminal commands (terminal(background=true)) inherit the
gateway's cgroup. A memory-heavy executor (Codex, tests, Node) can push
the whole cgroup past MemoryMax and trigger systemd-oomd to kill the
ENTIRE gateway — taking down the messaging control plane and silently
losing the active turn.
Root cause: tools/process_registry.py::spawn_local() uses
start_new_session=True (creates a process session/group, NOT a resource
cgroup). The spawned process tree stays in the gateway's systemd cgroup.
Fix: when running under a service manager (detected via the existing
is_gateway_supervisor_process() helper), wrap the pipe-mode spawn command
in 'systemd-run --user --scope --unit=hermes-worker-<id>' so the worker
gets its own transient cgroup. An OOM in the worker then kills only the
worker, not the gateway.
The systemd-run availability is probed once (a no-op /bin/true in a
transient scope) and cached, because the binary can exist on PATH while
the user D-Bus session is unavailable (system services, containers). If
unavailable, fall back to the current start_new_session=True behavior
with a debug log.
Scope: this covers the common background pipe-mode path. PTY mode
(PtyProcess.spawn) is left as future work — it uses a different spawn
mechanism and is used for interactive CLI tools where cgroup isolation
has additional considerations.
_write_checkpoint persisted s.command verbatim to ~/.hermes/processes.json.
Recovery only uses command for display/logging (the process is already
running; adoption re-validates PID + start time, never re-runs the
command), so masking is lossless.
kill_started_since duplicated kill_all's collect-under-lock/kill-outside-lock
loop line for line; it is now a thin delegate through new kill_all kwargs
(exclude_ids, source, consume_output). Public signatures unchanged — existing
callers and test monkeypatch seams keep working. kill_process's docstring now
names the deliberate consume_output=True exception for abandoned-turn reaping
so the deviation isn't 'fixed' later.
An agent turn can spawn a long-running background subprocess (e.g.
`next build`) and later be abandoned via inactivity timeout, /stop,
/new, or a client disconnect. Before this fix the gateway interrupted
the agent loop but never touched the subprocess: it kept running
inside the gateway's cgroup, unbounded, until memory pressure starved
the event loop and made every platform/cron look hung (#76115).
The process registry already knew how to kill a process tree — the
missing piece was per-turn ownership: nothing distinguished a process
that predates the turn (must survive), a process the turn started and
finished successfully (must survive), and a process an abandoned turn
left running (must be reaped).
- tools/process_registry.py: snapshot_running_ids() captures a turn's
starting baseline; kill_started_since() reaps only IDs created after
it, scoped to one task_id.
- gateway/turn_context.py: TurnContext carries process_task_id +
process_baseline so the timeout/interrupt paths can reach them.
- gateway/run.py: baseline is snapshotted right before the turn's
executor task starts; the inactivity-timeout path and the explicit
/stop|/new|disconnect interrupt path both reap via the same helper.
A daemon-thread watchdog backs up the asyncio-based timeout poll,
since a starved event loop is exactly the failure mode this bug
causes. The turn's own worker clears its ownership markers the
instant it finishes, closing a race where a /stop landing right
after normal completion could reap a background process the turn
deliberately left running.
Related but insufficient on their own: #37454 (cgroup ExecStopPost
reaper only fires on service restart) and #68915 (orphaned-pipe
grandchild detection, a registry bug not a turn-lifecycle gap).
Neither ties process cleanup to turn abandonment.
Port from openclaw/openclaw#112325: multibyte UTF-8 characters split
across a 4096-byte pipe or PTY read boundary were decoded statelessly
per chunk with errors='replace', corrupting both halves into U+FFFD
mojibake in background process output (poll/log/wait/completion
notifications). The foreground path already used an incremental decoder
(tools/environments/base.py::_wait_for_process); this applies the same
treatment to the background reader loops:
- _reader_loop (select and blocking paths): one
codecs.getincrementaldecoder('utf-8') per reader holds partial
sequences across chunks; the finally block flushes a truncated tail
as a single U+FFFD instead of dropping it.
- _pty_reader_loop: same treatment for ptyprocess byte chunks
(pywinpty str chunks pass through unchanged).
Genuinely invalid bytes keep errors='replace' behavior.
Second, deeper pass over tools/gateway/hermes_cli plus first pass over
the trees wave 1 missed (acp, acp_adapter, skills, computer_use, docker,
dashboard, conformance, monitoring, secret_sources, hermes_state,
providers). Same rubric as wave 1 (AGENTS.md test policy); security,
alternation/caching invariants, issue-number regressions, and E2E kept.
Real test-quality fixes found and rooted out along the way:
- tests/tools/test_command_guards.py made real auxiliary-LLM HTTPS calls
(DEFAULT_CONFIG smart-approval leaked in) — pinned approval
mode=manual via autouse fixture: 17.4s → 0.4s.
- test_model_switch_custom_providers.py / test_user_providers_model_switch.py
silently probed live provider catalogs (~2s/test) — stubbed
cached_provider_model_ids/provider_model_ids/fetch_api_models.
- test_telegram_noise_filter.py: 15-platform copy-paste matrix over
shared gateway.run logic → 3 representative platforms (55s → 3.9s).
- test_gateway_shutdown.py: stop()'s 5s interrupt-deadline loop spun on
MagicMock agents — interrupt.side_effect now clears _running_agents
(22s → 1.0s).
- test_gateway_inactivity_timeout.py poll-harness timings shrunk 3-5x
(24s → 1.1s); test_mcp_stability.py backoff/SIGTERM-grace sleeps
patched (15.4s → 2.5s); test_async_delegation.py negative-drain wait
5s → 0.5s.
- test_telegram_init_deadline.py: loop-block margin restored to 1.0s
with rationale comment — the watchdog-dump assertion needs the loop
blocked well past deadline+grace under parallel load (flaked once in
the 40-worker verification run at a 0.2s margin).
Verification: full hermetic suite via scripts/run_tests.sh —
2,438 files, 21,718 tests passed, 0 failed, 293.9s wall.
Suite totals vs original baseline: 46,820 → 19,757 test functions
(−57.8%), wall 583.5s → 293.9s (−50%), subprocess CPU 13,564s → 11,623s.
When a background terminal() command backgrounds its own long-lived
child (`node server.js &`, `sleep 300 &`), the grandchild inherits the
write end of the reader thread's stdout pipe. The direct bash child
exits promptly, but the pipe never reaches EOF while the grandchild
lives — so `_reader_loop`'s blocking `read1()` parked the thread
forever, `session.exited` never flipped on its own, and
`notify_on_complete` was silently lost. `_reconcile_local_exit`
(#17327) only runs lazily from poll()/wait(), so nothing autonomous
ever surfaced the exit; each occurrence also leaked a reader thread
and pipe fd for the grandchild's lifetime.
Fix: on POSIX, drain via select() with a short poll interval and stop
shortly after the direct child exits even if the pipe hasn't EOF'd —
the same pattern the foreground path uses in
tools/environments/base.py::_wait_for_process (#8340). Windows pipes
don't support select(), so the blocking path is kept there with the
existing lazy reconcile as the safety net; mocked/iterator stdout
streams (no usable fileno) also keep the historical path.
Fixes#68915
Follow-up to salvaged PR #70549:
- Replace fragile 'or' assertions with single precise checks that catch
partial-rewrite regressions (would have masked a missing closing brace)
- Add test_pty_path_uses_rewritten_command covering the PTY spawn path
that was modified but previously untested
Konstantin Khlopkov