fix(cli): soft-fail missing force-loaded --skills skill (no exit-1 worker crash) - #75
Closed
exiao wants to merge 368 commits into
Closed
fix(cli): soft-fail missing force-loaded --skills skill (no exit-1 worker crash)#75exiao wants to merge 368 commits into
exiao wants to merge 368 commits into
Conversation
…#32791) (NousResearch#54063) * docs(discord): document bot-to-bot comms as unsupported (NousResearch#32791) Multi-profile bot-to-bot conversation is not a supported topology. DISCORD_ALLOW_BOTS=none (the default) blocks all bot-originated messages; setting mentions/all across multiple Hermes profiles to make them reply to each other ack-loops because Discord's reply auto-mention satisfies the mention gate every turn. Document the safe default and the loop hazard so operators don't wire it up. * docs(discord): infographic for bot-to-bot unsupported stance (NousResearch#32791)
…ousResearch#33801) (NousResearch#54061) Secret redaction is display/output-scoped on main — write_file writes content verbatim, terminal/execute_code redact only output not the command/source. The real bug is in displayed tool OUTPUT (read_file, terminal, execute_code): _DB_CONNSTR_RE's password group [^@]+ was greedy across newlines, so on a multi-line block it scanned past the DSN line to the next stray '@' (a Python @decorator), replacing every intervening character — including line breaks — with ***. That dropped lines and concatenated the next line onto the f-string line, making read_file output look corrupted (the file on disk was always correct). Reported in NousResearch#33801. Fix: - Forbid whitespace in the userinfo/password groups ([^:\s]+ / [^@\s]+) so the match can never span a line break. A real DSN password never contains whitespace. This alone kills the catastrophic line-dropping. - Under code_file=True, preserve a password group that is a pure {...} brace expression — f"postgresql://{user}:{pass}@{host}" is an f-string template, not a live credential. Literal passwords are still masked. - Pass code_file=True at the terminal and execute_code output redaction call sites (file_tools already did) so code-execution output isn't corrupted by ENV/JSON/template false positives. Real prefixes, auth headers, JWTs, and private keys are still redacted. Verified E2E against the reporter's exact pydantic-settings module: file written verbatim, read_file shows the DSN f-string + @model_validator intact with zero *** corruption, while a literal postgresql://admin:pw@host DSN and a real sk- key are still masked. Reported-by: koishi70 Reported-by: pfrenssen
…Research#35205) (NousResearch#54076) * fix(telegram): clear send_path_degraded on successful reconnect _send_path_degraded was cleared only in _verify_polling_after_reconnect, 60s after reconnect and only if scheduled. A clean start_polling() reconnect left the flag stuck True, short-circuiting send() and blocking all outbound messages until the deferred probe ran (or forever if it never did). Clear the flag the moment start_polling() succeeds — that is the recovery signal. The deferred probe remains a defensive re-check that re-enters the reconnect ladder (re-setting the flag) if it detects a silent wedge. Fixes NousResearch#35205. * docs: add infographic for NousResearch#35205 telegram send-path fix
…search#301 (NousResearch#36658) Dashboard /chat spawns the TUI attached to the dashboard's in-memory gateway via HERMES_TUI_GATEWAY_URL. In that attach mode the already-running gateway replays `gateway.ready` (and `session.info`) the instant the socket connects, so those events land in GatewayClient.bufferedEvents *before* the consumer's mount-time subscribe effect (useMainApp.ts) calls drain(). drain() then emitted the buffered events synchronously, so the `gateway.ready` handler's patchUiState / setHistoryItems cascade ran while React was still inside the first commit — tripping "Too many re-renders" (Minified React error NousResearch#301) and breaking Dashboard chat after `hermes update`. Spawn / inline / sidecar modes never hit this: their `gateway.ready` only arrives after the Python child boots, on a later async tick. Fix: drain() defers the replay to the next microtask AND keeps `subscribed` false until that microtask runs. Keeping `subscribed` false in the gap means any live event arriving before the flush keeps buffering (publish() pushes when !subscribed) instead of emitting synchronously and jumping ahead of the chronologically-earlier replayed events — the flush re-drains the buffer right after flipping `subscribed`, preserving FIFO order. A drainGeneration token (bumped in resetStartupState) makes a queued flush a no-op if the transport was reset/killed in the meantime, avoiding use-after-teardown and duplicate/reordered exits. Regression tests: (1) drain() does not dispatch buffered events synchronously; (2) a live event arriving in the post-drain / pre-microtask window still delivers BEHIND the earlier-buffered event (FIFO). Both are red against the old synchronous behavior, green with this fix. Same class of fix as NousResearch#44528. Closes NousResearch#36658
…usResearch#34500) A custom_providers config that names the model under model.name (or model.model) resolved to an empty model, so the API request went out with model= — HTTP 400 from OpenAI-compatible backends. Display paths (hermes status/dump) already read model.name and showed the model, making the failure silent. The model id was read via 'default or model' at ~14 independent sites (cli, gateway, cron, curator, oneshot, fallback, profiles, ...), none of which honored 'name'. Rather than patch every site, canonicalize at the single load/save chokepoint: _normalize_root_model_keys() now promotes model.model/model.name -> model.default (precedence default > model > name) and drops the stale alias, so every reader — present and future — sees a populated default and config.yaml is migrated canonical on next save. The gateway, which bypasses load_config(), replays the same normalization in _load_gateway_config(). Co-authored-by: Bartok9 <danielrpike9@gmail.com> Credit: root-cause analysis and fix direction from @Bartok9 (NousResearch#34502, first) and @v86861062 (NousResearch#34527).
…cognized host (NousResearch#35166) On apt releases newer than the bundled Playwright recognizes (Ubuntu 26.04, Debian 14, and future distros), 'npx playwright install --with-deps chromium' hangs uninterruptibly at 'Installing Playwright Chromium with system dependencies' because Playwright's resolver maps the host to a platform with no download build (NousResearch#35166). Wrap every installer Playwright call in run_playwright_install(), which tries the native install first and, only if it fails or times out, retries once with PLAYWRIGHT_HOST_PLATFORM_OVERRIDE pinned to the newest known build (ubuntu24.04-<arch>). This is the escape hatch Playwright's maintainers bless for unrecognized platforms (microsoft/playwright#33434). Try-native-first (not a hardcoded distro/version table) is deliberate: - Self-correcting — when Playwright already supports the host (e.g. Ubuntu 26.04 on Playwright >=1.61) the first attempt succeeds and the override is never applied, so we never force a mismatched-glibc build onto a release Playwright handles correctly (microsoft/playwright#35114). - Zero-maintenance — new distro releases work the moment Playwright adds them. - Covers Debian 14+ and future releases, not just Ubuntu 26.04. An operator-set PLAYWRIGHT_HOST_PLATFORM_OVERRIDE is always respected (applied to the first attempt; retry skipped). Non-x64/arm64 arches have no fallback build and skip the retry. Refs NousResearch#35166
…p step interruptible Follow-up on NousResearch#54032 for NousResearch#35166: - Gate the PLAYWRIGHT_HOST_PLATFORM_OVERRIDE retry on the host being an apt release newer than Playwright recognizes (Ubuntu >24.04 / Debian >13) via playwright_host_unrecognized(), instead of retrying on ANY install failure. A network/disk/permission failure on a supported host now surfaces unchanged rather than getting a mismatched-glibc build forced onto it. - detect_os() now captures DISTRO_VERSION from os-release. - Fold in the interruptibility fix (was PR NousResearch#35304, self-closed): wrap the download in 'timeout --foreground -k 10' (probed, with plain-timeout fallback) so a terminal Ctrl+C reaches the child and a wedged download is force-killed after the deadline. - Add behavioral tests that source the helpers and assert the retry fires only on Ubuntu 26.04 / Debian 14, not on supported hosts, non-apt distros, native-success, operator-pinned override, or unsupported arch.
When an LLM API call returns HTTP 4xx with an empty parsed SDK `body` ({}),
`_summarize_api_error` fell through to a bare `str(error)`, so users saw only
"HTTP 400" with no provider detail (reported on Windows in NousResearch#36109). The SDK
leaves `body` empty in this case, but the httpx `response` still carries the
payload in `.text`.
- run_agent.py `_summarize_api_error`: when `body` is empty, fall back to
`response.text` — parse a JSON `error.message`/`message` when present, else
surface the raw (truncated) body. Platform-agnostic diagnostics.
- hermes_cli/oneshot.py: `hermes -z` now runs via `run_conversation` and returns
exit code 2 when the run is failed/partial with no usable final response, so
scripts can detect LLM failures (still 0 when a response — incl. an error
summary as output — is produced).
Tests: new tests/run_agent/test_summarize_api_error.py (empty-body JSON + raw
text, RED/GREEN verified) + oneshot exit-code/`run_conversation` wiring tests.
NOTE: NousResearch#36109's original root cause (Windows "all providers return empty 400")
is not reproducible on current main (heavy provider-transport churn since
v0.15.1). This change does not claim to fix that root cause — it makes any
empty-body API error LEGIBLE so a future occurrence shows the real provider
message instead of a bare HTTP 400. Relates to NousResearch#36109 (does not close it).
expand_whatsapp_aliases hardcoded get_hermes_home()/whatsapp/session, but
the adapter writes lid-mapping files via get_hermes_dir("platforms/whatsapp/
session", "whatsapp/session"). On installs without the legacy directory the
two paths diverge, so the resolver finds no mappings and returns the bare LID,
which misses the allowlist and silently drops the message. Resolve through the
same helper so both sides stay in lockstep on new and legacy layouts.
Add an _is_user_authorized E2E for the platforms/whatsapp/session layout on top of fesalfayed's resolver fix (NousResearch#36665) — guards the actual silently-dropped-LID-sender path from NousResearch#36664.
…chats (NousResearch#39293) Extend the gateway noisy-status filter beyond Telegram so internal compression lifecycle messages stay in logs instead of spamming Discord, Slack, and other messaging channels.
Force API-server error text through the existing secret redactor before returning OpenAI-compatible errors, response fallback text, response snapshots, and run failure events. This prevents credential-shaped provider failure text from crossing the API-server boundary while preserving debuggable sanitized messages.
Follow-up to the salvaged NousResearch#37733 fix. The contributor centralized redaction at _openai_error and the chat/responses failure paths, which covers the OpenAI-compatible envelopes transitively. Two sibling classes crossed the same authenticated HTTP boundary unredacted: - 8x cron-management endpoints returning {"error": str(e)} on 500 - the session-chat SSE error event ({"message": str(exc)}) Route both through the same _redact_api_error_text(force=True) helper. Add AUTHOR_MAP entry for coygeek and a TestRedactApiErrorText guard covering mask/force/limit/passthrough behavior.
When tools.environments.local can't be imported (partial install, import-time error), _is_hermes_provider_credential() returned False — fail-open. A skill could then register a Hermes provider credential (ANTHROPIC_API_KEY, etc.) as env passthrough; _scrub_child_env lets passthrough vars bypass the secret-substring net (rule 1), so the operator's real key would land in the execute_code child. Reopens the GHSA-rhgp-j443-p4rf bypass. Fail closed instead: on import failure, treat the name as a protected provider credential and refuse passthrough. Regression test exercises the full register -> scrub path under a simulated import failure. Co-authored-by: Hermes Agent <noreply@nousresearch.com>
Long-lived helpers spawned indirectly by tool calls (adb, platform bridges) were left in the service cgroup after the gateway's main process exited. When the kernel rejected the deferred cgroup-wide kill with EINVAL, systemd blocked Restart=always for 6+ minutes, taking down all platforms and cron windows (NousResearch#37454). Add a small ExecStopPost helper (gateway.cgroup_cleanup) that walks cgroup.procs and sends per-PID SIGKILLs — a different kernel code path than cgroup.kill, so it succeeds where the cgroup-wide write failed. KillMode=mixed is preserved so the gateway still reaps its own tool-call children before systemd intervenes (NousResearch#8202). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- read_text(encoding='utf-8') (PLW1514) - # windows-footgun: ok on signal.SIGKILL — module is Linux-only (reads /proc, /sys/fs/cgroup; runs from a systemd unit) - test lambda accepts the new encoding kwarg
Fix race condition in terminal environment snapshots that could corrupt PATH with declare -x entries. When concurrent terminal calls share the same snapshot file, the non-atomic 'export -p > snapshot.sh' write could be read mid-write by another process, causing partial/corrupted env vars to be sourced and mixed into PATH. The fix uses atomic file replacement: - Write to a temp file: export -p > snapshot.sh.tmp.303651 - Atomically replace: mv -f snapshot.sh.tmp.303651 snapshot.sh On POSIX, mv within the same filesystem is atomic, so source() will either see the old complete snapshot or the new complete one, never a partial/truncated file. Fixes NousResearch#38249
…lure path The atomic mv approach (kyssta-exe's commit) narrows but does not close the NousResearch#38249 race: the temp name used $$ (parent shell PID), which is identical across &-launched concurrent subshells. Two concurrent writers pick the same temp file, clobber each other mid-write, and mv then publishes a torn snapshot — a reader sourcing it absorbs declare-x/export fragments into PATH. - Use $BASHPID (actual per-subshell PID) so concurrent writers never collide. - Chain mv on export success (&&) and rm the temp on failure so a partial dump never replaces a good snapshot; apply the same to the init_session bootstrap. - shlex-quote the static temp-path portion (Windows/spaces), $BASHPID outside. - LocalEnvironment.cleanup sweeps orphaned snap.tmp.* temps. - Regression tests: string-shape + a behavioral concurrent writers/readers test that proves the snapshot never tears (would still tear with $$).
…teway-drain-microtask fix(tui): defer buffered gateway events to stop dashboard chat NousResearch#301 (NousResearch#36658)
Co-Authored-By: Paperclip <noreply@paperclip.ing>
…esearch#50005) (NousResearch#54126) When the Desktop forcibly closes its WebSocket mid-write, asyncio logs a full traceback for every pending connection-lost callback — 50+ identical WinError 10054 (ConnectionResetError) lines per disconnect on Windows, the equivalent ConnectionResetError/BrokenPipeError on POSIX. These are not actionable: they are the expected side effect of the peer hanging up before our writes drained. Install a loop exception handler on the gateway serving loop that collapses exactly this teardown class (ConnectionResetError/ConnectionAbortedError/ BrokenPipeError originating from _call_connection_lost) to a single debug line, forwarding every other loop error to the existing/default handler unchanged so genuine loop bugs still surface. Idempotent per loop.
…isclassification Rebase onto plugins/platforms/matrix/adapter.py (code moved from gateway/platforms/matrix.py). Same logic: _on_invite checks is_direct on invite events and calls _record_dm_room to persist in m.direct account data. Fixes NousResearch#44679
Fixes NousResearch#14238. During a compression/session split at the response boundary, the interim callback delivered unrelated commentary, setting response_previewed=True. The suppression logic treated that as proof the final reply had been delivered and skipped the normal send — the response was persisted to the child session but never sent to chat. Only suppress the normal final send when the stream consumer confirms final delivery (final_response_sent / final_content_delivered) or the exact final response text was delivered as a preview.
…module reload races (NousResearch#54775) The five _resolved_api_call_stale_timeout_base integration tests reloaded hermes_cli.config + hermes_cli.timeouts via importlib.reload to clear cached config. Under xdist that mutates module-global state shared across the worker process, so a sibling test could leave the config cache in a state that made get_provider_stale_timeout return a leaked value — intermittently failing test_reasoning_floor_applies_to_opus_4_thinking (shard 6 flake, NousResearch#52217 area). Patch run_agent.get_provider_stale_timeout per-test instead: floor-path tests get None (resolver falls through to the reasoning floor / env var / default), the explicit-config test gets 60.0 (priority-1 short-circuit). Same assertions, no shared-module mutation, deterministic under parallel execution.
… load When the dashboard gateway has no local session cookie, it rendered a click-through /login interstitial — even though the Nous portal's /oauth/authorize auto-approves any current member of the dashboard's org and is a silent 302 when the user already holds a portal session. For the common case (clicking a hosted-agent dashboard link while signed in to the portal) that interstitial click is pure friction. This makes the gate auto-initiate the OAuth redirect on an unauthenticated HTML document load instead of rendering the interstitial, when exactly one interactive provider is registered. A one-shot loop-guard cookie (hermes_sso_attempt, 60s TTL) ensures that a genuinely absent portal session (the portal bounces back still-unauthenticated) falls back to the /login page after exactly one bounce rather than ping-ponging forever. The marker is cleared on a successful callback and whenever the gate falls back to /login. Security: this removes a human CLICK, not a security check. The redirect lands on the existing /auth/login route and runs the unchanged PKCE auth-code flow; token verification, audience checks, redirect-URI match, and org-membership checks are all untouched. /api/* fetches still get the 401 JSON envelope (never a 302 a fetch() would follow opaquely), and with two or more providers the /login chooser still renders. Phase 1 of the cloud-auto-discovery work.
list_session_providers() already filters on supports_session=True, so the new helper re-filtered an already-filtered list. Call it directly at the single auto-SSO call site.
…NSERT OR IGNORE The gateway's get_or_create_session() creates a bare session row (source + user_id) before the agent exists. The agent's later create_session() carries the real model/model_config/system_prompt, but _insert_session_row used INSERT OR IGNORE — silently dropping that enrichment. Gateway sessions were left with NULL model and NULL billing metadata. Switch to INSERT ... ON CONFLICT(id) DO UPDATE with COALESCE so NULL columns get backfilled while values an earlier writer already set are never overwritten (a later bare write with source='unknown' can't clobber a real source/model). Credit: original report and fix direction by @LucidPaths (NousResearch#5048).
…dge path guard Salvages the two still-valid hardenings from NousResearch#5381 onto the relocated plugin adapters (the discord/feishu/whatsapp adapters moved to plugins/platforms/ since the PR was opened, and 4 of its 6 hunks are already on main or superseded). - feishu: rate limiter now denies untracked keys when the tracking table is at capacity after pruning stale entries (was: allow through without tracking). At-capacity-with-all-fresh-entries only happens under abuse, so allowing untracked requests let an attacker who flooded the table bypass the limiter entirely. Already-tracked keys and post-prune room are unaffected. - whatsapp: absolute file paths handed back by the Baileys bridge are now validated to resolve inside a known media cache dir before being attached. A compromised/buggy bridge could otherwise return an arbitrary path (e.g. /etc/passwd) that would be sent verbatim to the model. Guard resolves symlinks and accepts both the canonical cache/<kind> and legacy <kind>_cache layouts.
reset_had_activity gated on entry.total_tokens, which is never written (token counts migrated to agent-direct persistence) so it was always 0. That suppressed session-reset notifications for sessions that genuinely had activity. Switch to last_prompt_tokens, which is updated on every turn.
The reset-had-activity tests set total_tokens (dead state) to simulate activity; production records activity via last_prompt_tokens. Update the fixtures to match the field the fix and runtime actually use.
…ersal Session IDs can originate from untrusted input (e.g. the X-Hermes-Session-Id API header) and are interpolated raw into on-disk artifact filenames under ~/.hermes/sessions/. A traversal-shaped ID (../../../../etc/pwned) would let a caller write the session snapshot or request dump outside the sessions directory. _safe_session_filename_component() collapses every non [A-Za-z0-9_-] character to _, caps the length, and appends a short content hash when sanitization changed the string, always yielding a single traversal-free path segment. Closes NousResearch#5958.
…undary Defense-in-depth on top of _safe_session_filename_component (NousResearch#5958): Sink (makes the bad write impossible regardless of entry point): - run_agent._save_session_log: sanitize session_id before building the session_{sid}.json snapshot path. - agent_runtime_helpers.dump_api_request_debug: sanitize before building the request_dump_{sid}_{ts}.json path. Boundary (clean 400 instead of a silently-hashed filename): - api_server rejects path-traversal-shaped X-Hermes-Session-Id on the session-continuation path and the explicit /api/sessions create path, reusing gateway.session._is_path_unsafe (mirrors the native gateway's entry-boundary guard). Also enforces the session-header length cap on the continuation path. Tests: traversal session_id stays contained at the write site; sanitizer always yields a traversal-free segment; the API header rejects ../, absolute, and Windows-traversal IDs with 400.
Widen NousResearch#5961's _format_untrusted_prompt_value coverage to the Matrix room display name (**Matrix Room:**), a sibling attacker-controllable field the original fix missed. chat_name is user-settable, so an injected room name could render as literal markdown in the system prompt. Adds a regression test.
NousResearch#54834) The register path builds each profile-gateway slot in a sibling staging dir under /run/service (the scandir s6-svscan watches), then atomically renames it to the live gateway-<profile> name. The staging dir was named gateway-<profile>.tmp — a NON-dotfile — so a concurrent `s6-svscanctl -a` rescan (fired by the cont-init reconciler registering gateway-default, or by a sibling register) would supervise the half-built slot the moment it had a valid type/run: s6-supervise spawns AS ROOT and mkdirs supervise/ root-owned 0700, then the in-flight _seed_supervise_skeleton early-returns on the now-existing supervise/ and the next `mkdir supervise/event` hits PermissionError. That is the arm64-only CI flake on test_s6_unregister_removes_service_dir_in_live_container (PermissionError: /run/service/gateway-phase3test.tmp/supervise/event) — arm64-only because the native-arm runner's wider scheduling jitter lets the rescan land inside the ~ms seed window; amd64 ran 30/30 clean. Fix: dot-prefix the staging dir (.gateway-<profile>.tmp) in both register paths (S6ServiceManager.register_profile_gateway and container_boot._register_service). s6-svscan skips any scandir entry whose name begins with '.', so the half-built slot can never be supervised mid-build. The atomic rename to the dotless live name is unchanged. Verified on a real s6 image (amd64): a non-dotted staging dir is picked up by an svscanctl -a rescan (SUPERVISED owner=root) while a dot-prefixed one is ignored (NOT-SUPERVISED). Added a docker-harness regression test that asserts both, plus a unit test that the staging dir is dot-prefixed.
NVIDIA integrate.api.nvidia.com models such as minimaxai/minimax-m3 can return HTTP 200 with empty choices when max_tokens is omitted. Keep the output cap on auxiliary chat-completions routes, matching the main NVIDIA provider profile behavior.
Let users click the status bar context indicator to see how tokens are split across system prompt, tools, rules, skills, MCP, and conversation. Co-authored-by: Cursor <cursoragent@cursor.com>
…ontext-usage-popover feat(desktop): add context usage breakdown popover
…sResearch#7779) (NousResearch#54862) A manually-installed venv inside the cloned repo can be destroyed by the agent running a relative-path command against its own checkout (rm -rf venv, uv venv venv, etc.), silently wiping the running runtime mid-session. Moving the canonical manual-install venv to ~/.hermes/venvs/hermes-dev means no relative path from the agent's workspace resolves to its own runtime, making the bug class impossible without any command-detection code. Closes the root cause of NousResearch#7779. The managed install.sh layout is unchanged.
…ousResearch#54843) * feat(web_extract): truncate-and-store instead of LLM summarization web_extract no longer runs an auxiliary LLM over scraped pages. The extract backends (Firecrawl/Tavily/Exa/Parallel) already return clean, boilerplate- stripped markdown, so we return it directly: pages within a char budget (default 15000, web.extract_char_limit) come back whole; larger pages get a head+tail window plus an explicit footer giving the stored full-text path and the read_file call to page through the omitted middle. The full clean text is written to cache/web (mounted read-only into remote backends like the other cache dirs), so nothing is lost. Inline base64 images are converted to [IMAGE: alt] placeholders (token bombs dropped) while real http(s) image URLs are preserved as links so the agent can still web_extract/vision_analyze them. Removes process_content_with_llm + the chunked summarizer + check_auxiliary_model + _resolve_web_extract_auxiliary. context_references._default_url_fetcher is updated to the truncate path and its stale data.documents shape read is fixed to results (it was silently returning empty). Live before/after eval (firecrawl, 4 URLs): 11.7x faster overall (176.6s -> 15.1s); 10-60x on large pages. Quality identical; findability 4/4 (answer recoverable from stored full text on every truncated page). web_search is unchanged. No own scraper added; no changes to web_search. * fix(web_extract): add char_limit to execute_code web_extract stub The new web_extract char_limit param must appear in the code_execution_tool _TOOL_STUBS signature (and doc line) or test_stubs_cover_all_schema_params fails — the stub schema must cover every real schema param.
Subagent session pop-outs (`watch=1`) spectate a run driven elsewhere, so editing/steering the transcript from there makes no sense. Gate the composer and the user-bubble mutations on `isWatchWindow()`: - hide the composer (folds into `showChatBar`) - user prompts become a read-only button that toggles the 2-line clamp so long prompts stay fully readable, instead of opening the edit composer - drop the stop/restore actions and the checkpoint branch-picker Keyed off the narrow `isWatchWindow()` (not `isSecondaryWindow()`), so the new-session and cmd-click pop-outs are unaffected.
…atch-readonly feat(desktop): read-only spectator transcript for subagent watch windows
Reconciled 6 code/test/doc conflicts preserving fork patches + upstream: - auxiliary_client.py: kept fork provider/model mismatch guard + upstream MoA preset resolution (skip_step1 folded into combined Step-1 gate) - gateway/run.py: kept fork health-aware watchdog + upstream deadline bound on restart watcher loop - runtime_provider.py (x2): kept fork ANTHROPIC_BASE_URL env fallback + upstream _anthropic_base_url_override_ok validation - tools/web_tools.py: took upstream truncate-and-store rewrite; fork temperature-strip patch now moot (lived in deleted LLM-summary path) - test files (x3): kept both sides' added test methods/cases - configuration.md: kept both Fallback Announcements + Tool-Loop Guardrails sections
…exit-1
A missing force-loaded skill no longer raises ValueError("Unknown skill(s)")
and exit-1s before the agent starts. cli.main() now warns to stderr and
continues with the skills that resolved, so a dispatched kanban worker degrades
the run (can still work or block intelligently) rather than thrashing the
dispatcher into burning its whole retry budget on the identical wall.
Exit-1 is preserved for genuinely unusable invocations (no prompt, bad
profile); only the missing-skill case is softened. Unit test retargeted to
assert warn-not-raise.
Patch note: ~/.hermes/plans/hermes-patches/soft-fail-missing-force-loaded-skill.md
Ref: kanban t_d85833c1 (observed t_b04f835d ads-optimizer nano-banana-pro)
🔎 Lint report:
|
| Rule | Count |
|---|---|
unresolved-import |
45 |
invalid-argument-type |
30 |
unresolved-attribute |
22 |
invalid-assignment |
3 |
unsupported-operator |
2 |
invalid-method-override |
2 |
no-matching-overload |
1 |
invalid-return-type |
1 |
First entries
tests/run_agent/test_28161_anthropic_stream_pool_cleanup.py:40: [invalid-argument-type] invalid-argument-type: Argument to `AIAgent.__init__` is incorrect: Expected `((str, dict[Unknown, Unknown], /) -> None) | None`, found `str | bool`
gateway/run.py:19180: [invalid-argument-type] invalid-argument-type: Argument to function `_stream_confirmed_final_delivery` is incorrect: Expected `str`, found `(Unknown & ~AlwaysFalsy) | (str & ~AlwaysFalsy) | (list[Unknown] & ~AlwaysFalsy) | (int & ~AlwaysFalsy) | Literal[""]`
tools/process_registry.py:2203: [invalid-argument-type] invalid-argument-type: Argument to bound method `ProcessRegistry.list_sessions` is incorrect: Expected `str`, found `(str & ~AlwaysFalsy) | None`
cron/scheduler.py:2492: [invalid-argument-type] invalid-argument-type: Argument to `AIAgent.__init__` is incorrect: Expected `str`, found `Unknown | None`
tests/hermes_cli/test_kanban_write_txn_busy_retry.py:12: [unresolved-import] unresolved-import: Cannot resolve imported module `pytest`
tests/run_agent/test_28161_anthropic_stream_pool_cleanup.py:40: [invalid-argument-type] invalid-argument-type: Argument to `AIAgent.__init__` is incorrect: Expected `int | float`, found `str | bool`
hermes_cli/gateway.py:434: [no-matching-overload] no-matching-overload: No overload of function `run` matches arguments
gateway/slash_commands.py:3245: [unresolved-attribute] unresolved-attribute: Object of type `Self@_handle_resume_command` has no attribute `_set_session_reasoning_override`
tools/environments/local.py:627: [invalid-argument-type] invalid-argument-type: Argument to function `apply_subprocess_home_env` is incorrect: Expected `dict[str, str]`, found `dict[str | Unknown, str | Unknown | None]`
tests/gateway/test_session.py:1518: [unresolved-attribute] unresolved-attribute: Attribute `_conn` is not defined on `None` in union `None | SessionDB`
tests/agent/test_credential_pool.py:3087: [unresolved-attribute] unresolved-attribute: Object of type `dict[str, Any]` has no attribute `append`
tests/gateway/test_telegram_pending_update_probe.py:117: [unresolved-attribute] unresolved-attribute: Attribute `bot` is not defined on `None` in union `Unknown | None`
tests/hermes_cli/test_config.py:743: [unsupported-operator] unsupported-operator: Operator `in` is not supported between objects of type `str` and `str | None | bool | list[Unknown] | list[str]`
cron/scheduler.py:2491: [invalid-argument-type] invalid-argument-type: Argument to `AIAgent.__init__` is incorrect: Expected `list[str]`, found `Unknown | None`
tests/cli/test_terminal_interrupt_recovery.py:24: [unresolved-import] unresolved-import: Cannot resolve imported module `pytest`
tests/plugins/memory/test_memory_lazy_install.py:138: [unresolved-attribute] unresolved-attribute: Unresolved attribute `MemoryClient` on type `ModuleType`
tests/gateway/test_session.py:1513: [unresolved-attribute] unresolved-attribute: Attribute `end_session` is not defined on `None` in union `None | SessionDB`
tests/hermes_cli/test_env_custom_keys.py:11: [unresolved-import] unresolved-import: Cannot resolve imported module `fastapi.testclient`
tests/gateway/test_telegram_auth_check.py:20: [unresolved-import] unresolved-import: Cannot resolve imported module `gateway.platforms.telegram`
tests/gateway/test_clarify_active_session_bypass.py:6: [unresolved-import] unresolved-import: Cannot resolve imported module `pytest`
tests/agent/test_auxiliary_config_bridge.py:12: [unresolved-import] unresolved-import: Cannot resolve imported module `pytest`
tests/test_tui_gateway_loop_noise.py:7: [unresolved-import] unresolved-import: Cannot resolve imported module `pytest`
tools/vision_tools.py:1141: [invalid-argument-type] invalid-argument-type: Argument to function `async_call_llm` is incorrect: Expected `dict[Unknown, Unknown]`, found `str | list[dict[str, str | list[dict[str, str] | dict[str, str | dict[str, Unknown]]]]] | int | float`
tests/run_agent/test_24996_fallback_exhaustion_cooldown.py:36: [invalid-argument-type] invalid-argument-type: Argument to `AIAgent.__init__` is incorrect: Expected `dict[str, Any]`, found `Unknown | None`
tests/plugins/memory/test_holographic_shutdown_closes_db.py:17: [unresolved-import] unresolved-import: Cannot resolve imported module `pytest`
... and 81 more
✅ Fixed issues (8):
| Rule | Count |
|---|---|
invalid-argument-type |
5 |
unresolved-attribute |
2 |
invalid-return-type |
1 |
First entries
tools/web_tools.py:683: [invalid-argument-type] invalid-argument-type: Argument to function `async_call_llm` is incorrect: Expected `list[Unknown]`, found `dict[str, Any] | str | list[dict[str, str]] | int`
tools/web_tools.py:683: [invalid-argument-type] invalid-argument-type: Argument to function `async_call_llm` is incorrect: Expected `dict[str, Any] | None`, found `dict[str, Any] | str | list[dict[str, str]] | int`
tools/web_tools.py:683: [invalid-argument-type] invalid-argument-type: Argument to function `async_call_llm` is incorrect: Expected `int | float`, found `dict[str, Any] | str | list[dict[str, str]] | int`
tests/tools/test_vision_tools.py:259: [unresolved-attribute] unresolved-attribute: Object of type `Awaitable[str]` has no attribute `close`
tools/browser_tool.py:210: [invalid-return-type] invalid-return-type: Return type does not match returned value: expected `int`, found `None | int`
tools/web_tools.py:683: [invalid-argument-type] invalid-argument-type: Argument to function `async_call_llm` is incorrect: Expected `dict[Unknown, Unknown]`, found `dict[str, Any] | str | list[dict[str, str]] | int`
tools/web_tools.py:683: [invalid-argument-type] invalid-argument-type: Argument to function `async_call_llm` is incorrect: Expected `str`, found `dict[str, Any] | str | list[dict[str, str]] | int`
tools/process_registry.py:907: [unresolved-attribute] unresolved-attribute: Attribute `read` is not defined on `None` in union `IO[Any] | None`
Unchanged: 6083 pre-existing issues carried over.
Diagnostics are surfaced as warnings — this check never fails the build.
Owner
Author
|
Superseded by #76. This branch was cut from local live-config (368 commits ahead of origin), so its diff range dragged in an unrelated upstream commit with an unmapped author (christianpersico98@gmail.com), failing check-attribution on code I didn't touch. #76 is the same single commit rebased onto origin/live-config — clean diff, no attribution noise. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several enhancements, including a headless serve backend for the desktop app, parallel reference-model execution for the Mixture of Agents (MoA) provider, and robust fallback and retry mechanisms for API and transport errors. It also hardens security with improved credential redaction, SSRF protection, and systemd cgroup cleanup. Feedback from the review highlights opportunities to improve robustness by safely parsing the background process age configuration, ensuring custom providers are respected during auto-resolution, validating configuration dictionary types to prevent AttributeError, and wrapping the Discord liveness probe in a timeout to prevent indefinite hangs.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
+316
to
324
| bg_max_age = data.get("bg_process_max_age_hours") | ||
| return cls( | ||
| mode=mode if mode is not None else "both", | ||
| at_hour=at_hour if at_hour is not None else 4, | ||
| idle_minutes=idle_minutes if idle_minutes is not None else 1440, | ||
| notify=_coerce_bool(notify, True), | ||
| notify_exclude_platforms=tuple(exclude) if exclude is not None else ("api_server", "webhook"), | ||
| bg_process_max_age_hours=bg_max_age if bg_max_age is not None else 24, | ||
| ) |
There was a problem hiding this comment.
The configuration value bg_process_max_age_hours is retrieved from data but is not validated or cast to an integer. If a user configures it as a string (e.g., "24") or an invalid/non-positive value, it can cause unexpected behavior (like string multiplication "24" * 3600) or runtime TypeError crashes when compared with timestamps. We should safely parse and validate it, falling back to the default of 24 if invalid or non-positive.
Suggested change
| bg_max_age = data.get("bg_process_max_age_hours") | |
| return cls( | |
| mode=mode if mode is not None else "both", | |
| at_hour=at_hour if at_hour is not None else 4, | |
| idle_minutes=idle_minutes if idle_minutes is not None else 1440, | |
| notify=_coerce_bool(notify, True), | |
| notify_exclude_platforms=tuple(exclude) if exclude is not None else ("api_server", "webhook"), | |
| bg_process_max_age_hours=bg_max_age if bg_max_age is not None else 24, | |
| ) | |
| bg_max_age = data.get("bg_process_max_age_hours") | |
| try: | |
| bg_max_age_val = int(bg_max_age) if bg_max_age is not None else 24 | |
| if bg_max_age_val <= 0: | |
| bg_max_age_val = 24 | |
| except (ValueError, TypeError): | |
| bg_max_age_val = 24 | |
| return cls( | |
| mode=mode if mode is not None else "both", | |
| at_hour=at_hour if at_hour is not None else 4, | |
| idle_minutes=idle_minutes if idle_minutes is not None else 1440, | |
| notify=_coerce_bool(notify, True), | |
| notify_exclude_platforms=tuple(exclude) if exclude is not None else ("api_server", "webhook"), | |
| bg_process_max_age_hours=bg_max_age_val, | |
| ) |
References
- Use safe integer parsing helpers (e.g., validating for a minimum value of 1) when handling user-supplied or task-specific numeric limits like max turns. Ensure that invalid, non-positive, or malformed inputs fall back to safe defaults to avoid producing infinite or zero-turn loops.
Comment on lines
+1634
to
+1636
| _cfg_provider = _model_cfg.get("provider") | ||
| if isinstance(_cfg_provider, str) and _cfg_provider.strip().lower() in PROVIDER_REGISTRY: | ||
| return _cfg_provider.strip().lower() |
There was a problem hiding this comment.
If _cfg_provider is a custom provider configured in config.yaml, it will not be present in PROVIDER_REGISTRY. As a result, the early check is skipped, and if OPENAI_API_KEY is present in the environment, the resolver will mistakenly return "openrouter" instead of respecting the user's explicit custom provider choice. We should check resolve_custom_provider to ensure custom providers are also respected.
Suggested change
| _cfg_provider = _model_cfg.get("provider") | |
| if isinstance(_cfg_provider, str) and _cfg_provider.strip().lower() in PROVIDER_REGISTRY: | |
| return _cfg_provider.strip().lower() | |
| _cfg_provider = _model_cfg.get("provider") | |
| if isinstance(_cfg_provider, str): | |
| _cfg_provider_clean = _cfg_provider.strip().lower() | |
| if _cfg_provider_clean in PROVIDER_REGISTRY: | |
| return _cfg_provider_clean | |
| try: | |
| from hermes_cli.providers import resolve_custom_provider | |
| _custom = resolve_custom_provider(_cfg_provider_clean) | |
| if _custom: | |
| return _custom.id | |
| except Exception: | |
| pass |
| return None | ||
|
|
||
| try: | ||
| config = _get_env_config() |
There was a problem hiding this comment.
When retrieving a dictionary from a configuration object, validate that the retrieved value is a dictionary using isinstance and provide a fallback empty dictionary if it is not, to prevent AttributeError.
Suggested change
| config = _get_env_config() | |
| raw_config = _get_env_config() | |
| config = raw_config if isinstance(raw_config, dict) else {} |
References
- When retrieving a dictionary from a configuration object, validate that the retrieved value is a dictionary using
isinstanceand provide a fallback empty dictionary if it is not, to preventAttributeError.
Comment on lines
+1235
to
+1236
| await client.fetch_user(user.id) | ||
| fails = 0 |
There was a problem hiding this comment.
If the connection is completely wedged, client.fetch_user might hang indefinitely if the TCP connection is stuck in a half-open state and discord.py's internal timeout doesn't trigger. Wrapping it in asyncio.wait_for with a reasonable timeout ensures the liveness probe itself never hangs.
Suggested change
| await client.fetch_user(user.id) | |
| fails = 0 | |
| try: | |
| await asyncio.wait_for(client.fetch_user(user.id), timeout=15.0) |
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
20 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.
Problem
A kanban worker spawned with
hermes -p <lane> chat -q ... --skills <name>exit-1s before the agent starts when<name>isn't in that lane's ownskills/dir (per-profile dirs don't fall back to global~/.hermes/skills/). The dispatcher reads exit-1 as a crash, retries, hits the identical wall, and burns the whole failure budget without the agent ever running.Observed: t_b04f835d (ads-optimizer,
nano-banana-pro). Same class as NousResearch#29415 (avoid-ai-writing).Fix
cli.main()no longer raises onmissing_skills. It warns to stderr (⚠ Skipping unknown force-loaded skill(s): ...) and continues starting the agent with the skills that resolved. A missing optional skill degrades the run; an agent that starts can still do the work orkanban_blockintelligently. Exit-1 is preserved for genuinely unusable invocations (no prompt, bad profile).Verification
pytest tests/cli/test_cli_preloaded_skills.py→ 3 passed (unit test retargeted to warn-not-raise).--skills does-not-exist→ exit 0, stderr warning, agent producedPONG. NoUnknown skill(s)string.--skills render-cli→ exit 0, no warning, agent ran (PONG2).Companion audit (kanban t_d85833c1 Part 2)
All currently force-loaded lane skills already resolve as symlinks into the global tree (ads-optimizer: meta-ads-cli + nano-banana-pro; dev: bloom-cli, all
enabled). This PR makes any future gap a warning, not a crash.Patch note:
~/.hermes/plans/hermes-patches/soft-fail-missing-force-loaded-skill.md