Review round 3 (Gemini 3.8 Flash): counting every planned gateway row per profile
made a dirty baseline exit 1 even when the fleet was fully accounted for. With an
orphan row the restart phase killed (killed_pids) plus an active row that was
restarted, the killed row's own verdict ("stopped") was already in hand, yet its
count still marked the profile ambiguous and blocked credit for the row that did
restart.
The baseline count now skips rows bookkeeping already resolves (killed /
relaunched / name-matched) and counts only rows that still need a verdict, so the
ambiguity guard fires on genuine ambiguity instead of on any multi-row plan. The
debug message no longer claims a successor exists when the probe returned none.
Review round 3 (@GPT-OSS 120B): `kind not in _SERVE_KINDS` was read as
"excludes gateways" — the set is ("serve", "dashboard"), so the behaviour was
already gateway-only (a profile with a gateway plus a serve and a dashboard row
still credits the gateway), but the intent is now explicit: `kind == "gateway"`.
The served-kind test covers both non-gateway kinds for one profile to pin that.
Also documents what this fallback deliberately does NOT do, so the next reader
does not have to re-derive it:
- ambiguity resolves conservatively (pairing a successor to the planned process
needs a start-time identity neither the plan record nor the fleet row carries);
- credit is not gated on restart bookkeeping (the reported failure already had
bookkeeping; a self-respawned gateway has none, while the fleet row still
proves the successor runs the new code).
Review (@ehz0ah): one live successor cannot prove that two planned same-profile
gateways both restarted. `_collect_gateway_runtimes()` de-duplicates by PID, not
by profile, so duplicate/orphan states can put two `coder` rows in the plan —
and while the post-restart fleet probe publishes at most one row per profile,
both planned rows were credited from that single successor, letting an untouched
sibling vanish behind it and suppressing the unaccounted tripwire.
Successor credit now requires the profile to hold exactly ONE planned gateway
runtime (baseline identity); an ambiguous profile keeps the name-path verdict and
stays armed. Logged at debug so the reason is visible.
Reported repro, before -> after:
plan=[coder 76508, coder 76795], live_gateway_pids={"coder": {76796}},
empty bookkeeping
before: ['restarted', 'restarted'] tripwire: False
after: ['unaccounted', 'unaccounted'] tripwire: True
Review feedback (@kvnloo): when the post-restart fleet probe has no row for a
planned gateway's profile, the incarnation fallback silently does nothing and
the tripwire output reads identically to a genuinely missed restart. Log the
missing evidence at debug so the next report is diagnosable:
No post-restart gateway evidence for profile 'coder' (planned pid 76508 via
launchd) — reconciling on restart bookkeeping alone
Also make the call-site comment name the snapshot dependency the evidence
rests on (the same note #110343 carries at its wiring point).
`match_runtime_outcomes()` can only bridge a planned gateway runtime and the
restart bookkeeping by SERVICE NAME, and the two disagree whenever a service
serves a profile its name does not encode:
- macOS: the root-home LaunchAgent `ai.hermes.gateway` honours the install's
sticky active profile (`hermes profile use <name>`), so the process declares
`profile: coder` on its control socket while its label encodes the install
root. `_gateway_service_matches_profile("coder", "ai.hermes.gateway")` is
False for every naming rule, so the planned runtime is reported
`unaccounted` → `restart.incomplete` → exit 1 after a restart that verified
clean (fleet row: new PID, new SHA). The leftover `fleet_restart_pending`
marker then warns on every later invocation.
- Linux: a hash-suffixed unit for a custom HERMES_HOME has the same shape of
problem (#110238), which the open name-matching PRs only paper over for the
default profile.
Reconcile gateways on incarnation evidence too — the rule serve/dashboard
runtimes already use via `stale_serve_pids`: the post-restart fleet snapshot
(control-socket identity per profile) is passed in as `live_gateway_pids`, and
a planned PID that is gone while a gateway answers for the same profile counts
as `restarted`. Requiring a successor keeps the tripwire honest: no successor,
or the planned PID still answering, stays `unaccounted` and still exits 1.
`down` rows are excluded from the evidence — they report the pre-restart PID.
Fixes#110333
- _TS_RE now accepts the T-separated stamps update.log and
desktop-update-handoff.log actually write (posix.sh date +%Y-%m-%dT%H:%M:%S%z,
windows.ps1 yyyy-MM-ddTHH:mm:ssK, the update banner), so --since filtering
works for hermes logs update/handoff instead of silently dropping everything.
- test_logs' LOG_FILES stamp contract gains real writer samples for update/handoff.
- Replace the PR's source-shape (inspect.getsource) upload tests with behavioral
ones (monkeypatched uploader, --local capture); the upload/--local loops stay
data-driven over bundle.items() as on main.
When a Desktop-driven update fails at the Electron rebuild (updater
exit 6, 'Code and dependencies updated, but the Desktop app REBUILD
FAILED'), the only artifacts holding the root cause are
~/.hermes/logs/update.log (full stdout/stderr mirror of hermes update,
written by _UpdateOutputStream) and ~/.hermes/logs/desktop-update-handoff.log
(hand-off stages including the 'desktop --force-build --build-only' retry).
Both the updater's failure messages and the Desktop error box direct
users to 'hermes debug share', but the share captured neither file:
LOG_FILES and the debug.py capture chain covered only the classic
five logs, so update-failure issues (e.g. #100874) arrived with the
diagnostics that matter most missing.
Additionally, 'hermes logs list' (a directory scan) already listed
both files while LOG_FILES had no readable key for them, so
'hermes logs update' failed with 'Unknown log' against a file the
CLI itself displays.
Wire both files through the whole chain:
- hermes_cli/logs.py: add 'update' and 'handoff' LOG_FILES keys
(fixes list-vs-read inconsistency; tail/follow/filters come free).
- hermes_cli/debug.py: _capture_default_log_snapshots,
collect_debug_report (512 KB snapshot cap and the 100-line tail
budget both apply; a hand-built snapshot dict without the new keys
degrades to the old five-section report instead of KeyError),
collect_share_bundle full logs, the paste-upload label list in
build_debug_share, and the --local print list. The --nous bundle
and privacy notice inherit the new keys; force-redaction applies
via _capture_log_snapshot as for every other log.
- hermes_cli/subcommands/logs.py + website docs: name the new logs.
Tests (tests/hermes_cli/test_debug_share_update_logs.py): 12 new
tests — snapshot keys present with real content, missing files
degrade to the honest note without raising, report sections appear,
bundle keys omitted when absent, upload/--local label lists include
the new logs, LOG_FILES keys resolve, tail_log end-to-end. Sabotage
pass: 10 of 12 fail without the fix.
The WS-orphan reap interrupts the turn (deny-resolving the pending approval
queue) and teardown pops it via unregister_gateway_notify — both silently.
A reconnecting client's parked prompt card then answers resolved: 0 (or a
bare 4001 on approval.pending), so the prompt looked lost rather than
cancelled and the user had no signal the command would never run (#106678).
- session_lifecycle: _announce_cancelled_gateway_approvals broadcasts the
dropped entries (session ids, reason, request ids/count) before the
deny-resolve drains the queue, from both _interrupt_session_turn and
_teardown_session. Fail-open: a missing key, empty queue, or broadcast
failure never blocks the deny-resolve or teardown.
- _interrupt_session_turn gains orphan= so the reaper labels its cancels
ws_orphan_reap explicitly (never request_id prefix sniffing).
- contracts: ApprovalCancelledPayload event model + regenerated TS/OpenRPC.
- desktop: gateway-event lifecycle consumes approval.cancelled and clears
the parked prompt cards by request id (mismatch is a no-op, so a newer
prompt survives); the turn's BLOCKED tool result is the in-transcript
signal, same as the timeout path.
Supersedes #106693 (KoNit-K) and #126455 (Finn763); design credit to both.
Co-authored-by: KoNit-K <konit.block@protonmail.com>
Fixes#106678
An inline agent-generated image (FAL CDN URL) that the Desktop client
cannot reach renders as a collapsed frame with no usable fallback
(#74564): resolveMediaDisplaySrc passes inline https sources through
untouched, and a failed <img> load only records the broken-key.
- Gateway: GET /api/media/proxy fetches an allowlisted remote image URL
on the client's behalf (the gateway generated the image and holds a
working route to the CDN) and returns the same data_url shape as
/api/media. Host-allowlist (fal.media/fal.run/storage.googleapis.com
and subdomains), 20s timeout, image content-type check, 25MB cap.
- Renderer: gatewayImageProxyDataUrl wraps the new endpoint with
owner-connection/profile pinning; useMediaImage retries a failed
inline https load ONCE through the proxy and swaps in the returned
data URL. The retry is latched per source: a failing proxied data URL
or a remount never loops, and non-https sources never reach it.
Gated mode accepted only browser-minted ws-tickets and server-internal
credentials, but a token-mode Remote desktop connection bakes its
stored session token into ?token= (buildGatewayWsUrl) — so the WS
upgrade to /api/ws 403'd forever against a password-gated server even
though the same token authenticates REST fine. The REST bearer leg
already verifies session tokens via _verify_access_token; the WS leg
was the only inconsistent gate.
Gated ?token= is now verified against the dashboard auth session
providers (the same verify_session seam), stamps {user_id, provider}
identity, and audits success/failure (TOKEN_AUTH_SUCCESS/FAILURE). The
legacy in-process _SESSION_TOKEN stays rejected; a ProviderError (all
providers unreachable) rejects with token_unavailable instead of
crashing the upgrade; the bot-desktop display-ticket guard is kept.
Co-authored-by: salch-cred <141555468+salch-cred@users.noreply.github.com>
The fixture's _find_live_session_by_key/_child_run_active stubs predate the
profile_home parameter; the profile_name assertion compared against a bare
create_session (NULL) instead of the production stamp
(profile_name_for_home(...) or _current_profile_name()) that
_ensure_session_db_row applies on first-prompt persistence — the exact
contract materialization must match.
Co-authored-by: ruangraung <github@nadyahermes.anonaddy.com>
Co-authored-by: Brooklyn Nicholson <brooklyn.bb.nicholson@gmail.com>
A Desktop/TUI session created from the client can become permanently
unresumable if the backend dies or is restarted before the session's
first prompt. session.create deliberately writes no session row until
the first prompt arrives (avoiding "Untitled" litter), so the minted
key exists only in the backend's in-memory live registry. The managed
update pipeline restarts hermes-gateway.service and
hermes-dashboard.service at the end of every update; any session
minted but not yet prompted in that window is orphaned: the key exists
client-side (pinned tile, stored session id) but has no row anywhere,
and after a restart session.resume looks up the live registry and the
DB, misses both, and returns 4007 permanently.
Fix: in _resume_locate, before returning 4007, when the target is a
well-formed server-minted key (^\d{8}_\d{6}_[0-9a-f]{6}$) with no
persisted row and no live-session claim (any profile), materialize the
row via db.create_session(...) and re-resolve, resuming into a fresh
empty session. Fail-closed: garbage ids, 8-hex runtime ids, titles,
lazy watch windows, and keys claimed by any live session under another
profile still 4007 — an unscoped resume must never mint a phantom row
in the launch store.
Rebased onto current main by the maintainer landing crew: the resume
handler is split into _Resume helpers (methods_session.py), so the
materialize block lives in _resume_locate; the helpers stay in
server.py and resolve through the method_ctx rebinding. Test adapted
to the current handler shape (source parity with the env resolver,
profile_name stamped by the store).
Co-authored-by: ruangraung <github@nadyahermes.anonaddy.com>
Co-authored-by: Brooklyn Nicholson <brooklyn.bb.nicholson@gmail.com>
`_is_ancestor_pid` asked psutil for the whole parent chain at once:
any(parent.pid == pid for parent in psutil.Process().parents())
`Process.parents()` is eager: it walks all the way to the lowest pid
*before* returning a list, so the generator expression cannot
short-circuit when it reaches the matching ancestor. Its per-link
`parent()` tolerates only `NoSuchProcess`, so any process we may not
inspect anywhere above us raises `AccessDenied` out of the whole call,
and the `except Exception` guard discards every ancestor already found,
including the orchestrator one link down.
Under firejail with `ptrace_scope=1`, and in hardened containers,
`/proc/1` is exactly that unreadable process. The hand-off child then
failed to recognize its own parent's fresh marker, `UpdateLock.acquire`
treated a live orchestrator as a foreign concurrent updater, and the
in-app update exited 2 ("Another Hermes update is already running",
naming the hand-off script's own pid) on every attempt, indefinitely.
Walk the chain one link at a time and test each ancestor as it is
discovered, so a failure above a match can no longer erase it. A
failure encountered before any match still returns False, keeping the
conservative refusal for genuinely unprovable ancestry. The walk is
bounded by `_MAX_ANCESTRY_DEPTH` and skips a repeated pid so an
unexpected chain cannot spin.
Fixes#87514
A writer in the update's mutation window replaced every agent-job
prompt with the job's own name while the job count stayed identical,
so the count-based restore_cron_jobs_if_emptied net passed the loss
undetected (#82990). Add a field-level safety net mirroring the config
model-settings net: restore only the prompt field of a live agent job
whose id matches a snapshot job and whose live prompt is blank or
collapsed to the job name — a legitimate user edit that merely
differs is never stomped. Wired into the post-migration restore
sequence for the active profile and every sibling profile.
A providers.<slug> block with no base_url/url/api of its own is settings,
not a user-defined endpoint. Only remap to the custom: validation branch
when the user-config entry declares a base_url; otherwise openai-codex and
other listing-less built-ins keep their normal catalog path.
Closes#120020
providers.bedrock: {stale_timeout_seconds: 600} is the documented way to tune a
built-in provider's per-call timeout (agent/turn_recovery.py,
agent/thinking_timeout_guidance.py). resolve_user_provider() treated ANY dict
under providers.<name> as a full custom-endpoint definition, even with no
api/url/base_url — producing an empty openai_chat/api_key stub that shadowed
the real built-in overlay (bedrock_converse transport, aws_sdk auth).
For Bedrock this routed every /model switch through the generic
custom-endpoint /models probe against the Bedrock runtime host, which has no
such endpoint: the switch intermittently failed with "could not reach this
custom endpoint's model listing" depending on how fast the (irrelevant) probe
timed out.
resolve_user_provider() now requires a real endpoint (api/url/base_url) before
resolving an entry; a tuning-only block falls through to resolve_provider_full's
next rung, which picks up the built-in overlay correctly.
Three gaps stranded the app through a legitimately long or failed
source-update completion:
- update-gate: a fourth gate signal 'failed-receipt' (hasFailedReceipt on
UpdateGateDeps, optional) reports a FINISHED, failed update even when a
stale marker survives; waitForUpdateClearance gains abandonOn so the
boot path stops parking the full 20-minute budget on a receipt that
already says failed and boots the current build instead.
- backend-ready: while the backend prints venv_sync's source-completion
banners the 90s port-announce deadline is re-armed (5-min grace,
30-min total cap, real wall clock) so a healthy multi-minute repair is
not killed and re-run on every boot.
- venv_sync: relaunch-driven completion-tail retries are bounded — an
attempts record beside the pending marker keeps the first retry
immediate (the historical flaky-tail case), backs off after 2
consecutive failures (15-min window), and caps at 6 attempts, past
which the launch keeps the marker, skips the tail, and points at
'hermes update'; success clears both the marker and the record.
The skills security scanner had two blind spots (#63307 Part A):
1. destructive_home_rm only matched literal $HOME, not bare ~. A command
like 'rm -rf ~/Documents' scanned as safe for every trust tier.
2. No pattern detected the !`cmd` inline-shell DSL that
agent/skill_preprocessing.py expands via bash -c on skill view/load.
A skill carrying !`rm -rf ~/Documents` scanned safe.
Fix: extend destructive_home_rm to match ~/~ /~* (bare tilde forms),
add an inline_shell_exec pattern for the !`...` auto-exec snippet
(requires a non-space payload so a skill explaining the DSL is not
flagged), and bump SCANNER_VERSION to skills-guard-v7 so cached safe
verdicts are invalidated.
Also stop swallowing the scan verdict on the TUI-gateway skills.manage
install RPC (#63307 Part B): _skills_install returned installed:true on
every path, including a security-scan block, because its console was a
sink. It now keys the response off do_install's verdict and answers an
error envelope (5031) whose message/data carry the printed tail — the
scan verdict, the ambiguity candidate table, or the fetch failure —
instead of a silent success.
The Desktop one-click hub install path gets the same treatment from the
spawned CLI's exit code (hub-actions.ts already parses it).
Original scanner patterns by the PR author.
Co-authored-by: liuhao1024 <sunsky.lau@gmail.com>
The code-skew watchdog only covered SSH-isolated backends; Desktop-owned
local serves share the same 'updater never restarts me' property, so they
served stale modules until the app happened to restart them (R1). Extend
start_code_skew_watchdog wiring to is_desktop_owned_backend() backends.
Centralize the skew refusal: POST /api/model/set and the tui_gateway
model.options / model.save_key RPCs now refuse with the same restart
message the picker and the gateway /model switch already return (R2).
Rust live_marker_owner now REMOVES a stale marker (dead pid, past ceiling,
unparseable) on read — previously stale bytes were only ignored, so a crashed
updater whose Drop never ran refused every later acquire until the 20-minute
ceiling (#77259). pid_is_alive gains pid-0 and zombie handling (Linux /proc
state, macOS ps stat), closing the kill(pid,0) false-positive that kept a
dead-but-unreaped updater 'live'.
Python _pid_is_running gains the same zombie awareness via a new
_process_state probe (Linux /proc, macOS ps), which read_live_update inherits
through update_lock._pid_alive — so the CLI gate self-heals a zombie-owned
marker in seconds instead of 20 minutes (#120635, #125932).
Electron readLiveUpdateMarker consults a new posixProcessState probe
(injectable, fail-open to the signal-0 verdict) so the desktop boot gate no
longer parks on a zombie-owned marker.
Rust fix cherry-picked from #77885 (authorship kept); Python and Electron
are new parity work.
The test passed on the author's macOS but exited 1 on CI: with
--skip-build the build lock is never acquired (the probe asserted a
release that never happened), and the unmocked platform launch fixups
ran against the fake empty executable — on Linux,
_packaged_desktop_launch_command's sandbox helper is missing and sudo
is unavailable, so cmd_gui exits 1 before the handoff.
- Run the real acquire path (no --skip-build) so the lock is genuinely
held, then probe its release at the Electron handoff.
- Pin HERMES_HOME to tmp so the checkout-keyed lock path is isolated
from the runner's home.
- Stub _desktop_linux_sandbox_fixup and _installed_desktop_launch_target:
environment-dependent platform fixups are not this test's subject.
Two concurrent `hermes desktop`/`hermes update` invocations mutate the same
checkout-scoped node_modules and apps/desktop/release with no serialization:
the check-then-build preflight (_desktop_build_needed → npm install → pack) runs
unguarded, so the loser trips over the winner's half-installed tree
(ENOTEMPTY) or packs against a missing tsc.
Lock the whole mutable window behind DesktopBuildLock, a checkout-keyed flock
under the profile-common Hermes root (kept out of the checkout so read-only
prebuilt launches still work, released by the OS if a builder crashes):
- `hermes desktop` acquires it before the freshness check and refuses a
contended build with an explicit message (exit 2) instead of racing.
- `hermes update`'s desktop rebuild (build_update_products) waits for an
in-flight build rather than failing the update.
- the lock is released before the Electron handoff (source, packaged, and
--build-only paths), so an open Desktop window never blocks a rebuild,
and after a build failure so the next run isn't deadlocked.
Supersedes #94101 (lock design; its main.py wiring no longer applies after the
cmd_gui extraction) and #96581 (wait/exit-2 UX; not checkout-keyed).
Co-authored-by: worlldz <cryptoworlldz@gmail.com>
Co-authored-by: lepetitprince716-prog <lepetitprince716-prog@users.noreply.github.com>
A choices list whose entries were all blank quietly became an open-ended
question, so the model never learned its options were gone and Desktop
showed a free-text card for a choice prompt (#73152). A choice longer
than a surface renders was dropped downstream while choices_offered still
reported it as shown (#124127).
_normalize_questions now returns a tool error for both: a non-empty
choices list with no surviving label names the question, and a choice
over MAX_CHOICE_CHARS (8000, also declared as maxLength in the schema)
names its position and the limit. An omitted or empty choices list stays
open-ended, which is still the documented way to ask free-text.
Co-authored-by: Konstantin Khlopkov <47825603+kokhlo@users.noreply.github.com>
Co-authored-by: JonthanaHanh <92574114+JonthanaHanh@users.noreply.github.com>
The downgrade guard added a codesign probe inside the cmd_gui launch path,
which leaked /usr/bin/codesign calls into
test_packaged_launch_opens_the_refreshed_installed_app's captured
subprocess.run. The test pins the LAUNCH contract only — with no codesign
binary the signature checks are off and the swap proceeds, keeping it
focused (signature policy is pinned separately in
test_desktop_install_after_update.py).
The rebase of the signing-downgrade guard dropped the unconditional
`xattr -cr` from _desktop_macos_relaunchable_fixup (the commit that claimed
to reorder it after the identity decision deleted it instead), so locally
built bundles kept their quarantine attributes and still tripped the
'Hermes is damaged' Gatekeeper path the strip exists to prevent. Restore it
unconditionally ahead of the signing attempt: in the stage-and-swap flow it
targets the STAGED bundle (discarded when the fixup refuses), and in-place it
is harmless hygiene on a bundle whose signature is never replaced.
test_relaunchable_fixup_configured_identity_failure_never_falls_back_to_adhoc
now models the reconciled policy from #123748/#121857: it pins a
publisher-signed (Team ID) replace-target — refusal, no ad-hoc, remedy named —
while the locally-signed target's identifier-pinned ad-hoc retry stays covered
by test_relaunchable_fixup_failed_identity_uses_pinned_adhoc_before_legacy.
Review follow-up on the macOS signing-downgrade guard:
- _promote_staged_desktop_app no longer discards the fixup's False: a
refused signing (configured identity failed / codesign missing) now
discards staging and raises the same previous-app-kept RuntimeError
the integrity check uses, so the live bundle is never swapped for one
whose signature was never established. Regression drives the real
promotion path end-to-end (the earlier tests only called the fixup
directly).
- Every downgrade-refusal message now names the next action (publisher
identity for the locally-signed branch, the desktop signing setting
for team mismatch, bundle identifier alignment, codesign -vvv for
strict-verification failures), matching the running-app message.
- The fixup reads the identity decision BEFORE the quarantine-xattr
strip: a failed configured attempt no longer strips attributes off a
bundle it then declines to modify.
Red on upstream/main, green with the fix:
- _install_rebuilt_macos_bundles refuses to replace a Team-ID-signed
install with a locally signed build, a different Team ID, a different
bundle identifier, or an unverifiable rebuild; matching publisher
identities and ad-hoc installs keep swapping.
- _desktop_macos_relaunchable_fixup keeps the existing signature when the
configured desktop.macos_signing_identity fails instead of degrading
to legacy ad-hoc.
A locked login keychain makes the configured identity unusable during an
SSH update. Retry the identifier-pinned signer before the cdhash-only
legacy deep sign so TCC grants and entitlements survive.
Nested frameworks were signed with no entitlements, so under the hardened
runtime Electron Framework lost com.apple.security.cs.allow-jit and V8
crashed on launch. Apply the inherit plist to every nested bundle, matching
what electron-builder's afterSign step does in a real build.
The busy-queue envelope keeps the accept-time row id across an in-place
compaction, but archive_and_compact's concurrent-append path deactivates that
row and clones it to a fresh id; the drain's deactivate_message and the merge's
set_user_message_content then address the dead original, leaving the clone
active beside the replacement (prompt active twice) or the merge a no-op.
Adds SessionDB.resolve_active_row_id (the row itself while active, else the
single clone with same role/content/timestamp, None when ambiguous) and uses
it in both the drain deactivate and the merge update, dropping the envelope's
staged row when no live row remains.
Co-authored-by: Totoro-qaq <Totoro-qaq@users.noreply.github.com>
The submit-row fix closed the WRITES. These are the reads, found by a
parallel review, and each was verified by execution before being applied.
session.history served the parent. get_messages_as_conversation walks PARENT
pointers for include_ancestors, so a stale session_key materializes
root..parent and never the continuation: verified, a reconnect in the
post-rotation window rendered only the pre-rotation rows.
_adopt_out_of_band_turns keyset-probed the parent, so a Telegram reply or
cron delivery that landed in the continuation was invisible and the model
never saw it — the #42962/#86588 contract this function exists for.
That probe was not the whole fix, and this is why it needed executing rather
than reading: its second step, _load_durable_truncation_history, also keyed
off session_key. Fixing only the probe left the test red with the right
target and the right row visible in the DB. It is the third read in the same
path, so it moves with them.
Verified: 9/9 in the regression file. Sabotage: reverting all three files
fails both new tests, each naming the stale parent; restoring turns them
green. Out-of-band, sidecar, deferred-history and prompt-logging suites 22/22.
Four independent reviewers swept the #123635 diff. These are the findings that
changed code; each was verified before applying, not taken on report.
Register the recorded key instead of relying on the underscore sweep. The
staged dict becomes the turn's live user message, so _submit_row_session_id
rides every replay of that row. turn_context.py pops PERSISTENCE_ONLY_MESSAGE_FIELDS
from every outgoing copy and its comment says only chat-completions strips
underscore keys — an unregistered key survives to the transport, so a strict
OpenAI-compatible backend could 400 on it. Verified by execution: the registry
pop leaves the key in place and the transport sweeper is what removes it. The
membership is the contract; the underscore is the accident.
Gate each write on the key it writes to. _write_submit_user_row checked
session_key and appended to _submit_row_target_key(session); the two can
disagree after a rotation, so the guard described a write that was not the one
performed. The dead `key` local is gone.
Drop the isinstance/Any guard on _submit_row_owner_key. All three callers
narrow to a dict and check _row_id immediately before; the annotation existed
only to permit the non-dict the guard then rejected.
Log a failed model-switch marker persist at warning, not debug. Filing the
pivot in the live session means the write can now hit CompressionSessionClosedError
on a closed parent, which the old session_key target could not. At debug a
model switch silently loses its durable notice and nothing reaches errors.log —
the sibling persist one screen up already logs at warning.
Test-file: replace the E731 lambda with a named helper, and move _rotated_session
above its first use.
Verified: 7/7 in the regression file, 2113/2113 across the tui_gateway siblings
plus tests/hermes_state, 79/79 in the message-metadata/turn-context suites.
An external review of #123635 found two more sites in the same class: a
durable model-switch marker had no counterpart to the in-memory strip, and
message.react still resolved its row by the stale key.
_append_model_switch_marker strips the prior marker from session["history"]
so N switches leave one, but its durable append had no such step: every
switch added another ACTIVE row and all of them replayed on resume. The
invariant held in memory only. Filed in the live session, those rows are
the reporter's 130 stray markers. Add the durable counterpart
(deactivate_messages_by_display_kind) so the superseded rows are preserved
inactive, never deleted — the same contract as deactivate_message.
message.react resolves the row through latest_message_row_id, which filters
on one session_id. On a rotated session the newest user row lives in the
continuation, so the stale key reacted to the PARENT's last user row — the
previous turn — or 404'd when the parent had no text row. Address both the
lookup and the write through the same live-session key the submit row uses.
Also correct the submit-row helper's docstring: it named the recorded key
`_session_id`, but the constant is `_SUBMIT_ROW_SESSION_KEY`.
Verified: 7/7 in test_submit_row_session_target.py, 710/710 across the
tui_gateway sibling suites plus the reaction suites, 1403/1403 in
tests/hermes_state. Sabotage per module: reverting methods_session.py
fails only the react test, reverting server.py fails only the marker test.
The new test drives a real rotation and asserts the marker lands in the
continuation. The existing config.set test asserted the marker row was filed
under session_key while using an agent stub whose session_id is an arbitrary
"sid" — it pinned the stale-key coupling rather than an invariant, so it now
expects the live id the code (and the sibling system-prompt persist) uses.
_append_model_switch_marker writes a durable role=user pivot through the
agent's own db handle but addressed it by session_key, so on a rotated session
the notice landed under a parent the conversation no longer reads — the
reporter's 130 stray model_switch rows. It is the same stale-key hazard as the
submit row, and the sibling persist one screen up
(_persist_live_session_system_prompt) already resolves it the same way this
now does.
personality_switch is deliberately untouched: it only ever mutates
session["history"], never the DB.
The queue path shares the submit-row write and then addresses the row twice
more through session_id-qualified UPDATEs. Drive a real rotation and assert the
merge lands and the accept-time row is deactivated in the continuation, not in
the rotated-away parent.
Drives the real rotation (publish_compression_child + the routing host's
reopen of the parent) so the pre-fix failure is the reporter's exact shape: the
user row in the parent, the tool row and final text in the child. Sabotage:
reverting only the source fix fails 2 of the 3; the unrotated case passes on
both and pins ordinary sends to session_key.
A gateway whose last messaging adapter hits a retryable fatal error exits
with the failure verdict (EX_TEMPFAIL) so a supervising service manager
restarts it. The desktop app launches `hermes serve` directly with no
supervisor: the exit only severs the UI's websocket connections and drops
in-flight assistant messages, which then 'disappear' from chat (#118080 —
334 fatal events over 6 days on one profile, mostly email IMAP fetch
failures marked retryable).
Add gateway.on_all_adapters_down: exit | stay_alive (default exit — zero
behavior change for service-managed deployments). At both stranded-exit
sites in gateway/run_adapters.py, the stay_alive policy keeps the process
running (reconnect watcher owns recovery) instead of setting
_exit_with_failure; non-retryable adapter loss still exits. The desktop
launcher stamps GATEWAY_ON_ALL_ADAPTERS_DOWN=stay_alive last into the
child env (same rule as HERMES_GUEST_ONBOARDING); older runtimes ignore
the unknown variable and keep the historical behavior.
Fixes#118080
Without an explicit provider the probe runs the startup pin and then
the configured fallback chain. When the chain only resolves at its tail
(e.g. BWS-backed secrets not yet hydrated during backend boot), the
resolved runtime stops on the tail provider and the failure surfaced
as 'No usable credentials found for <tail>' — blaming a provider the
user never pinned and sending the post-update onboarding overlay's
reason box after the wrong provider (#124939).
Attribute the failure to the pin instead: the startup provider pin,
else the config model provider — the same primary notion
pre_agent_fallback_notice already uses. Auto mode (no pin anywhere)
keeps naming the resolved route's provider.
atomic_config_write() now refuses deletion-by-omission at any mapping depth:
atomic_roundtrip_yaml_save treats keys absent from the payload as deleted
recursively, while the old docstring described the call as a merge, so a
partial dict could silently erase unrelated config (#125107). A new
atomic_config_replace() names the deliberate full-state replacement contract
explicitly, and the owners that intentionally remove state (save_config,
provider reset/switch, credential mirror scrub, doctor key migration, profile
channel stripping, TUI full-state saves) route through it. Additive writers
stay on the guarded atomic_config_write.
Salvages #125152 (identical patch, cherry-picked; conflict in
_write_user_config resolved to pass atomic_config_replace through the new
telemetry wrapper) and carries the executable bit main expects on
scripts/check_config_yaml_writers.py.
Fixes#125107
Co-authored-by: JoaoMarcos44 <joaomarcosdias444@gmail.com>
The no-row-id fallback only recognised strict-prefix appends, so a compaction
by another surface (store shorter, summary row the view never saw) kept the
stale pre-compaction history and sent it to the model. Detect an unseen
_compressed_summary row and re-hydrate from the DB like the stamped path.
Through prompt.submit, _persist_submit_user_row has already written this turn's user
row when _adopt_out_of_band_turns runs; the stamped path stops below it (ceiling). The
unstamped fallback compared the whole store against the in-memory view, so the store was
always "strictly longer" by that one row and the prompt was adopted into history and then
appended again by the turn — the model saw it twice and the live record kept both copies.
Apply the same ceiling; cover the shape through the real prompt.submit path.
hermes-agent #81951 is not closed by this commit: the report's stamped-path repro already reconciles on main (#42962). This fixes the unstamped variant of the same divergence.
Native-vision user turns persist inline base64 in history text, so the
served user text never equals the Desktop optimistic @image ref and the
optimistic bubble is re-appended below every finished reply.
Display-only projection in _history_to_messages for role=user rows:
verified local image parts become @image: refs, stored/model content
untouched. Ambiguous or missing images keep inline form.
The session store's text-only projection appends one `[screenshot]`
line per image part, so flattened rows are projected too: trailing
placeholders are peeled (one per projected ref, else unprojected) and
dropped, mirroring the Desktop's extractImageRefs.
Closes#120083
After _recover_capture reopens the microphone, _run left self._cap
pointing at the closed capture, so _halt_thread could no longer abort a
blocking read wedged in the reopened stream. Re-arm self._cap on every
successful recovery.
Windows DirectSound devices report read_available == 0 forever even while
audio flows, so the #117096 poll guard spins without ever calling read() and
the wake word never fires with zero diagnostics. After _READ_STALL_FALLBACK_
SECONDS of starvation the capture now warns once, marks the stream's counter
unreliable, and reads blocking — frames keep arriving on such devices. A
blocking read that truly wedges is released by _halt_thread aborting the
capture once the join times out, preserving the #117096 halt guarantee.
Fixes#118552
