The initial (pre-running) connect awaited during gateway startup now uses
a capped 45s budget for Telegram instead of the full 180s (#67498) budget.
On timeout the platform is queued for the reconnect watcher, which retries
with the full budget and is_reconnect=True (preserving the offline update
queue, #46621). Combined with the parallel startup connects, an unreachable
Telegram no longer holds the whole gateway out of the running state.
The previous concurrency assertion (slow_start < fast_end) was true under
BOTH the serial and parallel implementations, so it proved nothing -- it
even passed against the old serial code on main. The only assertion that
distinguishes the two is that the fast platform finishes before the slow
one (fast_end before slow_end), which is only possible when the connects
overlap.
Switch the test to record connect start/end events in arrival order
(clock-resolution independent) and assert fast_end precedes slow_end. This
also fixes the Windows failure @zuowen7 reported: time.monotonic() has only
~15 ms resolution there, so two parallel connects could land on the same
tick and defeat any wall-clock comparison -- event ordering cannot.
Verified the new test fails against origin/main (serial) and passes against
this branch (parallel).
GatewayRunner.start() previously awaited each platform's connect() (with its
own timeout) in a serial for-loop. A single slow/failing platform (e.g.
Telegram behind a dead proxy) delayed every later platform's connect by a full
timeout window, cascading one platform's failure onto WeChat/QQ/etc.
Now the slow connect() calls run concurrently via asyncio.gather while the
serial pre-filter (checks, adapter creation, handler wiring) and the
single-threaded result aggregation (shared-state mutation, error handling)
are unchanged. A failing platform no longer blocks the others.
Adds regression tests proving connect() calls overlap and that one failing
platform leaves the others connected.
Layer 2 of the #81163 / #78050 fix: _get_platform_tools computed
plugin_ts_keys = _get_plugin_toolset_keys() but only used
CONFIGURABLE_TOOLSETS in the explicit-config filter, so a user-listed
plugin key like `a2a` in `platform_toolsets.cli: [hermes-cli, a2a]` was
silently dropped. The filter now unions configurable and plugin toolset
keys when evaluating has_explicit_config and when admitting per-key
entries.
Cherry-picked from PR #81190 (Layer 2 hunks only; Layer 1 is covered by
the provides_tools mechanism from PR #78842).
Rebased onto current main. `hermes_cli/plugins.py` grew 103KB -> 265KB
across 49 commits since the original branch point, and the attribution
mechanism this change hooks into was replaced along the way: the
`_tools_before` / `_plugin_tool_names` snapshot diff is now a
registration ledger sliced from `registration_start`, and `_plugin_id`
is `plugin_key`.
Re-anchored accordingly:
- Discovery-time pre-registration, module reuse, and the `provides_tools`
opt-in are unchanged.
- Attribution credits `_predeclared_tools` ahead of the ledger slice,
since those tools registered before `registration_start` and the slice
cannot see them.
- A failed materialization no longer carries attribution across. The
failure path now sweeps the whole ownership ledger for the plugin key,
not just the `registration_start:` slice, so the pre-registered tools
are disposed along with the adapter. Attribution and the registry now
agree at zero instead of reporting tools the process is not serving.
tests/hermes_cli/test_deferred_platform_client_tools.py 13/13.
test_plugins.py, test_plugins_cmd_list.py, test_plugin_cli_registration.py
65/65.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Follow-up on the salvaged #85764 commits, addressing review findings:
- _session_left_live_context now allowlists end_reason == 'compression'
or a fresh reset (_FRESH_RESET_END_REASONS) instead of accepting any
non-None end_reason. The wide predicate let 'branched' parents — whose
transcript /branch verbatim-copies into the child — surface as
same-lineage recall hits, returning content already in the caller's
live context (verified empirically vs main).
- _FRESH_RESET_END_REASONS is now derived from the canonical
hermes_state_common._RESET_END_REASONS (plus CLI 'new_session') instead
of a third hand-maintained copy, per that tuple's anti-drift comment.
Import verified cycle-free.
- Browse drops the Python re-check of parent_session_id rows:
list_sessions_rich (include_children=False) already applies the
canonical _LISTABLE_CHILD_SQL classifier, and the Python re-check
re-hid legacy pre-marker reset children the SQL same-key heuristic
deliberately admits. _has_reset_from_marker (now orphaned) removed.
- Tests: branched-parent exclusion regression guard (mutation-checked:
fails on the overbroad predicate) + legacy pre-marker reset child
browse guard. 48/48 pass.
The salvaged regression from #86582 predates the claim_job_for_fire
owner-fencing that landed with #70638; mock the claim and heartbeat so
the healthy job actually runs through the fenced flow.
Agent crons resolve OAuth credentials before the agent loop. A short
macOS/WARP DNS blip raised httpx.ConnectError ([Errno 8] nodename nor
servname provided) from xai-oauth token refresh, and the scheduler only
walked fallback_providers on AuthError — so Daily Focus Kickoff died
even when XAI_API_KEY / Anthropic were healthy.
Treat ConnectError/DNS OSError (and cause-chain equivalents) like
AuthError when selecting the fallback chain. Keep provider+model atomic.
Regression test covers the ConnectError path.
Review follow-up (egilewski): the previous commit only hedged the guidance
text; the exact Anthropic 400 was still classified, persisted, and surfaced
as confirmed billing exhaustion. Carry the ambiguity all the way through:
- agent/error_classifier.py: 'out of extra usage' matches on the 400 and
status-less paths now attach error_context {billing_unverified,
possible_content_filter}. Reason stays FailoverReason.billing (rotation +
fallback remain the right recovery either way); ClassifiedError grows a
billing_unverified property.
- agent/credential_pool.py: new FAILURE_REASON_BILLING_UNVERIFIED. An
unverified billing exhaustion gets the short transient cooldown instead of
the one-hour bench, regardless of pool size: a content-filter rejection
leaves the credential healthy and fails identically on every key, and the
hour-long sole-credential latch is what replayed the stored error and made
real fixes look ineffective. A true 402 keeps the full bench. The marker
persists with the entry so a restart cannot upgrade it back to a bench.
- agent/agent_runtime_helpers.py + run_agent.py: recover_with_credential_pool
threads billing_unverified and hands the pool 'billing_unverified' as the
persisted failure_reason.
- agent/conversation_loop.py: the fallback-switch status, max-retries status,
terminal label, and both structured terminal results hedge when the verdict
is unverified. New _billing_terminal_label + _billing_failure_result build
the returned terminal response in one place; the result dict now carries
billing_unverified and the billing_block gains 'unverified': true. The
confirmed-billing path (a real 402 or an API-key credit depletion) keeps
the original assertive wording, so the caveat no longer dilutes it.
Regression tests: classifier marking (400 + status-less + unambiguous-body
negative), pool cooldown TTLs + persistence round-trip, pool failure_reason
plumbing, and the returned terminal response for both unverified and
confirmed verdicts.
Note: tests/agent/test_credential_pool_routing.py::TestFailureAttribution::
test_unmatched_key_does_not_retry_only_pool_entry fails identically on
current main without this change (pre-existing, unrelated).
On an Anthropic subscription OAuth credential, every request failed with
HTTP 400 "You're out of extra usage. Add more at claude.ai/settings/usage".
That is not a billing condition: Anthropic's server-side content filter rejects
the first sentence of Hermes' own built-in SKILLS_GUIDANCE prompt, and the
rejection is surfaced with a billing-shaped message. Because the message points
at the usage settings page, it reliably sends people to buy quota they do not
need — the reporter lost three debugging sessions to it.
Bisected against the live API with the real 71,721-char assembled prompt: the
first SKILLS_GUIDANCE sentence alone reproduces the 400 and removing it alone
clears it. Size was ruled out (20 KB of unrelated filler returns 200) and so was
the system[0] identity gate (that returns 429, a different failure).
Three changes, all serving the same outcome — a subscription user can no longer
be misdirected by this 400:
- agent/prompt_builder.py: reword the triggering sentence to the phrasing the
reporter verified returns 200. Meaning, the skill_manage reference, and the
## Skill Safety Rule block are all preserved. The reword is empirically
validated rather than understood, so a comment records the bisect and warns
that any rewrite must be re-verified against an OAuth token, not an API key.
- agent/conversation_loop.py: the Anthropic branch of the billing guidance no
longer asserts exhaustion as fact. It hedges the opening line, names the
content-filter alternative, and gives the operator a way to tell the two apart
(if the usage page still shows quota, suspect a content rejection). It also
points at `hermes auth reset anthropic`, because the credential exhaustion
latch replays the stored error for ~60 min without issuing a request — which
makes a real fix look like it did not work.
- hermes_cli/auth.py: document that CLAUDE_CODE_OAUTH_TOKEN is an OAuth token,
not an API key, despite auth_type="api_key". It stays in api_key_env_vars
because that tuple doubles as the credential-discovery list; removing it would
stop Hermes finding a `claude setup-token` credential at all.
Docs updated to match the reworded prompt.
Fixes#82154
Simplify-code pass: gateway/run.py called estimate_messages_tokens_rough
6x on the same data in the anti-growth guard (condition + warning f-string).
Bind to _hyg_in_toks/_hyg_out_toks locals like the conversation_compression.py
guard already does. Also trim the comment from 10 lines to 4 (keep the WHY,
drop the WHAT) and remove an extra blank line before TestCompactedTurnsStaySearchable.
The gateway rotation guard (#83339) only protects the rotate path, but
in-place compaction commits inside compress_context() via
archive_and_compact — before the gateway can inspect the result. Add the
anti-growth check at the commit site so both paths are covered: a
compression whose rough output exceeds its input is a strict no-op
(original transcript kept durable, session identity untouched).
Covers the observed failure where session hygiene persisted 426 -> 426
messages and ~379K -> ~688K tokens.
The relative exclude-newer = "14 days" cutoff bricks installs whenever the
resolver cannot see (or accept) a package's upload date:
- defusedxml / python-olm / unpaddedbase64 (#80387, #79434): ancient frozen
releases (2021-2023) whose upload dates are often absent from mirror
indexes and stale uv HTTP caches. uv then filters them entirely
("there are no versions of defusedxml"), breaking [youtube]/[wecom]/
[matrix] resolution and daily `uv sync --locked` runs.
- setuptools / pillow / mcp (#78227, #75992, #76020): exact-pinned deps.
When the pinned version's upload date is invisible, the resolver filters
the ONLY acceptable candidate — setuptools==83.0.0 in
[build-system].requires meant the project could not even be built from a
git checkout on released v0.20.0. Exempting an exact pin costs nothing:
the version cannot float without a reviewed pin bump.
Changes:
- pyproject.toml: add all six to the existing exclude-newer-package
whitelist, with rationale comments per class.
- uv.lock: regenerated; diff is the whitelist metadata only (verified
zero version drift, still 249 packages).
- tests/test_packaging_metadata.py: new standing guard
test_build_system_requires_exempt_from_exclude_newer — every
[build-system].requires package must be whitelisted while a relative
exclude-newer cutoff is configured. Verified both directions (fails
when setuptools is removed from the whitelist).
- scripts/install.sh: fix the stale tier-name comparison ("all (with
RL/matrix extras)" vs actual "all") that mislabeled every successful
Tier-1 install as a fallback-tier install (#79434 bonus finding).
Verification: uv lock --check green on uv 0.11.19 and 0.12.5;
uv sync --extra all --locked green; uv pip install -e '.[all]' resolves;
whitelist mechanism A/B-proven on a minimal project (unsatisfiable ->
resolves; build-requires variant: uv build fails -> succeeds).
Reported-by: MichaelClawHub (#80387), liujianqiu (#79434), maxonliu (#78227)
Pre-creates a 0o6755 target, imports a member over it, and asserts the
published file is 0o755 with both elevated bits gone — plus that the
staged temp file never carried them either, so there is no window where
archive content sits behind an elevated mode.
The existing coverage in this class cannot see the failure: every mode
assertion masks with ``& 0o777``, which discards exactly the bits at
issue, and the fixtures chmod their targets to ordinary modes that never
had them set. Without the mask on the preserved mode this test reports
the published file still holding S_ISUID.
Skipped where the platform or filesystem refuses setuid on a user-owned
file, so the assertion never depends on running as root.
Follow-up on the atomic-import restore, delegating both metadata concerns to
the shared helpers instead of half-handling them locally.
Owner preservation was missing entirely. `tempfile.mkstemp` + `atomic_replace`
publishes a temp file owned by the *writing* user, so `sudo hermes import`
re-owned every restored file to root — on the disaster-recovery path, and on
exactly the Docker/NAS volume installs `utils._restore_file_owner` was added
for. `_extract_member_atomically` now captures `_preserve_file_owner(target)`
before staging and calls `_restore_file_owner` after the replace, before the
mode restore (chown clears setuid/setgid, so the mode has to go back last).
Mode handling was also only half applied before the replace: the `os.fchmod`
branch applied it to the temp fd, but the platforms without `fchmod` fell
through to a best-effort post-replace chmod, leaving the published file at
mkstemp's 0600 until that chmod landed — permanently if the process died in
between — and making `atomic_replace`'s EXDEV/EBUSY `shutil.copystat` fallback
copy 0600 onto the target. The mode is now applied to the temp file on both
branches, with the post-replace `_restore_file_mode` kept as the belt-and-
braces path.
This is the same shape `atomic_write_text` and `atomic_yaml_write` already
carry after 3556728a5 and 43fc86562; capture and restore now reuse
`utils._preserve_file_mode` / `_preserve_file_owner` / `_restore_file_mode` /
`_restore_file_owner` rather than re-deriving them, which also drops the local
`import stat`.
Tests (tests/hermes_cli/test_backup.py, class TestImportAtomicWrites):
- test_restore_preserves_existing_file_owner — forces a uid/gid so it does not
need root; asserts chown fires once, with the captured owner, on the
pre-existing file only (a newly created member has no prior owner).
Mutation-checked: dropping only the `_restore_file_owner` call reds it.
- test_mode_is_applied_before_the_replace_without_fchmod — `monkeypatch.delattr`
on `os.fchmod`, spies the temp file's mode at replace time. Reads 0o600
without the fix, 0o644 with it. Mutation-checked the same way.
`hermes import` wrote every zip member with `open(target, "wb")` followed by
`dst.write(src.read())`, at both restore sites in `run_import`. Opening for
write truncates the user's existing file to zero *before* any replacement
bytes exist, so a Ctrl-C, an ENOSPC, a corrupt zip member, or a crash leaves
`config.yaml`, `.env`, or an external provider config (e.g.
`~/.honcho/config.json`) empty with nothing behind it — during the
disaster-recovery path the user is running precisely because they already
lost something. The `_external/` branch writes outside HERMES_HOME, into
third-party configs under the user's home, so the blast radius is not
confined to Hermes state.
Both sites now stage the member into the target's own directory, fsync it,
and publish with `utils.atomic_replace`, so the target only ever moves from
its old contents to the complete new contents.
`atomic_replace` rather than a bare `os.replace`: it resolves a symlinked
target first, so deployments that link `config.yaml` into a dotfiles repo
keep the link instead of having it silently swapped for a regular file
(#16743), and it falls back to copy/fsync/unlink on EXDEV/EBUSY for
cross-device and bind-mount installs. Members stream through
`shutil.copyfileobj` instead of being read whole into memory. The temp file
is removed on any failure so a partial import leaves no residue, and
permission bits are carried across the replace so mkstemp's 0600 does not
silently tighten restored files.
This extends the module's own established idiom — `backup.py` already
publishes atomically via `os.replace` in `_atomic_output_path` and in the
snapshot writer — into the one path that still overwrote user files in place.
mark_running_jobs_interrupted skipped legacy fires without a registered
durable owner entirely — correct for the persisted last_status write
(no owner fence to protect a replacement run), but the gateway shutdown
path also uses the returned ID list to deliver interrupted-cron notices
while adapters are still connected (#82232). Keep the persistence skip,
but include the job in the returned list so the user is still told.
When the shutdown drain times out and kills an in-flight cron job, the
job's owner is never told. The cron worker does try: `_is_interrupted()`
forces the failure path with an honest "interrupted by gateway shutdown"
error, and failed jobs always deliver. But that worker is a thread, it
reaches `_deliver_result()` asynchronously, and by then
`_bounded_adapter_teardown()` has closed the transport. The reporter of
Worse, the loss is silent twice over: `_consume_interrupted_flag()`
returns True — the gateway already wrote `last_status` — so
`mark_job_run()` is skipped, and the `delivery_error` from the failed
send is discarded with it. The run's only trace is a generic line in
jobs.json.
The gateway already owns the right window. `_notify_active_sessions_of_
shutdown()` runs while adapters are up, precisely so shutdown messages
can be sent — but it iterates `_running_agents`, and cron work lives on
the scheduler's own thread pool. Same structural blindness already fixed
for counting (#60432) and draining (#63529), never fixed for notifying.
So notify from the post-interrupt phase, which is the last point where
the transport is still up: `_kill_tool_subprocesses()` now returns the
job IDs it marked, and `_notify_interrupted_cron_jobs()` sends each one's
owner a notice on the job's own resolved delivery targets. Adapter
teardown order is untouched — it is load-bearing for #53175 and #8202.
Jobs with `deliver: local`, and `deliver: origin` jobs with no resolvable
origin (#43014), resolve to zero targets and stay silent. Per-platform
`gateway_restart_notification: false` is honoured, matching the chat
path. Every failure is swallowed so a wedged adapter cannot extend
shutdown.
Second, when the interrupted flag short-circuits `mark_job_run()`, the
delivery failure is now persisted on its own via `update_job()`, so a
notice that still cannot be sent is at least recorded. `update_job()`
rather than a second `mark_job_run()`: the latter also advances
`next_run_at` and the repeat counter, and running that twice for one run
would skip a fire or auto-delete the job early.
Fixes#82232. Related: #82161, #82224.
CI slice 5/12 caught two ways the new cron budget broke `_stop_impl_body`
for callers that are not real GatewayRunner instances:
- `_FakeGateway` in test_shutdown_cache_cleanup.py borrows `_stop_impl`
without subclassing, so it never picked up the class-level
`_cron_drain_timeout` default and raised AttributeError. Read it through
the getattr-guard convention the same function already uses for its
liveness-guard machinery.
- The same double overrides `_drain_active_agents(self, timeout)`, so
passing the cron budget raised "takes 2 positional arguments but 3 were
given". The double now mirrors the real optional parameter. It is the
only override in the tree; test_startup_restart_race.py uses AsyncMock,
which accepts any signature.
Verified against a stashed clean tree: the 22 gateway test files that
still fail locally fail identically with and without this branch (80 = 80,
empty set difference both ways) — they are pre-existing Windows-only
failures (setsid, POSIX modes) unrelated to this change.
`agent.restart_drain_timeout` defaults to 0 and governed every class of
in-flight work at once. That default is deliberate for chat turns: the
gateway announces the restart to the user and pre-marks the session
resume_pending, so interrupting one is cheap and recoverable.
A cron run has neither property. Nobody is waiting on it, it is written
to jobs.json as a permanent failure, and a recurring job simply skips to
its next schedule. Sharing the chat budget meant `_drain_active_agents()`
short-circuited on `timeout <= 0` before entering the wait loop, so the
drain reported `drain took 0.00s, timed_out=True, cron_at_start=1,
cron_now=1` — it detected the job and killed it anyway.
Cron work now drains on its own deadline, `agent.cron_drain_timeout`
(default 30s, 0 opts out). The floor is clamped to the shutdown-watchdog
leash minus a teardown reserve, so the longer wait can never consume the
post-drain cleanup window: being SIGKILLed mid-cleanup would leave the
job wedged at `last_status=running`, strictly worse than the bug. Being
bounded also means a cron-triggered restart cannot deadlock on itself.
The `timeout <= 0` special case is gone — an expired deadline expresses
the legacy "interrupt immediately" behaviour, so `timed_out` is always
computed from real state instead of asserted up front. The drain-timeout
warning now reports the elapsed wait rather than the configured budget,
which is what made "timed out after 0.0s" so confusing in the report.
Chat-only shutdowns are unchanged: `restart_drain_timeout: 0` still
interrupts chat turns immediately.
Relates to #82161 (complements #82195, which removes the `hermes update`
self-deadlock that triggered the reported instance).
Filter manual dashboard/serve respawn candidates after update: skip
ephemeral --port 0 backends (Desktop-owned), dedupe normalized cmdlines,
and cap one restart per profile/HERMES_HOME so orphan counts no longer
grow across successive updates.
Compose the service-PID exclusion (#85743, RelaxJonh) and the recorded-PID +
parent-chain exemption (#86100, arccat-114) into one cross-platform rule:
- _get_service_pids() exclusion now runs unconditionally, not only under
is_macos() — it is the authoritative "supervised" signal for launchd and
any systemd unit visible on a host that got past the systemd gate.
- The recorded-healthy-gateway (get_running_pid()) + parent-chain exemption
now runs on every platform, not only Windows. A recorded, liveness-verified
gateway is by definition not an orphan "the pidfile/runtime record can't
see", so the reaper must never target it — this covers Windows Scheduled
Task / Startup VBS supervision, standalone launcher-started gateways
(the case #85743 alone would miss), and macOS/WSL equivalents.
True orphans (no service registration, no valid runtime record) are still
found and reaped, preserving the #51325/#75936 duplicate-port protection.
Existing macOS regression tests updated to pin get_running_pid to None for
their scenario; Windows regression tests from #86100 carry over unchanged.
Bug class: #83683 (root), #86287, #86098, #85738, #85368, #85344, #85044,
#84855, #84824, #84200.
The orphan reaper kills a healthy gateway (and its Scheduled-Task bootstrap
parent chain) every time the Desktop backend starts on Windows, because
_get_service_pids() only implements systemd/launchd and returns an empty
set on Windows — a supervised gateway is therefore indistinguishable from
an unsupervised orphan.
Exempt the recorded healthy gateway PID and its parent chain from the
orphan scan on Windows, mirroring the macOS launchd exemption (#85913).
The Scheduled-Task bootstrap's argv matches the gateway scan, so without
exempting the parent chain killing the bootstrap takes the detached
gateway down with it.
Fixes#86098
- tests/cron/test_sessiondb_init_hang.py: add threading/time imports the
salvaged late-close regression tests rely on.
- tests/test_hermes_state.py: drop
test_close_closes_wal_read_connection_created_on_worker_thread — main
replaced per-thread WAL reader ownership with the pooled read-connection
design (permits + checkout/return), so cross-thread reader draining no
longer exists in the form the test asserted.
run_job() submits SessionDB() to a one-worker executor and abandons the
worker (shutdown(wait=False)) when init exceeds the cron timeout. If the
constructor later completes inside that abandoned worker, the Future's
result — an open SessionDB holding .db/WAL/SHM handles — was orphaned and
never closed, leaking descriptors until EMFILE. Attach a done-callback on
the timeout path that retrieves and closes any eventual late result.
Salvage note: the lazy-recall ownership half of #72822 (_owns_session_db
tracked on AIAgent, owned handle closed in close()) already landed on main;
this carries the remaining cron timeout-abandon half with its regression
test.
SessionDB could leave native SQLite handles open when construction failed
partway through schema/pragma/FTS/repair/lock/interrupt handling. Other
short-lived callers (MCP reads/polling, session search, reactions, trace
upload, insights, shutdown recovery) opened temporary SessionDB handles
without a complete ownership boundary. API-server profile caches and
RetainDB shutdown had similar late-close races. Under sustained load this
exhausted file descriptors (EMFILE).
- Close partially initialized SessionDB connections on every constructor
exception path via a finally block guarded by an initialization-complete
flag.
- Close temporary/cross-profile SessionDB handles in finally blocks across
CLI, MCP, search, trace, reactions, insights, and recovery paths.
- Add API-server per-profile cache ownership and disconnect cleanup.
- Make RetainDB writer-queue shutdown exception-safe: track connections per
thread, close on worker exit, reject new enqueues after shutdown starts,
and sweep any connections left by short-lived threads.
- Add regression coverage for constructor failures, worker-thread readers,
API disconnect failures, shutdown recovery, RetainDB late enqueue, and
foreign-loop async clients.
Salvage notes: the original PR's per-thread WAL-reader ownership changes
were superseded by main's read-connection pool (permits + checkout/return);
its cron timeout-abandon fix is credited separately to #72822's earlier
identical fix.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A turn writing against a session already closed by compression died with
session_persistence_failed and a misleading "this is often a full disk"
dialog, even though the store was healthy and a live continuation existed
(#82001). Depth-1 recovery (find_live_compression_child) could not resolve
lineages with >=2 compression hops (root -> mid -> tip), reproduced
independently on two- and three-hop chains.
- run_agent.py flush chokepoint: on CompressionSessionClosedError, resolve
tip = db.get_compression_tip(old_id) (canonical bounded transitive walk),
adopt only when tip != old_id AND the tip row is live, retry the flush
exactly once (adoption budget); otherwise fail closed.
- gateway/session.py append_to_transcript: replace the depth-1 live-child
lookup with the same tip + liveness contract, so gateway transcript
reroutes follow full chains.
- agent/conversation_compression.py _adopt_live_compression_child: turn-start
recovery preflight now resolves via get_compression_tip with the same
liveness check, closing the last depth-1 consumer in this family.
- classify_persistence_error: new "compression_closed" bucket; the turn-end
explanation names compression rotation and tells the client to refresh the
session id instead of blaming a full disk.
Tests: depth-1 adoption, multi-hop chain adoption (agent + gateway), fail
closed with no continuation / stale-closed (ws_orphan_reap) tip, exactly-once
adoption budget, and error-wording guards (compression-closed never mentions
disk; real disk failures keep disk guidance).
Closes#82001
Co-authored-by: Al3xand3r1987 <125030427+Al3xand3r1987@users.noreply.github.com>
Co-authored-by: yuzilongleif-collab <235949691+yuzilongleif-collab@users.noreply.github.com>
A bare SessionDB() resolves the launch profile's default state.db, but
parents can hold non-default per-profile handles (tui_gateway opens
SessionDB(db_path=<profile_home>/state.db) for non-launch profiles and
hands them to agents via _transfer_db_to_agent). A child of such a
parent would write its transcript into the WRONG database — cross-
profile leakage that breaks parent_session_id lineage and
session_search. Open the dedicated handle at the parent handle's
db_path instead (AsyncSessionDB forwards .db_path via __getattr__, so
the gateway wrapper path works too). Regression test verified RED on
the pre-fix code.
Review follow-up (cc3f18197): if AIAgent() raises inside _build_child_agent
the freshly-opened dedicated handle has no owner and no child close() will
ever run — release it on the exception path so the sqlite fds don't
outlive the failed spawn. Also pin the degradation contract with a test:
a parent without a SessionDB still yields session_db=None children.
Cron run_job closes its per-job SessionDB in its finally block while a
fire-and-forget background delegation subagent is still flushing on a
daemon thread. The child shared the parent's SessionDB object, so every
subsequent flush hit the closed handle ('NoneType' object has no
attribute 'execute') and the child's whole transcript was silently
dropped. The same teardown-while-child-alive shape exists on gateway
session end and /new mid-delegation.
Each child now opens its own SessionDB connection (owned flag set at
construction so child.close() releases it), so no parent teardown can
close the child's handle out from under it.
Regression test proves the child gets a distinct live handle that
survives the parent's close().
On macOS (256 soft fd limit), routing the weixin/email pollers through a
local HTTP proxy leaked one TCP socket per failed poll/connect cycle
until the gateway hit `[Errno 24] Too many open files` and crashed
(launchd respawn loop). Live capture showed 216 of 256 fds pinned on
connections to the proxy, ~214 of them abandoned.
Code-side gaps fixed:
- email adapter, `connect()`: no try/finally around the IMAP test
connection — a failure in login/ID/select/search abandoned the
connected socket with no owner. Every reconnect-watcher retry builds
a fresh adapter, so each retry against an unreachable/proxied host
leaked another fd. Teardown now runs in `finally`.
- email adapter, IMAP teardown: `imaplib.IMAP4.logout()` only swallows
`OSError` internally; on a broken connection `LOGOUT` raises
`IMAP4.abort` before the internal `shutdown()`, leaving the socket
open. New `_close_imap()` helper chases a failed `logout()` with an
unconditional `shutdown()`; used in `connect()` and
`_fetch_new_messages()`.
- weixin adapter: repeated poll failures through a proxy strand
sockets in the aiohttp connector where the tight keepalive reaper
never sees them. The poll loop now recycles its ClientSession
(swap-then-close, safe for concurrent `_process_message` tasks)
after each MAX_CONSECUTIVE_FAILURES streak, tearing down the
connector and every socket it holds.
Targeted tests: tests/gateway/test_poller_fd_lifecycle.py (9 tests).
Reported by @EthanHunter1229 with measured fd captures.
After a client replacement (credential rotation, dead-connection cleanup,
or fallback+restore), agent.client may become a native OpenAI client
while agent.provider stays "moa". The _moa_prepared_request key was
passed through to the native SDK, causing TypeError on every turn.
Pop the key at the dispatch point (chat_completion_helpers.py:509).
The MoAClient facade already handles a missing key by falling through
to its normal resolution path.
Closes#78382
`_moa_prepared_request` is a private handshake between the conversation
loop and MoAChatCompletions.create. It is attached whenever
agent.provider == "moa", on the assumption that agent.client is still the
in-process MoA facade.
Credential rotation, provider fallback and dead-connection cleanup all
rebuild agent.client from _client_kwargs between attempts, and
pending_moa_prepared_request deliberately carries a prepared request
across exactly that boundary. The rebuilt client is a native OpenAI
client while provider stays "moa", so the key reaches an SDK that has
never heard of it:
TypeError: Completions.create() got an unexpected keyword argument
'_moa_prepared_request'
That error is non-retryable, so every remaining turn on the session
fails. Both dispatch paths are affected: the non-streaming one calls
agent.client directly, and _create_request_openai_client returns
agent.client unchanged for provider "moa".
Re-check the live client at the point the key is attached, which covers
both paths at once. When the facade is gone, send the prepared prompt
without the handshake and log the downgrade.
After `hermes update`, an existing state.db on an old schema made every
GET /api/sessions poll fail with sqlite3.OperationalError "no such
column: s.last_read_at" (or s.last_activity_at) until something
unrelated forced a writable open — the desktop sidebar showed "No
sessions yet" while every row sat intact on disk (#79531, #80037).
Two remaining root causes (the stale hand-written read probe was
already replaced by the SCHEMA_SQL-derived probe on main, prototyped in
draft PR #80030 by @Tilly-YL):
1. Migrations ran lazily: _init_schema/_reconcile_columns only ran on a
writable open, typically the user's first NEW session. The dashboard
backend now schedules one writable open of its own state.db from the
lifespan (daemon thread, never blocks the ready-probe socket, never
raises), so the store is brought current before the first session-
list poll on every `hermes serve` / `hermes dashboard` / Desktop
headless entrypoint.
2. _reconcile_columns caught sqlite3.OperationalError around every
ALTER TABLE ADD COLUMN and logged at DEBUG. Lock contention from
orphaned sibling backends made the ALTER fail silently — startup
"succeeded" with a half-reconciled schema, and the open-time lock
patience (#74478) never saw the error because it was swallowed
inside first. Now: "duplicate column" races stay at DEBUG,
locked/busy re-raises so _connect_and_init_with_lock_patience
retries the whole idempotent init with jittered backoff, and any
other failure (e.g. un-ADDable NOT NULL) logs at WARNING.
Regression tests: a store missing sessions.last_read_at is healed by
the eager startup reconcile and serves list_sessions_rich; a locked
ALTER propagates and is retried to success by the open lock patience;
duplicate-column races stay quiet; other ALTER failures warn.
Fixes#79531Fixes#80037
Reported-by: @yenhunghuang (#79531) and @FLOW3R0111 (#80037)
Root-cause analysis: @wangyi0177-eng (stale read probe) and
@www654cc-pixel (_reconcile_columns DEBUG-swallow under lock
contention); draft PR #80030 by @Tilly-YL prototyped the probe fix.
The Test-Node stage-and-swap relies on Rename-Item's same-directory
carve-out: -NewName accepts a path only when it shares the directory of
-Path (FileSystemProvider strips the directory and keeps the leaf), and
all four swap calls rename between $HermesHome\node and sibling
node.new-* / node.old-* paths. Pin that invariant so a future refactor
cannot introduce a cross-directory rename, which would throw on every
Windows install and read as a false "in use" deferral.
Source-level probe, matching the other tests/test_install_ps1_*.py
regressions (Linux CI cannot execute the Windows installer).
The Hermes-managed Node tree at %HERMES_HOME%\node is destructively
rewritten while the desktop app's Node processes execute from it:
the Node-26 heal did shutil.rmtree + move, the EBADENGINE repair ran
npm install --global --prefix into the tree, and install.ps1's
Test-Node did Remove-Item + Move-Item. Windows rejects those writes
with PermissionError: [WinError 5] on npm.cmd.
- _heal_managed_node_windows: stage the fully-downloaded tree in a
sibling node.new-* dir, then rename-swap (live tree -> node.old-*,
staged -> node). The live tree is never deleted before its
replacement is ready, so an interrupted heal cannot gut it; a
refused rename is the OS-level in-use signal and defers (returns
None) instead of forcing the write.
- heal_hermes_managed_node: an in-use deferral does not record the
once-per-process attempt, so the heal retries once the tree is free.
- managed_node_tree_in_use: cheap psutil pre-check (Windows only) that
avoids pointless 30-50MB re-downloads in long-lived processes.
- upgrade_managed_npm: defer the in-place npm self-upgrade while the
tree is in use, with a notice.
- install.ps1: Test-ManagedNodeInUse guard around Update-ManagedNpm and
the Test-Node install branch, which now rename-swaps instead of
delete-then-move.
An in-use-but-outdated tree keeps serving the old runnable Node (old
Node beats no Node), and every npm resolution re-evaluates the heal, so
the upgrade applies automatically on the next update with the app
closed.
Residual from PR #70910 after #77509 landed the message-list scrub: a
whitespace-only system content block carrying a cache_control marker
still reached the wire and 400'd the whole request ("text content
blocks must contain non-whitespace text"), wedging the session on every
retry. The block cannot be dropped (it carries the cache breakpoint),
so coerce its text to the shared non-whitespace placeholder when
extracting the system param, copying the block so caller message dicts
are never mutated.
Adds SHL0MS's request-level regression suite from #70910; four of its
five cases already pass on main via #77509 — the system-block case
fails without this fix.
Teknium