feat(gateway): add gateway.terminal_backend sandbox override for messaging sessions - #7
feat(gateway): add gateway.terminal_backend sandbox override for messaging sessions#7dsr-restyn wants to merge 10000 commits into
Conversation
dsr-restyn
left a comment
There was a problem hiding this comment.
Hermes Agent Code Review
Verdict: COMMENT — draft is solid; two real issues to fix before upstream submission, plus a few warnings.
🔴 Issues (fix before upstream)
1. Zombie containers on timeout (tools/execute_code_docker.py:43-47)
proc.kill() kills the Docker CLI client process, but the container itself keeps running inside Docker. This is a resource leak — long timeout scripts will accumulate orphaned containers. Fix: use docker run --name <uuid> and docker kill <uuid> on timeout.
2. container_id = None is declared but never used (tools/execute_code_docker.py:30)
Stale variable from incomplete kill logic (the intended approach was to capture the container ID for targeted kill). Remove it, or implement the container-ID-based kill properly.
⚠️ Warnings
3. Warning block re-reads config.yaml from disk (gateway/run.py:1040-1052)
The should_warn_insecure_gateway() call inside GatewayRunner.start() opens and parses config.yaml a second time, even though it was already read at module import as _cfg. Use _cfg (module-level) or pass the already-loaded config into GatewayRunner instead of re-reading from disk. If the file changes between startup and start(), the warning can see a different config than what was actually applied.
4. No validation of terminal_backend value (gateway/sandbox_config.py)
apply_gateway_backend_to_env() sets TERMINAL_ENV to whatever the config says, including typos. A value like "docer" would silently break all terminal execution. Add validation against the known set: {"local", "docker", "modal", "daytona", "ssh", "singularity"}.
💡 Suggestions
5. Docker path can't be interrupted by user messages (tools/code_execution_tool.py)
The local subprocess path checks _interrupt_event.is_set() in its poll loop. The Docker path uses proc.communicate(timeout=...) which blocks without checking the interrupt event. This means a user sending a message mid-execution won't cancel the Docker job. Acceptable for a first pass, but worth a docstring note and a follow-up issue.
6. No log message when gateway override is applied (gateway/sandbox_config.py)
apply_gateway_backend_to_env() sets TERMINAL_ENV silently. A logger.info("Gateway terminal backend override: %s (image: %s)", backend, image) would make startup logs clearly show that sandboxing is active.
✅ Looks Good
gateway/sandbox_config.pyis a clean, well-tested helper module — good separation of concerns- The if/else structure in
code_execution_tool.pycorrectly scopes the docker path; ANSI stripping and secret redaction still apply to both paths (they come after the if/else) - No collision with existing
gateway/config.pykeys — our new DEFAULT_CONFIGgateway:block is additive - 26 tests, good coverage of sandbox_config helpers and the docker subprocess wrapper
- Commit is atomic and well-scoped — no rewrites of working internals
Reviewed by Hermes Agent
|
|
||
| cmd.extend([image, "python3", script_path]) | ||
|
|
||
| container_id = None |
There was a problem hiding this comment.
🔴 Stale variable — container_id is declared here but never assigned or used. This is a leftover from planned container-kill-by-ID logic that wasn't completed. Either remove it or implement the intended pattern:
import uuid
container_name = f"hermes-sandbox-{uuid.uuid4().hex[:8]}"
cmd = ["docker", "run", "--name", container_name, "--rm", ...]
# then on timeout: subprocess.run(["docker", "kill", container_name])| except subprocess.TimeoutExpired: | ||
| logger.warning("Docker sandbox timed out after %s seconds, killing container", timeout) | ||
| # Try to kill via docker kill using the process | ||
| proc.kill() |
There was a problem hiding this comment.
🔴 Zombie container leak — proc.kill() terminates the Docker CLI process (the client), but the container itself keeps running on the Docker host. On a long timeout, orphaned containers accumulate.
Fix: use a named container so you can kill it:
container_name = f"hermes-sandbox-{uuid.uuid4().hex[:8]}"
# Add --name container_name to the docker run cmd
# Then on timeout:
subprocess.run(["docker", "kill", container_name], timeout=5, capture_output=True)
proc.kill()| # Warn if running with local backend (no sandbox isolation) | ||
| try: | ||
| from gateway.sandbox_config import should_warn_insecure_gateway | ||
| import yaml as _yaml |
There was a problem hiding this comment.
_cfg (around line 165). The two reads could see different values if the file changes between startup and .start(). Consider passing _cfg into GatewayRunner.__init__ or referencing the module-level variable:
if should_warn_insecure_gateway(_cfg): # use already-loaded config
logger.warning(...)|
|
||
|
|
||
| def apply_gateway_backend_to_env(config: dict) -> None: | ||
| """Set TERMINAL_ENV to gateway.terminal_backend if configured.""" |
There was a problem hiding this comment.
"docer") will be blindly set as TERMINAL_ENV and silently break all terminal tool execution for the session. Add:
_VALID_BACKENDS = {"local", "docker", "modal", "daytona", "ssh", "singularity"}
if backend not in _VALID_BACKENDS:
import logging
logging.getLogger(__name__).warning(
"Unknown gateway.terminal_backend %r — ignoring. Valid values: %s",
backend, ", ".join(sorted(_VALID_BACKENDS))
)
return| exit_code = _exit_code | ||
| status = "timeout" if _exit_code == -1 else "success" | ||
| else: | ||
| proc = subprocess.Popen( |
There was a problem hiding this comment.
💡 Missing interrupt support — the local path checks _interrupt_event.is_set() in the poll loop so a user message can cancel mid-run. The Docker path uses blocking proc.communicate(timeout=...) and has no interrupt check. Consider wrapping run_script_in_docker in a thread and polling _interrupt_event alongside it, or at minimum add a docstring note marking this as a known limitation.
8bd36cd to
2cd39e7
Compare
2cd39e7 to
e597c1b
Compare
e597c1b to
b13061a
Compare
…mpression-disabled guard Salvage of NousResearch#63862. is_output_cap_error() returns False for vLLM/LM Studio error messages that contain 'prompt contains ... input tokens' (treated as input-overflow signal). But parse_available_output_tokens_from_error() CAN extract a valid available_tokens from those same messages. The compression-disabled guard only checked is_output_cap_error(), so vLLM/LM Studio users with compression off still got a terminal failure instead of the max-tokens retry. Fix: also exempt when parse_available_output_tokens_from_error() returns a value — that function determines whether the retry path can actually handle the error, so it's the right predicate for the exemption. Added test: verify vLLM-format error with compression_disabled=False still triggers the max-tokens retry path. Co-authored-by: dmabry <dmabry@users.noreply.github.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
NousResearch#63955 made Hermes survive a broken `bash -l` (Ainz's `Directory \drivers\etc does not exist`) by falling back to non-login `bash -c`. But a non-login shell never sources /etc/profile, so it never gets `…\usr\bin` on PATH — and that dir holds every coreutil the file/terminal tools shell out to (cat, mktemp, mv, wc, head, stat, chmod, mkdir, find). Result: `write_file` returned bytes_written:0 with an EMPTY error (the failure text went to a missing binary's stderr) and terminal commands exited 127. The survive-broken-login-bash fix was only half-done: it stopped crashing but silently failed every write. Derive Git Bash's bin dirs (mingw64/bin, usr/bin, bin, …) from the resolved bash.exe and prepend them to the subprocess PATH on Windows, in /etc/profile precedence order so coreutils win over same-named System32 tools (find.exe, sort.exe) inside the shell. No-op off Windows and when a login snapshot is healthy (the snapshot re-exports the full PATH inside the shell), so this only bites on the broken-login fallback path. Adds _git_bash_bin_dirs() (derivation, cached) + _prepend_git_bash_dirs() (PATH merge), plus regression tests for PortableGit/MinGit layouts and the run-env injection ordering.
…s-nonlogin-coreutils-path fix(windows): put Git Bash coreutils on PATH for the non-login fallback
Self-review nits on the Thinking-widget fix: - type ReasoningTextPart as ReasoningMessagePartComponent and read the typed useMessagePartReasoning() directly, dropping the ad-hoc cast (the hook already returns text/status). - remove useRef, now unused after deleting useSmoothReveal. - trim the autopsy comments; the PR body carries the narrative.
…s dropdown survives (NousResearch#63886) The dashboard's dedicated memory-provider UI (4b184cb) excluded memory.provider from /api/config/schema server-side. Desktop's settings page builds its field list from that schema, so the Memory Provider dropdown silently vanished from Desktop after v0.18.1. - web_server.py: restore memory.provider as a select in _SCHEMA_OVERRIDES, with options built from plugins.memory discovery (was a stale hardcoded [builtin, honcho] list before the removal) - plugins/memory: add list_memory_provider_names() — directory-scan-only name listing, safe at module import time (no provider imports) - web ConfigPage: hide memory.provider client-side instead — the Plugins page owns the dedicated provider-switching UI there - tests: schema contract (select present, category memory, builtin sentinel) + invariant that every discoverable provider is selectable
The desktop provider panel previously rendered the curated declarations from hermes_cli/memory_providers.py: five hindsight fields, and no panel at all for undeclared providers like honcho (OAuth connect only). The dashboard provider-switching rework re-pointed the shared config route at raw plugin schemas, so the desktop began dumping every internal field (35 for hindsight) and grew a bespoke honcho panel. Serve both surfaces from the same route: ?surface=declared returns the curated schema (empty for undeclared providers) with the original config-file + env-store write semantics; the dashboard keeps the raw plugin schema unchanged. The desktop client opts into declared.
Include the Hermes client name and version with Gemini inference, model and tier checks, and TTS requests. Add focused coverage for the request headers and keep the Gemini-specific context scoped to Google Gemini endpoints.
…Research#64318) test_probe_sends_client_context_to_gemini and test_probe_omits_gemini_client_context_for_other_providers (added in b8eb89f) patch hermes_cli.models.urllib.request.urlopen, but probe_api_models routes requests through the _urlopen_model_catalog_request wrapper (open_credentialed_url from the urllib_security hardening), so the mock is never invoked and mock_urlopen.call_args is None -> TypeError. Every CI run on main and every PR has been failing test slice 7/8 on these two tests. Point the patches at _urlopen_model_catalog_request, the same target every sibling test in TestProbeApiModelsUserAgent already uses. 89/89 tests in the file now pass.
…usResearch#64319) Cache-decorated turns (apply_anthropic_cache_control converts string content to [{type: text, ..., cache_control}] lists — applied BEFORE the MoA facade since the NousResearch#57675 cache-cold fix) and multimodal turns (text + image_url parts) flattened to empty strings in _reference_messages, which only read str content. On turn 1 of a provider:moa session with a Claude aggregator the references received a single EMPTY user message: Anthropic-side providers 400'd ('messages: at least one message is required') while tolerant models answered 'no user request is present' (live incident Jul 14 2026, preset 'closed'). Fixes, in totality: - _reference_messages: extract visible text via agent/message_content.flatten_message_text for user/assistant/tool turns (skips image parts, so no base64 leaks into the advisory view); decorated and undecorated transcripts now produce a byte-identical advisory view (advisor cache prefix stays stable). - image-only user turns get a placeholder instead of an empty message (Anthropic rejects empty text blocks) or a silently dropped turn (would break user/assistant alternation). - degenerate-case fallback flattens structured content too. - _attach_reference_guidance: a decorated/multimodal trailing user turn now receives the guidance as a NEW text part appended AFTER the cache_control-marked part (cached prefix byte-stable) instead of falling through to a second consecutive user message (strict providers reject user/user). - conversation_loop MoA injection: multimodal user turns get the MoA context appended as a trailing text part instead of being dropped; user_prompt for the one-shot path flattens content lists instead of str()-ing them (which leaked base64 payloads into the prompt). Live-verified on the 'closed' preset (real OpenRouter wire, 2 user turns, tool loop): all 4 reference calls carry the full document + rendered tool state, end on user, zero tool-role/tool_calls; advisor cache_write 7968 then cache_read 5909+; aggregator cache_read 14880-15237 on iterations 2+. Co-authored-by: bo.fu <bo.fu@meituan.com>
…esearch#64280) OpenAI lets ChatGPT-plan Codex users bank rate-limit reset credits, but until now they could only be redeemed from the Codex CLI/app or the website. This wires the same backend API into Hermes: - /usage on the openai-codex provider now shows "You have N resets banked - use /usage reset to activate" (parsed from the rate_limit_reset_credits field the /usage endpoint already returns). - New /usage reset subcommand (CLI + gateway) redeems one banked credit via POST .../rate-limit-reset-credits/consume with a UUID idempotency key, mirroring codex-rs backend-client semantics (PathStyle /wham vs /api/codex, ChatGPT-Account-Id header, reset/nothing_to_reset/no_credit/already_redeemed outcomes). - Guard: redemption is refused while no rate-limit window is fully exhausted, since a banked reset restores the FULL 5h + weekly allowance and spending it early wastes it. /usage reset --force overrides. Zero banked credits and non-codex providers are refused with clear messages; nothing_to_reset reports the credit was NOT spent. - i18n: new gateway.usage.unknown_subcommand / reset_wrong_provider keys across all 16 locales; docs updated (cli.md, messaging index). Tested with unit tests plus a real-socket E2E against a local fake Codex backend exercising redeem/guard/force and the /usage hint.
…ool-only turns A cached _last_content_with_tools response from a housekeeping-only turn could survive a later substantive tool-only turn. When the model returned an empty response, Hermes incorrectly finalized the older housekeeping narration instead of invoking the post-tool empty-response nudge. Production impact: scheduled cron jobs could return early without completing their actual work (e.g., daily report job returning a housekeeping message instead of producing the report artifact). Root cause: The fallback state was only updated when a turn had both content AND tool_calls. A turn with tool_calls but empty visible content would skip state updates entirely, leaving stale fallback state intact. Fix: Classify tools in every tool-call turn (regardless of visible content). When any tool is substantive (non-housekeeping), clear the older fallback state before processing later empty responses. This prevents two-turn-old housekeeping narration from being treated as if it belonged to the immediately preceding substantive tool turn. Regression test added: tests/run_agent/test_conversation_fallback_state.py Fixes NousResearch#63860
… turn Salvage of NousResearch#63888. The original fix clears stale _last_content_with_tools on substantive tool-only turns but doesn't clear _mute_post_response, which a prior housekeeping turn may have set. This suppresses tool progress output via _vprint until the no-tool-call branch resets it at line ~4834 — after all tools have finished executing. Fix: also reset _mute_post_response = False when clearing stale fallback. Added test: verify pure housekeeping turns (content + only housekeeping tools) still set the fallback correctly — the original use case the fallback was designed for. Co-authored-by: liuhao1024 <sunsky.lau@gmail.com>
Add a bounded turn-end stop guard for kanban workers. When a worker tries to exit with finish_reason=stop without having called kanban_complete or kanban_block, inject up to two synthetic nudges so the conversation loop continues instead of exiting cleanly (which the dispatcher records as protocol_violation). Mirrors the existing verify-on-stop pattern: same ephemeral scaffolding flag (_kanban_stop_synthetic), same role-alternation contract, same _pending_verification_response fallback for budget exhaustion. Disabled by default (gated on HERMES_KANBAN_TASK env var set by the dispatcher); kill switch via HERMES_KANBAN_STOP_NUDGE=0. Salvaged from NousResearch#62262 by @mdc2122. The original branch was 272 commits behind main with ~538 files of stale-base reversions; this salvage applies only the 4 substantive files (agent/kanban_stop.py, conversation_loop.py insertion, run_agent.py _EPHEMERAL_SCAFFOLDING_FLAGS, tests/agent/test_kanban_stop.py).
…tion On Darwin, the default synchronous=NORMAL only calls fsync(), which Apple explicitly states does not guarantee data-on-platter or write-ordering. During a WAL checkpoint race with process termination (e.g., launchd shutdown), this can leave the main DB with half-written btree pages, resulting in btreeInitPage error 11 corruption. WAL mode's durability guarantee assumes the OS honors fsync barriers; macOS does not unless we explicitly set synchronous=FULL (which issues fsync() and F_FULLFSYNC via checkpoint_fullfsync=1). Previously, apply_wal_with_fallback() skipped setting synchronous=FULL when the DB was already in WAL mode, leaving connections at the unsafe synchronous=NORMAL default. This commit adds _enforce_macos_synchronous_full() to always enforce synchronous=FULL on macOS after any WAL activation. Fixes NousResearch#63531
synchronous=FULL issues plain fsync(), not F_FULLFSYNC. The F_FULLFSYNC barrier comes from checkpoint_fullfsync=1, set by the separate _apply_macos_checkpoint_barrier(). The original docstring conflated the two PRAGMAs.
… collisions
scan_skill_commands() had two collision bugs in the same loop body:
1. Core-command collision: a skill whose normalized slug matches a core
Hermes command name or alias (e.g. "skills", "learn", "bg") would
get an auto-generated /command that shadows the core command in the
gateway dispatch path (skill map is consulted before built-in
handlers). The skill command silently overrode the core command.
2. Inter-skill slug collision: the seen_names set deduped on the raw
frontmatter name, but the command map was keyed by the normalized
slug. Two distinct names collapsing to the same slug (e.g.
"git_helper" vs "git-helper") both passed the dedup, and the second
silently clobbered the first.
Fix: add two guards in scan_skill_commands() after slug normalization:
- resolve_command(cmd_name) check skips skills colliding with any core
CommandDef (name or alias), logging a warning. Uses the existing
resolve_command() API so aliases and case variants are covered
without a separate cache. The skill remains loadable via /skill.
- cmd_key in _skill_commands check dedups on the resolved slug,
first-wins (preserving local-before-external precedence), logging
a warning naming the shadowed skill.
Combines and supersedes NousResearch#31204 (@cyrkstudios), NousResearch#53450 (@Gridzilla),
NousResearch#50304 (@petrichor-op), and NousResearch#63305 (@Vissirexa).
Co-authored-by: cyrkstudios <cyrkstudios@users.noreply.github.com>
Co-authored-by: Gridzilla <Gridzilla@users.noreply.github.com>
Co-authored-by: petrichor-op <petrichor-op@users.noreply.github.com>
Co-authored-by: Vissirexa <Vissirexa@users.noreply.github.com>
…4-cli-close-persist fix(cli): persist close transcript without history alias
…ings - Fix no-useless-escape in i18n/ko.ts and i18n/uk.ts (remove backslash escapes inside single-quoted strings) - Add eslint-disable-next-line for no-control-regex in pty-mobile-input.ts (terminal data legitimately contains control characters) - Configure react-refresh/only-export-components with allowConstantExport so context providers that export hooks don't trigger the rule - Downgrade react-hooks v7 rules (set-state-in-effect, refs, preserve-manual-memoization, static-components) from error to warn — these are real concerns but the existing code uses common patterns (data loading on mount, ref-as-instance-var) that need careful refactoring
apps/shared had lint:fix but not the 'fix' alias that other workspaces have. The js-tests check job runs 'npm run fix' as a second step, so this workspace was failing with 'Missing script: fix'.
The hoisted shared eslint config catches pre-existing no-useless-escape and no-control-regex errors in apps/desktop that were only fixed in web/ in the previous commit. - no-useless-escape: remove unnecessary \) escapes inside character classes - no-control-regex: add eslint-disable-next-line comments for intentional \x1b terminal escape byte matching (same pattern as web/src/lib/pty-mobile-input.ts)
we need em for desktop :)
* fix(dashboard): add MCP auth to profile builder * fix(dashboard): preserve MCP rejection error contract * feat(dashboard): refine profile MCP picker
ran npm i
…sResearch#65186) * fix(js): never format package-lock.json prettier and eslint should never touch package-lock.json. main has a repo rule requiring team approval when lockfiles change, so an autofix PR touching it would hang waiting for review. - Add .prettierignore at repo root - Add '**/package-lock.json' to eslint shared config ignores * fix(ci): js-autofix pushes via PR instead of direct push to main Main now has repository rules requiring pull requests + required status checks ("All required checks pass"), so the workflow's direct push to main is rejected with GH013 every time eslint --fix produces changes. Switch apply-patch to push to a dedicated bot/js-autofix branch, create or update a PR, and enable auto-merge (squash). The PR auto-merges once CI passes. If CI fails or main moves, the PR is auto-closed and the branch deleted — the next run re-applies on the current state. The two-job security split is preserved: - generate-patch stays unprivileged (contents: read only) — it runs npm on an ephemeral runner with zero push permissions. - apply-patch (contents: write + pull-requests: write) still never runs npm, never installs anything, never executes repo code — it applies the trusted patch artifact and delivers it via PR.
Adds a "Tests for JavaScript / npm / package.json invariants belong in the JS suite" subsection under Testing, documenting that the CI classifier routes package.json / lockfile / .ts/.tsx changes to the frontend lane — so Python tests asserting about those files won't run on a JS-only PR. Includes a table mapping artifact types to the correct vitest workspace and run commands.
The CI change classifier routes package.json / package-lock.json /
.ts/.tsx changes to the frontend lane, not the Python lane. Four
Python tests asserted about these JS-side artifacts, so a PR touching
only those files would skip the Python suite — the regression goes
green on the PR and red on main (where the classifier fails open).
Ported all four to vitest so they run in the correct CI lane:
tests/test_package_json_lazy_deps.py
→ apps/desktop/electron/package-json-lazy-deps.test.ts
(camofox is lazy, agent-browser is eager, lockfile clean)
tests/test_desktop_electron_pin.py
→ apps/desktop/electron/desktop-electron-pin.test.ts
(electron dep is exact, matches build.electronVersion, lockfile agrees)
tests/test_assistant_ui_tap_compat.py
→ apps/desktop/electron/assistant-ui-tap-compat.test.ts
(@assistant-ui cluster shares one tap version + semver helper)
tests/test_dashboard_sidecar_close_on_disconnect.py
→ web/src/lib/chat-sidebar-session-params.test.ts
(sidecar session.create opts into close_on_disconnect + profile)
The ChatSidebar test was regex-matching .tsx source text (the
source-reading anti-pattern). Extracted sidecarSessionCreateParams()
from the component's effect into an exported pure function so the
test calls real code instead of pattern-matching a string.
Verified: 8 electron tests + 76 web tests pass; both typecheck clean.
tests/test_desktop_mac_entitlements.py asserts about apps/desktop/electron/*.plist — the same CI blind spot as the other ported tests: the change classifier routes apps/ changes to the frontend lane, so a PR touching only the plists would skip the Python suite and the regression would go green on the PR and red on main. Ported to tests-js/desktop-mac-entitlements.test.ts so it runs in the correct lane. All three tests carry over: the inherit plist grants audio-input (regression NousResearch#37718), every device.* entitlement on the main app is also inherited, and both files remain well-formed. Parsing uses the `plist` package (pinned to ^3.1.0, the version already present in the workspace lockfile, so no new transitive packages) plus `@types/plist` — Node's stand-in for plistlib. Verified: tests-js `npm run check` passes (typecheck + 9/9 tests), and a mutation run (removing audio-input from the inherit plist) turns both regression tests red.
The js-autofix workflow used 'gh pr create --json number --jq .number' to capture the PR number, but 'gh pr create' doesn't support --json. Extract the PR number from the URL that 'gh pr create' prints instead.
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
When the bot PR auto-merges, main moves — which the poll loop detects as 'main moved' and tries to close the PR. But the PR is already merged, so gh pr close fails with an error. Fix: when main moves, re-check PR state before closing. If it's already MERGED, exit cleanly. Also make gh pr close non-fatal (|| true) as a belt-and-suspenders guard against the same race on the other close paths.
…sResearch#60145) hermes-agent is public/OSS; the forensic-logging comment in _quarantine_nous_oauth_state named 'Fly' (the specific managed-hosting compute provider) twice. Reword generically ('a hosted agent', 'a managed log drain may be WARNING-only') — the behaviour is unchanged, only the comment. Follows the same scrub applied to the boot re-seed helper (NousResearch#59983) before merge; this one slipped through in NousResearch#59976.
* fix(auth): apply newer hosted bootstrap session * fix(auth): validate rebootstrap replacement seeds
eb2eb1b to
f285a31
Compare
…ex models (NousResearch#24182) * feat(codex-runtime): scaffold optional codex app-server runtime Foundational commit for an opt-in alternate runtime that hands OpenAI/Codex turns to a 'codex app-server' subprocess instead of Hermes' tool dispatch. Default behavior is unchanged. Lands in three pieces: 1. agent/transports/codex_app_server.py — JSON-RPC 2.0 over stdio speaker for codex's app-server protocol (codex-rs/app-server). Spawn, init handshake, request/response, notification queue, server-initiated request queue (for approval round-trips), interrupt-friendly blocking reads. Tested against real codex 0.130.0 binary end-to-end during development. 2. hermes_cli/runtime_provider.py: - Adds 'codex_app_server' to _VALID_API_MODES. - Adds _maybe_apply_codex_app_server_runtime() helper, called at the end of _resolve_runtime_from_pool_entry(). Inert unless 'model.openai_runtime: codex_app_server' is set in config.yaml AND provider in {openai, openai-codex}. Other providers cannot be rerouted (anthropic, openrouter, etc. preserved). 3. tests/agent/transports/test_codex_app_server_runtime.py — 24 tests covering api_mode registration, the rewriter helper (default-off, case-insensitive, opt-in, non-eligible providers preserved), version parser, missing-binary handling, error class. Does NOT require codex CLI installed. This commit is wire-only: the api_mode is recognized but AIAgent does not yet branch on it. Followup commits add the session adapter, event projector, approval bridge, transcript projection (so memory/skill review still works), plugin migration, and slash command. Existing tests remain green: - tests/cli/test_cli_provider_resolution.py (29 passed) - tests/agent/test_credential_pool_routing.py (included above) * feat(codex-runtime): add codex item projector for memory/skill review The translator that lets Hermes' self-improvement loop keep working under the Codex runtime: converts codex 'item/*' notifications into Hermes' standard {role, content, tool_calls, tool_call_id} message shape that agent/curator.py already knows how to read. Item taxonomy (matches codex-rs/app-server-protocol/src/protocol/v2/item.rs): - userMessage → {role: user, content} - agentMessage → {role: assistant, content: text} - reasoning → stashed in next assistant's 'reasoning' field - commandExecution → assistant tool_call(name='exec_command') + tool result - fileChange → assistant tool_call(name='apply_patch') + tool result - mcpToolCall → assistant tool_call(name='mcp.<server>.<tool>') + tool result - dynamicToolCall → assistant tool_call(name=<tool>) + tool result - plan/hookPrompt/etc → opaque assistant note, no fabricated tool_calls Invariants preserved: - Message role alternation never violated: each tool item produces at most one assistant + one tool message in that order, correlated by call_id. - Streaming deltas (item/<type>/outputDelta, item/agentMessage/delta) don't materialize messages — only item/completed does. Mirrors how Hermes already only writes the assistant message after streaming ends. - Tool call ids are deterministic (codex item id-based) so replays produce identical messages and prefix caches stay valid (AGENTS.md pitfall NousResearch#16). - JSON args use sorted_keys for the same reason. Real wire formats verified against codex 0.130.0 by capturing live notifications from thread/shellCommand and including one as a fixture (COMMAND_EXEC_COMPLETED). 23 new tests, all green: - Streaming deltas don't materialize (3 paths) - Turn/thread frame events are silent - commandExecution: 5 tests including non-zero exit annotation + deterministic id stability across replays - agentMessage + reasoning attachment + reasoning consumption - fileChange: summary without inlined content - mcpToolCall: namespaced naming + error surfacing - userMessage: text fragments only (drops images/etc) - opaque items: no fabricated tool_calls - Helpers: deterministic id stability + sorted JSON args - Role alternation invariant across all four tool-shaped item types This commit is a pure addition. AIAgent integration (the wire that uses the projector) is the next commit. * feat(codex-runtime): add session adapter + approval bridge The third self-contained module: CodexAppServerSession owns one Codex thread per Hermes session, drives turn/start, consumes streaming notifications via CodexEventProjector, handles server-initiated approval requests, and translates cancellation into turn/interrupt. The adapter has a single public per-turn method: result = session.run_turn(user_input='...', turn_timeout=600) # result.final_text → assistant text for the caller # result.projected_messages → list ready to splice into AIAgent.messages # result.tool_iterations → tick count for _iters_since_skill nudge # result.interrupted → True on Ctrl+C / deadline / interrupt # result.error → error string when the turn cannot complete # result.turn_id, thread_id → for sessions DB / resume Behavior: - ensure_started() spawns codex, does the initialize handshake, and issues thread/start with cwd + permissions profile. Idempotent. - run_turn() blocks until turn/completed, drains server-initiated requests (approvals) before reading notifications so codex never deadlocks waiting for us, projects every item/completed via the projector, and increments tool_iterations for the skill nudge gate. - request_interrupt() is thread-safe (threading.Event); the next loop iteration issues turn/interrupt and unwinds. - turn_timeout deadlock guard issues turn/interrupt and records an error if the turn never completes. - close() escalates terminate → kill via the underlying client. Approval bridge: Codex emits server-initiated requests for execCommandApproval and applyPatchApproval. The adapter translates Hermes' approval choice vocabulary onto codex's decision vocabulary: Hermes 'once' → codex 'approved' Hermes 'session' or 'always' → codex 'approvedForSession' Hermes 'deny' / anything else → codex 'denied' Routing precedence: 1. _ServerRequestRouting.auto_approve_* flags (cron / non-interactive) 2. approval_callback wired by the CLI (defers to tools.approval.prompt_dangerous_approval()) 3. Fail-closed denial when neither is wired Unknown server-request methods are answered with JSON-RPC error -32601 so codex doesn't hang waiting for us. Permission profile mapping mirrors AGENTS.md: Hermes 'auto' → codex 'workspace-write' Hermes 'approval-required' → codex 'read-only-with-approval' Hermes 'unrestricted/yolo' → codex 'full-access' 20 new tests, all green. Combined with prior commits this PR now has 67 tests across three modules: - test_codex_app_server_runtime.py: 24 (api_mode + transport surface) - test_codex_event_projector.py: 23 (item taxonomy projections) - test_codex_app_server_session.py: 20 (turn loop + approvals + interrupts) Full tests/agent/transports/ directory: 249/249 pass — no regressions to existing transport tests. Still no wire into AIAgent.run_conversation(); that integration commit is small and goes next. * feat(codex-runtime): wire codex_app_server runtime into AIAgent The integration commit. AIAgent.run_conversation() now early-returns to a new helper _run_codex_app_server_turn() when self.api_mode == 'codex_app_server', bypassing the chat_completions tool loop entirely. Three small surgical edits to run_agent.py (~105 LOC total): 1. Line ~1204 (constructor api_mode validation set): Add 'codex_app_server' so an explicit api_mode='codex_app_server' passed to AIAgent() isn't silently rewritten to 'chat_completions'. 2. Line ~12048 (run_conversation, just before the while loop): Early-return to _run_codex_app_server_turn() when self.api_mode is 'codex_app_server'. Placed AFTER all standard pre-loop setup — logging context, session DB, surrogate sanitization, _user_turn_count and _turns_since_memory increments, _ext_prefetch_cache, memory manager on_turn_start — so behavior outside the model-call loop is identical between paths. Default Hermes flow is unchanged when the flag is off. 3. End-of-class (line ~15497): New method _run_codex_app_server_turn(). Lazy-instantiates one CodexAppServerSession per AIAgent (reused across turns), runs the turn, splices projected_messages into messages, increments _iters_since_skill by tool_iterations (since the chat_completions loop normally does that per iteration), fires _spawn_background_review on the same cadence as the default path. Counter accounting: _turns_since_memory ← already incremented at run_conversation:11817 (gated on memory store configured) — codex helper does NOT touch it (would double-count). _user_turn_count ← already incremented at run_conversation:11793 — codex helper does NOT touch it. _iters_since_skill ← incremented in the chat_completions loop per tool iteration. Codex helper increments by turn.tool_iterations since the loop is bypassed. User message: ALREADY appended to messages by run_conversation pre-loop (line 11823) before the early-return reaches us. Helper does NOT append again. Regression test test_user_message_not_duplicated guards this. Approval callback wiring: Lazy-fetches tools.terminal_tool._get_approval_callback at session spawn time, passes to CodexAppServerSession. CLI threads with prompt_toolkit get interactive approvals; gateway/cron contexts get the codex-side fail-closed deny. Error path: Codex session exceptions become a 'partial' result with completed=False and a final_response that explicitly tells the user how to switch back: 'Codex app-server turn failed: ... Fall back to default runtime with /codex-runtime auto.' Same return-dict shape as the chat_completions path so all callers (gateway, CLI, batch_runner, ACP) work unchanged. 9 new integration tests in tests/run_agent/test_codex_app_server_integration.py: - api_mode='codex_app_server' is accepted on AIAgent construction - run_conversation returns the expected codex shape (final_response, codex_thread_id, codex_turn_id, completed, partial) - Projected messages are spliced into messages list - _iters_since_skill ticks per tool iteration - _user_turn_count delegated to standard flow (not double-counted) - User message appears exactly once (regression guard) - _spawn_background_review IS invoked (memory/skill review keeps working) - chat.completions.create is NEVER called (loop fully bypassed) - Session exception → partial result with /codex-runtime auto hint - Interrupted turn → partial result with error preserved Adjacent test runs confirm no regressions: - tests/run_agent/test_memory_nudge_counter_hydration.py: green - tests/run_agent/test_background_review.py: green - tests/run_agent/test_fallback_model.py: green - tests/agent/transports/: 249/249 green Still missing for full feature: /codex-runtime slash command, plugin migration helper, docs page, live e2e test gated on codex binary. Those are the remaining followup commits. * feat(codex-runtime): add /codex-runtime slash command (CLI + gateway) User-facing toggle for the optional codex app-server runtime. Follows the 'Adding a Slash Command (All Platforms)' pattern from AGENTS.md exactly: single CommandDef in the central registry → CLI handler → gateway handler → running-agent guard → all surfaces (autocomplete, /help, Telegram menu, Slack subcommands) update automatically. Surface: /codex-runtime — show current state + codex CLI status /codex-runtime auto — Hermes default runtime /codex-runtime codex_app_server — codex subprocess runtime /codex-runtime on / off — synonyms Files changed: hermes_cli/codex_runtime_switch.py (new): Pure-Python state machine shared by CLI and gateway. Parse args, read/write model.openai_runtime in the config dict, gate enabling behind a codex --version check (don't let users opt in to a runtime they have no binary for; print npm install hint instead). Returns a CodexRuntimeStatus dataclass that callers render however suits their surface. hermes_cli/commands.py: Single CommandDef entry, no aliases (codex-runtime is its own thing). cli.py: Dispatch in process_command() + _handle_codex_runtime() handler that delegates to the shared module and renders results via _cprint. gateway/run.py: Dispatch in _handle_message() + _handle_codex_runtime_command() that returns a string (gateway sends as message). On a successful change that requires a new session, _evict_cached_agent() forces the next inbound message to construct a fresh AIAgent with the new api_mode — avoids prompt-cache invalidation mid-session. gateway/run.py running-agent guard: /codex-runtime joins /model in the early-intercept block so a runtime flip mid-turn can't split a turn across two transports. Tests: tests/hermes_cli/test_codex_runtime_switch.py — 25 tests covering the state machine: arg parsing (10 cases incl. case-insensitive and synonyms), reading current runtime (5 cases incl. malformed configs), writing runtime (3 cases), apply() entry point covering read-only, no-op, codex-missing-blocked, codex-present-success, disable-no-binary-check, and persist-failure paths (8 cases). All green. Adjacent test suites confirm no regressions: - tests/hermes_cli/test_commands.py + test_codex_runtime_switch.py: 167/167 green - tests/agent/transports/: 283/283 green when combined with prior commits Still missing: plugin migration helper, docs page, live e2e test gated on codex binary. Followup commits. * feat(codex-runtime): auto-migrate Hermes MCP servers to ~/.codex/config.toml Translates the user's mcp_servers config from ~/.hermes/config.yaml into the TOML format codex's MCP client expects. Wired into the /codex-runtime codex_app_server enable path so users get their MCP tool surface in the spawned subprocess automatically. The migration runs on every enable. Failures are non-fatal — the runtime change still proceeds and the user gets a warning so they can fix the codex config manually. What translates (mapping verified against codex-rs/core/src/config/edit.rs): Hermes mcp_servers.<n>.command/args/env → codex stdio transport Hermes mcp_servers.<n>.url/headers → codex streamable_http transport Hermes mcp_servers.<n>.timeout → codex tool_timeout_sec Hermes mcp_servers.<n>.connect_timeout → codex startup_timeout_sec Hermes mcp_servers.<n>.cwd → codex stdio cwd Hermes mcp_servers.<n>.enabled: false → codex enabled = false What does NOT translate (warned + skipped per server): Hermes-specific keys (sampling, etc.) — codex's MCP client has no equivalent. Listed in the per-server skipped[] field of the report. What's NOT migrated (intentional): AGENTS.md — codex respects this file natively in its cwd. Hermes' own AGENTS.md (project-level) is already in the worktree, so codex picks it up without translation. No code needed. Idempotency design: All managed content lives between a 'managed by hermes-agent' marker and the next non-mcp_servers section header. _strip_existing_managed_block removes the prior managed region cleanly, preserving any user-added codex config (model, providers.openai, sandbox profiles, etc.) above or below. Files added: hermes_cli/codex_runtime_plugin_migration.py — pure-Python migration helper. Public API: migrate(hermes_config, codex_home=None, dry_run=False) returns MigrationReport with .migrated/.errors/ .skipped_keys_per_server. No external TOML dependency — minimal formatter handles strings/numbers/booleans/lists/inline-tables. tests/hermes_cli/test_codex_runtime_plugin_migration.py — 39 tests covering: - per-server translation (12): stdio/http/sse, cwd, timeouts, enabled flag, command+url precedence, sampling drop, unknown keys - TOML formatter (8): types, escaping, inline tables, error case - existing-block stripping (4): no marker, alone, with user content above, with user content below - end-to-end migrate() (8): empty, dry-run, round-trip, idempotent re-run, preserves user config, error reporting, invalid input, summary formatting Files changed: hermes_cli/codex_runtime_switch.py — apply() now calls migrate() in the codex_app_server enable branch. Migration failure logs a warning in the result message but does NOT fail the runtime change. Disable path (auto) explicitly skips migration. tests/hermes_cli/test_codex_runtime_switch.py — 3 new tests: test_enable_triggers_mcp_migration, test_disable_does_not_trigger_migration, test_migration_failure_does_not_block_enable. All 325 feature tests green: - tests/agent/transports/: 249 (incl. 67 new) - tests/run_agent/test_codex_app_server_integration.py: 9 - tests/hermes_cli/test_codex_runtime_switch.py: 28 (3 new) - tests/hermes_cli/test_codex_runtime_plugin_migration.py: 39 (new) * perf(codex-runtime): cache codex --version check within apply() Single /codex-runtime invocation could spawn 'codex --version' up to 3 times (state report, enable gate, success message). Each spawn is ~50ms, so the cumulative cost wasn't a crisis, but it was wasteful and turned a trivial slash command into something noticeably laggy on slower systems. Refactored to lazy-once via a closure over a nonlocal cache. First call spawns; subsequent calls in the same apply() reuse the result. Behavior unchanged — same return shape, same error handling, same install hint when codex is missing. Just one subprocess per call instead of three. Two regression-guard tests added: - test_binary_check_cached_within_apply: enable path → call_count == 1 - test_binary_check_cached_on_read_only_call: state-report path → call_count == 1 Total tests for /codex-runtime now 30 (was 28); all 143 codex-runtime tests still green. * fix(codex-runtime): correct protocol field names found via live e2e test Three real bugs caught only by running a turn end-to-end against codex 0.130.0 with a real ChatGPT subscription. Unit tests passed because they asserted on our own (incorrect) wire shapes; the wire format from codex-rs/app-server-protocol/src/protocol/v2/* is the source of truth and my initial reading of the README was incomplete. Bug 1: thread/start.permissions wire format Was sending {"profileId": "workspace-write"}. Real format per PermissionProfileSelectionParams enum (tagged union): {"type": "profile", "id": "workspace-write"} AND requires the experimentalApi capability declared during initialize. AND requires a matching [permissions] table in ~/.codex/config.toml or codex fails the request with 'default_permissions requires a [permissions] table'. Fix: stop overriding permissions on thread/start. Codex picks its default profile (read-only unless user configures otherwise), which matches what codex CLI users expect — they configure their default permission profile in ~/.codex/config.toml the standard way. Trying to be clever about profile selection broke every turn we tested. Live error before fix: 'Invalid request: missing field type' on every turn/start, even though our turn/start payload was correct — the field codex was complaining about was inside the permissions sub-object we shouldn't have been sending. Bug 2: server-request method names Was matching 'execCommandApproval' and 'applyPatchApproval'. Real names per common.rs ServerRequest enum: item/commandExecution/requestApproval item/fileChange/requestApproval item/permissions/requestApproval (new third method) Fix: match the documented names. Added handler for item/permissions/requestApproval that always declines — codex sometimes asks to escalate permissions mid-turn and silent acceptance would surprise users. Live symptom before fix: agent.log showed 'Unknown codex server request: item/commandExecution/requestApproval' and codex stalled because we replied with -32601 (unsupported method) instead of an approval decision. The agent reported back 'The write command was rejected' even though Hermes never showed the user an approval prompt. Bug 3: approval decision values Was sending decision strings 'approved'/'approvedForSession'/'denied'. Real values per CommandExecutionApprovalDecision enum (camelCase): accept, acceptForSession, decline, cancel (also AcceptWithExecpolicyAmendment and ApplyNetworkPolicyAmendment variants we don't currently use). Fix: rename _approval_choice_to_codex_decision return values; update auto_approve_* fallbacks; update fail-closed default from 'denied' to 'decline'. Test mapping table updated to match. Live test verified after fixes: $ hermes (with model.openai_runtime: codex_app_server) > Run the shell command: echo hermes-codex-livetest > .../proof.txt then read it back Approval prompt fired with 'Codex requests exec in <cwd>'. User chose 'Allow once'. Codex executed the command, wrote the file, read it back. Final response: 'Read back from proof.txt: hermes-codex-livetest'. File contents on disk match. agent.log confirms: codex app-server thread started: id=019e200e profile=workspace-write cwd=/tmp/hermes-codex-livetest/workspace All 20 session tests still green after wire-format updates. * fix(codex-runtime): correct apply_patch approval params + ship docs Live e2e revealed FileChangeRequestApprovalParams doesn't carry the changeset (just itemId, threadId, turnId, reason, grantRoot) — Codex's 'reason' field describes what the patch wants to do. Test config and display logic updated to use it. The first 'apply_patch (0 change(s))' display from the live test is now 'apply_patch: <reason>'. Adds website/docs/user-guide/features/codex-app-server-runtime.md covering enable/disable, prerequisites, approval UX, MCP migration behavior, permission profile delegation to ~/.codex/config.toml, known limitations, and the architecture diagram. Wired into the Automation category in sidebars.ts. Live e2e validation across the path matrix: ✓ thread/start handshake ✓ turn/start with text input ✓ commandExecution items + projection ✓ item/commandExecution/requestApproval → Hermes UI → response ✓ Approve once → command runs ✓ Deny → command rejected, codex falls back to read-only message ✓ Multi-turn (codex remembers prior turn's results) ✓ apply_patch via Codex's fileChange path ✓ item/fileChange/requestApproval → Hermes UI ✓ MCP server migration loads inside spawned codex (verified via 'use the filesystem MCP tool' prompt) ✓ /codex-runtime auto → codex_app_server toggle cycle ✓ Disable doesn't trigger migration ✓ Enable with codex CLI present succeeds + migrates ✓ Hermes-side interrupt path (turn/interrupt request issued cleanly even if codex finishes before the interrupt lands) Known live-validated limitations now documented in the docs page: - delegate_task subagents unavailable on this runtime - permission profile selection delegated to ~/.codex/config.toml - apply_patch approval prompt has no inline changeset (codex protocol doesn't expose it) 145/145 codex-runtime tests still green. * feat(codex-runtime): native plugin migration + UX polish (quirks 2/4/5/10/11) Major: migrate native Codex plugins (#7 in OpenClaw's PR list) Discovers installed curated plugins via codex's plugin/list RPC and writes [plugins."<name>@<marketplace>"] entries to ~/.codex/config.toml so they're enabled in the spawned Codex sessions. This is the 'YouTube-video-worthy' bit Pash highlighted: when a user has google-calendar, github, etc. installed in their Codex CLI, those plugins activate automatically when they enable Hermes' codex runtime. Implementation: - hermes_cli/codex_runtime_plugin_migration.py: new _query_codex_plugins() helper spawns 'codex app-server' briefly and walks plugin/list. Returns (plugins, error) — failures are non-fatal so MCP migration still works. - render_codex_toml_section() now takes plugins + permissions args. - migrate() defaults: discover_plugins=True, default_permission_profile= 'workspace-write'. Explicit None on either disables that side. - _strip_existing_managed_block() now also strips [plugins.*] and [permissions]/[permissions.*] sections inside the managed block, so re-runs replace plugins cleanly without touching codex's own config. Quirk fixes: #2 Default permissions profile written on enable. Without this, Codex's read-only default kicks in and EVERY write triggers an approval prompt. Now writes [permissions] default = 'workspace-write' so the runtime feels normal out of the box. Set default_permission_profile=None to opt out. #4 apply_patch approval prompt now shows what's changing. Codex's FileChangeRequestApprovalParams doesn't carry the changeset. Session adapter now caches the fileChange item from item/started notifications and looks it up by itemId when codex requests approval. Prompt shows '1 add, 1 update: /tmp/new.py, /tmp/old.py' instead of 'apply_patch (0 change(s))'. Side benefit: also drains pending notifications BEFORE handling a server request, so the projector and per-turn caches are up to date when the approval decision fires. Bounded to 8 notifications per loop iter to avoid starving codex's response. #5/#10 Exec approval prompt never shows empty cwd. When codex omits cwd in CommandExecutionRequestApprovalParams, fall back to the session's cwd. If somehow neither is available, show '<unknown>' explicitly instead of an empty string. Also surfaces 'reason' from the approval params when codex provides it — gives users more context on why codex wants to run something. #11 Banner indicates the codex_app_server runtime when active. New 'Runtime: codex app-server (terminal/file ops/MCP run inside codex)' line appears in the welcome banner only when the runtime is on. Default banner is unchanged. Tests: - 7 new tests in test_codex_runtime_plugin_migration.py covering plugin discovery (mocked), failure handling, dry-run skip, opt-out flag, idempotent re-runs, and permissions writing. - 3 new tests in test_codex_app_server_session.py covering the enriched approval prompts: cwd fallback, change summary on apply_patch, fallback when no item/started cache exists. - All 26 session tests + 46 migration tests green; 153 total in PR. * feat(codex-runtime): hermes-tools MCP callback + native plugin migration The big architectural addition: when codex_app_server runtime is on, Hermes registers its own tool surface as an MCP server in ~/.codex/config.toml so the codex subprocess can call back into Hermes for tools codex doesn't ship with — web_search, browser_*, vision, image_generate, skills, TTS. Also: 'migrate native codex plugins' (Pash's YouTube-video-worthy bit) — when the user has plugins like Linear, GitHub, Gmail, Calendar, Canva installed via 'codex plugin', Hermes discovers them via plugin/list and writes [plugins.<name>@openai-curated] entries so they activate automatically. New module: agent/transports/hermes_tools_mcp_server.py FastMCP stdio server exposing 17 Hermes tools. Each call dispatches through model_tools.handle_function_call() — same code path as the Hermes default runtime. Run with: python -m agent.transports.hermes_tools_mcp_server [--verbose] Exposed: web_search, web_extract, browser_navigate / _click / _type / _press / _snapshot / _scroll / _back / _get_images / _console / _vision, vision_analyze, image_generate, skill_view, skills_list, text_to_speech. NOT exposed (deliberately): - terminal/shell/read_file/write_file/patch — codex has built-ins - delegate_task/memory/session_search/todo — _AGENT_LOOP_TOOLS in model_tools.py:493, require running AIAgent context. Documented as a limitation and surfaced in the slash command output. Migration changes (hermes_cli/codex_runtime_plugin_migration.py): - _query_codex_plugins() spawns 'codex app-server' briefly to walk plugin/list and pull installed openai-curated plugins. Failures are non-fatal — MCP migration still completes. - render_codex_toml_section() now takes plugins + permissions args AND wraps the managed block with a MIGRATION_END_MARKER comment so the stripper can reliably find both ends, even when the block contains top-level keys (default_permissions = ...). - migrate() defaults: discover_plugins=True, expose_hermes_tools=True, default_permission_profile=':workspace' (built-in codex profile name — must be prefixed with ':'). All three opt-out via explicit args. - _build_hermes_tools_mcp_entry() builds the codex stdio entry with HERMES_HOME and PYTHONPATH passthrough so a worktree-launched Hermes points the MCP subprocess at the same module layout. Live-caught wire bugs fixed during this turn: 1. Permission profile config key is top-level , NOT a [permissions] table. The [permissions] table is for *user-defined* profiles with structured fields. Built-in profile names start with ':' (':workspace', ':read-only', ':danger-no-sandbox'). Was emitting which codex rejected with 'invalid type: string "X", expected struct PermissionProfileToml'. 2. Built-in profile is , NOT . Codex rejected with 'unknown built-in profile'. 3. Codex's MCP layer sends for tool-call confirmation. We weren't handling it, so codex stalled and returned 'MCP tool call was rejected'. Now: auto-accept for our own hermes-tools server (user already opted in by enabling the runtime), decline for third-party servers. Quirk fixes shipped (from the limitations list): #2 default permissions: workspace profile written on enable. No more approval prompt on every write. #4 apply_patch approval shows what's changing: cache fileChange items from item/started, look up by itemId when codex sends item/fileChange/requestApproval. Prompt: '1 add, 1 update: /tmp/new.py, /tmp/old.py' instead of '0 change(s)'. #5/#10 exec approval cwd never empty: fall back to session cwd, then '<unknown>'. Also surfaces 'reason' from codex when present. #11 banner shows 'Runtime: codex app-server' line when active so users understand why tool counts may not match what's reachable. Tests: - 5 new tests in test_codex_runtime_plugin_migration.py covering plugin discovery, expose_hermes_tools entry generation, idempotent re-runs, opt-out flag, permissions profile. - 3 new tests in test_codex_app_server_session.py covering enriched approval prompts (cwd fallback, fileChange summary). - 2 new tests for mcpServer/elicitation/request handling (accept hermes-tools, decline others). - New test file test_hermes_tools_mcp_server.py covering module surface, EXPOSED_TOOLS safety invariants (no shell/file_ops, no agent-loop tools), and main() error paths. - 166 codex-runtime tests total, all green. Live e2e validated against codex 0.130.0 + ChatGPT subscription: ✓ /codex-runtime codex_app_server enables, migrates filesystem MCP, registers hermes-tools, writes default_permissions = ':workspace' ✓ Banner shows 'Runtime: codex app-server' line in subsequent sessions ✓ Shell command runs without approval prompt (workspace profile works) ✓ Multi-turn — codex remembers prior turn's results ✓ apply_patch path via fileChange request approval ✓ web_search via hermes-tools MCP callback returns real Firecrawl results: 'OpenAI Codex CLI – Getting Started' end-to-end in 13s ✓ Disable cycle clean Docs updated: website/docs/user-guide/features/codex-app-server-runtime.md Full re-write covering native plugin migration, the hermes-tools callback architecture, the prerequisites change ('codex login is separate from hermes auth login codex'), the trade-off table now reflecting which Hermes tools work via callback, and the limitations list updated with what's actually unavailable on this runtime. * feat(codex-runtime): pin user-config preservation invariant for quirk #6 Quirk #6 from the limitations list — user MCP servers / overrides / codex-only sections in ~/.codex/config.toml that live OUTSIDE the hermes-managed block must survive re-migration verbatim. This already worked thanks to the MIGRATION_MARKER + MIGRATION_END_MARKER pair I added when fixing the default_permissions wire format (so the strip can find both ends of the managed region even with top-level keys like default_permissions). But it was an emergent property without a test pinning it. Now explicitly tested: - User MCP server above the managed block survives migration - User MCP server below the managed block survives migration - Both above + below survive a second re-migration - User content (model, providers, sandbox, otel, etc.) outside our region is left untouched Docs added a section "Editing ~/.codex/config.toml safely" explaining the marker contract — so users know they can add their own MCP servers, override permissions, configure codex-only options, etc. without fear of Hermes overwriting their work. 167 codex-runtime tests, all green. * docs(codex-runtime): clarify the actual tool surface — shell covers terminal/read/write/find Previous docs and PR description undersold what codex's built-in toolset actually provides. apply_patch alone made it sound like the runtime could only edit files in patch format — implying you'd lose terminal use, read_file, write_file, search/find. That was wrong. Codex's 'shell' tool runs arbitrary shell commands inside the sandbox, which covers everything you'd do in bash: cat/head/tail (read), echo> or heredocs (write), find/rg/grep (search), ls/cd (navigate), build/ test/git/etc. apply_patch is for structured multi-file edits on top of that. update_plan is its in-runtime todo. view_image loads images. And codex has its own web_search built in (in addition to the Firecrawl-backed one Hermes exposes via MCP callback). Docs now have a 'What tools the model actually has' section right after Why, breaking the surface into three clearly-labeled buckets: 1. Codex's built-in toolset (always on) — shell, apply_patch, update_plan, view_image, web_search; covers everything terminal- adjacent. 2. Native Codex plugins (auto-migrated from your codex plugin install) — Linear, GitHub, Gmail, Calendar, Outlook, Canva, etc. 3. Hermes tool callback (MCP server in ~/.codex/config.toml) — web_search/web_extract via Firecrawl, browser_*, vision_analyze, image_generate, skill_view/skills_list, text_to_speech. Plus a 'What's NOT available' callout listing the four agent-loop tools (delegate_task, memory, session_search, todo) that need running AIAgent context and can't reach the codex runtime. Trade-offs table broken out: shell, apply_patch, update_plan, view_image, sandbox each get their own row with a one-line description so users can see at a glance what's available natively. Architecture diagram updated to list the codex built-ins by name instead of 'apply_patch + shell + sandbox'. No code changes — purely docs clarification. 167 codex-runtime tests still green. * fix(codex-runtime): _spawn_background_review signature + review fork api_mode downgrade Two real bugs in the self-improvement loop integration that the previous test mocked away. Bug 1: wrong call signature The codex helper was calling self._spawn_background_review() with no args after every turn. That function actually requires: messages_snapshot=list (positional or keyword) review_memory=bool (at least one trigger must be True) review_skills=bool So the call would have raised TypeError at runtime — except the only test that exercised this path mocked _spawn_background_review entirely and just asserted spawn.called, so the wrong-arg shape never surfaced. Bug 2: review fork inherits codex_app_server api_mode The review fork is constructed with: api_mode = _parent_runtime.get('api_mode') So when the parent is codex_app_server, the review fork ALSO runs as codex_app_server. But the review fork's whole job is to call agent-loop tools (memory, skill_manage) which require Hermes' own dispatch — they short-circuit with 'must be handled by the agent loop' on the codex runtime. So the review fork would have run, decided to save something, called memory or skill_manage, and silently no-op'd. Fixed in run_agent.py:_spawn_background_review() — when the parent api_mode is 'codex_app_server', the review fork is downgraded to 'codex_responses' (same OAuth credentials, same openai-codex provider, but talks to OpenAI's Responses API directly so Hermes owns the loop). Also rewrote the codex helper's review wiring to match the chat_completions path: - Computes _should_review_memory in the pre-loop block (was already being computed; now passed through to the helper as an arg). - Computes _should_review_skills AFTER the codex turn returns + counters tick (line ~15432 pattern in chat_completions). - Calls _spawn_background_review(messages_snapshot=, review_memory=, review_skills=) only when at least one trigger fires. - Adds the external memory provider sync (_sync_external_memory_for_turn) that the chat_completions path runs after every turn. Tests: Replaced the broken test_background_review_invoked (which only asserted spawn.called) with three sharper tests: - test_background_review_NOT_invoked_below_threshold: single turn at default thresholds → no review fires (would have caught the original 'every turn calls spawn with no args' bug) - test_background_review_skill_trigger_fires_above_threshold: 10 tool_iterations at threshold=10 → review fires with messages_snapshot=list, review_skills=True, counter resets - test_background_review_signature_never_breaks: regression guard asserting positional args are always empty and kwargs include messages_snapshot New TestReviewForkApiModeDowngrade class: - test_codex_app_server_parent_downgrades_review_fork: drives the real _spawn_background_review function (no mock at that level), asserts the review_agent gets api_mode='codex_responses' when the parent was codex_app_server. Live-validated against real run_conversation: - Counter ticked from 0 to 5 after a 5-tool-iteration turn - _spawn_background_review fired exactly once with kwargs-only signature - review_skills=True, review_memory=False - messages_snapshot was 12 entries (5 assistant tool_calls + 5 tool results + 1 final assistant + initial system/user) - Counter reset to 0 after fire 170 codex-runtime tests, all green. Docs: added a Self-improvement loop section to the codex runtime page explaining both how the trigger logic stays equivalent and that the review fork is auto-downgraded to codex_responses for the agent-loop tools. Also clarified that apply_patch and update_plan ARE codex's built-in tools (the previous version made it sound like they were separate from 'codex's stuff' — they're not, all five tools listed in 'What tools the model actually has' section 1 are codex built-ins). * feat(codex-runtime): expose kanban tools through Hermes MCP callback Kanban workers spawn as separate hermes chat -q subprocesses that read the user's config.yaml. If model.openai_runtime: codex_app_server is set globally (which is the whole point of opt-in), every dispatched worker ALSO comes up on the codex runtime. That mostly works — codex's built-in shell + apply_patch + update_plan do the actual task work fine — but it had one critical break: the worker handoff tools (kanban_complete, kanban_block, kanban_comment, kanban_heartbeat) are Hermes-registered tools, not codex built-ins. On the codex runtime, codex builds its own tool list and these never reach the model, so the worker would do the work but not be able to report back, hanging until the dispatcher's timeout escalates it as zombie. Fix: add all 9 kanban tools to the EXPOSED_TOOLS list in the Hermes MCP callback. They dispatch statelessly through handle_function_call() just like web_search and the others — they read HERMES_KANBAN_TASK from env (set by the dispatcher), gate correctly (worker tools require the env var, orchestrator tools require it unset), and write to ~/.hermes/kanban.db. Why kanban tools work via stateless dispatch when delegate_task/memory/ session_search/todo don't: those four are listed in _AGENT_LOOP_TOOLS (model_tools.py:493) and short-circuit in handle_function_call() with 'must be handled by the agent loop' — they need to mutate AIAgent's mid-loop state. Kanban tools have no such requirement; they're pure side-effect functions against the kanban.db plus state_meta. Tools exposed: Worker handoff (require HERMES_KANBAN_TASK): kanban_complete, kanban_block, kanban_comment, kanban_heartbeat Read-only board queries: kanban_show, kanban_list Orchestrator (require HERMES_KANBAN_TASK unset): kanban_create, kanban_unblock, kanban_link Tests: - test_kanban_worker_tools_exposed: complete/block/comment/heartbeat in EXPOSED_TOOLS (regression guard for the would-hang-worker bug) - test_kanban_orchestrator_tools_exposed: create/show/list/unblock/link Docs: - New 'Workflow features' section in the docs page covering /goal, kanban, and cron behavior on this runtime - /goal: works fully via run_conversation feedback; only caveat is approval-prompt noise on long writes-heavy goals (mitigated by the default :workspace permission profile) - Kanban: enumerated which tools are reachable via the callback and why the env var propagates correctly through the codex subprocess to the MCP server subprocess - Cron: documented as 'not specifically tested' — same rules as the CLI apply since cron runs through AIAgent.run_conversation - Trade-offs table gained rows for /goal, kanban worker, kanban orchestrator 172/172 codex-runtime tests green (+2 from kanban tests). * docs(codex-runtime): wire /codex-runtime into slash-commands ref + flag aux token cost Three docs gaps caught during a final audit: 1. /codex-runtime was only in the feature docs page, not in the slash-commands reference. Added rows to both the CLI section and the Messaging section so users discover it where they'd look for slash command syntax. 2. CODEX_HOME and HERMES_KANBAN_TASK weren't in environment-variables.md. CODEX_HOME lets users redirect Codex CLI's config dir (the migration honors it). HERMES_KANBAN_TASK is set by the kanban dispatcher and propagates to the codex subprocess + the hermes-tools MCP subprocess so kanban worker tools gate correctly — documented as 'don't set manually' since it's an internal handoff. 3. Aux client behavior on this runtime. When openai_runtime= codex_app_server is on with the openai-codex provider, every aux task (title generation, context compression, vision auto-detect, session search summarization, the background self-improvement review fork) flows through the user's ChatGPT subscription by default. This is true for the existing codex_responses path too, but it's more visible / important here because users explicitly opted in for subscription billing. Added a 'Auxiliary tasks and ChatGPT subscription token cost' section to the docs page with a YAML example showing how to override specific aux tasks to a cheaper model (typically google/gemini-3-flash-preview via OpenRouter). Also documents how the self-improvement review fork gets auto-downgraded from codex_app_server to codex_responses by the fix earlier in this PR. No code changes — pure docs. 172 codex-runtime tests still green. * docs+test(codex-runtime): pin HOME passthrough, document multi-profile + CODEX_HOME OpenClaw hit a real footgun in openclaw/openclaw#81562: when spawning codex app-server they were synthesizing a per-agent HOME alongside CODEX_HOME. That made every subprocess codex's shell tool launches (gh, git, aws, npm, gcloud, ...) see a fake $HOME and miss the user's real config files. They had to back it out in PR NousResearch#81562 — keep CODEX_HOME isolation, leave HOME alone. Audit confirms Hermes' codex spawn doesn't have this problem. We do os.environ.copy() and only overlay CODEX_HOME (when provided) and RUST_LOG. HOME passes through unchanged. But it was an emergent property without a test pinning it, so adding a regression guard: test_spawn_env_preserves_HOME — confirms parent HOME survives intact in the subprocess env test_spawn_env_sets_CODEX_HOME_when_provided — confirms codex_home arg still isolates codex state correctly Docs additions: 'HOME environment variable passthrough' section — calls out the contract explicitly: CODEX_HOME isolates codex's own state, HOME stays user-real so gh/git/aws/npm/etc. find their normal config. Cites openclaw#81562 as the cautionary tale. 'Multi-profile / multi-tenant setups' section — addresses the related concern: profiles share ~/.codex/ by default. For users who want per-profile codex isolation (separate auth, separate plugins), documents the manual CODEX_HOME=<profile-scoped-dir> approach. Explains why we DON'T auto-scope CODEX_HOME per profile: doing so would silently invalidate existing codex login state for anyone upgrading to this PR with tokens already at ~/.codex/auth.json. Opt-in is safer than surprising users. 174 codex-runtime tests (+2 from HOME guards), all green. * fix(codex-runtime): TOML control-char escapes + atomic config.toml write Two footguns caught in a final audit pass before merge. Bug 1: TOML control characters not escaped The _format_toml_value() helper escaped backslashes and double quotes but passed literal control characters (\n, \t, \r, \f, \b) through unchanged. TOML basic strings don't allow literal control characters — a path or env var containing a newline would produce invalid TOML that codex refuses to load. Realistic exposure: pathological cases like a HERMES_HOME with a trailing newline (env var concatenation accident), or a PYTHONPATH with a tab from a multi-line shell heredoc. Fix: escape all five TOML basic-string control sequences (\b \t \n \f \r) in addition to \\ and \" that we already did. Order matters — backslash must come first or the other escapes get re-escaped. Bug 2: config.toml write wasn't atomic If the python process crashed between target.mkdir() and the write_text() finishing, a half-written config.toml could be left behind. On NFS / Windows / some FUSE mounts this is a real concern; on ext4/APFS small writes are usually atomic in practice but not guaranteed. Fix: write to a tempfile.mkstemp() temp file in the same directory, then Path.replace() (atomic same-dir rename on POSIX, ReplaceFile on Windows). On rename failure, clean up the temp file so repeated failed migrations don't pile up .config.toml.* files. Tests: - test_string_with_newline_escaped — \n in value → \n in output - test_string_with_tab_escaped — \t in value → \t in output - test_string_with_other_controls_escaped — \r, \f, \b - test_windows_path_escaped_correctly — backslash doubling - test_atomic_write_no_temp_leak_on_success — no .config.toml.* left over after a successful write - test_atomic_write_cleanup_on_rename_failure — temp file removed when Path.replace raises (simulated disk full) 180 codex-runtime tests, all green (+6 from this commit). Footguns audited but NOT fixed (with rationale): - Concurrent migrations race. Two Hermes processes hitting /codex-runtime codex_app_server within seconds of each other could cause one writer to lose entries. Low probability (you'd have to enable from two surfaces simultaneously) and low impact (just re-run migration). Adding fcntl/msvcrt locking is more code than it's worth here. The atomic rename above means each individual write is consistent — only the merge step is racy. - Codex protocol version drift. We pin MIN_CODEX_VERSION=0.125 and check at runtime but don't reject too-new versions. Right call — the protocol has been stable through 0.125 → 0.130. If OpenAI breaks it later we'd see the error in test_codex_app_server_runtime on CI before users hit it.
🚨 CRITICAL Supply Chain Risk DetectedThis PR contains a pattern that has been used in real supply chain attacks. A maintainer must review the flagged code carefully before merging. 🚨 CRITICAL: Install-hook file added or modifiedThese files can execute code during package installation or interpreter startup. Files: Scanner only fires on high-signal indicators: .pth files, base64+exec/eval combos, subprocess with encoded commands, or install-hook files. Low-signal warnings were removed intentionally — if you're seeing this comment, the finding is worth inspecting. |
f285a31 to
0554ad6
Compare
🚨 CRITICAL Supply Chain Risk DetectedThis PR contains a pattern that has been used in real supply chain attacks. A maintainer must review the flagged code carefully before merging. 🚨 CRITICAL: Install-hook file added or modifiedThese files can execute code during package installation or interpreter startup. Files: Scanner only fires on high-signal indicators: .pth files, base64+exec/eval combos, subprocess with encoded commands, or install-hook files. Low-signal warnings were removed intentionally — if you're seeing this comment, the finding is worth inspecting. |
2 similar comments
🚨 CRITICAL Supply Chain Risk DetectedThis PR contains a pattern that has been used in real supply chain attacks. A maintainer must review the flagged code carefully before merging. 🚨 CRITICAL: Install-hook file added or modifiedThese files can execute code during package installation or interpreter startup. Files: Scanner only fires on high-signal indicators: .pth files, base64+exec/eval combos, subprocess with encoded commands, or install-hook files. Low-signal warnings were removed intentionally — if you're seeing this comment, the finding is worth inspecting. |
🚨 CRITICAL Supply Chain Risk DetectedThis PR contains a pattern that has been used in real supply chain attacks. A maintainer must review the flagged code carefully before merging. 🚨 CRITICAL: Install-hook file added or modifiedThese files can execute code during package installation or interpreter startup. Files: Scanner only fires on high-signal indicators: .pth files, base64+exec/eval combos, subprocess with encoded commands, or install-hook files. Low-signal warnings were removed intentionally — if you're seeing this comment, the finding is worth inspecting. |
a986771 to
9cde8ea
Compare
🚨 CRITICAL Supply Chain Risk DetectedThis PR contains a pattern that has been used in real supply chain attacks. A maintainer must review the flagged code carefully before merging. 🚨 CRITICAL: Install-hook file added or modifiedThese files can execute code during package installation or interpreter startup. Files: Scanner only fires on high-signal indicators: .pth files, base64+exec/eval combos, subprocess with encoded commands, or install-hook files. Low-signal warnings were removed intentionally — if you're seeing this comment, the finding is worth inspecting. |
🚨 CRITICAL Supply Chain Risk DetectedThis PR contains a pattern that has been used in real supply chain attacks. A maintainer must review the flagged code carefully before merging. 🚨 CRITICAL: Install-hook file added or modifiedThese files can execute code during package installation or interpreter startup. Files: Scanner only fires on high-signal indicators: .pth files, base64+exec/eval combos, subprocess with encoded commands, or install-hook files. Low-signal warnings were removed intentionally — if you're seeing this comment, the finding is worth inspecting. |
Summary
gateway.terminal_backendconfig key that overridesterminal.backendfor all gateway (Telegram, Discord, etc.) sessionsexecute_code— whenTERMINAL_ENV=docker, the Python subprocess runs inside the container via a UDS socket bind-mountgateway.sandbox_imageandgateway.sandbox_lifetimeconfig keysMotivation
Closes NousResearch#4281.
The dangerous-command approval system explicitly skips approval checks when a container backend is active (the container is the security boundary). This means running the gateway with
terminal.backend: localgives no sandbox isolation AND no approval gate. This PR makes it easy to enforce Docker sandboxing for all gateway sessions without affecting local CLI usage.Changes
gateway/sandbox_config.py(new): helpers —apply_gateway_backend_to_env(),should_warn_insecure_gateway(),get_gateway_terminal_backend()tools/execute_code_docker.py(new): runs theexecute_codechild subprocess inside Docker with UDS socket bind-mounted for tool RPCgateway/run.py: apply gateway backend override at startup; emit warning when running with local backendhermes_cli/config.py: addgateway.terminal_backend,gateway.sandbox_image,gateway.sandbox_lifetimedefaultshermes_cli/status.py: show gateway sandbox backend + image in/statusoutputtests/gateway/test_sandbox_config.py(new): 12 unit tests for sandbox config helperstests/tools/test_execute_code_docker.py(new): 14 unit tests for Docker subprocess wrapper and routing logicTest Plan
pytest tests/tools/test_execute_code_docker.py tests/gateway/test_sandbox_config.py)TERMINAL_ENV=dockerenv varUsage
Notes for Reviewers
execute_code_docker.pywrapper uses--network=hostso the UDS RPC socket path is identical inside and outside the containerTERMINAL_ENV=dockerANDTERMINAL_DOCKER_IMAGEare set — so local CLI sessions are never affectedgateway.sandbox_lifetimekey is stored in config but lifetime enforcement (container cleanup) is left for a follow-up (the per-session task_id already provides isolation via existing DockerEnvironment)