feat(computer-use): draw self-localization action marker on follow-up captures - #2
Closed
trac3r00 wants to merge 166 commits into
Closed
feat(computer-use): draw self-localization action marker on follow-up captures#2trac3r00 wants to merge 166 commits into
trac3r00 wants to merge 166 commits into
Conversation
The cache used a '__default__' sentinel as its path key, so switching HERMES_HOME (profiles, tests) within one process kept serving the stale policy loaded from the previous home. Key the cache on the actual resolved default config path instead, so a home/config-path change naturally misses the cache. Trimmed from bundled PR NousResearch#3923 (the other sub-fixes are superseded on main); authored by @aydnOktay.
The Contents-API fallback's directory listing used a raw httpx.get with no retry, so a 429/403 rate limit aborted the whole skill download even though file fetches already retry via _github_get. Route the directory listing through the same helper (429/reset-aware backoff, 5xx retry, rate-limit flagging). Salvage of PR NousResearch#3033's intent — rerouted through the _github_get helper that landed after the PR was opened, instead of the PR's ad-hoc retry loops. Co-authored-by: 0xbyt4 <35742124+0xbyt4@users.noreply.github.com>
'KEY=os.getenv(...)' / 'os.environ[...]' / 'process.env.X' values are variable-name references in code snippets, not leaked secrets. Masking them corrupted pasted code in prose/log contexts (issue NousResearch#2852): ha_token=os.getenv('HOMEASSISTANT_TOKEN') -> ha_token=os.get...EN'). Skip these values inside _redact_env, which covers all three passes that share the closure (_ENV_ASSIGN_RE, _CFG_DOTTED_RE, _CFG_ANCHORED_RE). Real secret values are still masked. Salvage of PR NousResearch#2852-fix NousResearch#2854 — the PR's own placement (an unconditional pass before the code_file gate) would have reintroduced the code-file false-positive class; the skip is applied inside the existing gated pass instead. Tests adapted from the PR. Co-authored-by: crazywriter1 <sampiyonyus@gmail.com>
…ment Salvage of NousResearch#35362, evolved to also close the vision sandbox-escape (GHSA-gpxw-6wxv-w3qq). The two were the same root cause — vision read image bytes host-side while every other tool reads through the terminal backend — so one resolver fixes both the delivery gaps and the escape. Delivery (from NousResearch#35362, re-authored against current main since the branch was 4140 commits stale and vision_tools.py had been rewritten on both sides): - tools/image_source.py: one resolver for data:/http(s)/file/local/container image sources, returning raw bytes through a single magic-byte-sniff + 50MB-ingest chokepoint. Fixes 'no image attached' / 'Invalid image source' for every source type (NousResearch#7571, NousResearch#25118, NousResearch#29643, NousResearch#22328, NousResearch#32709, NousResearch#9077). - tools/credential_files.py: from_agent_visible_cache_path, the container->host cache reverse-map (inverse of the existing forward twin). - tools/vision_tools.py: both vision sites route through the resolver with task_id threaded from the handler; resolved bytes are materialized to a temp file so main's evolved encode/resize/embed-cap pipeline is reused verbatim (kept over the PR's older bytes-core resize to avoid touching browser_tool / conversation_compression callers). Security (fills NousResearch#35362's deliberately-stubbed _within_allowed_roots seam): - Under a non-local terminal backend the file tools are confined to the sandbox (SECURITY.md 2.2), but vision read host-side — a prompt-injected vision_analyze('/etc/passwd') exfiltrated host secrets, and read_file even redirects the model to vision_analyze for image paths. The resolver now enforces the same boundary: local backend reads any host path (chosen posture); non-local backend host-reads ONLY the media caches under HERMES_HOME (where the gateway/download media lives) and routes every other path to an in-sandbox base64 exec-read — which reads the CONTAINER's file, the same one 'cat' would, never the host's. Paths are resolve()-d so a symlink can't escape a cache; fail-closed when no sandbox env exists. This closes the escape AND delivers container-only images (NousResearch#32709) with the same mechanism. Tests: unified resolver + confinement model (tests/tools/test_image_source.py, incl. proof a non-cache host path under Docker yields container bytes not the host secret); existing vision tests updated to the resolver boundary; Docker integration test verified green against a real daemon (exec-read of a tmpfs /workspace file, a root-owned mode-600 file, and the host-secret invariant). Fixes GHSA-gpxw-6wxv-w3qq. Co-authored-by: banditburai <promptsiren@gmail.com>
…d code, async I/O
- resolve_image_source: bare cwd-relative filenames ('pic.png') resolve
again (main accepted them; the path-shape gate regressed them — review
by egilewski). Unknown explicit schemes (ftp://, s3://) still rejected.
- Local backend: nonexistent path now raises a clean 'image file not
found' instead of a misleading sandbox-fallback message.
- W4: remove the now-dead path-based _detect_image_mime_type (suffix-trust
SVG acceptance) so future callers can't reintroduce it.
- W3: SVG sources rejected with a dedicated actionable message + test.
- Polish: host read_bytes / temp-file write_bytes offloaded via
asyncio.to_thread (matches the container exec-read); unused
ResolveContext.cfg/extra_roots fields dropped; duplicate policy check
documented as intentional pre-flight short-circuit.
…dding vision_analyze embedded SVG (and BMP/TIFF) tool-results into conversation history with media_type image/svg+xml. Anthropic only accepts jpeg/png/ gif/webp, so the request fails with a non-retryable 400. Because the image is baked into immutable history and re-sent every turn, the session is permanently wedged on resume — retries re-send the same bad bytes. Add _normalize_to_supported_image(): SVG is rasterized to PNG (best-effort via cairosvg/svglib/rsvg-convert/inkscape), other non-supported raster formats are re-encoded to PNG via Pillow, and if conversion is impossible the tool returns an actionable error instead of a session-wedging payload. Wired into both the native-vision fast path and the auxiliary-API path so the whole bug class is covered, not just the one call site. All 99 existing vision tests pass.
… rasterizer; AUTHOR_MAP for NousResearch#52688 - _normalize_to_supported_image: ensure cache/vision exists before writing the converted PNG (fresh HERMES_HOME had no dir -> FileNotFoundError). - resolver: SVG passes through as image/svg+xml instead of erroring; the call sites rasterize to PNG via the salvaged normalize step (cairosvg / svglib / rsvg-convert / inkscape, best-effort with actionable error). - normalization offloaded via asyncio.to_thread at both call sites. - tests: resolver pass-through + rasterization + no-converter error paths. - AUTHOR_MAP: jonathan@mintrx.com -> JAlmanzarMint (PR NousResearch#52688 salvage).
The salvaged NousResearch#52688 rasterizer shell-out predates the TUI subprocess stdin= guard; a rasterizer that prompts on stdin could hang the tool under prompt_toolkit. DEVNULL it.
The container exec-read piped the whole file through base64 with no size guard — the 50MB cap was only enforced host-side AFTER the full payload had already streamed into host memory. A prompt-injected read of a huge container file (or /dev/zero) could balloon the gateway process. head -c (cap+1) bounds the read inside the sandbox; the +1 byte lets the host distinguish at-cap from over-cap and reject with SourceTooLarge. Input redirect replaces 'base64 --' (no argv exposure at all for leading-dash paths). Docker integration tests re-verified live.
The webhook adapter enforced max_body_bytes only via the Content-Length header; a Transfer-Encoding: chunked request (content_length=None) or a spoofed small Content-Length bypassed the cap entirely and read the full body (bounded only by aiohttp's implicit 1 MiB default, above any operator-configured smaller limit). - web.Application(client_max_size=max_body_bytes): aiohttp enforces the cap on every read path, chunked included - catch HTTPRequestEntityTooLarge -> 413 (was swallowed into generic 400) - post-read length re-check as defense in depth - chunked-upload regression test Manual port of PR NousResearch#3955 by @Gutslabs onto current main (handler had been restructured since); authorship preserved.
api_server already caps every read via client_max_size (chunked included), but when the limit tripped mid-read the handler's broad JSON except turned it into 400 'Invalid JSON'. Catch HTTPRequestEntityTooLarge in body_limit_middleware and return the OpenAI-style 413. Status-code polish extracted from PR NousResearch#3949 by @Gutslabs — the PR's core client_max_size change already exists on main.
…o usable entry
_try_anthropic() hard-failed (return None, None) when the anthropic
credential pool was present but had no selectable entry — e.g. the pooled
OAuth token expired and its refresh_token had gone stale, so
_select_pool_entry("anthropic") returned (True, None). This wedged every
auxiliary task routed to Anthropic (goal judge surfaced "no auxiliary
client configured") even when a perfectly valid ANTHROPIC_TOKEN /
credentials-file token was available. The main session stayed healthy
because it resolves the env token directly.
The openrouter path (_try_openrouter) and codex path already fall through
to their standalone credential on (True, None); anthropic was the only
provider that hard-failed. Make _try_anthropic fall through to
resolve_anthropic_token() on that branch so the three paths are symmetric:
a temporarily dead pool entry must not block auxiliary tasks when a valid
standalone credential exists.
Adds a regression test covering: (1) pool present + no entry + valid env
token -> client built from the env token, (2) pool present + no entry + no
resolvable token -> clean (None, None), (3) base_url defaults correctly
when falling through with pool_present=True.
…llback When OpenRouter routes to an endpoint that does not support tool/function calling, it returns HTTP 404 with the message 'No endpoints found that support tool use. Try disabling "browser_back".' The raw error body does not contain 'model not found' or any other _MODEL_NOT_FOUND_PATTERNS entry, so it falls through to FailoverReason.unknown with retryable=True. The retry loop wastes 3-5 attempts on the same deterministic rejection, then surfaces a confusing generic error instead of automatically failing over to a fallback model or provider. Adding the OpenRouter phrase to _MODEL_NOT_FOUND_PATTERNS classifies it as model_not_found (retryable=False, should_fallback=True), which triggers the client-error fast-fallback path in conversation_loop.py: the agent switches to a configured fallback model/provider before the user sees the error. Existing buffered guidance in conversation_loop.py (the 'support tool use' hint at line ~2967) remains intact and surfaces only if every fallback exhausts.
- ChatCompletionsTransport.normalize_response: convert integer finish_reason (e.g. 24) to string for Poolside compatibility - Chat completion helpers: handle integer tool_call.id during streaming by converting to string - Add Poolside as first-class CANONICAL_PROVIDERS entry (visible in CLI/TUI/desktop provider pickers)
- package-lock.json changes in NousResearch#58451 were unrelated peer-flag churn - CANONICAL_PROVIDERS 'poolside' entry from NousResearch#58374 has no ProviderConfig in hermes_cli/auth.py and no setup flow, so the picker entry would be dead; the wire-format coercions stand on their own
Per-session /model overrides supplied api_key and provider but omitted credential_pool, so billing rotation never ran on HTTP 402. Wire the pool on fast override, rehydrate, and apply paths; backfill from provider for legacy persisted overrides. Regression tests in tests/gateway/.
… auto-reset After a config change (e.g. switching model provider), the /new command must clear the per-session _last_resolved_model cache so the next turn resolves the model from the updated config instead of falling back to the stale cached value. Without this fix, if a transient config-cache miss occurs on the first post-/new turn, the NousResearch#35314 recovery path serves the old model from the cache — the user sees the old model being used even though they changed config.yaml and explicitly ran /new. Fix applies to both call sites that reset session model state: - GatewaySlashCommandsMixin._handle_reset_command (slash_commands.py) - GatewayRunner compression-exhausted auto-reset (run.py) Fixes NousResearch#58403
_normalize_custom_provider_entry() runs on every load_picker_context() call (per picker/inventory request) and warned each time for (a) the redundant `provider` key that Hermes' own config writer emits into provider entries and (b) any other unknown key. On Windows the serve launcher+worker pair share one rotating log via concurrent-log-handler's cross-process lock, so that per-load warning volume drove 'Cannot acquire lock after 20 attempts' retries that pegged a core, stalled the event loop ~14s, and dropped every desktop/TUI WebSocket while /health stayed green (gateway looked down; dashboard looked fine). - Accept `provider` as a known key (silently ignored) so self-written legacy configs don't warn. - Deduplicate the normalizer's warnings per (provider, signature) so a static config quirk is surfaced once, not on every inventory load. Adds regression tests for both. Fixes NousResearch#58265
…ener On Windows every Hermes process (gateway, serve, TUI/slash workers, MCP servers, CLI commands) writes the shared rotating logs through concurrent-log-handler's cross-process rotation lock. When the emitting thread is an asyncio event loop, a lock wait blocks the loop — stalling it for seconds and dropping WebSocket clients (the 'gateway keeps going down' symptom seen in NousResearch#58265). Route every file handler through a single QueueListener on a dedicated thread: loggers only enqueue (non-blocking); the listener does the file I/O and rotation-lock wait off the hot path. The QueueHandler funnels via the root logger; per-handler levels and component filters are preserved by respect_handler_level + handler.handle on the listener thread. An atexit hook stops the listener before logging.shutdown closes the file handlers. - _NonFormattingQueueHandler passes the raw record (in-process queue) so target handlers apply their own RedactingFormatter/filters. - flush_log_queue() drains synchronously (shutdown + tests). - rotating_file_handlers() exposes the handlers now behind the listener; tests updated to use it. Extends the NousResearch#58265 fix: the provider-key warn-storm was one amplifier of this contention; this takes the contention off the event loop entirely.
The QueueListener change routes rotating file handlers through an in-memory queue drained on a dedicated thread, with an atexit hook to flush on shutdown. But _exit_after_graceful_shutdown() uses os._exit, which bypasses atexit — so on the early-exit and NousResearch#53107 hard-exit paths the queued records (including the shutdown reason) were silently lost. Explicitly flush_log_queue() before os._exit, and correct the now-stale comment that claimed handlers are synchronous with nothing pending.
…ord copy Self-review (3-agent + codex) findings on the async QueueListener change: 1. (HIGH) The os._exit shutdown backstop called flush_log_queue(), whose stop() joins the listener thread unbounded. If that thread is wedged on the rotation lock — the exact failure this change survives — shutdown re-freezes. Add drain_log_queue(timeout): stop-only, bounded via a throwaway joiner thread. Also release PID/runtime locks BEFORE the drain so a slow drain can't strand them. 2. (MED) _log_queue/_queue_listener/_queued_file_handlers were read-modify- written without a lock across register/stop/flush/reset; a gateway-init race with a plugin/CLI path could leave two live listeners. Guard all four globals with a single _queue_state_lock. 3. (MED) _NonFormattingQueueHandler.prepare() enqueued the same LogRecord a synchronous handler on the emitting thread may still format/mutate. Return copy.copy(record) (preserves msg/args/exc_info for deferred RedactingFormatter) to remove the cross-thread mutation race. E2E-verified: bounded drain returns in ~500ms on a permanently-wedged listener; 4x20 concurrent flushes single-listener no-crash; args still format and secrets still redact through the copied record.
Extend the pre_tool_call plugin hook return contract with a new directive:
{"action": "approve", "message": "why this needs human confirmation"}
Previously a pre_tool_call hook could only veto a tool call (action: block)
or allow it silently. It could not escalate to the existing human-approval
flow. This unlocks user-defined runtime approval rules on ANY tool (HTTP
writes, file writes to sensitive paths, email sends), enforced at runtime —
resolving NousResearch#51221 as a pure plugin, with no core approval.py rule schema.
Mechanism:
- get_pre_tool_call_directive() returns (action, message) for block|approve;
get_pre_tool_call_block_message() kept as a block-only back-compat shim.
- resolve_pre_tool_block() is the single dispatch-site chokepoint: fetches
the directive and, for approve, invokes the human gate; fail-closed to a
block on denial, timeout, or gate exception. ALL FOUR tool-dispatch sites
now call it: tool_executor (concurrent + sequential), agent_runtime_helpers,
and model_tools.handle_function_call.
- request_tool_approval() escalates via the SAME machinery as Tier-2
dangerous commands: session/permanent allowlist, prompt_dangerous_approval
(CLI) / submit_pending (gateway), [o]nce/[s]ession/[a]lways/[d]eny,
timeout fail-closed, approvals.cron_mode for cron contexts.
Architecture: extracted the shared decision core into _run_approval_gate(),
called by BOTH check_dangerous_command() and request_tool_approval() so the
fail-closed / cron / gateway / yolo / persist policy lives in ONE place and
cannot drift. Fixed a latent divergence — the plugin path now honors --yolo.
Approval grain: [a]lways is keyed on tool_name + a hash of the reason (an
explicit plugin rule_key overrides), so distinct reasons on the same tool
persist independently instead of one 'always' blanketing the whole tool.
Non-interactive: cron honors approvals.cron_mode (parity with commands); any
other non-interactive non-gateway context fails CLOSED for the plugin path
(the command path keeps its historical fail-open default, unchanged).
No new config schema, no new env vars, no new hook events.
…lood-control edit storms (NousResearch#58563) Post-NousResearch#48648, oversized mid-stream edits truncate to a 4096-char preview instead of splitting. But when rich messages raise the consumer's overflow budget to 32k, the consumer keeps accumulating past 4096 and keeps issuing progressive edits every edit_interval — each one truncating to the SAME preview text. Telegram counts every one of those no-op requests against the flood budget: a long streamed reply fires ~1 identical edit per 0.8s for the rest of the stream, trips flood control (200s+ penalties), and the final delivery hangs behind inline flood sleeps. Users see the bot stuck 'streaming' and the chat unresponsive. Fix at the chokepoint: track the last truncated preview per (chat_id, message_id) and skip the API call when the new truncation is identical. Previews still update when the visible prefix actually changes (e.g. chunk-count marker 1/2 → 1/3). State clears on finalize and when content shrinks back under the cap, so dedup can never mask a real edit. Live repro: 19,956-char streamed reply, transport=edit, rich available — 4x flood-control hits within ~700ms, 250s penalties, hung final delivery. E2E harness on the same stream: 14 edit calls on main vs 7 with the fix (the delta is pure no-op duplicates; scales with stream length).
_do_reconnect() succeeded but never called YuanbaoAdapter.set_active(adapter), leaving get_active() permanently returning None after any WS disconnect/reconnect cycle. This caused cron delivery to silently fail because _send_yuanbao() checks get_active_adapter() and gives up immediately when it returns None. Fix: call set_active(adapter) after successful reconnect, matching the pattern in connect(). Fixes NousResearch#58363
…ALLOW_ALL_USERS The Cloud setup wizard and docs tell operators to set WHATSAPP_CLOUD_ALLOWED_USERS (and WHATSAPP_CLOUD_ALLOW_ALL_USERS), but the adapter DM intake gate only read WHATSAPP_CLOUD_ALLOW_FROM + WHATSAPP_CLOUD_DM_POLICY (default open, opted-in only via GATEWAY_/WHATSAPP_ALLOW_ALL_USERS). So an allowlist set via the documented var silently dropped every inbound (_should_process_message -> None -> HTTP 200, no dispatch, no log line). - _allow_from also reads WHATSAPP_CLOUD_ALLOWED_USERS - dm_policy defaults to allowlist when an allowlist is present (else open) - _open_dm_opted_in() also honors WHATSAPP_CLOUD_ALLOW_ALL_USERS Explicit DM_POLICY / ALLOW_FROM still win -> backward compatible.
…env vars Follow-up for salvaged NousResearch#58448 which shipped without tests.
…ousResearch#10270) (NousResearch#59130) * fix(docker): heal pairing-dir ownership after `docker exec` writes (NousResearch#10270) The official Docker image runs the gateway as the unprivileged `hermes` user (uid 10000) via `gosu`, but `docker exec` defaults to root. Approval files written by `docker exec <container> hermes pairing approve <code>` end up as `-rw------- root:root`, and the post-gosu gateway process cannot read them. The approval is silently ignored — the user keeps hitting 'Unauthorized user' on every message. The entrypoint's existing top-level chown is gated on the top-level $HERMES_HOME being mis-owned, so on warm boots (where /opt/data is already hermes:hermes) the recursive chown is skipped — meaning a container restart does NOT self-heal the bug either. Three-part fix: 1. docker/entrypoint.sh: chown the platforms/pairing/ (and legacy pairing/) subtree on every container start, regardless of the top-level decision. The directory is tiny (a few JSON files), so the unconditional chown is effectively free. Container restart now self-heals. 2. gateway/pairing.py: PairingStore._load_json was swallowing PermissionError under its bare 'except OSError' branch, which is what made this a silent failure. Split it out: log a WARNING that names the file, the gateway's uid, the file's owner/mode, and the exact docker exec -u hermes workaround. Still falls back to {} so the gateway stays up. 3. website/docs/user-guide/security.md: add a Docker tip to the pairing-CLI section pointing users at `docker exec -u hermes …` up front. Reproduced end-to-end in a containerized harness — before the fix the gateway sees 0 approved users after `docker exec` + restart; after the fix it sees the expected 1, and the file on disk goes from `root:root 600` back to `hermes:hermes 600` on next start. Fixes NousResearch#10270 * fix(pairing): gate os.geteuid for Windows in PermissionError warning
…ities The "Test server" probe (`_probe_single_server`, used by the Desktop/dashboard MCP tab, `hermes mcp add`, and `hermes mcp test`) called `prompts/list` and `resources/list` on every server unconditionally whenever `details` was requested. This ignored the user's `tools.prompts` / `tools.resources` config and the server's own advertised capabilities. Servers that don't implement those optional families (e.g. Unreal Engine's MCP server, which answers `Call to unknown method "prompts/list"`) therefore logged a hard error during discovery, and setting `tools.prompts: false` — the documented workaround — had no effect because the probe never consulted it. Mirror the runtime gating in `tools.mcp_tool._select_utility_schemas`: only probe a family when it is enabled in config AND advertised in the server's `initialize` capabilities. Falls back to the previous always-try behaviour when no capability info was captured.
Assert the "Test server" probe skips prompts/list when tools.prompts is false, skips both families when the server advertises neither capability (the Unreal MCP server case), probes both when advertised and enabled, and falls back to the legacy always-try behaviour when no capability info was captured.
Blank Slate's _blank_slate_minimal_toolsets() adds every TOOLSETS entry to agent.disabled_toolsets except file and terminal. The coding posture toolset (session-level, selected by agent/coding_context.py) slips through because the loop only skips hermes-* composites and includes-only groups. At runtime, model_tools.get_tool_definitions() resolves coding and subtracts its tools — terminal, read_file, write_file, patch, search_files, process — erasing the entire Blank Slate minimal surface. The agent ends up with only cronjob. Skip posture toolsets in the disabled-list computation. Posture toolsets are not user-facing capabilities to disable; they are per-session selections that should never appear in agent.disabled_toolsets. Fixes NousResearch#57315
…led_toolsets (NousResearch#57315) The disabled_toolsets subtraction loop in _compute_tool_definitions preserved shared core tools only for hermes-* platform bundles (NousResearch#33924), subtracting bundle_non_core_tools(); every other name took the else branch and got a full resolve_toolset() subtraction. The `coding` toolset is a posture toolset (posture: True) that re-lists the shared _HERMES_CORE_TOOLS it does not own, so disabled_toolsets=["coding"] stripped those core tools from the whole schema (34 tools collapsed to a handful; terminal/read_file/write_file/web_search/execute_code gone). Extend the core-preserving branch to also match posture toolsets, so they subtract only the non-core delta. Only `coding` carries posture: True, so atomic toolsets stay fully removable. The bundle-misconfiguration info log is gated to hermes-* names, since its wording is bundle-specific and disabled_toolsets=["coding"] is a legitimate config written by older `hermes setup` runs. Adds a regression test (TestDisabledToolsetsPostureToolset) alongside the existing NousResearch#33924 bundle tests.
Overlap-invariant regression test from PR NousResearch#58686 — no toolset in the blank-slate disabled_toolsets may share a tool with a kept toolset, since the subtraction happens at tool granularity (NousResearch#57315, NousResearch#58281).
…ort nanoclaw#2895) (NousResearch#59261) Port from nanocoai/nanoclaw#2895's never-silently-drop guarantee. Before: saveMedia() in scripts/whatsapp-bridge/bridge_helpers.js awaited downloadMedia() with no try/catch. A failed CDN fetch (expired media URL, transient network error — Baileys throws 'Failed to fetch stream from https://mmg.whatsapp.net/...') rejected out of extractBridgeEvent, which bridge.js awaits inside its messages.upsert for-loop with no per-message guard — dropping the failed message AND every remaining message in the same upsert batch, silently. After: - saveMedia catches download/write failures, records the media type, and logs a console.warn instead of rejecting. - appendMediaFailureNote() (exported pure helper, mirroring the file's testable-helper convention) surfaces '[<type> could not be downloaded]' in the event body, so the agent learns media was sent rather than the attachment vanishing. Applied before the '[<type> received]' fallback so an uncaptioned failed image reads as a failure, not an arrival. The reuploadRequest recovery half of nanoclaw#2895 is already wired in bridge.js (downloadMediaMessage(..., { reuploadRequest: sock.updateMediaMessage })); this ports the containment half hermes was missing. Tests: 3 new cases in bridge.native.test.mjs (note formatting, uncaptioned failure containment, captioned failure note). All 5 bridge test files pass.
…sk key is empty (NousResearch#9318) When an auxiliary task is configured with provider=custom and an explicit base_url but an empty api_key, the custom_key fallback chain in resolve_provider_client() jumped straight to the no-key-required placeholder without consulting model.api_key from config.yaml. Users on self-hosted gateways who share the same endpoint and credentials for both the main model and auxiliary tasks got 401 auth errors. Add _read_main_api_key() following the same pattern as _read_main_model() and _read_main_provider(): checks _RUNTIME_MAIN_API_KEY (runtime override) first, then config.yaml model.api_key. Insert it into the fallback chain before no-key-required so real credentials are used when available, while local servers without auth still get the placeholder.
Follow-up to the NousResearch#55911 salvage: inherit model.api_key only when the aux base_url resolves to the same hostname as the main model's base_url (runtime override or config). A misconfigured aux endpoint on a different host keeps the fail-safe no-key-required placeholder instead of leaking the main credential cross-host.
A `hermes update` that bumps the spectrum-ts pin rewrites the Photon sidecar's package-lock.json but never reinstalls node_modules. The sidecar then spawns against the old install and the v8 postinstall patch throws "@spectrum-ts/imessage dist not found", so the gateway retries the photon platform every 300s forever without ever repairing the deps. Observed in the wild: a June pin bump to spectrum-ts 8.0.0 left node_modules at 3.1.0, and inbound/outbound iMessage stayed dead for days with the reconnect loop faithfully restarting into the identical broken state. _start_sidecar only checked that node_modules exists, not that it matches the lockfile, so restart never became repair. Detect the skew with the same signal npm ci uses: the top-level package-lock.json being newer than npm's node_modules/.package-lock.json install marker. When stale, reinstall (npm ci, falling back to npm install) before spawning. The reinstall runs via asyncio.to_thread so a cold install can't block the event loop and stall every other platform's traffic; worst case it heals on the next reconnect tick instead. First-run "deps not installed" behavior is unchanged, and a missing/unreadable marker fails safe to "not stale" so start is never blocked. Reuses the existing npm ci -> npm install fallback from `hermes photon install-sidecar`. Adds unit tests for the staleness signal (stale / fresh / missing-marker).
Follow-up to the salvaged NousResearch#57943: a wedged npm (dead registry, network blackhole) ran unbounded inside asyncio.to_thread, holding the photon connect path hostage. Cap npm ci / npm install at 600s; on timeout, log and leave the stale deps in place so the readiness check reports the real error and the next reconnect tick retries.
_handle_webhook() called request.read() with no size guard. Since the endpoint is publicly reachable, an attacker can send an arbitrarily large POST body to exhaust gateway memory. Add _TWILIO_WEBHOOK_MAX_BODY_BYTES (64 KiB — well above any real Twilio payload) and gate on both Content-Length and actual read size, returning HTTP 413 with an empty TwiML Response on oversized requests. Mirrors the guard already present in the Raft adapter.
Follow-up to the salvaged NousResearch#54620: the post-read length check bounds processing but a chunked body is still buffered by aiohttp first. client_max_size enforces the same 64 KiB cap mid-read on every path (NousResearch#58536/NousResearch#58902/NousResearch#59180 pattern).
The salvaged NousResearch#25296 fixture's _FakeRequest.read() calls json.dumps but the test module never imported json — the NameError was swallowed by the handler's generic except → 400, failing 10 payload tests.
Follow-up to the salvaged NousResearch#54944: before this, aiohttp's implicit 1 MiB default client_max_size tripped BEFORE the intended 3 MB Meta cap could apply on read() paths — the explicit value makes the documented limit real while the bounded reader keeps chunked bodies from buffering past 3 MB (NousResearch#58536/NousResearch#58902/NousResearch#59180 pattern).
Follow-up to the salvaged NousResearch#54938: the bounded reader gives a proper 413 + anomaly telemetry for oversized chunked bodies; client_max_size makes aiohttp enforce the same 1 MiB cap on every other read path (NousResearch#58536/NousResearch#58902/NousResearch#59180 pattern). Test fixture's fake Application now accepts kwargs.
… captures
Codex-style self-localization for the computer_use tool. cua-driver paints a
tinted agent cursor on the user's physical screen, but that overlay never
lands in the PNG the model receives — so after a click/drag the model has no
visual confirmation of where its own action went. Game loops, drags, and
precise manipulation degrade because the model is blind to its landing point.
This composites a lightweight marker onto the follow-up capture PNG (the one
produced by capture_after=True) at the resolved landing point of the action
just executed:
* click / double_click / right/middle click -> translucent ringed crosshair
centered on the click point.
* drag -> start ring, end ring, and a connecting arrow shaft.
Design:
* New pure module tools/computer_use/action_marker.py owns the compositing.
Pillow is treated as an optional dependency and imported lazily; when it
is unavailable (or the input is not a decodable image), compositing
degrades to a no-op and returns the original bytes, so the capture path
never breaks.
* CuaDriverBackend records the resolved landing point on click()/drag()
(explicit x/y, or an element index resolved to its cached center pixel
from the last snapshot) and hands it over one-shot via
consume_action_marker(), so a later markerless capture never re-stamps a
stale point.
* tool._maybe_follow_capture composites the marker into the follow-up
screenshot before building the response. Gated by a new show_action_marker
schema flag (default true).
* This extends the existing capture path rather than adding a core tool,
preserving the narrow-waist contract. The marker is the *last action's*
landing point, semantically distinct from the SOM overlay's numbered
next-click candidates; the two coexist.
TDD: unit tests decode the composited PNG back to pixels and assert the marker
lands at the exact action point (click crosshair through the point; drag
endpoints + shaft), that far regions stay untouched, that image dimensions are
preserved, and that the layer degrades gracefully with no coordinates / bad
PNG bytes. An E2E test drives tool._dispatch end-to-end through a stub backend
and verifies the returned multimodal screenshot carries the marker, that
show_action_marker=false yields a clean screenshot, and that the marker is
one-shot.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
40 issues found across 288 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tools/computer_use/permissions.py">
<violation number="1" location="tools/computer_use/permissions.py:67">
P1: Secret stripping can be bypassed on sanitizer/import failure, so `cua-driver` may receive inherited provider credentials. Keeping graceful degradation but adding a fail-closed scrub fallback (instead of returning raw `env`) would preserve probe reliability without reintroducing the secret-leak path.</violation>
</file>
<file name="tests/gateway/test_10710_auto_reset_evicts_cached_agent.py">
<violation number="1" location="tests/gateway/test_10710_auto_reset_evicts_cached_agent.py:124">
P1: The new `test_auto_reset_cleanup_clears_last_resolved_model` assertion checks `_references_name(node, "_last_resolved_model")` which walks the AST for `ast.Constant` nodes matching that string — but production code accessing `self._last_resolved_model` or `_last_resolved_model` (attribute/variable) produces `ast.Attribute` or `ast.Name` nodes, not `ast.Constant`. The test would fail even when the correct `pop` call is present in the cleanup block, or worse, it could silently pass on a false match (e.g. a docstring or comment-like constant that happens to contain the string). Update `_references_name` to also match `ast.Attribute.attr` and `ast.Name.id` against the literal.</violation>
</file>
<file name="tools/computer_use/cua_backend.py">
<violation number="1" location="tools/computer_use/cua_backend.py:982">
P1: CLI fallback failures can be misreported as successful tool actions. `_call_tool_via_cli()` always returns `isError=False`, so `_action()` treats error responses as `ok=True`; propagate subprocess/error state into `isError` to preserve correctness.</violation>
<violation number="2" location="tools/computer_use/cua_backend.py:1689">
P2: Follow-up screenshots can show a stale click marker instead of the most recent action site. `click()` stores `_last_action_marker` before `_action()` succeeds, so failed/no-follow-up clicks leave pending state that later captures may consume; recording the marker only after a successful click avoids misleading localization.</violation>
</file>
<file name="plugins/platforms/photon/adapter.py">
<violation number="1" location="plugins/platforms/photon/adapter.py:178">
P1: The `_reinstall_sidecar_deps` function (called during reconnect when sidecar deps are stale) only catches `subprocess.TimeoutExpired` from `subprocess.run`, but `subprocess.run` can also raise `OSError` (permissions, deleted npm binary between `shutil.which` and execution). This propagates out of the function and crashes the reconnect flow in `_start_sidecar` via `asyncio.to_thread`, contradicting the documented "best-effort" contract. Wrap the `subprocess.run` calls with an outer `try/except OSError` (or broad `except Exception`) that logs the error and returns, consistent with the best-effort intent.</violation>
</file>
<file name="tools/environments/docker.py">
<violation number="1" location="tools/environments/docker.py:915">
P1: Air-gapped mode can be lost after recovery: when `docker_network=false`, a later "No such container" retry can still reattach to a network-enabled labeled container and restore egress unexpectedly. The new NetworkMode guard runs only in `__init__`; mirroring it in `_recreate_container()` would keep isolation consistent across retries.</violation>
</file>
<file name="hermes_cli/profiles.py">
<violation number="1" location="hermes_cli/profiles.py:1064">
P2: Profile exports can become impossible to import when the source contains symlinks. `export_profile` now preserves links in the tar, but `import_profile` explicitly rejects link members, so using the exported backup as a restore path fails for those profiles.</violation>
</file>
<file name="hermes_cli/web_server.py">
<violation number="1" location="hermes_cli/web_server.py:712">
P2: The `updates.refresh_cua_driver` schema override uses `"type": "bool"` but the codebase convention (from `_infer_type`) is `"type": "boolean"` for all Python `bool` values. The frontend likely doesn't recognize `"bool"` and may render this as a plain text input instead of a toggle. Use `"boolean"` to match every other bool field.</violation>
</file>
<file name="tools/computer_use/tool.py">
<violation number="1" location="tools/computer_use/tool.py:892">
P2: Disabling `show_action_marker` currently suppresses drawing but also skips marker consumption, so a later `capture_after` can display a stale marker from an earlier action. Consuming the pending marker even when drawing is disabled keeps the one-shot contract and prevents misleading follow-up screenshots.</violation>
</file>
<file name="tools/computer_use/doctor.py">
<violation number="1" location="tools/computer_use/doctor.py:64">
P2: A sanitizer runtime error currently causes doctor to fall back to the raw environment and pass provider secrets to the third-party `cua-driver` process. Narrowing this to import-only fallback keeps stripped-down environments working without silently bypassing sanitization on real sanitizer failures.</violation>
</file>
<file name="mcp_serve.py">
<violation number="1" location="mcp_serve.py:464">
P2: In pre-migration fallback mode, the bridge can miss newly registered conversations because polling now skips whenever `state.db` mtime is unchanged, even though `_load_sessions_index()` may be sourcing routing keys from `sessions.json`. Restoring a sessions-index change signal (or bypassing the mtime skip when fallback index is in use) would keep fallback behavior correct.</violation>
</file>
<file name="agent/auxiliary_client.py">
<violation number="1" location="agent/auxiliary_client.py:2071">
P2: Auxiliary credential inheritance can now send the main API key to a different service on the same hostname but different port. The new same-host check uses hostname-only matching, so tightening this to host+effective-port (or full origin) would avoid cross-service credential leakage.</violation>
</file>
<file name="hermes_logging.py">
<violation number="1" location="hermes_logging.py:630">
P2: Concurrent logging setup can register the same log file handler twice, so one log record may be written multiple times. The duplicate-path check runs outside the queue-state lock, but append is unconditional inside `_register_queued_handler`; moving/duplicating the dedupe check under the same lock would keep registration atomic.</violation>
</file>
<file name="tools/hook_output_spill.py">
<violation number="1" location="tools/hook_output_spill.py:96">
P2: `hooks.output_spill.enabled` can be misread when users quote booleans in YAML, so spilling stays on even with `"false"`. This comes from `bool(enabled_raw)` on raw config; parsing common false-like strings (`false/0/no/off`) would make the flag behave as configured.</violation>
</file>
<file name="hermes_cli/status.py">
<violation number="1" location="hermes_cli/status.py:561">
P2: `hermes status` can report active gateway sessions when none are live: a DB count of 0 enters the fallback path and re-counts `sessions.json`. Since `sessions.json` is known to retain stale mappings, consider using DB count whenever it is available and only falling back when DB read failed.</violation>
</file>
<file name="scripts/whatsapp-bridge/bridge.js">
<violation number="1" location="scripts/whatsapp-bridge/bridge.js:257">
P3: `normalizePollUpdateOptions` declares a third `meId` parameter that is never referenced in the function body. Neither of the two call sites (line 460 in `messages.update`, line 625 in `messages.upsert`) passes a third argument, so the parameter is both unused at definition and always `undefined` at runtime. It dead-weights the function signature and creates a misleading impression that the function filters by a user identity when it never does. Remove the `meId` parameter to keep the signature honest.</violation>
<violation number="2" location="scripts/whatsapp-bridge/bridge.js:283">
P2: Poll vote diagnostics are always printed, so normal bridge runs will leak poll-update metadata and add noisy stdout traffic. Wrapping this log behind `WHATSAPP_DEBUG` would keep the diagnostics available without exposing routine vote details in production logs.</violation>
</file>
<file name="run_agent.py">
<violation number="1" location="run_agent.py:1326">
P2: Copilot enterprise endpoints can miss first-turn `x-initiator=user`, so Copilot-specific request behavior is inconsistent across accounts. The new `_is_copilot_url()` uses fragile substring matching instead of hostname matching, so using `_is_github_copilot_url()` (and host-match for GitHub Models) would avoid false negatives/positives.</violation>
</file>
<file name="scripts/release.py">
<violation number="1" location="scripts/release.py:280">
P2: `"perkintahmaz50@gmail.com": "devatnull"` is added 3 separate times (lines 280, 456, 495) in addition to the first annotated occurrence on line 50. Python dicts silently let the last duplicate key win, so lines 50, 280, and 456 are dead entries — they inflate `AUTHOR_MAP` with no runtime effect, and the detailed PR #58704 attribution comment on line 50 becomes misleading since that entry is overwritten. Remove the 3 duplicate bare copies and keep only the first annotated entry.</violation>
</file>
<file name="gateway/status_phrases.py">
<violation number="1" location="gateway/status_phrases.py:109">
P2: Status phrase directory loading can still read YAML outside `HERMES_HOME` via symlinked files, even though absolute/`..` paths are blocked. Consider skipping symlink children when enumerating phrase files so the containment guarantee holds for directory-based catalogs too.</violation>
</file>
<file name="gateway/platforms/api_server.py">
<violation number="1" location="gateway/platforms/api_server.py:674">
P2: The middleware catch of `HTTPRequestEntityTooLarge` doesn't prevent the handler's inner `except Exception` from intercepting it first. `_handle_chat_completions` (line 2075) and `_read_json_body` (line 1668) both wrap `request.json()` in `except Exception`, which catches `HTTPRequestEntityTooLarge` and returns 400 "Invalid JSON" — the very outcome the comment says we're avoiding. The middleware's except only fires for handlers without a broad except around body reads. To fix: either re-raise `HTTPRequestEntityTooLarge` from the handler's except clause, or check for it specifically before the generic JSON catch.</violation>
</file>
<file name="plugins/platforms/sms/adapter.py">
<violation number="1" location="plugins/platforms/sms/adapter.py:125">
P2: When `request.read()` raises `HTTPRequestEntityTooLarge` (413) due to the new `client_max_size` on `web.Application`, the handler's broad `except Exception` catches it and returns status 400 with a misleading log message. Twilio receives a 400 for what should be a 413 — semantically wrong and harder to debug. The pre-read `content_length` check handles the Content-Length path correctly, but for chunked bodies without Content-Length the exception path returns the wrong status. Either catch `HTTPRequestEntityTooLarge` explicitly and return 413, or remove `client_max_size` and rely on the manual checks.</violation>
<violation number="2" location="plugins/platforms/sms/adapter.py:308">
P2: Dead code: `if len(raw) > _TWILIO_WEBHOOK_MAX_BODY_BYTES` after `await request.read()`. With `client_max_size=65536` on `web.Application`, aiohttp's `request.read()` raises `HTTPRequestEntityTooLarge` during streaming when the body exceeds the limit — execution never reaches this check with an oversized body. Keeps readers wondering what defense layer this provides. Remove the post-read check (it's safe because `client_max_size` already enforces the limit), or drop `client_max_size` and keep the manual checks as the sole enforcement.</violation>
</file>
<file name="tools/mcp_tool.py">
<violation number="1" location="tools/mcp_tool.py:310">
P2: Highest-severity MCP server logs are currently downgraded to ERROR, so critical incidents may be missed by handlers that key off CRITICAL. Mapping `critical/alert/emergency` to `logging.CRITICAL` would preserve intended severity.</violation>
</file>
<file name="website/docs/user-guide/features/credential-pools.md">
<violation number="1" location="website/docs/user-guide/features/credential-pools.md:14">
P2: The warning states OpenAI prompt caches are "scoped to the account/API key" — this is inaccurate. OpenAI prompt caching is scoped per **organization**, not per API key: multiple keys under the same org share the same cache. A pool rotation to a different key within the same org does NOT invalidate the cache. Drop or rephrase "OpenAI" from the scoping claim, or clarify the per-org (not per-key) behavior to avoid misleading users into thinking every rotation always costs a full re-read.</violation>
</file>
<file name="agent/codex_runtime.py">
<violation number="1" location="agent/codex_runtime.py:626">
P2: Interleaved Responses streams can misroute assistant text because delta classification uses one global `active_message_phase` instead of the delta’s message identity. This can hide final answer tokens from `on_text_delta`/`output_text`; consider tracking phase by `item_id` (or `output_index`) and resolving each delta against that map.</violation>
</file>
<file name="hermes_cli/gateway.py">
<violation number="1" location="hermes_cli/gateway.py:25">
P2: The PATH augmentation module-level code can produce a leading colon (`:`) when `PATH` is empty or unset, which puts the current working directory first in the POSIX executable search order. This is a security concern: if someone runs a Hermes command from a directory where an attacker has placed a malicious `launchctl` or `systemctl` binary, it would be found before the system one. The root cause is that `"".split(os.pathsep)` returns `['']` (a list with one empty string), so the set includes `''`. A safer approach is to filter out empty components from the current PATH before computing the union, and avoid the leading colon when reconstructing.</violation>
</file>
<file name="gateway/platforms/msgraph_webhook.py">
<violation number="1" location="gateway/platforms/msgraph_webhook.py:243">
P2: Chunked request bodies that exceed `client_max_size` get a 400 status instead of the correct 413. The generic `except Exception` around `request.read()` catches aiohttp's `HTTPRequestEntityTooLarge` and returns 400. Since the Content-Length check before it already returns 413 for the Content-Length case, and the `len(raw_body)` check after it also returns 413, this path is specifically reachable when a chunked body exceeds `client_max_size` during streaming read. Add an explicit `except web.HTTPRequestEntityTooLarge: return web.Response(status=413)` before the generic except to return the correct status code, consistent with `webhook.py` and `api_server.py`.</violation>
</file>
<file name="plugins/model-providers/zai/__init__.py">
<violation number="1" location="plugins/model-providers/zai/__init__.py:82">
P2: Invalid or misspelled effort values currently get coerced to `reasoning_effort="high"`, so a bad config silently changes model behavior instead of falling back to server defaults. Consider only mapping known effort values and omitting `reasoning_effort` for unknown inputs.</violation>
</file>
<file name="tools/image_source.py">
<violation number="1" location="tools/image_source.py:111">
P2: Some valid `file://` image sources resolve to the wrong path and fail as not found. The URI is parsed by string slicing instead of URI parsing, so `localhost` authorities and `%`-encoded path bytes are not handled.</violation>
</file>
<file name="tools/approval.py">
<violation number="1" location="tools/approval.py:382">
P2: This pattern now hardline-blocks `rm -rf "/ *"`, even though that is a single quoted literal path rather than root deletion. The `|/ \*` alternative is unnecessary here and should be removed so quoted literal paths are not treated as root wipes.</violation>
</file>
<file name="tests/gateway/test_aiohttp_body_caps.py">
<violation number="1" location="tests/gateway/test_aiohttp_body_caps.py:18">
P2: These tests verify `client_max_size` wiring via source-code string matching (`inspect.getsource` + `in`), which breaks on innocuous formatting changes (line wrapping, intermediate comments). Consider a runtime assertion instead — for example, create the adapter/runner and inspect the runner's `_handler._request_handler._client_max_size` attribute, or mock the Application constructor to assert it was called with the right kwarg. That way the test checks actual behavior, not formatted source text.</violation>
</file>
<file name="model_tools.py">
<violation number="1" location="model_tools.py:760">
P2: The `_schema_accepts_kind` helper treats `allOf` like `anyOf`/`oneOf` by returning True when ANY branch accepts the kind. Per JSON Schema, `allOf` requires that a value satisfy ALL sub-schemas simultaneously — the function should check that ALL branches accept the kind, not just any one. While no current tool schemas use `allOf`, using the wrong combinator could produce false positives for any future schema that composes type constraints via `allOf`, leading a JSON-encoded string to be incorrectly parsed into a container when the schema doesn't actually permit it. The fix is to use `all(...)` for the `allOf` branch and `any(...)` for the `anyOf`/`oneOf` branches.</violation>
</file>
<file name="tools/vision_tools.py">
<violation number="1" location="tools/vision_tools.py:324">
P3: Failed SVG/Pillow normalization can leave stray temp PNGs in the vision cache, which slowly grows disk usage on repeated failures. The new conversion path allocates `out_path` up front but does not clean it on error returns; consider deleting `out_path` before returning an error.</violation>
</file>
<file name="plugins/platforms/telegram/adapter.py">
<violation number="1" location="plugins/platforms/telegram/adapter.py:649">
P3: Long-lived bots can accumulate stale overflow-preview cache entries when oversized streams end before `finalize=True`, because cleanup is currently tied to finalize/same-message shrink paths only. Consider bounding this map (LRU/size cap) or clearing stale keys on disconnect/stream abort so aborted messages don't create unbounded state.</violation>
</file>
<file name="plugins/model-providers/opencode-zen/__init__.py">
<violation number="1" location="plugins/model-providers/opencode-zen/__init__.py:34">
P3: GLM-5.2 alias detection and effort mapping are now implemented in two provider modules, which makes future behavior changes easy to miss in one path. A shared helper for GLM-5.2 normalization/mapping would reduce drift risk between `opencode-zen` and `zai`.</violation>
</file>
<file name="hermes_cli/tools_config.py">
<violation number="1" location="hermes_cli/tools_config.py:1046">
P3: Timeout handling has a race where process-tree termination can raise after the fallback `proc.kill()`, causing a generic failure path instead of clean timeout reporting. Wrapping the fallback kill in its own guarded try keeps timeout recovery deterministic.</violation>
</file>
<file name="cli.py">
<violation number="1" location="cli.py:8902">
P3: Stacked skill parsing is case-sensitive for additional `/skill` tokens, while the primary slash command dispatch is case-insensitive. A mixed-case input like `/foo /Bar do x` can silently skip `/Bar`; normalizing extra command tokens before resolution would keep behavior consistent.</violation>
</file>
<file name="locales/fr.yaml">
<violation number="1" location="locales/fr.yaml:110">
P3: Missing French space before colon in `agent:` — existing entries consistently use `Error :`, `config.yaml :`, etc. with the French espace insécable before the colon. Add a space to match established convention.</violation>
</file>
<file name="tests/agent/test_skill_bundles.py">
<violation number="1" location="tests/agent/test_skill_bundles.py:235">
P3: The `missing` variable is unpacked from `build_bundle_invocation_message`'s return tuple but never asserted. For consistency with nearby tests (`test_loads_all_skills` asserts `missing == []`, `test_skips_missing_skills` asserts `missing == ["skill-ghost"]`), consider adding `assert missing == []` or using `_` to signal intentional disuse.</violation>
</file>
Note: This PR contains a large number of files. cubic only reviews up to 200 files per PR, so some files may not have been reviewed. cubic prioritizes the most important files to review.
Re-trigger cubic
| except Exception: | ||
| return dict(os.environ) | ||
| return env |
There was a problem hiding this comment.
P1: Secret stripping can be bypassed on sanitizer/import failure, so cua-driver may receive inherited provider credentials. Keeping graceful degradation but adding a fail-closed scrub fallback (instead of returning raw env) would preserve probe reliability without reintroducing the secret-leak path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tools/computer_use/permissions.py, line 67:
<comment>Secret stripping can be bypassed on sanitizer/import failure, so `cua-driver` may receive inherited provider credentials. Keeping graceful degradation but adding a fail-closed scrub fallback (instead of returning raw `env`) would preserve probe reliability without reintroducing the secret-leak path.</comment>
<file context>
@@ -47,13 +47,24 @@ def _driver_cmd(override: Optional[str]) -> str:
+ return _sanitize_subprocess_env(env)
except Exception:
- return dict(os.environ)
+ return env
</file context>
| "_set_session_reasoning_override" in calls | ||
| and _assigns_false(node, "was_auto_reset") | ||
| ): | ||
| assert _references_name(node, "_last_resolved_model") and "pop" in _calls( |
There was a problem hiding this comment.
P1: The new test_auto_reset_cleanup_clears_last_resolved_model assertion checks _references_name(node, "_last_resolved_model") which walks the AST for ast.Constant nodes matching that string — but production code accessing self._last_resolved_model or _last_resolved_model (attribute/variable) produces ast.Attribute or ast.Name nodes, not ast.Constant. The test would fail even when the correct pop call is present in the cleanup block, or worse, it could silently pass on a false match (e.g. a docstring or comment-like constant that happens to contain the string). Update _references_name to also match ast.Attribute.attr and ast.Name.id against the literal.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/gateway/test_10710_auto_reset_evicts_cached_agent.py, line 124:
<comment>The new `test_auto_reset_cleanup_clears_last_resolved_model` assertion checks `_references_name(node, "_last_resolved_model")` which walks the AST for `ast.Constant` nodes matching that string — but production code accessing `self._last_resolved_model` or `_last_resolved_model` (attribute/variable) produces `ast.Attribute` or `ast.Name` nodes, not `ast.Constant`. The test would fail even when the correct `pop` call is present in the cleanup block, or worse, it could silently pass on a false match (e.g. a docstring or comment-like constant that happens to contain the string). Update `_references_name` to also match `ast.Attribute.attr` and `ast.Name.id` against the literal.</comment>
<file context>
@@ -90,3 +90,49 @@ def test_evict_cached_agent_method_exists():
+ "_set_session_reasoning_override" in calls
+ and _assigns_false(node, "was_auto_reset")
+ ):
+ assert _references_name(node, "_last_resolved_model") and "pop" in _calls(
+ node
+ ), (
</file context>
| ec = parsed.get("element_count") | ||
| summary = f"{ec} elements" if ec is not None else "" | ||
| data = f"{summary}\n{tree}" if summary else tree | ||
| return {"data": data, "images": images, "structuredContent": structured, "isError": False} |
There was a problem hiding this comment.
P1: CLI fallback failures can be misreported as successful tool actions. _call_tool_via_cli() always returns isError=False, so _action() treats error responses as ok=True; propagate subprocess/error state into isError to preserve correctness.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tools/computer_use/cua_backend.py, line 982:
<comment>CLI fallback failures can be misreported as successful tool actions. `_call_tool_via_cli()` always returns `isError=False`, so `_action()` treats error responses as `ok=True`; propagate subprocess/error state into `isError` to preserve correctness.</comment>
<file context>
@@ -800,11 +880,132 @@ def _restart_session_locked(self) -> None:
+ ec = parsed.get("element_count")
+ summary = f"{ec} elements" if ec is not None else ""
+ data = f"{summary}\n{tree}" if summary else tree
+ return {"data": data, "images": images, "structuredContent": structured, "isError": False}
+ finally:
+ if shot_file and os.path.exists(shot_file):
</file context>
Suggested change
| return {"data": data, "images": images, "structuredContent": structured, "isError": False} | |
| is_error = bool(proc.returncode != 0 or (isinstance(parsed, dict) and parsed.get("error"))) | |
| return {"data": data, "images": images, "structuredContent": structured, "isError": is_error} |
| logger.warning("[photon] cannot reinstall stale sidecar deps: npm not on PATH") | ||
| return | ||
| try: | ||
| result = subprocess.run( # noqa: S603 |
There was a problem hiding this comment.
P1: The _reinstall_sidecar_deps function (called during reconnect when sidecar deps are stale) only catches subprocess.TimeoutExpired from subprocess.run, but subprocess.run can also raise OSError (permissions, deleted npm binary between shutil.which and execution). This propagates out of the function and crashes the reconnect flow in _start_sidecar via asyncio.to_thread, contradicting the documented "best-effort" contract. Wrap the subprocess.run calls with an outer try/except OSError (or broad except Exception) that logs the error and returns, consistent with the best-effort intent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At plugins/platforms/photon/adapter.py, line 178:
<comment>The `_reinstall_sidecar_deps` function (called during reconnect when sidecar deps are stale) only catches `subprocess.TimeoutExpired` from `subprocess.run`, but `subprocess.run` can also raise `OSError` (permissions, deleted npm binary between `shutil.which` and execution). This propagates out of the function and crashes the reconnect flow in `_start_sidecar` via `asyncio.to_thread`, contradicting the documented "best-effort" contract. Wrap the `subprocess.run` calls with an outer `try/except OSError` (or broad `except Exception`) that logs the error and returns, consistent with the best-effort intent.</comment>
<file context>
@@ -137,6 +143,77 @@ def check_requirements() -> bool:
+ logger.warning("[photon] cannot reinstall stale sidecar deps: npm not on PATH")
+ return
+ try:
+ result = subprocess.run( # noqa: S603
+ [npm, "ci"],
+ cwd=str(_SIDECAR_DIR),
</file context>
| # don't get their container churned on every startup. | ||
| mode_mismatch = False | ||
| actual_mode = None | ||
| if not network: |
There was a problem hiding this comment.
P1: Air-gapped mode can be lost after recovery: when docker_network=false, a later "No such container" retry can still reattach to a network-enabled labeled container and restore egress unexpectedly. The new NetworkMode guard runs only in __init__; mirroring it in _recreate_container() would keep isolation consistent across retries.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tools/environments/docker.py, line 915:
<comment>Air-gapped mode can be lost after recovery: when `docker_network=false`, a later "No such container" retry can still reattach to a network-enabled labeled container and restore egress unexpectedly. The new NetworkMode guard runs only in `__init__`; mirroring it in `_recreate_container()` would keep isolation consistent across retries.</comment>
<file context>
@@ -897,6 +897,45 @@ def __init__(
+ # don't get their container churned on every startup.
+ mode_mismatch = False
+ actual_mode = None
+ if not network:
+ actual_mode = self._container_network_mode(container_id)
+ mode_mismatch = actual_mode != "none"
</file context>
Comment on lines
+1046
to
+1047
| except (OSError, ProcessLookupError): | ||
| proc.kill() |
There was a problem hiding this comment.
P3: Timeout handling has a race where process-tree termination can raise after the fallback proc.kill(), causing a generic failure path instead of clean timeout reporting. Wrapping the fallback kill in its own guarded try keeps timeout recovery deterministic.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At hermes_cli/tools_config.py, line 1046:
<comment>Timeout handling has a race where process-tree termination can raise after the fallback `proc.kill()`, causing a generic failure path instead of clean timeout reporting. Wrapping the fallback kill in its own guarded try keeps timeout recovery deterministic.</comment>
<file context>
@@ -860,38 +971,114 @@ def _run_cua_driver_installer(label: str = "Installing", verbose: bool = True) -
+ os.killpg(os.getpgid(proc.pid), _signal.SIGKILL) # windows-footgun: ok — POSIX branch only
+ else:
+ proc.kill()
+ except (OSError, ProcessLookupError):
+ proc.kill()
+
</file context>
Suggested change
| except (OSError, ProcessLookupError): | |
| proc.kill() | |
| except (OSError, ProcessLookupError): | |
| try: | |
| proc.kill() | |
| except (OSError, ProcessLookupError): | |
| pass |
| build_stacked_skill_invocation_message, | ||
| split_stacked_skill_commands, | ||
| ) | ||
| extra_keys, user_instruction = split_stacked_skill_commands(rest) |
There was a problem hiding this comment.
P3: Stacked skill parsing is case-sensitive for additional /skill tokens, while the primary slash command dispatch is case-insensitive. A mixed-case input like /foo /Bar do x can silently skip /Bar; normalizing extra command tokens before resolution would keep behavior consistent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cli.py, line 8902:
<comment>Stacked skill parsing is case-sensitive for additional `/skill` tokens, while the primary slash command dispatch is case-insensitive. A mixed-case input like `/foo /Bar do x` can silently skip `/Bar`; normalizing extra command tokens before resolution would keep behavior consistent.</comment>
<file context>
@@ -8890,7 +8891,39 @@ def process_command(self, command: str) -> bool:
+ build_stacked_skill_invocation_message,
+ split_stacked_skill_commands,
+ )
+ extra_keys, user_instruction = split_stacked_skill_commands(rest)
+ if extra_keys:
+ stacked_result = build_stacked_skill_invocation_message(
</file context>
| const recentlyProcessedPollUpdates = createOutboundIdTracker(512); | ||
| const messageStore = createBoundedMessageStore(512); | ||
|
|
||
| function normalizePollUpdateOptions(aggregation, pollUpdateMessage, meId) { |
There was a problem hiding this comment.
P3: normalizePollUpdateOptions declares a third meId parameter that is never referenced in the function body. Neither of the two call sites (line 460 in messages.update, line 625 in messages.upsert) passes a third argument, so the parameter is both unused at definition and always undefined at runtime. It dead-weights the function signature and creates a misleading impression that the function filters by a user identity when it never does. Remove the meId parameter to keep the signature honest.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/whatsapp-bridge/bridge.js, line 257:
<comment>`normalizePollUpdateOptions` declares a third `meId` parameter that is never referenced in the function body. Neither of the two call sites (line 460 in `messages.update`, line 625 in `messages.upsert`) passes a third argument, so the parameter is both unused at definition and always `undefined` at runtime. It dead-weights the function signature and creates a misleading impression that the function filters by a user identity when it never does. Remove the `meId` parameter to keep the signature honest.</comment>
<file context>
@@ -227,6 +251,107 @@ const MAX_QUEUE_SIZE = 100;
+const recentlyProcessedPollUpdates = createOutboundIdTracker(512);
+const messageStore = createBoundedMessageStore(512);
+
+function normalizePollUpdateOptions(aggregation, pollUpdateMessage, meId) {
+ const selected = [];
+ for (const option of aggregation || []) {
</file context>
Suggested change
| function normalizePollUpdateOptions(aggregation, pollUpdateMessage, meId) { | |
| function normalizePollUpdateOptions(aggregation, pollUpdateMessage) { |
Comment on lines
+110
to
+111
| denied_reason_singular: "❌ Commande refusée. Raison transmise à l'agent: \"{reason}\"" | ||
| denied_reason_plural: "❌ Commandes refusées ({count} commandes). Raison transmise à l'agent: \"{reason}\"" |
There was a problem hiding this comment.
P3: Missing French space before colon in agent: — existing entries consistently use Error :, config.yaml :, etc. with the French espace insécable before the colon. Add a space to match established convention.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At locales/fr.yaml, line 110:
<comment>Missing French space before colon in `agent:` — existing entries consistently use `Error :`, `config.yaml :`, etc. with the French espace insécable before the colon. Add a space to match established convention.</comment>
<file context>
@@ -107,6 +107,8 @@ gateway:
no_pending: "Aucune commande en attente de refus."
denied_singular: "❌ Commande refusée."
denied_plural: "❌ Commandes refusées ({count} commandes)."
+ denied_reason_singular: "❌ Commande refusée. Raison transmise à l'agent: \"{reason}\""
+ denied_reason_plural: "❌ Commandes refusées ({count} commandes). Raison transmise à l'agent: \"{reason}\""
</file context>
Suggested change
| denied_reason_singular: "❌ Commande refusée. Raison transmise à l'agent: \"{reason}\"" | |
| denied_reason_plural: "❌ Commandes refusées ({count} commandes). Raison transmise à l'agent: \"{reason}\"" | |
| denied_reason_singular: "❌ Commande refusée. Raison transmise à l'agent : \"{reason}\"" | |
| denied_reason_plural: "❌ Commandes refusées ({count} commandes). Raison transmise à l'agent : \"{reason}\"" |
|
|
||
| result = build_bundle_invocation_message("/combo", platform="telegram") | ||
| assert result is not None | ||
| msg, loaded, missing = result |
There was a problem hiding this comment.
P3: The missing variable is unpacked from build_bundle_invocation_message's return tuple but never asserted. For consistency with nearby tests (test_loads_all_skills asserts missing == [], test_skips_missing_skills asserts missing == ["skill-ghost"]), consider adding assert missing == [] or using _ to signal intentional disuse.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/agent/test_skill_bundles.py, line 235:
<comment>The `missing` variable is unpacked from `build_bundle_invocation_message`'s return tuple but never asserted. For consistency with nearby tests (`test_loads_all_skills` asserts `missing == []`, `test_skips_missing_skills` asserts `missing == ["skill-ghost"]`), consider adding `assert missing == []` or using `_` to signal intentional disuse.</comment>
<file context>
@@ -213,6 +213,53 @@ def test_skips_missing_skills(self, bundles_env):
+
+ result = build_bundle_invocation_message("/combo", platform="telegram")
+ assert result is not None
+ msg, loaded, missing = result
+ assert loaded == ["skill-a"]
+ assert "SECRET DISABLED CONTENT." not in msg
</file context>
Suggested change
| msg, loaded, missing = result | |
| msg, loaded, _ = result |
trac3r00
pushed a commit
that referenced
this pull request
Jul 18, 2026
Completes the review's ask for "adapter-to-session-key integration coverage for Discord and a non-Discord platform" on NousResearch#20096. Drives a concrete adapter's real BasePlatformAdapter.build_source with an injected gateway_runner, asserts the matched route's profile is stamped on the source, and that build_session_key scopes the key under agent:<profile>: (versus the shared agent:main: namespace). Covers Discord and Telegram — the Telegram case is the bug-#2 path that previously fell through to default. Adds a regression anchor: without gateway_runner, profile stays None and the key lands in agent:main (the silent fallback the fix removes for non-Discord). Co-Authored-By: Claude <noreply@anthropic.com>
trac3r00
pushed a commit
that referenced
this pull request
Jul 18, 2026
…st (NousResearch#65214) Moves the fireworks entry in CANONICAL_PROVIDERS from its old slot (after GMI Cloud) to directly below Nous Portal, ahead of OpenRouter. Order propagates automatically to hermes model, the setup wizard, Telegram /model, and the desktop provider catalog.
trac3r00
pushed a commit
that referenced
this pull request
Jul 18, 2026
…etry Combines the two salvaged fixes so they compose instead of conflict: _persist_session_title (NousResearch#50575) now writes through set_auto_title_if_empty (NousResearch#51483) when the store provides it — the collision-dedup retry and the manual-/title race protection apply together. Predicate failure (a manual title landed while generation was in flight) returns None: nothing written, no callback. Legacy stores without the atomic method keep the plain set_session_title path, including the vanished-session RuntimeError. Tests cover both store shapes plus the race-skip path; E2E verified against a real SQLite SessionDB (collision -> 'Weekly Report #2', manual title preserved, cron dedup, blank guard). AUTHOR_MAP entry for rasitakyol.
trac3r00
pushed a commit
that referenced
this pull request
Jul 18, 2026
…er (NousResearch#66432) Mirror CANONICAL_PROVIDERS so Fireworks sits directly under Nous Portal (always visible) ahead of OpenRouter across onboarding, Settings → Providers, and the API-key catalog.
Owner
Author
|
[Bob] Closing — this was submitted to NousResearch/hermes-agent upstream. This fork-level PR is a duplicate. |
trac3r00
pushed a commit
that referenced
this pull request
Jul 20, 2026
…arch#65919) * fix(desktop): preserve interim assistant text wiped at message.complete When the agent emits interim text (commentary alongside tool calls, or the attempted final answer before a verify-on-stop nudge), all UI surfaces streamed it live but then wiped it at message.complete — keeping only the final response. The user saw text appear during inference, then disappear. This is the complete fix across all three layers: agent core, gateway transport, and all UI surfaces (desktop + Ink TUI). The verify-on-stop and pre_verify paths flagged the assistant's attempted final answer as _verification_stop_synthetic, suppressing it from both state.db and the UI. The user only saw the terse post-verification reply. Now the assistant response is real content: it's persisted to state.db and emitted as an interim message via _emit_interim_assistant_message(force_display=True) before the verification loop runs. Only the synthetic nudge messages keep the synthetic flags. The turn finalizer drops nudges from live history and compares content (not just role) to avoid duplicating a published candidate. Message sequence repair collapses verification candidates in the consecutive-assistant merge. Wire agent.interim_assistant_callback both at construction (_agent_cbs()) and per-turn (defense-in-depth), emitting a new message.interim event with {text, already_streamed}. Gated on display.interim_assistant_messages (default true). Cleared in the finally block so a stale closure can't fire on a later turn. Add message.interim to the GatewayEventName union (apps/shared) and a typed payload to the TUI's GatewayEvent discriminated union. The TUI already had the segment-anchoring machinery (flushStreamingSegment + finalTail) but had no handler for message.interim. Added recordInterimMessage + interimBoundaryIndex to seal segments mid-turn, and updated recordMessageComplete to only dedupe segments after the interim boundary. Replaced the fragile sealed-set approach with a proper interimBoundaryPending state flag on ClientSessionState. finalizeInterimAssistantMessage finalizes the streaming bubble in place (or creates a standalone one), rotates the stream ID so next deltas create a new bubble, and sets the flag. When the final text equals an already-sealed interim, they stay as distinct messages. Extracted mergeFinalAssistantText() as a pure function in chat-messages.ts, used by both completeAssistantMessage and finalizeInterimAssistantMessage. Split the bidirectional dedup predicate: reasoning is a restatement only when the final FULLY covers it. A short final ("Done.") no longer swallows a longer reasoning block that merely starts with it. Honor display.interim_assistant_messages (default true) across all layers: the tui_gateway gates the callback, the desktop wires it to a nanostores atom via use-hermes-config. Updated hermes_cli/config.py and cli-config.yaml.example comments to document the Desktop behavior. _split_segment_tokens now accepts posix=False and _find_ad_hoc_match tries both posix modes so ad-hoc verification scripts with Windows backslash paths are matched correctly. (response_previewed forwarding from NousResearch#53553 is not included — our emit-interim + persist approach makes it unnecessary since the attempted answer is now surfaced before the verification loop.) - tsc: clean (desktop + TUI + shared) - vitest desktop: 73/73 pass (7 interim-sealing + 5 mergeFinalAssistantText + 4 config atom) - vitest TUI: 83/83 pass (4 new message.interim tests) - python: 390 tests pass (340 tui_gateway + 33 verification/finalizer + 6 config gating + 3 evidence + 8 continuation budget) Co-authored-by: Liam Zhang <yingliang-zhang@users.noreply.github.com> Co-authored-by: Lucas D'Alessandro <lucasfdale@users.noreply.github.com> Co-authored-by: Eric Manganaro <superposition@users.noreply.github.com> Co-authored-by: sweetcornna <sweetcornna@users.noreply.github.com> Co-authored-by: DECK6 <DECK6@users.noreply.github.com> Co-authored-by: matantsevs <matantsevs@users.noreply.github.com> Co-authored-by: gitcommit90 <gitcommit90@users.noreply.github.com> * fix: prefix-match interim streamed content to avoid benign duplicate bubbles _interim_content_was_streamed used exact equality (streamed == visible_content), so a final response that was the streamed text plus a trailing delta — or a partial stream before the verify nudge fired — failed the match and left _response_was_previewed false. The turn then showed two bubbles (interim + identical final) instead of settling the interim in place. Relax to a prefix check (visible_content.startswith(streamed)) in both the core match and the desktop's settle-in-place gate. The TUI already used prefix matching via finalTail. The reverse direction (streamed longer than final) is intentionally not matched — that could suppress a needed resend in the gateway path where already_streamed=True calls on_segment_break(). * test(desktop): add partial-stream-then-nudge dedup edge case Third edge case for the interim-sealing dedup: model streams part of its answer via message.delta, verify nudge fires, interim seals the streamed prefix, then the final response is the same text plus a trailing delta. Asserts one bubble (not two) containing the full final text. Acceptance protocol #2 — covers all three dedup edges: 1. interim == final (existing) 2. interim = strict prefix of final (existing) 3. partial-stream-then-nudge (this commit) --------- Co-authored-by: Liam Zhang <yingliang-zhang@users.noreply.github.com> Co-authored-by: Lucas D'Alessandro <lucasfdale@users.noreply.github.com> Co-authored-by: Eric Manganaro <superposition@users.noreply.github.com> Co-authored-by: sweetcornna <sweetcornna@users.noreply.github.com> Co-authored-by: DECK6 <DECK6@users.noreply.github.com> Co-authored-by: matantsevs <matantsevs@users.noreply.github.com> Co-authored-by: gitcommit90 <gitcommit90@users.noreply.github.com>
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.
Motivation
cua-driver draws its tinted agent cursor only on the user's physical screen — the PNG the model receives has no marker, so the model can't see where its own click/drag landed. This breaks games, drag precision, and any continuous see→act→see loop. This adds Codex-style self-localization: the action's landing point is composited onto the follow-up capture so the model can confirm 'I just clicked here.'
What
action_marker.py(new): pure PIL compositing module. Lazy Pillow import; returns the original bytes unchanged when Pillow is absent (graceful degrade).cua_backend.py: click/drag record the resolved landing pixel (element index → center pixel cache);consume_action_marker()is one-shot so a later markerless capture never re-stamps.tool.py:_maybe_follow_capturecomposites the marker ontocapture_after=Truefollow-ups.schema.py:show_action_markerflag (default true).Verification
10 tests pass with Pillow installed (decode the composited PNG and assert marker pixels at the landing point, far corners untouched, drag path marked, no-op without coords, one-shot not re-stamped). Falls back cleanly to the original screenshot when Pillow is missing.
Summary by cubic
Big stability and UX update across the agent, gateway, and CLI: stacked slash-skill commands, smarter Copilot billing headers, long‑running status phrases, safer webhooks/transports, more reliable logging and computer-use captures, and several guardrails and fixes.
New Features
/skill-a /skill-b do XYZloads all leading skills (up to 5) on CLI and gateway.x-initiator: useron the first API call of each user turn; follow-ups default toagent.busy_steer_ack_enabled.stt.echo_transcripts) and desktop setting.approveescalates to human gate;/denycan include a reason relayed back to the agent.agent.log.Bug Fixes / Hardening
webhook,api_server,bluebubbles,msgraph_webhook, and proxy; cap proxy SSE line buffer.QueueListenerto avoid event-loop stalls; bounded flush on shutdown.os.getenv(...)-style values.finish_reasonand tool-callid.Written for commit 6865645. Summary will update on new commits.