Review cleanups on the orphan-reap startup grace:
- Drop the docstring claim that the Desktop boot sweep is "the one caller
that races a launch": web_server._spawn_gateway_restart also reaps
(grace-less) before its coalesce check, so the claim was wrong.
- Cut the 7-line lifespan comment to one line; the full reasoning lives in
the _reap_unsupervised_gateway_orphans docstring, so the two can't drift.
- Drop the never-asserted seen["extra_exclude"] and the constant-only
`_REAP_MIN_AGE_SECONDS > 0` assert; a grace-less revert already fails the
`seen["min_age_s"] == _REAP_MIN_AGE_SECONDS` check.
Co-authored-by: Halldrix <halldrix@users.noreply.github.com>
Collapse TestReaperStartupGrace into a single test pinning the grace
invariant: a booting gateway and an undeterminable age are spared under a
positive grace while a stale orphan is still reaped. The no-grace default
is already covered by the existing reaper tests.
The standalone _gateway_process_age_s wrapper only re-wrapped
dashboard_procs._process_age_seconds in a try/except. Inline it as a local
fail-closed predicate (same shape as dashboard_procs._is_stale_orphan) so
the grace lives entirely inside the one reaper that uses it; an
undeterminable age still never widens the reap.
Co-authored-by: Halldrix <halldrix@users.noreply.github.com>
The historical_task instructions already explain that the compressor inserts
a bounded, redacted snapshot after generation; repeating it in the reverse
signal paragraph only adds prompt tokens.
The prior check only guarded one of the three quote-forcing directives the
fix removed; a partial revert of '<exact latest user request>' or 'write the
reverse signal verbatim' would reintroduce long-quote stalls unnoticed.
Also call the classmethod directly and hoist the patch import to module level.
A deterministic fallback summary replaced the older handoff in the transcript but never updated _previous_summary. The next compaction kept the stale in-memory summary and dropped the fallback row from its window, so the fallback's user asks, files and last dropped turns never reached the summarizer. Store the fallback body in _previous_summary the same way a normal summary is stored.
(cherry picked from commit 35417d2e1ffbb775c3eaff17b26623896afa56c1)
_write_full_zip_backup_locked chose clean/salvage/discard in _publish_path
and then re-derived the same choice with an inverse test after the with
block. If only one copy changed later, .stat() could hit a path that was
never published and raise out of a "never raises" helper. _publish_path now
records the destination and the stat/return reuse it.
The `destination is None` discard branch in _atomic_output_path had no
teeth: publishing the empty all-failed archive over out_path kept every
test green. The serialization test now asserts an all-failed automatic run
leaves the previous good archive's members unchanged. Also refresh a stale
comment that still described a renamed salvage archive.
Review cleanups on the incomplete-backup salvage path:
- The incomplete / nothing-salvaged warnings joined every per-entry
error into one log line; a broken tree can fail thousands of entries,
so log the first 10 plus "(+N more)".
- Drop the _entry_error helper: its per-entry logger.debug duplicated
the summary warning, so errors are now collected by a plain lambda.
- claw migrate: a None pre-migration backup can mean an incomplete run
whose salvage was kept, so point the user at the possible
pre-migration-*.incomplete.zip instead of claiming there is no
restore point at all.
- Wrap an overlong create_pre_update_backup docstring line.
No partial can land under the complete out_path name: publish is a
single os.replace from the hidden partial, and any failure (including
the replace itself) unlinks the partial in _atomic_output_path.
_write_full_zip_backup_locked published the partial archive to out_path via
_atomic_output_path and only then renamed it to the .incomplete.zip salvage
name, so an incomplete run destroyed a pre-existing good backup at out_path
(test_zip_captures_live_wal_and_cleans_failed_staging[True] regressed vs base).
_atomic_output_path now takes an optional publish_path callable evaluated at
publish time: the full-zip writer publishes the hidden partial straight to
out_path when clean, to the salvage path when some entries failed, and
discards it (returns None) when every entry failed, since an empty salvage
archive restores nothing. out_path is never touched on an incomplete run.
Incomplete automatic backups kept the normal <prefix><ts>.zip name, so they
counted toward retention: the next complete run pruned by count and deleted
the last complete backups, and repeated failing runs piled up. Rename them to
<stem>.incomplete.zip, exclude that suffix from _prune_prefixed_zips, and cap
salvage archives at one.
The failed member's bytes also stayed in the file behind a valid local header,
visible to streaming readers as a ghost entry (#124564). Truncate at the first
dropped header and rewind start_dir so later members overwrite it.
Also: name skipped paths in one merged warning, fix a stale comment, drop a
redundant str(), update None-return docstrings, remove an empty duplicate
section heading, and warn in claw migrate when no pre-migration backup was made.
Keep the automatic-backup partial-member test and the pre-update rotation
test; drop the duplicate-name central-directory test, which exercises the
same _discard_failed_zip_members boundary and pushes the stack past the
two-invariant-test budget for this salvage.
Why: the test parametrized a label string that a ternary in the body mapped
back to an exception, and its name still said "api_timeout" although it also
covers an APIConnectionError carrying the stall marker. Parametrize the two
exception instances directly (with ids) and rename the test to
test_transport_errors_stay_terminal_network_failure. No behaviour change; still
one parametrized test.
The isinstance(e, TimeoutError) guard was untested: an APIConnectionError
whose text contains the stall marker must stay a terminal network failure
(#29559/#94448). Parametrize the existing api-timeout test with that input
(red when the guard is removed), and move both #124077 tests into
TestStreamingClosedFailure reusing _fail_on_main instead of a duplicate helper.
The stall check relied on the lowercased error string, which only works
while CODEX_STREAM_STALL_MARKER happens to be all-lowercase. Match against
str(e) so the shared marker stays authoritative regardless of case, and
fold the two duplicate #124077 comments into one explaining the split
(stall -> retry-ladder timeout; transport timeouts stay terminal).
The zoneinfo/pydantic plugin warm-up imports were author-machine workarounds
that ran at collection time in CI; remove them. Collapse the class to one
helper and two tests: the Codex stall takes the timeout ladder without the
terminal network-failure flag, and APITimeoutError still sets it.
Co-authored-by: happy5318 <5318happy@users.noreply.github.com>
(cherry picked from commit 1cda73e0ac2d073066741a964b5166b16e9caf34)
The cherry-picked fix set streaming_closed=False for every timeout, which
also stripped the terminal abort-and-preserve-session behaviour from real
network timeouts (openai APITimeoutError, httpx Read/ConnectTimeout), the
deliberate #29559/#25585/#94448 design. Narrow it: only a TimeoutError whose
message says "stalled" (the Codex aux stream guard) becomes a timeout that
takes the 60/300/900s ladder and does not arm _last_summary_network_failure.
Other timeouts classify exactly as on main.
Co-authored-by: happy5318 <5318happy@users.noreply.github.com>
(cherry picked from commit e013c38a017b4709d4598a0a07c71ea26519312c)
## Thinking Path
When the Codex auxiliary stream guard aborts a compaction summary mid-stream
it raises `TimeoutError("Codex auxiliary Responses stream stalled: no new
output for 60.0s ...")`. The message contains neither "timeout" nor
"timed out", so `_classify_summary_failure` returned `timeout=False` while
`_is_connection_error` (which matches the type name "Timeout") returned
`streaming_closed=True`. The terminal network-failure flag then armed an
unconditional abort (`_TERMINAL_SUMMARY_FAILURES`), bypassing the retry
ladder and the deterministic fallback summary — on turn-start preflight
compression that ends in "Auto-resetting session after compression
exhaustion", wiping the session.
### What Changed
`agent/context_compressor.py` `_classify_summary_failure`:
- `timeout` is now computed first, and additionally matches `isinstance(e,
TimeoutError)` and the "stalled" message shape (the actual text the Codex
guard emits).
- `streaming_closed` is `_is_connection_error(e) and not timeout` — a
timeout keeps its retry-ladder semantics and can never arm the terminal
network-failure abort.
### Tests
New `TestSummaryFailureClassification124077` in
`tests/agent/test_context_compressor.py`:
- classify: Codex stall → `timeout=True, streaming_closed=False`.
- classify: plain `ConnectionError` stays `streaming_closed=True` (no
regression on the premature-close class).
- classify: a "timed out" message on a non-TimeoutError type stays a
timeout and is excluded from `streaming_closed`.
- integration: the stalled-summary path in `_generate_summary` does NOT arm
`_last_summary_network_failure`.
### Verification
- RED/GREEN double proof via git stash: pre-fix 3 failed, post-fix 4/4 pass.
- Regression: 15 existing failure-classification tests pass
(network_failure / premature_stream / empty_content / auth / truncation).
- ruff clean on changed files.
### Notes
Local test env: this checkout's venv python is a symlink into
`<home>/hermes-agent/.hermes-runtime/...`, so stdlib `zoneinfo` first-import
and pydantic's plugin `distributions()` scan under the real-home IO guard
needed the collection-time warmups at the top of the test file. CI
interpreters are not symlinked into the home — those two warm-up blocks are
no-ops there.
## Related
#124077 (issue). Family: #124078 (the same stall's template trigger),
#108104 (`auxiliary.compression.no_progress_timeout`).
(cherry picked from commit c5399fbdee44f2db1172f22632cbf348a2c7cab5)
(cherry picked from commit bce26a99791c09911d8a008a1922c512f5c1fcea)
The byte-stability test for the handoff block only re-asserted determinism of
a constant gated on tool-name membership; stable-tier rebuild stability is
already covered by test_system_prompt_restore and test_skills_auto_load. Its
one unique check (block appears exactly once) moves into the positive branch
of the parametrized injection test, and the _prompt helper now takes only the
tool names since every caller used the same model/gates.
The rationale comment lived twice (prompt_builder constant and the
system_prompt call site); keep only the call-site ordering note.
Keep the stack at <=2 invariant tests: the guidance is injected only when
delegate_task is in the toolset (and after the generic keep-working
blocks), and the stable prompt tier stays byte-identical across rebuilds
so the prompt-cache prefix does not drift.
Since 4317ed0e71 the heartbeat schema allows 0 (disabled) and clamps
positive values to 60, but the validation error still said "min 60",
steering models away from the valid 0. Flagged on #119202.
elide() already returns short text unchanged, so the eval runner's length
guard was a second copy of the same check. The timeout-diagnostic test
read its log as utf-8-sig although the writer never emits a BOM, and it
interrupted a stub child that never ran.
Keeps #122392's test_timeout_diagnostic_marks_long_goal_as_non_original,
moved into the existing timeout-diagnostic test file and using its
fixture/stub instead of a new file. Red on base (bare marker).
Co-authored-by: Halldrix <12357213+Halldrix@users.noreply.github.com>
The subagent timeout diagnostic and the session_search eval harness still
appended a bare "...[truncated]" marker, the imitable wording #121548
replaced everywhere else. Route both through agent.compression_marker.elide
so every elision in the tree mints the same counted, guard-matched marker.
Salvaged from #122392 (only the two call-site hunks; base already ships
the elide helpers the PR re-defined). Refs #121572.
Packaged windows claim `com.nousresearch.hermes` as their Wayland app id
(electron-builder bakes product-identity.cjs's `appId` into
extraMetadata.desktopName; Electron hands that string to the compositor
verbatim), while the entry was written as `hermes.desktop` with
`StartupWMClass=Hermes` — so GNOME matched neither StartupWMClass nor a
`<app_id>.desktop` file name and every launch fell back to the placeholder
icon, with the raw app id in the tooltip.
- write `<app_id>.desktop` with `StartupWMClass=<app_id>` (Name= stays "Hermes")
- retire a leftover `hermes.desktop` once the new entry is on disk, and only
when the file still names this app and launcher management is enabled;
foreign files at that path are left alone
- nix/desktop.nix derives the entry file name from the module instead of
hardcoding it
- tests: the installed entry carries the app id, legacy retirement, foreign-file
preservation, opt-out preservation, plus a node probe asserting
APP_ID == product-identity.cjs appId
Known consequence: an existing taskbar pin points at the old entry id and has to
be re-added once.
Let the shared status field builder accept an explicit owning home and have the
multiplexed TUI/Desktop session.status path pass its session profile_home.
Unscoped CLI/gateway callers keep the historical process-home fallback.
Add focused coverage for a secondary-profile session and for the launch-profile
fallback.
Fixes#124500.
Clicking the project-group trunk + passed the null wire path through, which
downstream treats as the reserved Home/detached signal, silently creating a
global session. Fall back to the first repo root for path-less explicit
projects; Home keeps null. Covers click and new-session drag cwd.
clipboardPasteFrom was called with unbounded clipboard text; the gateway's
RfbClientFilter closes the display WebSocket on any ClientCutText over
_MAX_CUT_TEXT (256 KiB), turning an oversized paste into a full stream
disconnect instead of a dropped paste. Clamp before forwarding and add a
test pinning the ceiling.
Take over over a bot's remote screen never sent the client's text
clipboard to noVNC: screen-pane.tsx built the RFB session but had no
paste handler and never called clipboardPasteFrom, so Cmd/Ctrl+V into
a remote input did nothing (#123089). The gateway already gates
ClientCutText as lease-checked input, so the missing leg was purely
client-side.
Add an explicit-gesture `paste` listener on the canvas, forwarded only
while this viewer's RFB client is not view-only (i.e. holds the
lease); no polling, no clipboard logging. Listener is torn down with
every detach so it never outlives its RFB client.
The hermes-media://stream resolveLocalFile boundary ran
resolveMediaRequestPath on the path the protocol handler had already
percent-decoded and slash-stripped (parseMediaProtocolTarget), so the
second decode+strip turned every absolute POSIX media path
(/home/.../.hermes/...) into a cwd-relative one. The resolver then
pinned it under the app's cwd and the file ENOENTed into the handler's
silent 404 — inline TTS/audio playback dead with no log line, which is
the Linux symptom in hermes-agent 123823. A second decodeURIComponent
also threw URIError on filenames containing a literal %.
Restore the single decode at the protocol layer: the boundary now only
bridges (resolveMediaStreamFile), matching what 7722761581 intended as
byte-for-byte behavior but broke when it rewired main.ts.
Fixes https://github.com/NousResearch/hermes-agent/issues/123823
Opening a deleted/renamed artifact reported "No application found to open
URL": macOS LaunchServices answers a non-existent path with
kLSApplicationNotFoundErr (-10814), and the open route handed resolved
paths to the OS without ever stat-ing. On current main the file route is
reveal-in-folder only, which is worse on a miss — showItemInFolder of a
non-existent path is a silent no-op on macOS, so the click does nothing
at all.
- hardening.ts: assertExistingPathForOpen — pre-open stat wired into
openExternalFile before the reveal-dedupe window is touched (a miss
must not poison dedupe against a legitimate retry). Only
ENOENT/ENOTDIR become an honest missing-file failure.
- external-open.ts: reportPreOpenStatFailure — the guard's
classification lives in the electron-free module with the rest of the
open logic, so it unit-tests without electron. A miss is reported and
the route stops; every other stat failure (EACCES on a locked volume,
ELOOP, Windows EPERM, ENAMETOOLONG) is logged and the OS is still
asked, so an existing-but-locked file keeps its real error — a stat
failure never fabricates a miss and never swallows the click.
- preload.ts: the failure dialog merged in #122023 had no preload
forwarder for hermes:external-open-failed, so every open failure was
silently swallowed in every window (the optional-chained bridge is
invisible to typecheck). Adds the onExternalOpenFailed listener plus a
test tripwire so the dead wiring cannot come back.
- The failure payload gains code: 'missing-file'; the dialog shows
localized "File not found" copy for it instead of the no-browser
text (i18n types + en).
Fixes https://github.com/NousResearch/hermes-agent/issues/122027
Co-authored-by: Andrew Ho <andrewho.sf@gmail.com>
Co-authored-by: Enough1122 <10966420+Enough1122@users.noreply.github.com>
A session.reclaimed (or roster-activity) wake re-resolves the open bot
chat so the next send doesn't eat a stale runtime id — but it went through
the full navigating open, so with the Kanban board (or any other route)
in the foreground the route flipped to the Bot Chat. Background events
must offer, not hijack.
host.openSession gains refreshInPlace: the wake still consults the
registry, dials the owner backend, and stamps the owner hint, but never
calls the core open (route/tile), never re-publishes the bots workspace
scope, and never flips the all-profiles view. It refreshes through the
same levers the SDK's own hydration probe uses — the tile delegate's
resumeTile(refreshTranscript), or the armed requestSessionResume that
the route-resume effect consumes only while the route already points at
the session. A chat that isn't on screen re-opens nothing.
openBotCanonicalChat threads this as background: true, and a background
re-resume never MINTS a missing registry row. The reclaim listener and
the roster-activity refresh both pass it.
Fixes https://github.com/NousResearch/hermes-agent/issues/121874
MarkdownLink dropped formatted link labels: childrenToText only handled
plain strings, so an inline-code label ([`v1.0.1`](url)) fell through to a
title fetch or a URL-slug fallback that title-cased the identifier.
Flatten element children so the authored label wins with exact casing,
and keep the casing of separator-less slug tokens that look like
identifiers (digits, dots, mixed case) instead of title-casing them.
Fixes https://github.com/NousResearch/hermes-agent/issues/121321
