Skip to content

fix(terminal): route desktop (Windows/WSL) SSH commands via stdin - #2

Merged
DaveVoyles merged 358 commits into
mainfrom
fix/desktop-windows-ssh-stdin
Jul 10, 2026
Merged

fix(terminal): route desktop (Windows/WSL) SSH commands via stdin#2
DaveVoyles merged 358 commits into
mainfrom
fix/desktop-windows-ssh-stdin

Conversation

@DaveVoyles

Copy link
Copy Markdown
Owner

Summary

  • terminal(host="desktop", ...) connected via SSH fine but failed on real commands (exit_code 126, unexpected EOF while looking for matching ''') — a bash-quoting mismatch.
  • Root cause: ssh joins the trailing argv into one string and sends it for the remote side to re-parse. A POSIX login shell strips shlex.quote()'s single-quotes correctly; desktop's Windows OpenSSH/WSL remote re-parse does not, so any command-line-embedded script arrives mangled.
  • Fix: KNOWN_SSH_HOSTS["desktop"] now carries remote_shell="windows", threaded through to SSHEnvironment. _run_bash branches — POSIX hosts keep the existing shlex.quote()'d bash -c path (unchanged); desktop sends the script over the SSH stdin channel instead (bash -s), sidestepping the remote command-line re-parse entirely.

Test plan

  • tests/tools/test_ssh_environment.py — 3 new tests covering POSIX quoting (unchanged) and the new stdin-based Windows path (script-via-stdin, login flag, real stdin_data appended after the script)
  • tests/tools/test_ssh_environment.py + tests/tools/test_terminal_host_param.py — 26/26 pass
  • Live end-to-end via terminal_tool(host="desktop", ...): compound commands, nested quotes, and pipes all correct (pwd confirms WSL at /mnt/c/Users/DaveV)
  • Live end-to-end via terminal_tool(host="macmini", ...): unchanged, still correct — no regression on POSIX hosts
  • Broader tests/tools/ suite (7150+ tests) — no new failures vs. baseline

🤖 Generated with Claude Code

teknium1 and others added 30 commits July 7, 2026 12:42
Same 10062 degrade-gracefully pattern as _run_simple_slash: create the
thread anyway, skip the ephemeral followups that need a live
interaction token. Non-expiry defer errors still raise.
…#58962)

A session wedged against an unresponsive OpenAI-compatible provider can hit the stale-stream detector on every turn and loop forever, burning the full 180s x retries each turn with no response. Issue NousResearch#58962 reports 494 consecutive failures over 3+ days on a single session.

The streaming retry path already caps retries WITHIN a turn (HERMES_STREAM_RETRIES, default 2) but has no cross-turn cap. Once a session's conversation state makes every turn stale, it retries indefinitely across turns and never notifies the user.

Add a per-session consecutive-stale-stream counter on the agent:
- incremented on every stale-stream kill in the outer poll loop;
- reset to 0 only when a stream actually completes;
- when it reaches HERMES_STREAM_STALE_GIVEUP (default 5), the next turn aborts immediately with a clear, actionable RuntimeError instead of spending 180s x retries again.

This is distinct from the existing stale-stream work (local-provider hard ceiling NousResearch#44938, backoff/parse-error NousResearch#60031): those bound a single hung stream, while this bounds repeated cross-turn staleness and surfaces a user-visible error.

Adds tests/run_agent/test_stream_stale_circuit_breaker.py covering the short-circuit, the success-reset, and the increment.
Follow-up for the salvaged NousResearch#60332 circuit breaker. The breaker latches:
once the streak trips, interruptible_streaming_api_call raises before any
stream is attempted, so the on-success reset can never run again. The
error text tells the user to switch models and retry — but neither
switch_model() nor try_activate_fallback() cleared the streak, so a
freshly selected healthy provider kept short-circuiting forever (only
/new recovered), and the automatic fallback chain was wedged the same way.

Reset the streak at both swap sites (after a successful rebuild only;
rollback/exhaustion paths keep the latch). 4 tests.
…-swap resets

Review findings on the salvaged NousResearch#60332 breaker, fixed as follow-ups:

- restore_primary_runtime() now resets the streak (third provider-swap
  path; without it a recovered primary was short-circuited before a
  single attempt and could never be re-proven healthy except via /model).
- interruptible_api_call (non-streaming) now carries the same breaker
  (guard at entry, bump on stale_call_kill, reset on success). Quiet-mode
  / subagent / headless sessions — the profile most like NousResearch#58962's
  unattended 494-failure session — take this path and had the identical
  infinite stale-retry class.
- Partial-stream stub return now resets the streak (chunks were received,
  provider demonstrably responsive).
- Consolidated the triple-duplicated counter arithmetic into shared
  helpers (_stale_streak/_bump_stale_streak/_reset_stale_streak/
  _check_stale_giveup) with one canonical comment block; error message
  now says 'consecutive stale attempts' (the counter counts kills, not
  turns — a single turn can produce several).

4 new tests (restore resets / no-op restore keeps latch / non-streaming
short-circuit / non-streaming success reset).
Auxiliary Bedrock resolution always used the Anthropic Bedrock SDK, which
only works for Claude foundation-model IDs. Non-Claude models such as
openai.gpt-oss-20b-1:0 now use a Bedrock Converse adapter, matching the
main agent's bedrock_converse transport.
Assert gpt-oss Bedrock IDs resolve to BedrockAuxiliaryClient while Claude
IDs keep the Anthropic SDK path, including async mode.
…nverse shim

Review findings on the salvaged shim: (a) OpenAI callers may pass stop as
a bare string but Converse's stopSequences requires a list — normalize;
(b) call_llm(stream=True) (MoA aggregator) can reach this client and the
shim silently returned a complete response — keep that behavior (the
streaming consumer's got-final-object path downgrades gracefully) but log
it, and log dropped tool_choice, instead of silently ignoring both.
+2 regression tests.

Follow-up to the salvage of NousResearch#60217 by @xxxigm.
…usResearch#55614)

Salvage of NousResearch#55614 by @kartik-mem0 (mem0 maintainer). Adds a SelfHostedBackend
that talks to a self-hosted Mem0 Docker server over httpx (X-API-Key auth,
/search + /memories routes), gated behind `host`. Also folds in the mem0
research-team recall tuning that rides with it: rerank defaults to false across
all modes, the mem0_list tool is removed (5->4 tools), search guidance is
de-shouted, and self-hosted get_all reports the true stored total (NousResearch#52921).

Supersedes the self-hosted portion of NousResearch#52487 (@liuhao1024, first-submitted).

Closes NousResearch#52478
Fixes NousResearch#52921
…edence

Follow-up on the salvaged NousResearch#55614. The PR added host-based routing to
_create_backend (precedence: oss > host > platform) but two sibling surfaces
didn't mirror it:

- system_prompt_block() checked host before oss, so an oss+host config ran
  OSS but told the model it was self-hosted HTTP. Reordered to match routing.
- Platform setup (hermes memory setup mem0 --mode platform) left a stale host
  in mem0.json; since host beats platform, the user kept routing to the
  self-hosted server. save_config merges (no delete), so clear host to ""
  rather than pop() so the merge actually overwrites it.

Adds regression tests for both (mutation-checked).
…M0_HOST env override

Review follow-ups on the salvage:

- get_all() pruned from the ABC and all three backends: mem0_list (its
  only caller) was removed by the recall-tuning commit, leaving new,
  tested, unreachable code — including SelfHostedBackend's _MAX_TOP_K
  over-fetch workaround. Tests for it dropped; fake-class stubs remain
  harmlessly. (The NousResearch#52921 true-total fix lives on in the PR history if
  a lister ever returns.)
- The persisted rerank config key was write-only (setup prompted for it,
  nothing read it). initialize() now parses it into _rerank_default and
  mem0_search uses it when the model doesn't pass rerank explicitly;
  per-call args still win. Guard test added.
- Platform-mode setup now warns when MEM0_HOST is set in the environment:
  the json host-clear can't help there (_load_config seeds host from the
  env var, docs tell users to put it in .env) — the user would silently
  keep routing to the self-hosted server.
- SelfHostedBackend: connect-level retries (httpx.HTTPTransport(retries=2))
  so a single transient blip doesn't count toward the provider breaker;
  transport now injectable and the test helper uses the real __init__
  instead of mirroring it via __new__.
- plugin.yaml description no longer leads with reranking (off by default,
  platform-only); docs em-dash typo fixed.
Implements a professional, standalone HTML export feature for Hermes sessions.

Key changes:
- Adds 'hermes sessions export <file>.html' support to the CLI.
- Implements a dark-mode-first, responsive HTML generator in 'hermes_cli/session_export_html.py'.
- Single session export features a focused, centered 90% width layout.
- Multi-session export adds a fixed sidebar with session switching and real-time search filtering.
- ZERO external dependencies; all styles and JS are embedded for offline portability.
Salvage follow-up integrating PR NousResearch#30481 (@simplast) and PR NousResearch#57683
(@catbearlove1-lang) into the unified export surface:

- --format html: standalone self-contained HTML transcript (single
  session or multi-session with sidebar), works with all shared filters
  and --redact; requires a file output path.
- --only user-prompts: prompt-only export (jsonl records or md sections)
  via the shared session_export renderer; the separate export-prompts
  subcommand from the original PR is subsumed by this flag.
- AUTHOR_MAP entries for both contributors; docs EN + zh-Hans.
Opt-in discord.approval_mentions (config.yaml, bridged to
DISCORD_APPROVAL_MENTIONS) prepends <@id> mentions for numeric
allowlist entries to exec-approval prompts, with a scoped
AllowedMentions override (users only). Default off - no surprise
pings. Reapplied onto the content-mirror layout from NousResearch#60245: mentions
prepend to the visible content block and its truncation budget.

Original implementation from PR NousResearch#39719; commits arrived bot-authored,
re-attributed to the contributor.
`_save_anthropic_oauth_creds` wrote the Anthropic OAuth token file with
`os.replace(tmp, path)` followed by a post-hoc `chmod(0o600)`. Between the
rename and the chmod the token file existed at the default umask (0o644 on most
hosts) — a window in which another local user could read the access/refresh
tokens.

Write via `utils.atomic_json_write(..., mode=0o600)`, which creates the temp
with mode 0o600 *before* any content is written, fsyncs, atomically replaces,
preserves the existing file's owner, and cleans up its temp on failure. This
matches the `atomic_json_write(mode=0o600)` call already used elsewhere in this
module for the credential-pool write, and NousResearch#56644's owner preservation.

Tests updated for the new mechanism, plus a check that the write goes through
`atomic_json_write(mode=0o600)` (mutation-verified).
The salvaged SelfHostedBackend made self-hosted servers reachable via
mem0.json / MEM0_HOST, but the setup wizard still offered only Platform
and OSS — exactly the gap users hit (Discord report: 'At memory setup
there's only 2 options'). Adds a third wizard mode:

- interactive picker: Platform / Self-hosted server / Open Source
- non-interactive: hermes memory setup mem0 --mode selfhosted
  --host http://... [--api-key ...] [--dry-run]
- host -> mem0.json (behavioral), API key -> .env as MEM0_API_KEY
  (secret), optional key for AUTH_DISABLED servers
- best-effort reachability check against the server, non-fatal
- README + memory-providers docs updated with the wizard path
…t trace' (NousResearch#60507)

* feat(trace): upload sessions to HF Agent Trace Viewer

Salvage trace upload as a smaller CLI-first feature: deterministic Claude Code JSONL export, fail-closed redaction, lazy Hugging Face dependency, and no gateway slash-command wiring.

* chore(trace): drop external porting references from docstrings

Describe the trace-upload design in Hermes' own terms.

* feat(sessions): fold trace upload into 'sessions export --format trace'

Integrates the HF Agent Trace Viewer exporter (PR NousResearch#36145) onto the
unified export surface instead of a separate 'hermes trace' subcommand:

- --format trace: Claude Code JSONL to stdout/file, or one
  <id>.trace.jsonl per session for filtered bulk export; defaults to
  the most recent session when no --session-id/filters given.
- --upload pushes to the user's private HF traces dataset (--public to
  opt out of private); reads HF_TOKEN with guided setup when missing.
- traces are secret-redacted by default (force mode); --no-redact opts
  out after review; redaction failure blocks export (fail closed).
- hermes_cli/trace.py + subcommands/trace.py removed; agent/trace_upload.py
  is the single engine. Docs EN + zh-Hans; 4 new CLI tests.
teknium1 and others added 25 commits July 9, 2026 18:04
His noreply email has no numeric-id+ prefix, so the attribution CI's
auto-resolve pattern doesn't match it.
Every MemoryStore instance opened its own SQLite connection guarded by
its own RLock. Several providers coexist in one process (the main agent
plus every delegate_task subagent), so instances pointing at the same
memory_store.db raced as independent WAL writers. Combined with writes
that were not rolled back on error, one connection could leave an open
write transaction that pinned the write lock and made every other
connection's writes fail with "database is locked" for the full busy
timeout.

Instances for the same database now share ONE process-wide connection
and ONE re-entrant lock, so access is fully serialized and
cross-connection contention is impossible. The shared connection is
refcounted: closing one instance never tears it out from under a live
sibling, and the last close releases it. The connection runs in
autocommit (isolation_level=None) so a write that raises mid-method can
never leave a dangling transaction holding the write lock; the existing
explicit commit() calls become harmless no-ops.

The provider's shutdown() now calls the refcount-guarded close() instead
of just dropping the reference: leaving finalization to GC kept the
connection (and its write lock) alive indefinitely on long-running
gateways, prolonging the exact contention this fix removes. The last
provider now releases the connection deterministically while siblings
stay live; regression tests fail without the wiring.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Follow-ups for salvaged PR NousResearch#43819: the registry key was
str(Path(db_path).expanduser()) — a symlinked or relative path to the
same DB file got its own connection, silently reintroducing the exact
multi-writer contention the registry prevents. Key on Path.resolve()
(OSError-tolerant fallback). Adds a symlink regression test and the
AUTHOR_MAP entry for adambiggs.
switch_model() rebuilds _client_kwargs from scratch (api_key + base_url)
but does not call _apply_client_headers_for_base_url(), so provider-
specific headers like OpenRouter HTTP-Referer and X-Title are lost.
Subsequent requests show "Unknown" in OpenRouter dashboard logs.

Call _apply_client_headers_for_base_url() after rebuilding _client_kwargs
and before creating the new client.

Fixes NousResearch#61099
Three tests for the NousResearch#61099 salvage: OpenRouter attribution headers
present after switching to openrouter.ai, Kimi User-Agent sentinel
present after switching to api.kimi.com, and stale headers cleared
when switching to a provider with no URL-specific headers.
2/3 fail on unpatched main (DID NOT ATTACH), confirming the bug.
Sibling site of the load_cli_config fix (NousResearch#58277): _deep_merge treated a
YAML-null section (terminal: with no value) as an override, replacing
the entire DEFAULT_CONFIG dict for that section with None. Every
downstream consumer expecting a mapping was a latent crash, and default
sub-keys were silently lost. A None override of a dict default is now
ignored, matching the CLI loader's behavior. Scalar-null overrides are
unchanged.
GatewayConfig.from_dict(), PlatformConfig.from_dict(), SessionResetPolicy.from_dict(), and StreamingConfig.from_dict() assumed their input sections were mappings. A malformed scalar from legacy gateway.json or an internal caller could crash config loading with AttributeError before env overrides/defaults had a chance to recover.

Coerce non-mapping sections to empty dicts, skip malformed platform entries, and keep valid sibling platform configs loading normally.

Tests cover scalar platform blocks, scalar nested reset/streaming sections, and malformed PlatformConfig home_channel/extra values.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
The streaming fallback path read yaml_cfg.get("gateway", {}).get("streaming") when top-level streaming was absent or malformed. If a user accidentally set gateway to a scalar value, config loading crashed with AttributeError instead of ignoring the malformed block and using defaults.

Read the gateway block once, verify it is a mapping before accessing nested streaming, and keep the existing gateway.platforms fallback using the same checked value.

Adds a regression test for config.yaml containing gateway: disabled.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
…econnect compat

The base adapter's  signature was updated to include
, which the reconnect watcher passes as
 during reconnection. All other platform adapters were
updated, but QQAdapter was missed, causing:

    TypeError: QQAdapter.connect() got an unexpected keyword argument 'is_reconnect'

This leads to an infinite retry loop since every reconnect attempt fails
immediately with the same TypeError.

Fix: add  to QQAdapter.connect()'s signature.
QQBot has no server-side update queue, so the flag is accepted only for
interface conformance.

Test: new test_connect_accepts_is_reconnect_param verifies both
adapter.connect() and adapter.connect(is_reconnect=True) succeed without
raising.
…onnect

The gateway reconnect watcher forwards is_reconnect=True to every
adapter.connect() call on every retry. Adapters whose signature omits
the kwarg raise TypeError at every reconnect attempt and stay silently
disconnected — the exact bug that shipped for QQAdapter and only
surfaced after messages stopped flowing on the QQ channel for hours.

This test statically parses every adapter.py under gateway/platforms/
and plugins/platforms/ (via AST, so third-party SDKs like slack_sdk,
matrix-nio, aiohttp, telegram, etc. are NOT required in the test env)
and asserts every *Adapter class with an async connect() accepts
is_reconnect — either as a keyword-only argument or absorbed by
**kwargs.

Also fixes plugins/platforms/wecom/callback_adapter.py:WecomCallbackAdapter,
which the new test caught as a second offender. Same class of bug: bare
'async def connect(self)' signature would die on the first reconnect.

Companion to NousResearch#59429 (which fixed the original QQAdapter offender).
Addresses Copilot review on NousResearch#61348: the HTML-escaped role, while safe from
injection (quotes are escaped), still contains whitespace when a crafted role
is supplied, which splits the class attribute into several unintended CSS
classes. Keep the escaped role for the display badge, and reduce the raw role
to a single safe CSS token (alnum/-/_) for the class name. Real roles
(user/assistant/system/tool) are unchanged, so the existing .message-<role>
rules still match.
_load_context_cache() returned None when context_length_cache.yaml
contained 'context_lengths:' (no value) — YAML parses this as
{'context_lengths': None} and dict.get(key, default) only returns
the default when the key is absent, not when the value is None.

This caused AttributeError in every downstream caller (issue NousResearch#47135).

Fix: use 'or {}' instead of default= so both absent key and
None value return an empty dict.

Fixes NousResearch#47135
…ousResearch#61797)

skills: null crashed with AttributeError, and a bare scalar
disabled: my-skill was split into a set of characters. Both now
normalize the same way agent.skill_utils._normalize_string_set does:
null -> empty set, scalar -> single-item set. Non-dict skills
sections are ignored.

Closes NousResearch#13026.
…ributeError

When stt.local, tts.edge, or other config subsections are explicitly set
to null in config.yaml (which happens by default on a fresh --voice
setup), stt_config.get('local', {}) returns None instead of {} because
YAML null preserves the key.  The chained .get('model') then crashes
with 'NoneType' object has no attribute 'get'.

Apply the defensive (x or {}) pattern to every place a config subsection
is read via .get('xxx', {}).  Covers local, edge, openai, mistral, and
elevenlabs subsections in both transcription_tools.py and tts_tool.py.

Closes NousResearch#47318
Sibling sites of the salvaged NousResearch#47334 fix: xai/openai/elevenlabs/gemini/
mistral/piper/neutts subsection reads in transcription_tools.py and
tts_tool.py used .get(key, {}) which passes a present-but-null value
through as None. All provider-subsection reads now use .get(key) or {}.

Providers without a DEFAULT_CONFIG entry (e.g. stt.xai) were still
receiving None even after the load_config() deep-merge fix, since the
merge can only fill sections that have defaults.
# Conflicts:
#	scripts/release.py
The desktop host's remote-side re-parse of the joined ssh command line
doesn't preserve shlex.quote()'s POSIX quoting, so bash -c <quoted cmd>
arrived mangled (exit 126, unexpected EOF). Windows hosts now carry
remote_shell="windows" in KNOWN_SSH_HOSTS and send the script over the
SSH stdin channel (bash -s) instead, sidestepping the remote re-parse
entirely. POSIX hosts are unaffected — they keep the original
shlex.quote()'d bash -c path.

Verified live against macmini (unchanged, correct) and desktop
(compound commands, nested quotes, pipes all correct).
@DaveVoyles
DaveVoyles merged commit 1ddf615 into main Jul 10, 2026
30 checks passed
@DaveVoyles
DaveVoyles deleted the fix/desktop-windows-ssh-stdin branch July 10, 2026 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.