sync(upstream): daily batch 222465d84709 - #216
Merged
Merged
Conversation
atomic_json_write() calls os.fsync(), which blocks until the write reaches stable storage. build_channel_directory() already offloads its builders with asyncio.to_thread (NousResearch#60794) but still called the persist step directly on the loop, so the Discord heartbeat waited on a disk flush.
Mirrors test_discord_builder_runs_off_event_loop_thread. Verified to FAIL against unpatched v0.19.0 and pass with the fix.
Completes the bug class from NousResearch#83906 — the same blocking fsync-on-event-loop pattern existed in two more async gateway paths: - slash_commands.py _handle_restart_command: two atomic_json_write calls for .restart_notify.json and .restart_last_processed.json were blocking on fsync inside an async function. Now offloaded via asyncio.to_thread. - run.py _clear_restart_failure_count: called from _handle_message_with_agent (async, per-turn path) after a successful agent turn. Made the method async and offloaded the atomic_json_write call via asyncio.to_thread. Caller updated to await. Shutdown-path calls in _stop_impl_body (_increment_restart_failure_counts, planned restart notification marker) are intentionally left synchronous — the event loop is draining/stopping and offloading adds complexity for no benefit.
…s-index workflows Three separate reds on main. Two are fixed here; the third needs no code. 1. tests/gateway/test_multiplex_busy_input_mode.py (blocks every merge) Fails "Python tests / Run tests slice 5/12" and therefore "All required checks pass". Semantic merge conflict between two PRs merged ~1h apart: a31be48 fix(gateway): respect routed profile busy modes (added the test) c8f235a feat(gateway): allow selective multiplex profile serving (added the gate) c8f235a taught _profile_name_for_source to reject a route whose target profile is not in the served set (profiles_to_serve). Each PR was green on its own base; neither ran against the other's merge result. The test asserts a route to profile "research" resolves to that profile's busy mode, but never patches profiles_to_serve — so it reads the runner's REAL on-disk profiles. "research" is not among them, the route is rejected before the busy-mode snapshot is consulted, and the assertion gets the gateway default: WARNING gateway.run: Rejecting profile route 'research-chat': target profile 'research' is not served AssertionError: assert 'interrupt' == 'steer' Patch profiles_to_serve for the assertion — the same seam every sibling test in tests/gateway/test_profile_resolution.py already patches (test_route_inside_allowlist_resolves, test_route_outside_allowlist_rejects). This also removes an ambient-state dependency: the test previously passed or failed based on which profiles happened to exist on the machine running it. Verified passing under an empty HERMES_HOME. Test-only. The serving gate from c8f235a is correct and left intact. 2. Skills-index workflows: local action used without actions/checkout check-freshness has failed on all 12 of its last 12 scheduled runs: ##[error]Can't find 'action.yml', 'action.yaml' or 'Dockerfile' under '.../.github/actions/get-app-token'. Did you forget to run actions/checkout before running your local action? ./.github/actions/get-app-token is a LOCAL composite action and cannot resolve without the repo on disk. skills-index-freshness.yml had no checkout step at all. The step is gated on `status != 'ok'`, so the watchdog broke exactly when it was supposed to file its issue — the live index is currently 521.4h stale (limit 26h) and nobody was told. An audit of all workflows for this bug class found one more instance: skills-index.yml's `trigger-deploy` job, which re-triggers the docs deploy so a refreshed index reaches the live site. Its sibling `build-index` job checks out; this one did not. That is plausibly why the index went stale in the first place. Both are fixed; the audit now reports zero remaining jobs that use a local action without a prior checkout. Pinned to the same actions/checkout SHA used by the other 35 call sites. 3. "Publish inline E2E evidence" — no fix needed Failed once at 13:33Z on a transient TLS error reaching api.github.com ("certificate is not valid for any names") while installing a gh extension. The last 25 runs of that workflow are 25/25 success. Infra blip, not a code defect.
The hand-off script's WinForms window was a 720x420 dashboard: streaming log box, wide marquee, warning label. Updating is a wait, not a dashboard -- it is now the same shape as the other update surfaces (NousResearch#75895): a fixed 280x320 panel, marquee loader, one title, one static line, following the OS light/dark theme (charcoal #232323 seeds, never brand blue). Failure gets a terse finale instead of a wall of log: 'Failed to update' + 'Run "hermes debug share" in a terminal to send a report' + Close (held max 5 minutes, then the relaunched Desktop re-surfaces the result banner as before). The result-json message points at debug share too. With nothing streamed to the window, the per-line stdout pump is gone: Invoke-HermesStep drains both pipes async (no deadlock on chatty children, no frozen marquee on quiet ones) and writes full output to the hand-off log afterwards, where hermes debug share picks it up.
scripts/desktop-update.ps1 moves to scripts/desktop-update/windows.ps1 (a compat forwarder stays at the old path for one asar/checkout skew cycle) and gains the shim: scripts/desktop-update/ui.html rendered in a chromeless Edge app window, fed done|error over a loopback /progress endpoint. The page is NousResearch#75895's hand-off screen ported verbatim (Fourier Flow loader, one title, one line, OS light/dark, charcoal dark seeds); failure is the terse card pointing at hermes debug share. The WinForms card stays as the no-Edge fallback, same shape. Salvaged from the web-shell spike: TcpListener runspace server, Edge --app spawn with throwaway profile, degradation ladder, -SelfTestUi. Co-authored-by: Teknium <127238744+teknium1@users.noreply.github.com>
scripts/desktop-update/posix.sh is the mac/linux twin of windows.ps1: the Desktop spawns it detached and QUITS; it waits the app out, runs plain hermes update (retry-once across the update boundary, truthful desktop-rebuild completion), swaps/relaunches the .app bundle (mac) or the release/*-unpacked binary when its sandbox helper is launchable (linux), writes .hermes-update-result.json, and drives the same shim. Repo-owned, so every update refreshes the code that drives the next one. resolvePosixScriptHandoff mirrors the Windows resolver (with the flat-path fallback covering the scripts/ reorg skew).
applyUpdatesPosixInApp is gone: mac/linux Update now quits into the detached posix orchestrator, same shape as Windows. Deletes everything the in-app path dragged into main.ts -- runStreamedUpdate, the rebuild retry, the relaunch-outcome matrix (update-relaunch.ts/update-rebuild.ts and tests), shellQuote, resolveHermesCliBinary -- and with the app dead before the update starts, the HERMES_DESKTOP_CHILD_PID reaper-exclusion dance (NousResearch#37532) is structurally unnecessary on the desktop path.
scripts/desktop-update/repro.sh drives the real code paths against a disposable HERMES_HOME under /tmp: shim/shim-fail (UI dry runs), fresh (literal install.sh), behind N (rewound checkout driven forward by the orchestrator), error (broken venv -> abort + result file). Exposed as npm run update:shim / update:shim:fail / update:repro:* from apps/desktop.
…escaping Address helix4u's review: - finish() now delivers the outcome BEFORE publishing it: mac bundle swap and the linux relaunch gate run first, then the result file, marker removal, and the shim event -- the app launch itself goes last so it can't race the result write. A gated/skewed linux install (AppImage/ deb/rpm, broken sandbox helper) surfaces its message in the result file AND holds the shim window open with it instead of closing on a false 'Opening Hermes...'. - mac swap is transactional with a checked rollback; a failed install restores the previous bundle and the result says so (exit 7 when even rollback fails). Failed 'open' rewrites the result truthfully. - linux gate is an exact port of the deleted update-relaunch.ts logic: anchored path-segment match on <root>/apps/desktop/release/linux-unpacked, chrome-sandbox absent = namespace build = fine, present = root+setuid required, with the real opt-outs (ELECTRON_DISABLE_SANDBOX, --no-sandbox among replayed args, or the Desktop vouching) instead of the invented HERMES_DESKTOP_NO_SANDBOX. collectRelaunchArgs/sandboxFallbackFromEnv live in updater-process.ts again; the Desktop passes filtered launch args (after --) and --relaunch-cwd so a deep-link or --no-sandbox launch survives the update. - result/status JSON strings are escaped (git permits '"' in branch names) and the result write is atomic (tmp + rename). - coverage: resolvePosixScriptHandoff + ported helpers in updater-process.test.ts (19 pass); repro.sh gate / npm run update:repro:gate asserts the whole gate matrix and round-trips a hostile branch name through the result JSON.
…repro The posix orchestrator inherited the Desktop's cwd, and parts of the update pipeline resolve the tree they mutate from the working directory -- the sandboxed behind-repro caught it updating the DEVELOPER'S primary checkout (cwd at spawn time) while reporting success against the sandbox. cd "$INSTALL_ROOT" before running hermes update, matching the cwd:updateRoot contract of the deleted in-app path. Verified: rerun leaves the outside checkout untouched (reflog clean). repro.sh fresh used a --no-interactive flag install.sh doesn't have; non-TTY stdin (</dev/null) + --skip-setup is the real non-interactive contract.
…hestrators gille's round-2 review: the terminal lifecycle claimed outcomes the launch hadn't delivered yet. - posix finish() reorders: outcome -> durable result+marker -> LAUNCH WITH ACCEPTANCE -> terminal event. mac acceptance is open's exit code (launchd rejects broken bundles loudly); linux verifies the setsid child is still alive 1.5s after spawn, so an instant exec failure downgrades to a held 'manual' state + truthful result instead of a vanished 'done'. Gated skew/manual outcomes publish a real 'manual' event (new third shim state -- still zero logic in the page). - Renderer-free linux recovery: when no chromium-family browser exists, manual/error outcomes fire notify-send/zenity/kdialog best-effort so a gated non-relaunch is never a silent disappearance. - windows.ps1 mirrors the contract: Start-DesktopRelaunch returns verified acceptance (WMI pid alive / fallback process alive; dying before the window appears counts as failure), and the finally block downgrades to Show-ManualFinale + rewritten result when the launch didn't land. Error path still relaunches after showing itself. - repro.sh launch / npm run update:repro:launch: real-orchestrator matrix for instant-exit relaunch downgrade and skew-message surfacing. - posix.sh cds into the install root before hermes update (found by the sandboxed behind-repro: parts of the update resolve the mutated tree from cwd, which is the Desktop's cwd -- it updated the DEVELOPER'S checkout while reporting success against the sandbox).
…covery surface gille's round 3: - cd into the install root FAILS CLOSED (set -u without set -e let a failed cd continue hermes update in the caller's tree -- the exact wrong-tree class the correction exists to kill). Honest result, exit 3. - A supplied mac relaunch target that is missing is a REJECTED launch -> manual downgrade; the launch matrix asserts the downgrade instead of codifying the old false success. A mac swap-failure DONE_NOTE now still relaunches the kept/rolled-back bundle before publishing manual. - notify_fallback: every rung falls through on EXECUTION failure (a notify-send that can't reach D-Bus no longer eats the message), mac gets osascript (present on every macOS -- Safari-only machines have no chromium shim), and the no-surface terminal case is an explicit logged contract: the result file carries the outcome to the next boot. - update:repro:fresh passes --non-interactive explicitly (prompt_yes_no falls back to /dev/tty, so </dev/null was not equivalent).
Round 4 of helix4u's review — the durable fallback is now real: - Result protocol gains `manual`: an ok result the user still must act on (reopen the app, reinstall the GUI package, fix the sandbox helper). Both orchestrators set it on every DONE_NOTE/downgrade path; the Desktop consumer surfaces manual results in a real dialog on next boot instead of a log line — the browserless-Linux disappearance now ends at a visible dialog, worst case one boot later. Older result files without the field parse as manual:false (covered). - notify ladder verifies EXECUTION, not existence: zenity/kdialog must survive their first second (an instant death means no display and falls through); the no-surface case is an explicit best-effort contract whose guaranteed channel is the result dialog. - mac DONE_NOTE + failed relaunch of the kept/rolled-back bundle is no longer swallowed (`|| true` dropped): the durable message carries both facts. - launch/gate matrices assert `manual` in the result JSON; consumer round-trip tested in handoff-result.test.ts.
Fix NousResearch#78906 当部署同时启用 basic 密码 provider 与一个 OAuth/OIDC session provider 时， list_session_providers() 会把密码 provider 也计入 "exactly one candidate" 判断（密码 provider 虽是 session provider，但下一行就会因 supports_password 被原生 OAuth broker 流程拒绝），导致 len == 2、自动选择被跳过，桌面端 空 provider 登录返回 404 "Unknown provider: ''"。 修复：自动选择只在可 broker 的 provider（supports_session 且非 supports_password）中计数，与 /api/status 的 native_pkce 能力宣告使用同一 "brokerable" 定义；当没有任何可 broker provider 时保留原有选择逻辑， 让显式的 400 错误继续解释密码 provider 不支持原生 OAuth。 新增回归测试：basic+OIDC 并存时自动选中 OIDC、单 OAuth provider 自动 选中、多 OAuth provider 歧义 404、纯密码部署保留 400。
A manual:true hand-off result is the durable action-required channel: on a browserless Linux box with no working notifier, the boot dialog is the first and only place the message ever surfaces. The 30-minute freshness gate discarded it if the user reopened Hermes later, stranding exactly the machine the channel exists to serve. Parse before the age check and skip the window for manual results; the file is still unlinked before any age check, so it's surfaced at most once. Ordinary results still expire. Regression: a stale ordinary result is discarded (and consumed) while a stale manual result is still returned once.
…owngrading The Browser Use CLI became the default browser backend, but nothing provisioned it: users without uv/uvx (field report from DongyangHe on macOS) silently fell back to the built-in browser tools with no notice. - install_cli() in tools/browser_use_cli.py: uv tool install browser-use via the managed uv (bootstrapped on demand), linked into $HERMES_HOME/bin (UV_TOOL_BIN_DIR) - _find_cli() now also probes $HERMES_HOME/bin for browser-use/uvx — Hermes' managed uv is not on the user's PATH - hermes tools post_setup actually installs (Camofox standard) instead of printing instructions - install.sh / install.ps1 provision the CLI at install time (best-effort, non-fatal, honors --skip-browser) - CLI startup shows a one-line notice (24h rate-limited) when the default backend downgraded to the built-in tools
… fallback - install.ps1 must stay pure ASCII (PowerShell 5.1 ANSI code-page decoding, NousResearch#66994/NousResearch#67000): em-dash -> '--' - tests/test_managed_runtime_resolution.py: install_cli()'s shutil.which('uv') is a reviewed fallback AFTER ensure_uv() misses
…-f2b15435 feat(browser): auto-install the Browser Use CLI instead of silently downgrading
… press (NousResearch#83677) * fix(relay): stop sibling gateways answering another instance's button press A Discord button press arrives on the passthrough plane, and the connector fans a passthrough forward out to EVERY live gateway session of the tenant (relayServer.routeBusMessage delivers `passthrough` via sessionsByTenant), unlike a message, which it narrows to the admitted instance set. The prompt went out from exactly one instance and _pending_prompts is process-local, so every sibling gateway saw an answer for a prompt it never minted, could not tell that from its own prompt expiring, and fell through to chat dispatch -- where the option-shaped text ("/c1") is not a real command and run.py replied "Unknown command `/c1`". One copy per sibling, under the single real ack. Prompt ids are now minted as `<per-process nonce>.<8 hex>`, so an answer can be attributed to the process that minted it. A prompt answer is always consumed, never re-dispatched as chat: a sibling's prompt and a repeat answer are both dropped silently, and an expired prompt of our own gets a short "no longer waiting" notice from the owning gateway only. Ids stay inside the connector codec's contract ([A-Za-z0-9_.-], <=32 chars, 64-byte callback budget -- verified against promptCodec.ts: 52 bytes worst case with a full-length option id). An id with no nonce segment (a prompt in flight across an in-place upgrade) is still treated as ours. Tests: 4 added, each verified to fail without the fix. Full relay suite green (160 tests). * style(tests): ruff-format the added relay prompt tests
…ousResearch#84074) * feat(relay): ambient token endpoint mode for gateway.idp.token_url When gateway.idp.token_url is configured WITHOUT client_id/client_secret, treat the URL as a metadata-server-style ambient credential endpoint: plain GET, response body is the token (raw JWT or {"access_token": ...} JSON envelope). Covers workload-identity proxies such as Domino's $DOMINO_API_PROXY/access-token, which mint short-lived user-scoped OIDC tokens with no client registration. Previously this configuration was a hard error (client_id/client_secret missing), so no working deployment changes behaviour: creds present keeps the OAuth2 client_credentials POST, no token_url keeps Nous Portal. The misconfig error now self-diagnoses (names the ambient fallback and how to select the client_credentials grant instead). * fix(relay): reject short plain-text bodies in ambient token shape gate Review finding: the shape gate accepted any base64url-alphabet word, so an IdP answering the ambient GET with a terse error body ('unauthorized', 'error', 'null') had that word returned as a bearer token instead of the fail-closed misconfiguration error. Tighten the gate to JWT-like dotted tokens (3+ segments) or long opaque tokens (>= 32 chars); short bare words now raise the self-diagnosing ambient error. * fix(relay): partial IdP client credentials keep the loud error, never select ambient GET The ambient-endpoint dispatch used 'not client_id or not client_secret', so configuring exactly one credential (a mistyped client_credentials setup) silently issued a GET at the IdP token endpoint and then raised 'no client_id/client_secret configured' — factually wrong for that operator, and a stray request the old hard error never made. Ambient mode now requires NEITHER credential; a partial pair raises immediately, names the missing key, and issues no HTTP request (tests assert urlopen is never called). Docstring and relay.md now say 'neither' instead of 'without'. * fix(relay): ambient JSON envelope requires a string access_token, no coercion Review finding (P2): the JSON-envelope branch accepted any truthy access_token via str() coercion — a number became '12345…', a boolean became 'True', an object became its Python repr — bypassing the fail- closed contract and deferring the failure to the connector, where it hides the real endpoint problem. The envelope value must now be a non-empty string, the same contract the client_credentials path enforces on its token response. Deliberately NO shape gate on envelope values: an envelope is an intentional token response (mode-1 symmetry), and opaque tokens may use the standard-base64 alphabet the raw-body gate rejects. Mutation check: reverting the branch to str() coercion sends the 3 coercion tests red (3 failed, 15 passed). --------- Co-authored-by: Ben Barclay <ben@nousresearch.com>
…ndow Detached update hand-off on every OS: quit → hermes update → reopen, with one dumb shim window
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
…stead of relying on Fly autostop (NousResearch#84295) Fly Proxy autostop judges idle exclusively on inbound proxied connections. It cannot see an in-flight agent turn (outbound-only LLM traffic), and since Fly's mid-2026 proxy change an open outbound socket (the relay WS) no longer holds a machine awake. With autostop:"suspend", Fly suspended machines while they were still processing long-running jobs, and could suspend before the gateway flipped the relay destination (the buffered-event black hole). The scale-to-zero watcher now owns the suspend: after the idle predicate holds (no running agents, no live background work, inbound-quiet) and the go_dormant() quiesce completes (relay drained + flipped), it POSTs /v1/apps/{app}/machines/{id}/suspend on the local /.fly/api flaps socket. Suspend is skipped when the quiesce fails or inbound lands mid-quiesce (flip-before-freeze), and off-Fly the step is a no-op (fail-awake). Pairs with the NAS change that provisions scale-to-zero machines with autostop:"off" (gateway-owned suspend); wake is unchanged (Fly-proxied wakeUrl poke + autostart).
…nel (NousResearch#84300) Cron jobs targeting a relay-fronted logical platform (e.g. Discord behind the relay connector) failed twice over: 1. Target resolution read only the legacy <PLATFORM>_HOME_CHANNEL env mirror. The canonical home_channel block that /sethome persists to config.yaml — the only store that exists in a relay-fronted deployment, where no native env var is exported — was never consulted, so deliver='discord' silently resolved to nothing and the job fell back to local-only. 2. Even with a resolved target, the delivery loop's native configured/enabled gate rejected the platform ('not configured/enabled') although resolve_delivery_transport had already produced a live relay transport fronting it. A relay-fronted platform is deliberately NOT natively enabled (its credential lives in the connector), so the native gate must not apply to a relay transport. Resolution now falls back from the env mirror to config.get_home_channel(platform) for both chat_id and thread_id (thread affinity only when the chat id came from the same config block), which also makes the 'all' routing token pick up relay-fronted platforms. The delivery gate honours a resolved relay transport, mirroring the enablement rule resolve_delivery_transport already applied; the standalone (no-relay) path keeps the historical gate byte-identical.
…-zero busy check (NousResearch#84327) _scale_to_zero_has_live_background_work() counted every task in _background_tasks — but _spawn_supervised parks all permanent watchers there (session-expiry, kanban, reconnect, the scale-to-zero watcher itself, ...). An armed gateway therefore considered itself busy forever and never went dormant or suspended. Verified live on staging (hermes-agent-stg-test-6698, 2026-08-12): armed at 05:25, fully idle for 25+ minutes, zero 'going dormant' lines. Fly's coarse proxy autostop used to mask the bug; once the gateway took ownership of the suspend (NousResearch#84295) it became load-bearing. _spawn_supervised now tags its tasks and the busy check skips them. Transient tasks (startup-resume events, delegation, tracked processes) still block suspend. New tests exercise the REAL _spawn_supervised path rather than a stubbed _background_tasks set — the stubbing is exactly why the earlier tests missed this (same call-site trap as the F25 arm bug); the key test fails on main and passes with the fix.
…on events (NousResearch#84318) The relay interactions passthrough lane (_discord_interaction_to_event) built its SessionSource with platform=Platform.RELAY and no delivered_via_upstream_relay marker — unlike the relay text lane (ws_transport._event_from_wire), which maps the connector's platform to the logical enum and stamps the authenticated-upstream flag. Consequences of the mismatch: - /sethome sent as a Discord slash command persisted the home channel under platforms.relay.home_channel (invisible to cron delivery, which looks up the logical platform) and mirrored it into the dead RELAY_HOME_CHANNEL env var — so cron jobs with deliver='discord' kept falling back to local-only even after the resolution/delivery fixes. The absent trust marker also meant via_relay=False, so the handler's 'Relay does not authenticate this logical home target' guard — designed to reject exactly this misfiled shape — never engaged. - Session keys forked: the connector binds the interaction's follow-up capability under buildSessionKey with platform 'discord' and chat_type 'group' (interactionSessionSource), while the gateway keyed the same interaction as relay/channel. - _capture_scope skipped recording _platform_by_chat (it ignores the generic 'relay'), losing the egress sender hint for the chat. Stamp Platform.DISCORD (the lane statically parses Discord interaction wire payloads), chat_type 'group' for guild channels (native-adapter and connector parity), and delivered_via_upstream_relay=True (parity with the text lane; set locally, never read off the wire). With this, slash-command /sethome files under platforms.discord and passes the via_relay guard legitimately, and cron delivery over relay works end to end with the NousResearch#84300 resolution fixes.
…apters + dashboard forwarder) (NousResearch#84339) * fix(gateway): pass live adapters to cron fire webhook's fire_due The Chronos fire webhook (/api/cron/fire) called provider.fire_due(job_id, adapters=None, loop=loop), so every externally-triggered fire delivered through the standalone path even with a live gateway in-process. E2EE platforms and relay-fronted logical platforms (whose ONLY send path is the live relay adapter — no native credential exists on the box) failed every external fire with "platform 'X' not configured/enabled", while the same job delivered fine under the built-in ticker (gateway/run.py passes runner.adapters). Resolve the runner (self.gateway_runner → app['gateway_runner'] → _gateway_runner_ref(), the same chain the drain check uses) and forward its adapters. No runner → adapters=None, preserving the historical standalone path byte-identically. Note: does not by itself fix Fly-hosted scale-to-zero deployments where NAS's callback lands on the DASHBOARD process (internal_port 9119) — _fire_cron_job_for_profile there has no gateway runner in-process. That topology needs a separate fire handoff (design pending). * fix(cron): dashboard forwards Chronos fires to the gateway (503 when unreachable) The dashboard's /api/cron/fire executed cron jobs in the DASHBOARD process via _fire_cron_job_for_profile with adapters=None. On hosted deployments (Fly proxy exposes only the dashboard's port) that made every managed-cron fire deliver through the standalone send path, which cannot serve relay-fronted logical platforms (their only sender is the live relay adapter in the gateway process — no native credential exists on the box) or E2EE rooms. It also ran the whole agent turn inside the dashboard: wrong process for memory/session ownership and fire-claim attribution. Restore the invariant that the GATEWAY owns cron execution: - Dashboard route: after verifying the NAS JWT and resolving the job's profile, FORWARD the fire to the gateway api_server's own /api/cron/fire on loopback, NAS bearer preserved (the gateway re-verifies the JWT — defense in depth, no new trust link), and pass the gateway's response through. Gateway unreachable → 503 so NAS retries per the Chronos contract (non-2xx = retryable; the store CAS de-dupes the eventual double fire). Deliberately NO local-execution fallback. - Endpoint resolution mirrors gateway/config.py's api_server load order per target profile (config.yaml extra.port → API_SERVER_PORT from process env or the profile's .env → 8642), with /p/<profile>/ prefix routing under multiplex. - docker/stage2-hook.sh: generate a strong API_SERVER_KEY into .env on first boot when absent (never overwrites an operator value), so the loopback api_server passes its startup guard on hosted images. The fire route itself is NAS-JWT-authed; the key gates the rest of the api_server surface. The listener binds 127.0.0.1 by default and the Fly service exposes only the dashboard port. - _fire_cron_job_for_profile kept but deprecated (late-binding seam compatibility); no route calls it. - docs/chronos-managed-cron-contract.md: document the two-hop inbound topology and the 503-retry semantics. Depends on the previous commit (fire webhook passes live adapters to fire_due) — together they make NAS→dashboard→gateway fires deliver over relay end to end. * fix(cron): read the profile api_server port via the canonical config loader CI guard test_config_read_guard flagged the new _gateway_fire_endpoint for a raw yaml.safe_load of the profile's config.yaml — the exact drift class the guard exists to kill (raw reads miss the managed-scope overlay, ${ENV_VAR} expansion, and root-model normalization). Read through load_config() under a HERMES_HOME override scoped to the target profile instead (the same pattern the deprecated _fire_cron_job_for_profile uses for its store scope), and pull the port with cfg_get. Test updated to stub load_config rather than write a raw config.yaml. * fix(gateway): only messaging platforms count for the scale-to-zero arm gate The stage2 hook now generates API_SERVER_KEY for every Docker container, and key presence force-enables the api_server platform. The scale-to-zero arm gate counted every enabled platform, so the loopback api_server listener made messaging_is_relay_only_or_absent False on every hosted instance — silently disarming the feature (the not-armed log would show enabled platforms=['relay','api_server']). The arm gate and the not-armed logger now share one helper that filters to enabled MESSAGING platforms, excluding LOCAL/API_SERVER/WEBHOOK — the same non-messaging exclusion set _connect_platforms already uses. A genuinely enabled direct-socket platform (Discord/Telegram) still disarms. Two of the three new tests fail without this fix.
The previous commit's regex-based stripper removed EVERY heredoc body,
which review flagged as bypassable: a fake '<<EOF' marker inside a
comment or quoted string enters the unterminated path and swallows a
later REAL background operator, and unquoted ('cat <<EOF' — expansion
runs) or shell-consumed ('bash <<'EOF'' — body IS shell) bodies are
executable content that must stay visible to the guard.
Replace it with tools/shell_heredoc.strip_inert_heredoc_bodies(), a
conservative shell-state scanner: a body is masked ONLY when every
delimiter on the opener is quoted (no expansion), every heredoc is
terminated by an exact delimiter line, the opener composes a single
command (no list/pipeline operators, no nested $()/backtick/process-
substitution scope), and the consumer is an allowlisted non-shell
interpreter (python/osascript/cat). Anything ambiguous is returned
unchanged — a false positive on exotic syntax is acceptable; hiding a
real background operator is not. Masked bodies become newlines so line
structure is preserved for MULTILINE regexes.
The helper is a standalone stdlib-only module (precedent:
tools/ansi_strip.py) because the same heredoc-as-data false-positive
class exists in the blocked-command regex checks (NousResearch#83104) and the
gateway lifecycle guard (NousResearch#81721/NousResearch#79835, cron/lifecycle_guard.py) —
which must not import the terminal-tool module graph.
Adapted from Wolfram Ravenwolf's security-hardened rework of NousResearch#63788
(69c7663); test scenarios for the
bypass cases derive from his suite.
Co-authored-by: Wolfram Ravenwolf <github.com@wolfram.ravenwolf.de>
Efficiency review (measured with timeit probes) found two unbounded costs on adversarial inputs: - The masked-range rebuild copied the whole string once per range (O(n*k)): 50k tiny heredocs took 1.7s. Replaced with a single-pass segment join over the (sorted, non-overlapping) ranges: 152ms, and newlines are now counted on the original command instead of re-slicing. - After the last '<<' occurrence no opener can start, but the scanner still walked the remaining text per-char: one heredoc followed by a 1MB tail cost ~150ms. An rfind bound breaks out of the unit loop once the scan passes it: 0.3ms. Typical commands are unaffected (the '<<' fast path already returns first). 30/30 guard tests pass; mutation check re-run on the final stack (no-op mutation -> 11 tests fail, restore -> green).
MiniMax-M3 ships server-side automatic prefix caching on the
Anthropic-compatible endpoint (content-keyed, no marker needed —
see platform.minimax.io/docs/api-reference/text-prompt-caching).
cache_control markers are NOT on its explicit-cache support list
(which covers only M2.7/M2.5/M2.1/M2).
Emitting markers on M3:
- wasted serialization overhead
- risked perturbing the server-side prefix hash
- gave users a false sense of explicit-cache savings (the
cache_read_input_tokens field carries a +128 constant floor
and cache_creation_input_tokens is always 0 for M3)
Also add an opt-in debug=True parameter to normalize_usage() that
emits a debug-level log line carrying the observable cache fields.
This is the only reliable cache signal for M3 — off by default,
debug-level, scoped to the anthropic_messages wire, so production
callers see no impact.
Pin both changes with 8 new tests:
- 4 M3 tests covering provider, host, and custom-provider paths
- 1 regression guard ensuring M2.x caching is unaffected
- 3 observability tests (off-by-default, on-with-M3, on-with-Claude)
Verified end-to-end against api.minimaxi.com/anthropic/v1/messages
with MiniMax-M3[1m]: identical system prompt hit-rate with and
without markers; cache_read field is unreliable (128 floor),
input_tokens drop (8467 -> 1) is the real hit signal.
…ervability Follow-up fixes on top of the salvaged NousResearch#83678 commit: 1. Hoist the MiniMax-M3 marker exclusion ABOVE the native-Anthropic early return. provider="anthropic" pointed at a MiniMax /anthropic proxy is a supported override (_anthropic_base_url_override_ok), and the is_native_anthropic branch matched on provider alone — returning (True, True) before the M3 exclusion was reached. Two regression tests pin the proxy route (M3 off, M2.7 still on). 2. Reuse the existing _model_name_suggests_minimax_m3() helper from agent/model_metadata.py instead of a second inline substring copy. 3. Drop the debug kwarg on normalize_usage() — it had zero production callers and duplicated standard logging level gating. The cache-observability line is now a plain logger.debug scoped to MiniMax providers on the Anthropic wire only, so the "+128 floor" note can no longer appear for native Anthropic where it is false. Tests updated accordingly (MiniMax logs, native Anthropic does not).
Salvaged PR NousResearch#83678's commit is authored under a generic local agent identity with no linked GitHub account; map it to the PR opener for release attribution (same pattern as hermes-agent@users.noreply.local).
…lete, screenshots, OS detection (NousResearch#84419) Sweep of open Windows issues affecting day-to-day agent operation (explicitly excluding install/setup and locale classes): - hermes_cli/_subprocess_compat.py: new split_command_line() — Windows- safe command-line tokenizer (posix=False + quote stripping) so backslash paths survive. POSIX behavior unchanged (plain shlex.split). - hermes_cli/console_engine.py (NousResearch#83934): console commands like 'sessions export C:\Users\me\out.jsonl' no longer silently mangle the path into a relative filename in the cwd. - agent/shell_hooks.py (NousResearch#78293): hook commands with backslash paths now spawn, resolve their script path, and pass hooks doctor instead of reporting 'not executable'. All three shlex sites routed through the shared splitter. - agent/prompt_builder.py (NousResearch#51755): system prompt now reports Windows (11) on Windows 11 — platform.release() returns 10 for both; distinguish via sys.getwindowsversion().build >= 22000. - hermes_cli/commands.py (NousResearch#42016): @ autocomplete no longer crashes the prompt_toolkit event loop when rg emits a path on a different mount (device paths \.\nul, other drive letters) — relpath ValueError is skipped per-entry. - tools/browser_use_cli.py (NousResearch#83884): screenshot-path detection now matches Windows drive-letter paths (C:\... and C:/...) in addition to POSIX; Browser Use screenshots attach on Windows. - tools/skills_hub.py + tools/skills_guard.py (NousResearch#62310): the two 'MUST stay symmetric' skill content hashes actually agree on Windows now. Bundle keys are normalized to POSIX separators before hashing, and the disk digest sorts by rel-posix STRING (case-sensitive) instead of Path objects (case-insensitive on Windows). Fixes permanent false-positive update_available for every installed skill. Tests: tests/tools/test_windows_agent_loop_papercuts.py — 16 cases covering each fix, including a disk-vs-bundle hash symmetry check built with native Windows separators and a mixed-case filename.
…eservation (NousResearch#84426) Two follow-ups from live Windows sessions: 1. agent/prompt_builder.py: extend the Windows shell hint with the native-binary path rule. Hermes disables MSYS path conversion for its bash, so agents passing /c/Users/... or /tmp/... to NATIVE programs (git -C, node, python, rg) hit 'cannot change to' / 'not found' while the same path works in bash builtins — observed repeatedly in a live session (git -C failures, git apply /tmp/x.patch failures). The hint now says: forward-slash native form (C:/Users/x) for native tools, $LOCALAPPDATA/Temp over /tmp for scratch files native tools read. (/tmp is pure model habit from Linux training data — nothing instructs it — so the hint is the right layer.) 2. tests: pin LF/CRLF preservation through write_file and patch_replace. A live session saw a repo-LF file come back full-CRLF after an edit (4699-line diff churn); not reproducible through current tool APIs, so pin the correct behavior — LF files stay LF, CRLF files stay CRLF, no mixed endings — to catch any regression on the Windows write path.
…d paths (NousResearch#84428) Fixes NousResearch#69472. On a Windows host every destructive native command passed approval silently — DANGEROUS_PATTERNS were POSIX-shaped, and the normalizer strips backslashes as shell escapes so no Windows path could ever match a path rule. Probed live before the fix: 15 of 15 destructive Windows commands (Remove-Item -Recurse -Force, del /s /q, iwr | iex, taskkill /F, Format-Volume, diskpart, icacls /grant Everyone, vssadmin delete shadows, bcdedit /set, reg delete, cipher /w, ...) sailed through undetected. Two changes: 1. Windows destructive tier in DANGEROUS_PATTERNS: PowerShell deletes (bare Remove-Item -Recurse/-Force), cmd builtins with /s|/q switches, iwr|iex remote execution (pipe and subexpression forms), taskkill /F / Stop-Process -Force, volume/disk destruction (Format-Volume, Clear-Disk, diskpart, format.com, cipher /w), icacls Everyone-grant / /reset, backup destruction (vssadmin delete shadows, wbadmin delete, bcdedit /set), reg delete / Remove-ItemProperty -Force, and service stop/delete (Stop-Service -Force, sc stop|delete). Each pattern requires the destructive flag so graceful/read-only usage (taskkill /IM without /F, reg query, icacls inspect, sc query, plain del file) does not prompt. Patterns live in the main list, not a win32-gated tier: a Linux-hosted Hermes can drive a Windows box over SSH. 2. Windows-path detection variant in _command_detection_variants: when the raw command contains a drive-letter/UNC backslash path, also yield a variant with backslashes flattened to forward slashes BEFORE normalization strips them, plus Windows spellings of the credential path rules (Users/<u>/.ssh, AppData/{Local,Roaming}/hermes .env). Gated on a real path shape so POSIX escape semantics are untouched. Tests: tests/tools/test_approval_windows.py — 48 cases (27 destructive flagged, 13 benign not flagged, 5 credential paths in both separator spellings, 4 POSIX-escape non-regressions). The 8 pre-existing failures under '-k approval' on this Windows host are identical on unmodified main (ordering artifacts + known symlink cases) and unrelated.
…form skills (NousResearch#84429) Two Windows agent-loop friction fixes: 1. tools/mcp_tool.py (NousResearch#56536): shutil.which(cmd, path=env_path) reads executable extensions from the PARENT process PATHEXT, not the MCP subprocess env — a stdio MCP config supplying both PATH and PATHEXT could fail to resolve a command its own env can locate, and startup then got a bare command name. On Windows, when the first which() call misses and the config env carries PATHEXT (any key casing), retry the resolution with the config's PATHEXT temporarily applied. 2. skills/ + optional-skills/ (NousResearch#50606): 42 SKILL.md files that declare platforms: [.., windows] used python3 in their command examples. python3 does not exist on native Windows (the toolchain probe in the system prompt reports python3=missing), so every copy-pasted example burned a failed agent turn before self-correction. Replaced the command word python3 -> python (python3-config / python3.x version strings untouched). python is the spelling that exists in every Hermes-managed environment (Windows native, uv-managed venvs on all three OSes); agents on POSIX hosts additionally see the probed toolchain line and adapt either way.
The 3-sentence identical-edit message was snapshot-asserted verbatim in two tests. House style avoids exact-string change-detector assertions; both tests now import the constant from tools/fuzzy_match so rewording the message can't silently break them.
…hema skill_manage's patch action uses the same fuzzy_find_and_replace engine as the file patch tool and surfaces the identical-strings error verbatim — and unlike the file path it has NO is_already_applied no-op rescue, so identical old/new ALWAYS errors there. Mirror the new_string description so the schema warns before the error fires (sibling-site parity with tools/file_tools.py PATCH_SCHEMA).
The apply phase already skips a hunk whose -/+ lines are identical (patch_parser.py '(search_lines == replace_lines): continue'), but the validation phase lacked the guard: such a hunk reached fuzzy_find_and_replace, whose identical-strings error names old_string/new_string — parameters that don't exist in patch mode — and failed the whole atomic patch that apply would have accepted. Mirror the apply-phase skip in validation; regression test drives a mixed degenerate+live patch end-to-end (short text dodges the is_already_applied >=8-char rescue).
…thon (NousResearch#84452) * fix(windows): SSH ControlMaster gating + stop hijacking the user's python Two Windows environment-integrity fixes: 1. tools/environments/ssh.py (NousResearch#73927): Windows OpenSSH has no Unix-domain-socket ControlMaster support, so unconditionally passing ControlPath/ControlMaster/ControlPersist failed EVERY tool call on a Windows-hosted ssh terminal backend with 'getsockname failed: Not a socket'. Gate the three multiplexing options behind a module-level _SSH_MULTIPLEX = (os.name != 'nt'); the scp upload path is gated the same way. On Windows the backend now works without connection pooling (each command a fresh connection); POSIX behavior is unchanged. The teardown 'ssh -O exit' is naturally inert because the socket never exists on Windows. 2. scripts/install.ps1 (NousResearch#83797): the installer put the whole venv\Scripts directory on the user PATH, which contains python.exe / pythonw.exe / pip.exe and so silently hijacked the 'python' command in every terminal on the machine — unrelated projects started resolving python to Hermes' runtime interpreter. Now copy only the launchers (hermes.exe, hermes-acp.exe) into a dedicated $InstallDir\bin and put THAT on PATH. Existing installs are migrated: the legacy venv\Scripts entry is stripped from the user PATH on the next install/update. The new bin dir is under $InstallDir (…\hermes-agent), which the uninstall PATH sweep already matches via its \hermes-agent marker. Updated the stale hermes_cli/update_cmd.py docstring that described the old venv\Scripts-on-PATH layout. Tests: SSH ControlMaster gating pinned both directions (multiplex on → flags present; off → absent but BatchMode/StrictHostKeyChecking retained). install.ps1 parses clean via the PowerShell AST parser. * docs: update windows-native install docs for the bin\ launcher layout CI (test_windows_native_docs) pins the docs and installer to the same PATH layout. The NousResearch#83797 fix moved the PATH entry from venv\Scripts to a dedicated $InstallDir\bin holding only the hermes launchers, so update the Windows-native guide to match: PATH-after-install section, the install-steps list, the directory-layout table, the Get-Command verification line, and the 'command not found' pitfall. Test now asserts the bin\ layout and guards against a regression back to venv\Scripts on PATH. * fix: keep install.ps1 pure ASCII (PowerShell 5.1 codepage safety) The two comments I added in the NousResearch#83797 PATH-hijack fix used em-dashes, tripping tests/test_install_ps1_ascii_only.py — Windows PowerShell 5.1 reads a BOM-less .ps1 in the system ANSI codepage (not UTF-8), so a non-ASCII byte can misdecode into a stray quote and desync the parser (issues NousResearch#66994/NousResearch#67000). Replace the em-dashes with ASCII '--'.
Review follow-up on the salvaged handler: a non-string 'code' (int, dict, list) reached code.strip() and surfaced as a generic 'Tool execution failed: AttributeError' — the same unrecoverable shape the salvage exists to eliminate. Add an isinstance guard beside the 'command' check that names the received type and shows the correct call form; narrow the docstring to what the handler actually does. Regression test drives int/dict/list through registry.dispatch and asserts no AttributeError leaks (mutation-checked: removing the guard fails 3 subtests).
Whole-bug-class sibling of the execute_code fix: terminal(code=...) — the reverse confusion — fell through to command=None and failed with 'Invalid command: expected string, got NoneType', naming neither the stray 'code' argument nor execute_code as the right tool. Mirror the guard in _handle_terminal (verified live: the opaque NoneType error reproduces on main). Mutation-checked: removing the guard fails the new regression test.
…default Follow-up to the salvaged NousResearch#81201 commits: - Short-circuit _uses_hermes_python_environment when the child IS the running interpreter (path or realpath match). The default strict-mode path no longer spawns a probe subprocess at all, and a flaky probe of sys.executable can never drop the hermes root from PYTHONPATH (protects the test_repo_root_modules_are_importable invariant). The realpath leg also covers uv-style venvs whose bin/python resolves to the same binary. - Stop caching failed probes: _python_environment_prefix now uses a success-only dict cache instead of lru_cache, so one transient timeout under load no longer sticks for the process lifetime. - Deduplicate the subprocess probe scaffolding shared with _is_usable_python into _probe_python(). - Log once when the hermes root is omitted so import-behavior changes are diagnosable from user reports. - Tests: fail the composition tests loudly if execute_code never reaches Popen (was vacuously passing on exceptions); assert the staging dir is literally first in PYTHONPATH (was truthiness only); add guards for probe-failure retry and the no-probe short-circuit.
/simplify-code findings on the full PR diff: - _is_usable_python had the same sticky-failure bug the previous commit fixed in _python_environment_prefix: lru_cache pinned a transient probe failure (fork pressure, timeout) as False forever, silently locking project mode to sys.executable. Both probes now share a success-only bounded dict cache via _cache_probe_result() with FIFO eviction at _PROBE_CACHE_MAX (the old < cap guard stopped caching new entries instead of evicting, re-probing entry 33+ on every call). - The hermes-root-omitted logger.info fired on every external-env call in project mode; now deduped once per interpreter path per process (matching the tirith/mcp warn-once convention). - Regression test: _is_usable_python probe failures are retried, not cached (mutation-verified).
batumilove
added
the
ci-reviewed
|
Important Review skippedToo many files! This PR contains 154 files, which is 54 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (154)
You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
૮ >ﻌ< ა ci reviewran on 7437277 — sync(upstream): merge 222465d batch
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
15 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Batched official upstream changes into the fork patch stack.
c0106e50e7ecedb3ce34e785d949725dc4e0e457222465d84709379b65173b0283a6eea87516acfaVerification:
No live deployment or service action occurs until required CI and review pass.