fix(signal): drop empty metadata events and surface reactions - #4
Merged
Conversation
Signal metadata events (expiration timer, profile key, group updates, remote deletes) arrive as dataMessage envelopes with no message text. Previously these passed through the entire pipeline as empty-text MessageEvents, spinning up agent sessions that produced confused 'empty message' replies. - Handle reactions: convert dataMessage.reaction to [reacted <emoji>] text; silently drop reaction removals (isRemove) - Guard: after all text/attachment extraction, drop events with no text and no media; log dataMessage keys at INFO for diagnostics Patch note: ~/.hermes/plans/runtime-patches/signal-empty-message-guard.md
🚨 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. |
exiao
pushed a commit
that referenced
this pull request
May 14, 2026
…rch#25071) * tui: make URLs clickable + hover-highlight in any terminal Problem ------- URLs printed by `hermes --tui` were not clickable in basic macOS Terminal.app. Cmd+click did nothing, the cursor didn't change shape — like nothing was detected — even though arrow buttons and other Box onClick handlers worked fine. Root cause ---------- Two layers of dead plumbing: 1. `<Link>` only emitted the underlying `<ink-link>` (which carries the hyperlink metadata into the screen buffer) when `supportsHyperlinks()` said yes. On Apple_Terminal that's false, so the per-cell hyperlink field stayed empty, so `Ink.getHyperlinkAt()` had nothing to return on click. The visible underline was just decorative. 2. `Ink.openHyperlink()` calls `this.onHyperlinkClick?.(url)`, but `onHyperlinkClick` was never assigned anywhere in the codebase. The click pipeline (`App.tsx → onOpenHyperlink → Ink.openHyperlink`) ran but bailed silently on the optional chain. Bonus discovery: even when wired up, there was no hover affordance — terminal apps can't change the system mouse cursor, so users had no visual signal that a cell was clickable. Arrow buttons in the chrome worked because they had explicit `<Box onClick>` styling; inline link URLs didn't. Fix --- - `Link.tsx`: always emit `<ink-link>` regardless of terminal capability. The renderer's `wrapWithOsc8Link` already gates the actual OSC 8 escape on `supportsHyperlinks()` further down — so terminals that don't understand OSC 8 still don't see the escape, but the screen-buffer metadata (which the click dispatcher reads) is now populated everywhere. - `ink.tsx + root.ts`: add `onHyperlinkClick?: (url: string) => void` to `Options` / `RenderOptions`, wire it to the existing `Ink.onHyperlinkClick` field in the constructor. - `src/lib/openExternalUrl.ts`: small platform-aware opener using `child_process.spawn` with arg-array (no shell) — http(s) only, rejects `file:`, `javascript:`, `data:`, etc., so a hostile model can't trigger arbitrary local handlers via `<Link url="file:///...">`. Detached + stdio ignore so closing the TUI doesn't kill the browser and Chrome stderr doesn't leak into the alt screen. - `entry.tsx`: pass `onHyperlinkClick: openExternalUrl` to `ink.render`. - `hyperlinkHover.ts` + Ink hover wiring: track the URL under the pointer in `Ink.hoveredHyperlink`, update it from `dispatchHover`, and inverse- highlight every cell of the matching link in the render-pass overlay (same pattern as `applySearchHighlight`). This is the cursor-hover affordance for clickable links — terminals don't expose cursor shape, so we light up the link itself. - `types/hermes-ink.d.ts`: add `onHyperlinkClick` to the `RenderOptions` shim so consumers (`entry.tsx`) type-check against the new option. Tests ----- - `src/lib/openExternalUrl.test.ts` (15 cases): http(s) accepted; file/js/ data/mailto/ftp/ssh rejected; macOS open(1), Windows cmd.exe start with empty title slot, Linux xdg-open dispatch; shell-metacharacter URLs pass through unmolested as a single argv element; synchronous spawn failure returns false. Verified empirically in Apple Terminal 455.1 (macOS 15.7.3): clicking a URL opens in default browser, hovering inverts the link cells, and moving away clears the highlight. Full TUI suite: 713 passing, 0 type errors. Reverts ------- The earlier attempt that version-gated Apple_Terminal in `supports-hyperlinks.ts` was based on a wrong assumption — Terminal.app silently strips OSC 8 sequences but does not render them as clickable hyperlinks. Reverted to the original allowlist. * tui: address Copilot review — explorer.exe on win32 + comment fixes - openExternalUrl: switch win32 from `cmd.exe /c start` to `explorer.exe`. cmd.exe's `start` builtin reparses the URL through cmd's tokenizer, so `&`, `|`, `^`, `<`, `>` either split the command or get reinterpreted — breaking both the protocol-allowlist safety story AND plain http(s) URLs with `&` in query strings. `explorer.exe <url>` invokes the registered protocol handler directly with no shell. - openExternalUrl.test.ts: rename the win32 test to reflect the new contract and add two regression tests — one with `&|^<>` metachars, one with the common analytics-URL `&` query-param pattern — both pinned to single-argv-element delivery via explorer.exe. - Link.tsx: fix misleading comment. OSC 8 escapes are emitted unconditionally by the renderer (`wrapWithOsc8Link` in render-node-to-output.ts, `oscLink` in log-update.ts). Non-supporting terminals silently strip the sequence, which is why hover/click affordance has to come from the in-process overlay rather than the terminal's own link rendering. Verified: 715/715 tests pass, type-check + build clean. * tui: address Copilot review #2 — async spawn errors + hover scope + docs 1. openExternalUrl: attach a no-op `'error'` listener on the spawned child BEFORE unref(). spawn() returns a ChildProcess synchronously even when the binary is missing (ENOENT on xdg-open / explorer.exe), unreachable, or otherwise unusable; the failure surfaces later as an 'error' event. An unhandled 'error' on an EventEmitter crashes Node, which would tear down the whole TUI. The listener is a deliberate no-op — we already returned `true` synchronously and the user just doesn't see the browser pop. 2. openExternalUrl.test.ts: add a regression test using a real EventEmitter to simulate the async-error path. Pins both the listener-attached contract and the "doesn't throw on emit" behavior. Was 17/17, now 18/18. 3. ink.tsx dispatchHover: bypass `getHyperlinkAt()` and read `cellAt(...).hyperlink` directly. `getHyperlinkAt` falls back to `findPlainTextUrlAt` for cells without an OSC 8 hyperlink, but the render-pass overlay (`applyHyperlinkHoverHighlight`) only matches on `cell.hyperlink === hoveredUrl` — so plain-text URLs would burn re-renders without ever producing the highlight. Hover is now a strictly 1:1 fit for what the overlay can paint. Plain-text URLs still get the click action via the existing dispatch path. 4. root.ts + ink.tsx doc comments: replace the misleading "typically `open` / `xdg-open` / `start` shell" wording with the actual safe recipe — argv-array spawn into `open` / `xdg-open` / `explorer.exe`, with an explicit warning that `cmd.exe /c start` reparses the URL through cmd's tokenizer and is unsafe + breaks `&`-query URLs. Verified: 716/716 tests pass, type-check + build clean. * tui: address Copilot review #3 — hover damage, alt-screen cleanup, opener allowlist 1. ink.tsx onRender: stop folding steady-state hover into hlActive. hlActive forces a full-screen damage diff so previous-frame inverted cells get re-emitted when the highlight set changes. The transition IS the trigger — enter / leave / change-to-other-link. While the pointer just sits on a link the painted cells don't change and the per-cell diff handles the no-op. Folding the steady state in would burn a full-screen diff on every frame. Added a lastRenderedHoveredHyperlink tracker and gate the hlActive bump on `hovered !== lastRendered`. 2. ink.tsx setAltScreenActive: clear hoveredHyperlink (and the tracker) when toggling alt-screen state. Hover dispatch is alt-screen-gated, so once we leave there's no path to clear it. Without this, remounting <AlternateScreen> would paint a phantom hover from the previous session until the next mouse-move arrived. 3. openExternalUrl.ts openCommand: allowlist linux + the BSD family for xdg-open and return null for everything else (aix, sunos, cygwin, haiku, etc.). Previously the default-fallback always returned xdg-open, which made the caller's `if (!command) return false` dead and yielded a misleading `true` on platforms that probably don't have xdg-open. New tests cover the null path AND the openExternalUrl-returns-false-without-spawning behavior. Verified: 718/718 tests pass, type-check + build clean. * tui: address Copilot review #4 — doc comment accuracy 1. openExternalUrl return-value doc: now lists all three false paths (URL rejected / no opener for platform / synchronous spawn throw) plus a note that async 'error' events still return true because the spawn was attempted. 2. ink.tsx onHyperlinkClick field doc: clarifies the callback receives either an OSC 8 hyperlink OR a plain-text URL detected by findPlainTextUrlAt — App.tsx routes both into the same callback. 3. hyperlinkHover applyHyperlinkHoverHighlight doc: drops the misleading 'caller forces full-frame damage' promise. Caller decides; for hover the current caller only forces full damage on transitions. No behavior change. 718/718 tests pass. * tui: address Copilot review #5 — lint fixes 1. ink.tsx: reorder `./hyperlinkHover.js` import before `./screen.js` to satisfy perfectionist/sort-imports. 2. Link.tsx: drop unused `fallback` parameter destructuring + the trailing `void (null as ...)` dead-statement (would trip no-unused-expressions). Kept `fallback?: ReactNode` on the Props interface as a documented compat shim so existing call sites still compile, with a comment explaining why it's no longer wired up. 3. openExternalUrl.test.ts: replace `typeof import('node:child_process').spawn` inline annotations (forbidden by @typescript-eslint/consistent-type-imports) with a `SpawnLike` type alias backed by a real `import type { spawn as SpawnFn }`. No behavior change. 718/718 tests pass, type-check clean, lint clean on all modified files.
exiao
pushed a commit
that referenced
this pull request
May 14, 2026
…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 #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.
exiao
pushed a commit
that referenced
this pull request
May 27, 2026
…y prefixed Companion to the NousResearchGH-25255 incoming-strip fix from @hayka-pacha. Without this, build_anthropic_kwargs unconditionally added 'mcp_' to every tool name in step 3, so a native MCP server tool registered as 'mcp_composio_X' was sent as 'mcp_mcp_composio_X' on the wire. The incoming strip only removes ONE prefix, which still worked on first call, but on subsequent calls the model pattern-matched the single-prefixed form from message history and produced names that stripped to 'composio_X' — registry miss, dispatch fail. The history-rewrite block (#4) already has this guard. Apply the same guard to the schema-rewrite block (#3) so round-trip is symmetric. Added 4 outgoing-side tests. Existing 7 incoming-side tests still pass. Author map: hayka-pacha added for PR NousResearch#25270 salvage attribution. Refs NousResearchGH-25255.
exiao
pushed a commit
that referenced
this pull request
Jun 3, 2026
…NousResearch#34192) (NousResearch#34382) NousResearch#34192 reports Hostinger's 'Hermes WebUI' catalog crashes on startup with: /usr/bin/tini: No such file or directory The image moved from tini to s6-overlay as PID 1 (/init) earlier in 2026. Orchestration templates that still pin /usr/bin/tini as the entrypoint \u2014 like the Hostinger Hermes WebUI catalog \u2014 have no binary to exec and the container crashes immediately. Hermes has no control over the Hostinger catalog template, but we can make the image backward-compatible by symlinking /usr/bin/tini -> /init during the s6-overlay install step. External wrappers that exec /usr/bin/tini will land on the same s6-overlay reaper they would have landed on if they'd used the canonical /init entrypoint. The image's own ENTRYPOINT continues to be /init verbatim \u2014 the shim is purely for legacy external wrappers, not for the image's own runtime path. Once affected catalogs are updated, the symlink can be removed. Other issues NousResearch#34192 raises that are NOT addressed by this PR: * Problem #2 (UID 1024 vs 10000 mismatch): already fixed by NousResearch#33148 (S6_KEEP_ENV=1) and NousResearch#32412 (with-contenv shebangs). The Hostinger template likely needs to update its env-var propagation. * Problem #3 (incompatible session formats): RFC for pluggable SessionDB is tracked in NousResearch#23717. * Problem #4 (Telegram polling conflict): an operations problem on Hostinger's side, not in this codebase. This PR is scoped to the one issue that can be fixed inside Dockerfile: the missing /usr/bin/tini binary. Tests (3 in test_dockerfile_tini_compat_shim.py): - test_tini_compat_symlink_present Guard: the symlink line must exist in Dockerfile. - test_tini_compat_comment_explains_why The NousResearch#34192 anchor comment must be present so future readers know why the shim is there (avoid accidental removal). - test_entrypoint_still_init_not_tini Sanity check: ENTRYPOINT remains /init (s6-overlay). The shim is only for external wrappers. Refs: NousResearch#34192 Partial fix: addresses the immediate tini-binary crash. Catalog-side fixes still needed by Hostinger for the UID and session-format problems documented in the issue. Co-authored-by: Cursor <cursoragent@cursor.com>
exiao
pushed a commit
that referenced
this pull request
Jun 11, 2026
…eSessionPage (NousResearch#43487) When auto-compression rotates the session tip (old #4 → new #5), the incoming page carries the new tip but the previous list still holds the old one. The old tip's id differs from the new tip's id, so the existing id-only dedup in mergeSessionPage() preserves both as separate sidebar rows. Add lineage-level dedup: build a set of incoming lineage keys (`_lineage_root_id ?? id`) and filter survivors whose lineage key matches any incoming row. This mirrors the existing sessionPinId() logic used for pin stability. Fixes NousResearch#43483
exiao
added a commit
that referenced
this pull request
Jun 28, 2026
…ency merge, Kanban prefix) Five findings from gemini + Codex on the typed-block notification change: - P2 (Codex): required `kind` broke live worker prompts. _handle_block now rejects kind-less worker blocks, but agent/prompt_builder.py rule #4 and the two hermes_cli/goals.py kanban goal templates still told workers to call kanban_block(reason=...) with no kind, so a worker following the prompt hit "kind is required" and couldn't terminate. Updated all three to pass kind and enumerate the four kinds. - P2 (Codex): dependency blocks couldn't mention pending merges. The merge-ready reason heuristic ran before kind was considered, so a legitimate kind='dependency' block ("waiting on parent PR pending merge") was rejected instead of routing to todo for auto-resume. Scoped the redirect to kind != "dependency"; also shrinks the false-positive surface gemini flagged. Added test_block_dependency_may_mention_pending_merge. - Medium (gemini): legacy/untyped block notification was missing the "Kanban" prefix that done/gave_up/crashed/timed_out notifications use. Restored "⏸ {tag}Kanban {task_id} blocked{suffix}" and updated the assertion. Kanban + prompt_builder + goals suites green. Patch note: kanban-block-self-labeling-review-fixes.md
exiao
added a commit
that referenced
this pull request
Jun 29, 2026
…tag) (#56) * fix(kanban): self-labeling blocked-task notifications (kind + header tag) Block pushes in the Kanban Workers Signal group were a wall of identical `⏸ … blocked: <paragraph>` alerts with no way to tell "Eric must decide" from "worker hit a routing wall" from "merge-ready". Make every block typed and self-labeling end to end. - kanban_db.block_task: BLOCK_KIND_HEADERS + ensure_reason_header() normalize the stored reason / run summary / event payload to lead with ERIC DECISION: / ROUTING: / RETRY: for needs_input / capability / transient. dependency (→todo) and None (legacy/dispatcher) untouched (back-compat preserved). - tools/kanban_tools._handle_block: REQUIRE kind on the worker tool (reject an un-typed block instead of silently storing a generic one); redirect merge-ready / awaiting-merge reasons to kanban_complete (constitution 2a). Schema marks kind required + documents the headers. - gateway/kanban_watchers: _format_block_notification() leads the push with 🔴 ERIC DECISION / 🟠 ROUTING / 🟡 RETRY — <id>: <title>, full reason below; legacy untyped block keeps the ⏸ … blocked shape. Tests: new test_kanban_block_notify_labeling.py (drives the real notifier watcher); extended test_kanban_block_kinds.py (header normalization), test_kanban_tools.py (kind required + merge-ready redirect), and fixed two existing callers to pass kind. 145 focused tests green. Patch note: ~/.hermes/plans/hermes-patches/kanban-block-self-labeling.md Worker SOUL half (the behavioral populator) is staged for review, NOT applied: ~/.hermes/plans/hermes-patches/_soul-block-staged.diff (16 lanes). * test(cli): isolate ignore-user-config project fallback from stale repo-root config test_user_config_skipped_when_flag_set failed deterministically in CI slice 8 (passed locally). load_cli_config() falls back to the project config at Path(cli.__file__).parent/cli-config.yaml when HERMES_IGNORE_USER_CONFIG=1 skips the user config. That file is gitignored, but another test in the same slice leaves a real cli-config.yaml at the repo root and run_tests.sh's per-file subprocesses share one filesystem, so the stale file leaks in and its model.default fails the "defaults only" assertion. PR #55's file deletions reshuffled slice membership and landed victim + polluter together. Harden the victim: monkeypatch cli.__file__ into tmp_path in _reload_cli so the project fallback resolves to an empty dir. The test now uses built-in defaults regardless of any stale repo-root cli-config.yaml. Verified all 11 tests pass with a leaked file present. Patch note: test-ignore-user-config-project-fallback-isolation.md * fix(kanban): address block self-labeling review (kind prompts, dependency merge, Kanban prefix) Five findings from gemini + Codex on the typed-block notification change: - P2 (Codex): required `kind` broke live worker prompts. _handle_block now rejects kind-less worker blocks, but agent/prompt_builder.py rule #4 and the two hermes_cli/goals.py kanban goal templates still told workers to call kanban_block(reason=...) with no kind, so a worker following the prompt hit "kind is required" and couldn't terminate. Updated all three to pass kind and enumerate the four kinds. - P2 (Codex): dependency blocks couldn't mention pending merges. The merge-ready reason heuristic ran before kind was considered, so a legitimate kind='dependency' block ("waiting on parent PR pending merge") was rejected instead of routing to todo for auto-resume. Scoped the redirect to kind != "dependency"; also shrinks the false-positive surface gemini flagged. Added test_block_dependency_may_mention_pending_merge. - Medium (gemini): legacy/untyped block notification was missing the "Kanban" prefix that done/gave_up/crashed/timed_out notifications use. Restored "⏸ {tag}Kanban {task_id} blocked{suffix}" and updated the assertion. Kanban + prompt_builder + goals suites green. Patch note: kanban-block-self-labeling-review-fixes.md * fix(kanban): guard merge-ready heuristic against negation (P2 thread) * refactor(kanban): generic 'DECISION NEEDED' block header (decouple from a named user) Rename the needs_input block-kind header from 'ERIC DECISION' to the name-free 'DECISION NEEDED' so the self-labeling notification doesn't hardcode a single user. Pure label rename: kinds, routing, and idempotent header-stamping logic are unchanged. Updates the constant, its doc/comment echoes, and the test assertions/fixtures that pinned the literal. Patch note: ~/.hermes/plans/hermes-patches/kanban-block-self-labeling.md * docs(prompt): add required kind to remaining kanban_block examples (Codex P2) The required-kind guard rejects kanban_block(reason=...) with no kind, but two worker-guidance examples still omitted it: the review-required exception and the headless-clarify path. Following either literally would now return 'kind is required' and leave the task running instead of surfacing the blocker. Both are human-decision cases → kind="needs_input". * fix(kanban): phrase-anchored merge-ready redirect + review-required marker (Codex P2 x3) Three issues with the merge-ready→complete redirect heuristic: - review-required handoffs (the wording the worker prompts mandate) were not in the marker set, so a finished-code block bypassed the redirect and still parked in blocked. Add review-required / review required markers. - the global 'any negation anywhere' check mis-fired both ways: it suppressed a real affirmative redirect on an unrelated 'no blockers remain', and was the only thing stopping a negated 'not ready to merge' real blocker. Replace with a regex that fires only when a negator directly qualifies a merge-ready phrase (not <=2 words> <marker>). Adds fail-before/pass-after tests for all three cases. * fix(kanban): route review-required to kanban_complete in prompt + docs (Codex P2 x2) The merge-ready redirect now treats review-required as a complete-not-block marker, but the worker prompt still told workers to kanban_block(review-required) — a self-contradiction that gets rejected. Point the prompt at kanban_complete instead, and update the public kanban docs (required kind on kanban_block, and the kanban_block lifecycle examples) so docs-following agents don't hit the new kind-required error. * rework(kanban): infer block-notification header from reason, drop required kind Collapse PR #56 to a notification-only feature. The 'kind' machinery is upstream (optional); only this PR had made it required + added headers + a merge-redirect, which is too much agent-facing contract to maintain. Revert every agent-facing change to upstream/live-config shape: kanban_block kind is optional again, no required constraint, no merge-ready redirect, no reason header-stamping, prompt/goals/docs/SOULs untouched. Move the self-labeling header entirely into the gateway notifier: _infer_block_header() classifies the free-text reason (routing / retry / decision-needed, default decision-needed) so the Signal push triages at a glance with zero agent contract. An explicit optional kind, if passed, still overrides inference. Misclassification is cosmetic: full reason rides in the body, default is the act-on-it tag, so the worst case is glancing at one extra alert, never a dropped one. Patch note: ~/.hermes/plans/hermes-patches/kanban-block-inferred-header.md * test(kanban): align untruncate block test with self-labeling header The notification-only rework changed the blocked-event push from the legacy '⏸ … blocked' shape to a self-labeling header inferred from the reason text. test_blocked_reason_is_not_clipped_at_160 still asserted the literal 'blocked' substring; a reason with no routing/retry hints now leads with '🔴 DECISION NEEDED'. Assert the header instead — the untruncation intent (full reason survives, tail not clipped) is unchanged. * fix(kanban): scope HTTP-status block hints to error context (Codex P2) Bare 403/429/503/502 substrings in the routing/retry hint lists also matched an unrelated ticket reference like 'merge PR NousResearch#403?', wrongly downgrading a human-decision block to ROUTING/RETRY. The default header must stay DECISION NEEDED. Split the numeric codes into _ROUTING_STATUS_CODES / _RETRY_STATUS_CODES and match via _has_status_code(): strip issue/PR/ticket refs first, then require a standalone \bNNN\b token. 'returned 403' still classifies as routing; 'merge PR NousResearch#403?' stays DECISION NEEDED. Patch note: ~/.hermes/plans/hermes-patches/kanban-block-inferred-header.md New test: test_infer_issue_number_is_not_a_status_code * fix(kanban): decision wording wins over access/status hints in block classifier (Codex P2 x4) Tighten _infer_block_header so the at-a-glance header signal isn't mislabeled by incidental substrings: - Human CHOICE/question wording wins first (a '?' or decision phrasing), so 'Should I retry the failed migration or roll back?' and 'I can't reach a decision ... without input' stay 🔴 DECISION NEEDED instead of being downgraded to 🟡 RETRY / 🟠 ROUTING by a bare retry/can't-reach substring. - Status-code matching now requires explicit HTTP/error context, so a non-HTTP id (HERMES-429, ...NousResearch/issues/429) no longer triggers RETRY. - Retry/transient evidence is checked before access→routing, so a reason with both ('no access right now: 429 rate limit') is bucketed 🟡 RETRY. Notification-labeling only; misclassification stays cosmetic (full reason always rides in the body, default is the act-on-it tag). Regression tests added per case (red-before/green-after). * fix(kanban): model block-classifier precedence explicitly (Codex P2 x3) Enumerate the input space and restructure _infer_block_header as an explicit 4-rung ladder instead of an order-dependent substring race: 1. human CHOICE/question wording -> DECISION NEEDED 2. POSITIVE (non-negated) transient evid -> RETRY 3. hard access/lane wall -> ROUTING 4. default -> DECISION NEEDED Three Codex findings, one pass: - Hyphenated HTTP markers (HTTP-429, status-503) were eaten whole by the issue-ref strip before _has_status_code saw the number, so a real transient fell through to DECISION. A negative lookahead exempts http/https/status/ code/error- prefixes so the numeric code survives. - Negated retry (do not retry / don't try again) is an operator routing instruction, not a transient signal. New _NEGATED_RETRY_RE strips negated retry phrases before the retry-hint scan (_has_positive_retry_evidence), so a credential block defers to its hard access evidence -> ROUTING. - Bare "which " dropped from _DECISION_HINTS so ordinary relative clauses ("The deploy API, which returned 429, ... try again later") classify by their retry/status evidence -> RETRY; genuine "which ... ?" questions still hit the trailing-? path -> DECISION. Notification-labeling only; misclassification stays cosmetic (full reason always rides in the body, default is the act-on-it tag). Regression test per finding (red-before/green-after); existing classifier + notify/untruncate/mixin consumers (32 tests) stay green; ruff clean. Patch note: ~/.hermes/plans/hermes-patches/kanban-block-inferred-header.md * fix(kanban): complete negation + status-code input space (Codex P2 x2) Round-2 same-class gaps Codex flagged on the new head: - Modal/adjective negations: _NEGATED_RETRY_RE only matched direct "do not retry"/"don't try again". Broadened to modal negations (should/must/can/will/would + n't, plus contractions) and an optional qualifier span (not safe/ok/supposed to retry), plus bare standalone "not". So "should not retry until provisioned" / "not safe to retry until credentials exist" / "must not try again" now defer to access evidence -> ROUTING. - Common status codes: added 401 -> ROUTING (unauthorized = access wall) and 408/504 -> RETRY (request/gateway timeout = transient), so "API returned 401" / "HTTP 504 from gateway" classify without the worker also spelling out unauthorized/timeout. Codes still require HTTP/error context, so a bare "Finished 401 of the rows" stays DECISION. New tests: test_infer_modal_negated_retry_defers_to_access_evidence, test_infer_common_status_codes_bucket_correctly. 23 classifier tests + the notify/untruncate/mixin consumers (34 total) stay green; ruff clean. Patch note: ~/.hermes/plans/hermes-patches/kanban-block-inferred-header.md * fix(kanban): single-source HTTP-context words to kill prefix drift (Codex P2 x4) Codex surfaced one new substring edge case per round; root cause was two regexes disagreeing about "what reads as HTTP": _HTTP_CONTEXT_RE accepted api/gateway/server/request while the issue-ref hyphen-exemption only protected http/status/code/error, so api-429/gateway-504 got context from one regex but were stripped by the other. Collapse that: one _HTTP_CONTEXT_WORDS tuple now drives BOTH _HTTP_CONTEXT_RE and the <word>-NNN exemption in _ISSUE_REF_RE. Four round-3 findings, one structural pass: - pull-URL refs (a PR URL ending NousResearch/pull/429): added `pull` to the issue-ref keyword arm so a PR-review handoff is not read as a status code -> DECISION. - Hyphenated api-429/gateway-504 now survive the strip (exemption derives from _HTTP_CONTEXT_WORDS) -> RETRY. - Negated transient class: _NEGATED_RETRY_RE -> _NEGATED_TRANSIENT_RE scrubs negated forms of the WHOLE transient vocabulary (retry/try again/transient/ temporary/flaky/clear), so a "not transient -- missing API key" / "won't clear on its own" reason -> ROUTING. Helper renamed _has_positive_retry_evidence -> _has_positive_transient_evidence. - decide:/decide,/to decide added to _DECISION_HINTS so "Need Eric to decide: retry or revert" -> DECISION. New tests: test_infer_pull_url_number_is_not_a_status_code, test_infer_negated_transient_word_defers_to_access, test_infer_decide_colon_is_a_decision, test_infer_hyphenated_api_gateway_status_marker_is_transient. 27 classifier tests + notify/untruncate/mixin consumers (38 total) stay green; ruff clean. Patch note: ~/.hermes/plans/hermes-patches/kanban-block-inferred-header.md * fix(kanban): strip whole URLs + cover determiner-negated transient (Codex P2 x2) Round-4 escapes, each closed at its root: - Numeric URL path ids: a code like an actions-run URL ending /runs/504 (any URL, not just /pull or /issues) was read as a status code because the URL's https scheme satisfied the whole-message HTTP-context gate. New _URL_RE strips whole http(s):// tokens in _has_status_code BEFORE the context check and code scan, so a pure "review this <url>" handoff has no remaining context -> DECISION. Only inline status wording (API returned 504) counts. Generalizes past per-path-keyword arms. - Determiner/adjective negation: "not a retry issue", "not a transient issue", "not retryable" slipped past _NEGATED_TRANSIENT_RE (determiner between negator and token; adjective form). Added an optional (a|an|the|any) determiner span and retryable/recoverable adjective forms, so those access blocks defer to routing -> ROUTING. A POSITIVE "retryable 503" still classifies -> RETRY. New tests: test_infer_numeric_url_path_id_is_not_a_status_code, test_infer_determiner_negated_transient_defers_to_access. 29 classifier tests + notify/untruncate/mixin consumers (40 total) stay green; ruff clean. Patch note: ~/.hermes/plans/hermes-patches/kanban-block-inferred-header.md --------- Co-authored-by: testuser <testuser@erics-air.mynetworksettings.com>
exiao
added a commit
that referenced
this pull request
Jul 13, 2026
Codex second P1 on PR #99 (source-#3 gap): dropping creds to None under a secret scope did not actually suppress the global Claude Code fallback. A scoped profile with only ANTHROPIC_API_KEY (or no Anthropic secret) still reached _resolve_claude_code_token_from_credentials(None), which re-reads the global ~/.claude file and returns the DEFAULT profile's token before the scoped API-key fallback — authenticating /usage and Anthropic calls as the wrong profile. Skip source #3 entirely while a secret scope is active so only the profile's own scoped secrets (sources #1/#2/#4/#5) resolve. Adds a regression test for a scoped ANTHROPIC_API_KEY with a refreshable global Claude Code cred present (fails without the guard: returns GLOBAL-DEFAULT; passes with it).
exiao
added a commit
that referenced
this pull request
Jul 14, 2026
… key On the profile_only pool path, an explicit ANTHROPIC_API_KEY only dropped the hermes_pkce entry, so an auto-seeded env:ANTHROPIC_TOKEN / env:CLAUDE_CODE_OAUTH_TOKEN OAuth singleton left in the profile auth.json could still be returned by source #4 — reviving the Claude Code OAuth masquerade that picking the API-key path explicitly opts out of. Match load_pool()'s api_key_path_explicit pruning: require the API key be set AND no OAuth env tokens present, then drop every auto-seeded OAuth entry (env:* sources + hermes_pkce + claude_code) while keeping manual pool entries and the api_key entry. Regression test seeds a stale env:ANTHROPIC_TOKEN OAuth entry + a manual entry in a non-default profile pool with a scoped API key and asserts the stale env OAuth token is not what resolves (red before: it was returned).
exiao
added a commit
that referenced
this pull request
Jul 14, 2026
…ex scope The pool-consult gate skipped _resolve_anthropic_pool_token whenever a scoped ANTHROPIC_API_KEY was present and the scope was NOT a non-default profile. Under multiplexing the DEFAULT profile also runs inside a secret scope and owns ~/.hermes/auth.json, so a manually-added OAuth entry there lost its documented source-#4-before-#5 precedence and /usage + Anthropic calls resolved the API key instead of the configured OAuth credential. Consult the pool for any multiplex scope (default included) while keeping a non-multiplex single-profile cron scope's API key authoritative (no borrowed global pool token). Regression test added (red before/green after).
exiao
added a commit
that referenced
this pull request
Jul 14, 2026
…ecret scope (#99) * fix(anthropic): scope resolve_anthropic_token to active profile secret scope Under gateway profile multiplexing, /usage for a secondary Anthropic profile could show the DEFAULT profile's limits. resolve_anthropic_token read ANTHROPIC_TOKEN / CLAUDE_CODE_OAUTH_TOKEN / ANTHROPIC_API_KEY from os.getenv directly, never via agent.secret_scope.get_secret, so even with the scope contextvar propagated into the worker thread the token still resolved to the process/default value. Route the three reads through get_secret(..., '') so the active _SECRET_SCOPE wins under multiplexing; with no scope / multiplex-off get_secret transparently reads os.environ (single-profile deployments and non-gateway callers stay byte-identical). run_oauth_setup_token's os.getenv reads are left alone (interactive setup, not a per-turn multiplexed path). Regression test mirrors tests/gateway/test_credits_usage_profile_scope.py: scoped profile-B token wins, no-scope falls back to os.environ. Verified fail-before/pass-after. Patch note: ~/.hermes/plans/hermes-patches/anthropic-token-secret-scope.md Task: t_601f23d1 (follow-up to PR #98 Codex Anthropic P1) * fix(anthropic): don't let global Claude creds override scoped token under multiplexing Codex P1 on PR #99: resolve_anthropic_token reads the host's global ~/.claude / Keychain credentials, then _prefer_refreshable_claude_code_token could return that DEFAULT-profile refreshable credential in place of the requesting profile's scoped ANTHROPIC_TOKEN (or resolve it as source #3), authenticating /usage and Anthropic calls as the wrong profile. Under an active secret scope the global cred record is not the requesting profile's, so drop it to None; only the profile's own scoped secrets resolve. Adds a regression test with refreshable global creds present (the prior test stubbed creds to None and missed this). * fix(anthropic): skip global Claude Code source under active secret scope Codex second P1 on PR #99 (source-#3 gap): dropping creds to None under a secret scope did not actually suppress the global Claude Code fallback. A scoped profile with only ANTHROPIC_API_KEY (or no Anthropic secret) still reached _resolve_claude_code_token_from_credentials(None), which re-reads the global ~/.claude file and returns the DEFAULT profile's token before the scoped API-key fallback — authenticating /usage and Anthropic calls as the wrong profile. Skip source #3 entirely while a secret scope is active so only the profile's own scoped secrets (sources #1/#2/#4/#5) resolve. Adds a regression test for a scoped ANTHROPIC_API_KEY with a refreshable global Claude Code cred present (fails without the guard: returns GLOBAL-DEFAULT; passes with it). * fix(anthropic): scope global-cred suppression to non-default profiles Codex P2 on PR #99: under gateway.multiplex_profiles the DEFAULT profile's own turns also run inside _profile_runtime_scope, so current_secret_scope() is non-None even for the profile that owns the global ~/.claude / Keychain credential. The prior guard suppressed the global Claude Code source for ANY active scope, breaking Anthropic auth and /usage for a default profile that relies on Claude Code OAuth (rather than .env/pool creds). Gate the suppression on _scope_is_non_default_profile(): only a non-default scoped profile drops the global creds and skips the source-#3 fallback; the default profile (owner of ~/.claude) keeps resolving it. get_active_profile_name reads the per-turn HERMES_HOME override, so it reflects the active profile. Updates the two prior regression tests to a non-default profile scope and adds a default-profile regression (fails with the old scope-active gate: returns None; passes now: resolves the global Claude Code credential). * fix: isolate scoped Anthropic pool credentials * fix: honor suppressed profile OAuth credentials * fix: preserve scoped Anthropic credential priority * fix: preserve local OAuth pool precedence * fix: scope usage lookups without breaking cron * fix: preserve default env token under scoped usage * fix(anthropic): prune stale env OAuth pool entries under explicit API key On the profile_only pool path, an explicit ANTHROPIC_API_KEY only dropped the hermes_pkce entry, so an auto-seeded env:ANTHROPIC_TOKEN / env:CLAUDE_CODE_OAUTH_TOKEN OAuth singleton left in the profile auth.json could still be returned by source #4 — reviving the Claude Code OAuth masquerade that picking the API-key path explicitly opts out of. Match load_pool()'s api_key_path_explicit pruning: require the API key be set AND no OAuth env tokens present, then drop every auto-seeded OAuth entry (env:* sources + hermes_pkce + claude_code) while keeping manual pool entries and the api_key entry. Regression test seeds a stale env:ANTHROPIC_TOKEN OAuth entry + a manual entry in a non-default profile pool with a scoped API key and asserts the stale env OAuth token is not what resolves (red before: it was returned). * fix(anthropic): honor a blank scoped Anthropic slot as an explicit clear _read_anthropic_secret tested scope values for stripped truthiness, so a profile that intentionally disabled a path by writing a blank slot to its .env (ANTHROPIC_TOKEN= / ANTHROPIC_API_KEY=, as the setup helpers do) looked like a scope with no Anthropic secret. The default-profile fallback then read os.environ, letting a service-level ANTHROPIC_TOKEN override the profile's explicit clear. Treat any Anthropic slot PRESENT in the scope (blank or not) as the profile having spoken and suppress the process-env fallback. The empty-scope ({}, no slot declared) case still falls back, which is the legitimate default-profile-from-service-env path. Regression test: a default-profile scope with a blank ANTHROPIC_TOKEN slot must resolve to None, not the service-env token (red before). * fix(anthropic): keep manual pool OAuth precedence for default multiplex scope The pool-consult gate skipped _resolve_anthropic_pool_token whenever a scoped ANTHROPIC_API_KEY was present and the scope was NOT a non-default profile. Under multiplexing the DEFAULT profile also runs inside a secret scope and owns ~/.hermes/auth.json, so a manually-added OAuth entry there lost its documented source-#4-before-#5 precedence and /usage + Anthropic calls resolved the API key instead of the configured OAuth credential. Consult the pool for any multiplex scope (default included) while keeping a non-multiplex single-profile cron scope's API key authoritative (no borrowed global pool token). Regression test added (red before/green after). * fix(anthropic): normalize pool priorities on profile-only reads The profile_only pool path builds CredentialPool directly (to skip load_pool's global seeders on the isolated profile) but also skipped _normalize_pool_priorities, so a manually-added OAuth entry that hermes auth add appended with a larger priority number sorted behind a seeded singleton (env:ANTHROPIC_TOKEN / hermes_pkce) and the seeded token resolved first. Apply the same manual-over-seeded normalization load_pool runs before constructing the pool, restoring manual precedence. Regression test: a profile pool with a seeded OAuth (priority 0) and a manual OAuth (priority 1) must resolve to the manual token (red before: seeded won).
exiao
added a commit
that referenced
this pull request
Jul 15, 2026
… during seeding Addresses codex review on #116: _seed_from_singletons built its (source, creds) list by eagerly calling read_claude_code_credentials() before the _is_suppressed() check, so pool seeding (reached via resolve_anthropic_token source #4 -> load_pool) still touched ~/.claude / the macOS Keychain despite the suppression marker. Pair each source with a lazy reader and check suppression first, so a suppressed claude_code source is never read. Regression: red-before/green-after on _seed_from_singletons with a booby-trapped reader.
exiao
added a commit
that referenced
this pull request
Jul 15, 2026
…ion (#116) * fix(anthropic): honor claude_code source suppression in token resolution Follow-up to #115 (codex P2). resolve_anthropic_token()'s Claude-file read (source #3) ignored an explicit user suppression of the claude_code source: hermes auth remove anthropic leaves ~/.claude/.credentials.json in place (Claude Code owns it) and records a suppression marker, but the resolver still read the file and probed Anthropic with the removed credential. Gate the global-cred read on is_source_suppressed(anthropic, claude_code) for every profile including the default. Red-before/green-after regression added. * fix(anthropic): gate claude-file read before I/O; keep suppression out of pool isolation Addresses codex review on #116: - Check is_source_suppressed BEFORE read_claude_code_credentials so a suppressed claude_code source never touches the Claude file or the macOS Keychain, and gate source #3 on it too (creds=None alone lets that resolver re-read the global file). - Do NOT fold claude_code suppression into suppress_global_creds: that flag also drives profile_only= on the pool lookup, so folding it in wrongly bypassed the pool's global-root fallback for a named profile that suppressed only the Claude-file source but has a valid inherited/manual OAuth pool entry. Suppression now affects only the Claude-file path. Added a regression asserting the file is never read when suppressed. * fix(credential-pool): check claude_code suppression before reading it during seeding Addresses codex review on #116: _seed_from_singletons built its (source, creds) list by eagerly calling read_claude_code_credentials() before the _is_suppressed() check, so pool seeding (reached via resolve_anthropic_token source #4 -> load_pool) still touched ~/.claude / the macOS Keychain despite the suppression marker. Pair each source with a lazy reader and check suppression first, so a suppressed claude_code source is never read. Regression: red-before/green-after on _seed_from_singletons with a booby-trapped reader.
exiao
added a commit
that referenced
this pull request
Jul 22, 2026
The Modal worker hardcoded ~/.hermes/profiles/memo-evaluator and read a HERMES_MODAL_MEMO_EVALUATOR_PROFILE env var to override it. That breaks custom/Docker/profile HERMES_HOME layouts (AGENTS.md rule #9) and uses a HERMES_* env var for non-secret path config (rule #4). Resolve the profile via get_profile_dir('memo-evaluator') inside the modal.is_local() guard, which anchors to the profiles root and is HERMES_HOME-aware. Drops the env var entirely. Addresses claude[bot] CHANGES_REQUESTED (profile-safety, blocking).
exiao
pushed a commit
that referenced
this pull request
Aug 3, 2026
…e-review #4) The compression heartbeat's terminal 'context compression completed' stamp force-persists against the PARENT session id (agent.session_id at stamp time). After the out-of-place rotation the parent is archived but kept advertising a fresh last_activity_at + terminal label forever. Clear the parent row's activity labels best-effort after a committed rotation (keeps last_activity_at so idle clocks stay continuous; the child carries live labels). Regression asserts the archived parent's labels are cleared while the child's lineage is intact (sabotage-verified).
exiao
pushed a commit
that referenced
this pull request
Aug 11, 2026
…on delegation callbacks (NousResearch#82592) * fix(gateway): stop frozen-preview finals and dropped idle-session delegation callbacks Two relay-plane delivery losses from the 2026-08-09 staging incident: 1. stream_consumer: the skip-redundant-finalize branch recorded _accumulated as the delivered turn-final payload even when the last ACKED edit was an earlier throttled preview snapshot, so delivered_final_matches reconciled True and the gateway suppressed the corrective final send — the user was left with a cut-off message ending in the streaming cursor. Extracted _mark_skip_redundant_finalize(): records the last acked wire payload (cursor-stripped), so a preview/final mismatch now returns False and the normal final send fires. 2. run.py: _classify_completion_target classified every ended parent session terminal unless it ended by compression. Idle/timeout session ends are the norm on scale-to-zero relay deployments and the chat route remains valid; completed async delegation results were terminally dropped. Ended parents now classify deliver unless the end was an explicit user boundary (session_reset / user_exit / session_switch). * fix(relay): drain in-flight outbound frames before transport teardown disconnect() failed every pending outbound future immediately with 'relay transport closed', so a trailing finalize edit racing turn teardown was lost even though the connector socket could still serve it. Bounded drain grace (5s) lets in-flight requests resolve; silent connectors still tear down promptly. asyncio.wait (not gather+wait_for) so a timeout doesn't cancel futures owned by the fail-remaining loop. * fix(gateway): route completion injection through the alias-aware transport resolver Third relay-plane delivery loss from the 2026-08-09 staging incidents: a delegation batch completed while the gateway was up, the watcher drained the event, and delivery vanished with no log line. _inject_watch_notification resolved its adapter with a literal p.value == platform_name scan of self.adapters — a relay-fronted gateway registers ONE adapter under Platform.RELAY fronting N logical platforms, so 'slack' never matched and the injection returned None ('no gateway route'), silently dropping the completion. The handoff path already documents this exact trap and uses resolve_delivery_transport; the injection path now does the same (native wins; relay eligible only when it fronts the logical platform), with the literal scan kept as fallback for stub runners and exotic platforms. * fix(relay): clamp disconnect drain grace to the runner's adapter-disconnect budget Review finding (JoaoMarcos44, NousResearch#82592): a fixed 5.0s drain in front of the three 1.0s sequential teardown awaits gives an 8.0s worst case inside the runner's 5.0s asyncio.wait_for(adapter.disconnect()) — tripping it cancels teardown mid-drain, skips the fail-pending loop, and leaves outbound callers blocked until _OUTBOUND_TIMEOUT_S (30s). The effective grace is now budget - 3*TEARDOWN - margin (env-aware via the same HERMES_GATEWAY_ADAPTER_DISCONNECT_TIMEOUT the runner reads), so the drain can never push teardown past its caller's budget; a budget too small for any drain disables it cleanly. * test(gateway): pin the final-send suppression contract across a behaviour matrix The gateway skips its own final send when the stream consumer claims the turn final already reached the user. Every incident in that family — NousResearch#71643 (stale finalize snapshot), NousResearch#78541 (payload-less multi-message split), NousResearch#82656 (frozen preview left with a visible cursor) — is the same failure: the consumer claimed delivery for text the platform never rendered, so the corrective send was suppressed and the answer was lost with no retry. Each was fixed with a scenario test pinned to one branch of GatewayStreamConsumer.run(). The got_done handler now has five sibling branches that each set the suppression flags and record a turn-final payload, and nothing checks them as a group: a new branch, or a new early `return True` in _send_or_edit, can reintroduce the class without failing a test. Pin the invariant instead of the branch — if the consumer offers the gateway any signal it would trust, the complete final text must have reached the wire — and assert it across {edit always / dies / never / lies} x {send always / never} x {fresh-final on / off} x {clean / interrupted stream}. The adapter records only frames that actually rendered, so an ACK the platform drops does not count as delivery. 24 honest-transport scenarios hold the invariant as a hard assertion. The 16 lying-transport scenarios are checked too; the single combination that still violates it is reported as an expected failure documenting the open exposure rather than asserting it away. Refs NousResearch#82656 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(gateway,relay): prime relay egress routing for synthetic injections + cap stale completion replay Defect #4 from the 2026-08-09 staging incidents (upgrade-robustness): after every gateway restart the durable async-delegation replay injected completions correctly (post-741663cf1) but their replies bounced at the connector — 'slack egress declined: target not routed to an onboarded tenant'. The relay adapter re-attaches tenant discriminators (metadata.scope_id / metadata.user_id) from per-chat caches warmed ONLY by inbound traffic; synthetic turns race those cold caches on every deploy, scale-to-zero wake, and crash recovery. - relay adapter: prime_routing_cache() — feeds a synthetic event's session-store origin through the same _capture_scope used for real inbound (never raises). - run.py injection path: prime the resolved adapter before handle_message (duck-typed; native adapters unaffected). - async_delegation: 48h staleness cap in restore_undelivered_completions — a pending completion older than the cap is terminally dropped (payload stays queryable) instead of re-run as a fresh full-context turn; the post-restart replay of a July session burned a 102K-token context. Also carried: JoaoMarcos44's suppression behaviour-matrix harness (cherry-picked from NousResearch#82676, authorship preserved) — 39 passed + 1 xfail (the documented ACK-then-drop transport-honesty residue). * test: use recent timestamps in restored-ownership fixtures test_restore_stamps_restored_flag persisted its completion with epoch-era toy timestamps (dispatched_at=1.0), which the new 48h replay staleness cap correctly classifies as stale — the fixture then exercised the cap instead of the restored-flag contract (CI slice 4 failure). Timestamps are now now-relative; the staleness behavior itself is pinned separately in test_relay_injection_egress_priming.py. * fix(gateway,relay): close four review findings on the relay delivery fixes Review follow-ups on this branch (NousResearch#82592): 1. HIGH — classifier/resolver mismatch (falsely-acknowledged loss). _classify_completion_target now returns "deliver" for idle-ended parents, but _resolve_async_delegation_session still dropped every non-compression-ended pin: the durable row was acked at adapter acceptance, then the injection died inside the pipeline with no retry — strictly worse than the honest terminal drop on main, and the delivery leg defect #2's fix depends on did not exist. The resolver now retargets non-user-boundary ends (idle/timeout/ lifecycle) to the chat's current session — session_entry already IS the routing key's current session for the same chat — while user boundaries (session_reset / new_session / user_exit / session_switch) stay fail-closed. Both sides share one module-level _USER_BOUNDARY_END_REASONS so the verdict and the routing decision cannot drift again; a coherence test asserts deliver-verdicts resolve non-None across representative end reasons. 2. HIGH — drain clamp missed adapter-level spend. The effective drain grace budgeted drain + 3x teardown, but RelayAdapter.disconnect spends revocation-monitor teardown + go_idle time BEFORE the transport drain inside the same runner wait_for; worst case still blew the budget and cancelled teardown mid-drain (skipping the fail-pending loop). The adapter now measures its own elapsed time and threads the REMAINING budget into transport.disconnect(budget_s=...); legacy/stub transports without the keyword fall back to the no-arg signature. 3. P1 — _request_response racing disconnect() could register a future after the fail-pending loop already ran, stranding the caller for the full _OUTBOUND_TIMEOUT_S (30s). Fail fast with the same "relay transport closed" error once _closing is set. 4. P1 — _build_process_event_source's last-resort reconstruction dropped scope_id, so a scoped relay completion whose session-store origin was unavailable primed no tenant discriminator and could still bounce off the connector's fail-closed egress guard. scope_id now threads through the reconstructed SessionSource, with a warning when a scoped chat reconstructs without one. All four: RED reproduced with the fix reverted, GREEN after; relay/ delegation delivery families pass (43 + 71 + 179 across the touched suites); full tests/gateway run shows only failures already failing identically on merge base 2446c8b (env/dep issues). * fix(gateway,relay): make pending-frame failure cancellation-safe; persist completion routing origin Two remaining review findings on this branch (NousResearch#82592): 1. Cancellation could strand outbound waiters past the fail-pending loop. transport.disconnect() failed pending futures only at the END of the drain + three teardown awaits; a cancellation landing mid-drain (the runner's wait_for budget, an outer cleanup deadline) skipped the loop entirely and left registered futures unresolved — their callers blocked until _OUTBOUND_TIMEOUT_S (30s). The budget threading added earlier shrinks the window but is not a hard guarantee. The fail-pending loop (and the going_idle ack failure) now run in a `finally`, so no exit path — normal, error, or cancelled — can leave a registered future unresolved. Idempotent: done futures are skipped, a second disconnect() pass is a no-op. 2. Durable completions did not persist their routing origin, so the scope_id threading in the fallback SessionSource reconstruction had nothing to carry on the exact path it exists for (restart replay with session store + source cache gone): the async-delegation event producers never populated scope_id and the durable rows never stored it. Dispatch now snapshots the originating turn's scope_id/user_id/user_name from the session context (_capture_routing_origin — a new HERMES_SESSION_SCOPE_ID contextvar bound by the gateway at session-bind time alongside the existing vars), stores them in the existing task_json payload (no schema migration), and re-attaches them to all three completion-event shapes (live single, live batch, crash-recovery rebuild). The gateway's fallback reconstruction then primes both discriminators after a restart. Tests: cancellation mid-drain -> every pending future resolves with "relay transport closed" (mutation: moving the loop out of the finally goes RED); second-pass disconnect idempotence; end-to-end dispatch -> owner-death recovery -> event carries scope_id -> fallback SessionSource primes it (mutations: dropping the dispatch capture or the task_json persistence both go RED); live completion event carries the origin. 94 passed + 1 xfailed across the delivery/delegation suites; tests/tools delegation family 73 passed (2 collection errors pre-existing on merge base 2446c8b). --------- Co-authored-by: joaomarcos <joaomarcosdias444@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Ben Barclay <ben@nousresearch.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
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Signal metadata events (expiration timer changes, profile key updates, group updates, remote deletes, stickers without fallback text) arrive as
dataMessageenvelopes with nomessagefield. The adapter dispatches them asMessageEvent(text=""), which passes through the entire gateway pipeline with no guard, spins up a full agent session, and the model responds with confused "looks like you're sending empty messages!" replies.Reactions also arrive as
dataMessagewith areactionsub-object but nomessagetext, triggering the same empty-message path.Fix
Two additions to
_handle_envelope()ingateway/platforms/signal.py:Reaction handling — after text extraction, if
dataMessage.reactionis present with no text, convert to[reacted <emoji>]. Reaction removals (isRemove) are silently dropped.Empty message guard — after all text/attachment extraction, if we end up with no text AND no media, log the
dataMessagekeys at INFO (diagnostic) and return early instead of dispatching to the agent.Tests
111 passed, 0 failed (
test_signal.py+test_signal_format.py).Patch note:
~/.hermes/plans/runtime-patches/signal-empty-message-guard.md