Conversation
…#1) When a user replies in a Slack thread where the bot has an active conversation session, the bot now processes the message even without an explicit @mention. This improves UX for ongoing threaded discussions. Changes: - Added set_session_store() to BasePlatformAdapter for adapters to check active sessions - Modified SlackAdapter to detect thread replies and check if a session exists for that thread before requiring @mentions - Updated GatewayRunner to inject the session store into adapters - Added comprehensive tests for the new behavior Fixes: Thread replies without @jarvis are now processed if there is an active session, matching user expectations for conversation flow Co-authored-by: eizus <hello@cdr.xyz>
The edit_message method was sending raw content directly to Slack's chat_update API without converting standard markdown to Slack's mrkdwn format. This caused broken formatting and malformed URLs (e.g., trailing ** from bold syntax became part of clickable links → 404 errors). The send() method already calls format_message() to handle this conversion, but edit_message() was bypassing it. This change ensures edited messages receive the same markdown → mrkdwn transformation as new messages. Closes: PR NousResearch#5558 formatting issue where links had trailing markdown syntax. Co-authored-by: eizus <hello@cdr.xyz>
… $299) - Replace 4-tier structure (Free/Writer/Pro/Agency) with 3-tier - Basic: $1/mo = 1 site, 10 quotes, 1 social - Starter: $10/mo = 3 sites, 1k quotes, 10 social - Agency: $299/mo = 100 sites, 100k quotes, 500 social - Update blog post pricing section - Adjust grid layout from 4 to 3 columns
3 tasks
…d contamination The seeding logic at lines 772-805 was copying the ENTIRE parent DM session history into new thread sessions. This caused messages from all threads in a DM to be mixed together — when a user was in thread B, they would see context from thread A. The Slack adapter already handles thread context correctly by fetching fresh thread history from Slack's API and injecting it into the message text (slack.py lines 890-898). The seeding was redundant and harmful. Fixes: cross-thread context leakage in Slack DM threads
jarvisxyz
force-pushed
the
2025-04-07.eizus.fix-dm-thread-seeding
branch
from
April 7, 2026 20:22
a715001 to
43d87a0
Compare
jarvisxyz
force-pushed
the
main-with-fixes
branch
2 times, most recently
from
April 10, 2026 08:03
7511330 to
0848a79
Compare
jarvisxyz
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.
jarvisxyz
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 NousResearch#16). - JSON args use sorted_keys for the same reason. Real wire formats verified against codex 0.130.0 by capturing live notifications from thread/shellCommand and including one as a fixture (COMMAND_EXEC_COMPLETED). 23 new tests, all green: - Streaming deltas don't materialize (3 paths) - Turn/thread frame events are silent - commandExecution: 5 tests including non-zero exit annotation + deterministic id stability across replays - agentMessage + reasoning attachment + reasoning consumption - fileChange: summary without inlined content - mcpToolCall: namespaced naming + error surfacing - userMessage: text fragments only (drops images/etc) - opaque items: no fabricated tool_calls - Helpers: deterministic id stability + sorted JSON args - Role alternation invariant across all four tool-shaped item types This commit is a pure addition. AIAgent integration (the wire that uses the projector) is the next commit. * feat(codex-runtime): add session adapter + approval bridge The third self-contained module: CodexAppServerSession owns one Codex thread per Hermes session, drives turn/start, consumes streaming notifications via CodexEventProjector, handles server-initiated approval requests, and translates cancellation into turn/interrupt. The adapter has a single public per-turn method: result = session.run_turn(user_input='...', turn_timeout=600) # result.final_text → assistant text for the caller # result.projected_messages → list ready to splice into AIAgent.messages # result.tool_iterations → tick count for _iters_since_skill nudge # result.interrupted → True on Ctrl+C / deadline / interrupt # result.error → error string when the turn cannot complete # result.turn_id, thread_id → for sessions DB / resume Behavior: - ensure_started() spawns codex, does the initialize handshake, and issues thread/start with cwd + permissions profile. Idempotent. - run_turn() blocks until turn/completed, drains server-initiated requests (approvals) before reading notifications so codex never deadlocks waiting for us, projects every item/completed via the projector, and increments tool_iterations for the skill nudge gate. - request_interrupt() is thread-safe (threading.Event); the next loop iteration issues turn/interrupt and unwinds. - turn_timeout deadlock guard issues turn/interrupt and records an error if the turn never completes. - close() escalates terminate → kill via the underlying client. Approval bridge: Codex emits server-initiated requests for execCommandApproval and applyPatchApproval. The adapter translates Hermes' approval choice vocabulary onto codex's decision vocabulary: Hermes 'once' → codex 'approved' Hermes 'session' or 'always' → codex 'approvedForSession' Hermes 'deny' / anything else → codex 'denied' Routing precedence: 1. _ServerRequestRouting.auto_approve_* flags (cron / non-interactive) 2. approval_callback wired by the CLI (defers to tools.approval.prompt_dangerous_approval()) 3. Fail-closed denial when neither is wired Unknown server-request methods are answered with JSON-RPC error -32601 so codex doesn't hang waiting for us. Permission profile mapping mirrors AGENTS.md: Hermes 'auto' → codex 'workspace-write' Hermes 'approval-required' → codex 'read-only-with-approval' Hermes 'unrestricted/yolo' → codex 'full-access' 20 new tests, all green. Combined with prior commits this PR now has 67 tests across three modules: - test_codex_app_server_runtime.py: 24 (api_mode + transport surface) - test_codex_event_projector.py: 23 (item taxonomy projections) - test_codex_app_server_session.py: 20 (turn loop + approvals + interrupts) Full tests/agent/transports/ directory: 249/249 pass — no regressions to existing transport tests. Still no wire into AIAgent.run_conversation(); that integration commit is small and goes next. * feat(codex-runtime): wire codex_app_server runtime into AIAgent The integration commit. AIAgent.run_conversation() now early-returns to a new helper _run_codex_app_server_turn() when self.api_mode == 'codex_app_server', bypassing the chat_completions tool loop entirely. Three small surgical edits to run_agent.py (~105 LOC total): 1. Line ~1204 (constructor api_mode validation set): Add 'codex_app_server' so an explicit api_mode='codex_app_server' passed to AIAgent() isn't silently rewritten to 'chat_completions'. 2. Line ~12048 (run_conversation, just before the while loop): Early-return to _run_codex_app_server_turn() when self.api_mode is 'codex_app_server'. Placed AFTER all standard pre-loop setup — logging context, session DB, surrogate sanitization, _user_turn_count and _turns_since_memory increments, _ext_prefetch_cache, memory manager on_turn_start — so behavior outside the model-call loop is identical between paths. Default Hermes flow is unchanged when the flag is off. 3. End-of-class (line ~15497): New method _run_codex_app_server_turn(). Lazy-instantiates one CodexAppServerSession per AIAgent (reused across turns), runs the turn, splices projected_messages into messages, increments _iters_since_skill by tool_iterations (since the chat_completions loop normally does that per iteration), fires _spawn_background_review on the same cadence as the default path. Counter accounting: _turns_since_memory ← already incremented at run_conversation:11817 (gated on memory store configured) — codex helper does NOT touch it (would double-count). _user_turn_count ← already incremented at run_conversation:11793 — codex helper does NOT touch it. _iters_since_skill ← incremented in the chat_completions loop per tool iteration. Codex helper increments by turn.tool_iterations since the loop is bypassed. User message: ALREADY appended to messages by run_conversation pre-loop (line 11823) before the early-return reaches us. Helper does NOT append again. Regression test test_user_message_not_duplicated guards this. Approval callback wiring: Lazy-fetches tools.terminal_tool._get_approval_callback at session spawn time, passes to CodexAppServerSession. CLI threads with prompt_toolkit get interactive approvals; gateway/cron contexts get the codex-side fail-closed deny. Error path: Codex session exceptions become a 'partial' result with completed=False and a final_response that explicitly tells the user how to switch back: 'Codex app-server turn failed: ... Fall back to default runtime with /codex-runtime auto.' Same return-dict shape as the chat_completions path so all callers (gateway, CLI, batch_runner, ACP) work unchanged. 9 new integration tests in tests/run_agent/test_codex_app_server_integration.py: - api_mode='codex_app_server' is accepted on AIAgent construction - run_conversation returns the expected codex shape (final_response, codex_thread_id, codex_turn_id, completed, partial) - Projected messages are spliced into messages list - _iters_since_skill ticks per tool iteration - _user_turn_count delegated to standard flow (not double-counted) - User message appears exactly once (regression guard) - _spawn_background_review IS invoked (memory/skill review keeps working) - chat.completions.create is NEVER called (loop fully bypassed) - Session exception → partial result with /codex-runtime auto hint - Interrupted turn → partial result with error preserved Adjacent test runs confirm no regressions: - tests/run_agent/test_memory_nudge_counter_hydration.py: green - tests/run_agent/test_background_review.py: green - tests/run_agent/test_fallback_model.py: green - tests/agent/transports/: 249/249 green Still missing for full feature: /codex-runtime slash command, plugin migration helper, docs page, live e2e test gated on codex binary. Those are the remaining followup commits. * feat(codex-runtime): add /codex-runtime slash command (CLI + gateway) User-facing toggle for the optional codex app-server runtime. Follows the 'Adding a Slash Command (All Platforms)' pattern from AGENTS.md exactly: single CommandDef in the central registry → CLI handler → gateway handler → running-agent guard → all surfaces (autocomplete, /help, Telegram menu, Slack subcommands) update automatically. Surface: /codex-runtime — show current state + codex CLI status /codex-runtime auto — Hermes default runtime /codex-runtime codex_app_server — codex subprocess runtime /codex-runtime on / off — synonyms Files changed: hermes_cli/codex_runtime_switch.py (new): Pure-Python state machine shared by CLI and gateway. Parse args, read/write model.openai_runtime in the config dict, gate enabling behind a codex --version check (don't let users opt in to a runtime they have no binary for; print npm install hint instead). Returns a CodexRuntimeStatus dataclass that callers render however suits their surface. hermes_cli/commands.py: Single CommandDef entry, no aliases (codex-runtime is its own thing). cli.py: Dispatch in process_command() + _handle_codex_runtime() handler that delegates to the shared module and renders results via _cprint. gateway/run.py: Dispatch in _handle_message() + _handle_codex_runtime_command() that returns a string (gateway sends as message). On a successful change that requires a new session, _evict_cached_agent() forces the next inbound message to construct a fresh AIAgent with the new api_mode — avoids prompt-cache invalidation mid-session. gateway/run.py running-agent guard: /codex-runtime joins /model in the early-intercept block so a runtime flip mid-turn can't split a turn across two transports. Tests: tests/hermes_cli/test_codex_runtime_switch.py — 25 tests covering the state machine: arg parsing (10 cases incl. case-insensitive and synonyms), reading current runtime (5 cases incl. malformed configs), writing runtime (3 cases), apply() entry point covering read-only, no-op, codex-missing-blocked, codex-present-success, disable-no-binary-check, and persist-failure paths (8 cases). All green. Adjacent test suites confirm no regressions: - tests/hermes_cli/test_commands.py + test_codex_runtime_switch.py: 167/167 green - tests/agent/transports/: 283/283 green when combined with prior commits Still missing: plugin migration helper, docs page, live e2e test gated on codex binary. Followup commits. * feat(codex-runtime): auto-migrate Hermes MCP servers to ~/.codex/config.toml Translates the user's mcp_servers config from ~/.hermes/config.yaml into the TOML format codex's MCP client expects. Wired into the /codex-runtime codex_app_server enable path so users get their MCP tool surface in the spawned subprocess automatically. The migration runs on every enable. Failures are non-fatal — the runtime change still proceeds and the user gets a warning so they can fix the codex config manually. What translates (mapping verified against codex-rs/core/src/config/edit.rs): Hermes mcp_servers.<n>.command/args/env → codex stdio transport Hermes mcp_servers.<n>.url/headers → codex streamable_http transport Hermes mcp_servers.<n>.timeout → codex tool_timeout_sec Hermes mcp_servers.<n>.connect_timeout → codex startup_timeout_sec Hermes mcp_servers.<n>.cwd → codex stdio cwd Hermes mcp_servers.<n>.enabled: false → codex enabled = false What does NOT translate (warned + skipped per server): Hermes-specific keys (sampling, etc.) — codex's MCP client has no equivalent. Listed in the per-server skipped[] field of the report. What's NOT migrated (intentional): AGENTS.md — codex respects this file natively in its cwd. Hermes' own AGENTS.md (project-level) is already in the worktree, so codex picks it up without translation. No code needed. Idempotency design: All managed content lives between a 'managed by hermes-agent' marker and the next non-mcp_servers section header. _strip_existing_managed_block removes the prior managed region cleanly, preserving any user-added codex config (model, providers.openai, sandbox profiles, etc.) above or below. Files added: hermes_cli/codex_runtime_plugin_migration.py — pure-Python migration helper. Public API: migrate(hermes_config, codex_home=None, dry_run=False) returns MigrationReport with .migrated/.errors/ .skipped_keys_per_server. No external TOML dependency — minimal formatter handles strings/numbers/booleans/lists/inline-tables. tests/hermes_cli/test_codex_runtime_plugin_migration.py — 39 tests covering: - per-server translation (12): stdio/http/sse, cwd, timeouts, enabled flag, command+url precedence, sampling drop, unknown keys - TOML formatter (8): types, escaping, inline tables, error case - existing-block stripping (4): no marker, alone, with user content above, with user content below - end-to-end migrate() (8): empty, dry-run, round-trip, idempotent re-run, preserves user config, error reporting, invalid input, summary formatting Files changed: hermes_cli/codex_runtime_switch.py — apply() now calls migrate() in the codex_app_server enable branch. Migration failure logs a warning in the result message but does NOT fail the runtime change. Disable path (auto) explicitly skips migration. tests/hermes_cli/test_codex_runtime_switch.py — 3 new tests: test_enable_triggers_mcp_migration, test_disable_does_not_trigger_migration, test_migration_failure_does_not_block_enable. All 325 feature tests green: - tests/agent/transports/: 249 (incl. 67 new) - tests/run_agent/test_codex_app_server_integration.py: 9 - tests/hermes_cli/test_codex_runtime_switch.py: 28 (3 new) - tests/hermes_cli/test_codex_runtime_plugin_migration.py: 39 (new) * perf(codex-runtime): cache codex --version check within apply() Single /codex-runtime invocation could spawn 'codex --version' up to 3 times (state report, enable gate, success message). Each spawn is ~50ms, so the cumulative cost wasn't a crisis, but it was wasteful and turned a trivial slash command into something noticeably laggy on slower systems. Refactored to lazy-once via a closure over a nonlocal cache. First call spawns; subsequent calls in the same apply() reuse the result. Behavior unchanged — same return shape, same error handling, same install hint when codex is missing. Just one subprocess per call instead of three. Two regression-guard tests added: - test_binary_check_cached_within_apply: enable path → call_count == 1 - test_binary_check_cached_on_read_only_call: state-report path → call_count == 1 Total tests for /codex-runtime now 30 (was 28); all 143 codex-runtime tests still green. * fix(codex-runtime): correct protocol field names found via live e2e test Three real bugs caught only by running a turn end-to-end against codex 0.130.0 with a real ChatGPT subscription. Unit tests passed because they asserted on our own (incorrect) wire shapes; the wire format from codex-rs/app-server-protocol/src/protocol/v2/* is the source of truth and my initial reading of the README was incomplete. Bug 1: thread/start.permissions wire format Was sending {"profileId": "workspace-write"}. Real format per PermissionProfileSelectionParams enum (tagged union): {"type": "profile", "id": "workspace-write"} AND requires the experimentalApi capability declared during initialize. AND requires a matching [permissions] table in ~/.codex/config.toml or codex fails the request with 'default_permissions requires a [permissions] table'. Fix: stop overriding permissions on thread/start. Codex picks its default profile (read-only unless user configures otherwise), which matches what codex CLI users expect — they configure their default permission profile in ~/.codex/config.toml the standard way. Trying to be clever about profile selection broke every turn we tested. Live error before fix: 'Invalid request: missing field type' on every turn/start, even though our turn/start payload was correct — the field codex was complaining about was inside the permissions sub-object we shouldn't have been sending. Bug 2: server-request method names Was matching 'execCommandApproval' and 'applyPatchApproval'. Real names per common.rs ServerRequest enum: item/commandExecution/requestApproval item/fileChange/requestApproval item/permissions/requestApproval (new third method) Fix: match the documented names. Added handler for item/permissions/requestApproval that always declines — codex sometimes asks to escalate permissions mid-turn and silent acceptance would surprise users. Live symptom before fix: agent.log showed 'Unknown codex server request: item/commandExecution/requestApproval' and codex stalled because we replied with -32601 (unsupported method) instead of an approval decision. The agent reported back 'The write command was rejected' even though Hermes never showed the user an approval prompt. Bug 3: approval decision values Was sending decision strings 'approved'/'approvedForSession'/'denied'. Real values per CommandExecutionApprovalDecision enum (camelCase): accept, acceptForSession, decline, cancel (also AcceptWithExecpolicyAmendment and ApplyNetworkPolicyAmendment variants we don't currently use). Fix: rename _approval_choice_to_codex_decision return values; update auto_approve_* fallbacks; update fail-closed default from 'denied' to 'decline'. Test mapping table updated to match. Live test verified after fixes: $ hermes (with model.openai_runtime: codex_app_server) > Run the shell command: echo hermes-codex-livetest > .../proof.txt then read it back Approval prompt fired with 'Codex requests exec in <cwd>'. User chose 'Allow once'. Codex executed the command, wrote the file, read it back. Final response: 'Read back from proof.txt: hermes-codex-livetest'. File contents on disk match. agent.log confirms: codex app-server thread started: id=019e200e profile=workspace-write cwd=/tmp/hermes-codex-livetest/workspace All 20 session tests still green after wire-format updates. * fix(codex-runtime): correct apply_patch approval params + ship docs Live e2e revealed FileChangeRequestApprovalParams doesn't carry the changeset (just itemId, threadId, turnId, reason, grantRoot) — Codex's 'reason' field describes what the patch wants to do. Test config and display logic updated to use it. The first 'apply_patch (0 change(s))' display from the live test is now 'apply_patch: <reason>'. Adds website/docs/user-guide/features/codex-app-server-runtime.md covering enable/disable, prerequisites, approval UX, MCP migration behavior, permission profile delegation to ~/.codex/config.toml, known limitations, and the architecture diagram. Wired into the Automation category in sidebars.ts. Live e2e validation across the path matrix: ✓ thread/start handshake ✓ turn/start with text input ✓ commandExecution items + projection ✓ item/commandExecution/requestApproval → Hermes UI → response ✓ Approve once → command runs ✓ Deny → command rejected, codex falls back to read-only message ✓ Multi-turn (codex remembers prior turn's results) ✓ apply_patch via Codex's fileChange path ✓ item/fileChange/requestApproval → Hermes UI ✓ MCP server migration loads inside spawned codex (verified via 'use the filesystem MCP tool' prompt) ✓ /codex-runtime auto → codex_app_server toggle cycle ✓ Disable doesn't trigger migration ✓ Enable with codex CLI present succeeds + migrates ✓ Hermes-side interrupt path (turn/interrupt request issued cleanly even if codex finishes before the interrupt lands) Known live-validated limitations now documented in the docs page: - delegate_task subagents unavailable on this runtime - permission profile selection delegated to ~/.codex/config.toml - apply_patch approval prompt has no inline changeset (codex protocol doesn't expose it) 145/145 codex-runtime tests still green. * feat(codex-runtime): native plugin migration + UX polish (quirks 2/4/5/10/11) Major: migrate native Codex plugins (#7 in OpenClaw's PR list) Discovers installed curated plugins via codex's plugin/list RPC and writes [plugins."<name>@<marketplace>"] entries to ~/.codex/config.toml so they're enabled in the spawned Codex sessions. This is the 'YouTube-video-worthy' bit Pash highlighted: when a user has google-calendar, github, etc. installed in their Codex CLI, those plugins activate automatically when they enable Hermes' codex runtime. Implementation: - hermes_cli/codex_runtime_plugin_migration.py: new _query_codex_plugins() helper spawns 'codex app-server' briefly and walks plugin/list. Returns (plugins, error) — failures are non-fatal so MCP migration still works. - render_codex_toml_section() now takes plugins + permissions args. - migrate() defaults: discover_plugins=True, default_permission_profile= 'workspace-write'. Explicit None on either disables that side. - _strip_existing_managed_block() now also strips [plugins.*] and [permissions]/[permissions.*] sections inside the managed block, so re-runs replace plugins cleanly without touching codex's own config. Quirk fixes: #2 Default permissions profile written on enable. Without this, Codex's read-only default kicks in and EVERY write triggers an approval prompt. Now writes [permissions] default = 'workspace-write' so the runtime feels normal out of the box. Set default_permission_profile=None to opt out. #4 apply_patch approval prompt now shows what's changing. Codex's FileChangeRequestApprovalParams doesn't carry the changeset. Session adapter now caches the fileChange item from item/started notifications and looks it up by itemId when codex requests approval. Prompt shows '1 add, 1 update: /tmp/new.py, /tmp/old.py' instead of 'apply_patch (0 change(s))'. Side benefit: also drains pending notifications BEFORE handling a server request, so the projector and per-turn caches are up to date when the approval decision fires. Bounded to 8 notifications per loop iter to avoid starving codex's response. #5/#10 Exec approval prompt never shows empty cwd. When codex omits cwd in CommandExecutionRequestApprovalParams, fall back to the session's cwd. If somehow neither is available, show '<unknown>' explicitly instead of an empty string. Also surfaces 'reason' from the approval params when codex provides it — gives users more context on why codex wants to run something. #11 Banner indicates the codex_app_server runtime when active. New 'Runtime: codex app-server (terminal/file ops/MCP run inside codex)' line appears in the welcome banner only when the runtime is on. Default banner is unchanged. Tests: - 7 new tests in test_codex_runtime_plugin_migration.py covering plugin discovery (mocked), failure handling, dry-run skip, opt-out flag, idempotent re-runs, and permissions writing. - 3 new tests in test_codex_app_server_session.py covering the enriched approval prompts: cwd fallback, change summary on apply_patch, fallback when no item/started cache exists. - All 26 session tests + 46 migration tests green; 153 total in PR. * feat(codex-runtime): hermes-tools MCP callback + native plugin migration The big architectural addition: when codex_app_server runtime is on, Hermes registers its own tool surface as an MCP server in ~/.codex/config.toml so the codex subprocess can call back into Hermes for tools codex doesn't ship with — web_search, browser_*, vision, image_generate, skills, TTS. Also: 'migrate native codex plugins' (Pash's YouTube-video-worthy bit) — when the user has plugins like Linear, GitHub, Gmail, Calendar, Canva installed via 'codex plugin', Hermes discovers them via plugin/list and writes [plugins.<name>@openai-curated] entries so they activate automatically. New module: agent/transports/hermes_tools_mcp_server.py FastMCP stdio server exposing 17 Hermes tools. Each call dispatches through model_tools.handle_function_call() — same code path as the Hermes default runtime. Run with: python -m agent.transports.hermes_tools_mcp_server [--verbose] Exposed: web_search, web_extract, browser_navigate / _click / _type / _press / _snapshot / _scroll / _back / _get_images / _console / _vision, vision_analyze, image_generate, skill_view, skills_list, text_to_speech. NOT exposed (deliberately): - terminal/shell/read_file/write_file/patch — codex has built-ins - delegate_task/memory/session_search/todo — _AGENT_LOOP_TOOLS in model_tools.py:493, require running AIAgent context. Documented as a limitation and surfaced in the slash command output. Migration changes (hermes_cli/codex_runtime_plugin_migration.py): - _query_codex_plugins() spawns 'codex app-server' briefly to walk plugin/list and pull installed openai-curated plugins. Failures are non-fatal — MCP migration still completes. - render_codex_toml_section() now takes plugins + permissions args AND wraps the managed block with a MIGRATION_END_MARKER comment so the stripper can reliably find both ends, even when the block contains top-level keys (default_permissions = ...). - migrate() defaults: discover_plugins=True, expose_hermes_tools=True, default_permission_profile=':workspace' (built-in codex profile name — must be prefixed with ':'). All three opt-out via explicit args. - _build_hermes_tools_mcp_entry() builds the codex stdio entry with HERMES_HOME and PYTHONPATH passthrough so a worktree-launched Hermes points the MCP subprocess at the same module layout. Live-caught wire bugs fixed during this turn: 1. Permission profile config key is top-level , NOT a [permissions] table. The [permissions] table is for *user-defined* profiles with structured fields. Built-in profile names start with ':' (':workspace', ':read-only', ':danger-no-sandbox'). Was emitting which codex rejected with 'invalid type: string "X", expected struct PermissionProfileToml'. 2. Built-in profile is , NOT . Codex rejected with 'unknown built-in profile'. 3. Codex's MCP layer sends for tool-call confirmation. We weren't handling it, so codex stalled and returned 'MCP tool call was rejected'. Now: auto-accept for our own hermes-tools server (user already opted in by enabling the runtime), decline for third-party servers. Quirk fixes shipped (from the limitations list): #2 default permissions: workspace profile written on enable. No more approval prompt on every write. #4 apply_patch approval shows what's changing: cache fileChange items from item/started, look up by itemId when codex sends item/fileChange/requestApproval. Prompt: '1 add, 1 update: /tmp/new.py, /tmp/old.py' instead of '0 change(s)'. #5/#10 exec approval cwd never empty: fall back to session cwd, then '<unknown>'. Also surfaces 'reason' from codex when present. #11 banner shows 'Runtime: codex app-server' line when active so users understand why tool counts may not match what's reachable. Tests: - 5 new tests in test_codex_runtime_plugin_migration.py covering plugin discovery, expose_hermes_tools entry generation, idempotent re-runs, opt-out flag, permissions profile. - 3 new tests in test_codex_app_server_session.py covering enriched approval prompts (cwd fallback, fileChange summary). - 2 new tests for mcpServer/elicitation/request handling (accept hermes-tools, decline others). - New test file test_hermes_tools_mcp_server.py covering module surface, EXPOSED_TOOLS safety invariants (no shell/file_ops, no agent-loop tools), and main() error paths. - 166 codex-runtime tests total, all green. Live e2e validated against codex 0.130.0 + ChatGPT subscription: ✓ /codex-runtime codex_app_server enables, migrates filesystem MCP, registers hermes-tools, writes default_permissions = ':workspace' ✓ Banner shows 'Runtime: codex app-server' line in subsequent sessions ✓ Shell command runs without approval prompt (workspace profile works) ✓ Multi-turn — codex remembers prior turn's results ✓ apply_patch path via fileChange request approval ✓ web_search via hermes-tools MCP callback returns real Firecrawl results: 'OpenAI Codex CLI – Getting Started' end-to-end in 13s ✓ Disable cycle clean Docs updated: website/docs/user-guide/features/codex-app-server-runtime.md Full re-write covering native plugin migration, the hermes-tools callback architecture, the prerequisites change ('codex login is separate from hermes auth login codex'), the trade-off table now reflecting which Hermes tools work via callback, and the limitations list updated with what's actually unavailable on this runtime. * feat(codex-runtime): pin user-config preservation invariant for quirk #6 Quirk #6 from the limitations list — user MCP servers / overrides / codex-only sections in ~/.codex/config.toml that live OUTSIDE the hermes-managed block must survive re-migration verbatim. This already worked thanks to the MIGRATION_MARKER + MIGRATION_END_MARKER pair I added when fixing the default_permissions wire format (so the strip can find both ends of the managed region even with top-level keys like default_permissions). But it was an emergent property without a test pinning it. Now explicitly tested: - User MCP server above the managed block survives migration - User MCP server below the managed block survives migration - Both above + below survive a second re-migration - User content (model, providers, sandbox, otel, etc.) outside our region is left untouched Docs added a section "Editing ~/.codex/config.toml safely" explaining the marker contract — so users know they can add their own MCP servers, override permissions, configure codex-only options, etc. without fear of Hermes overwriting their work. 167 codex-runtime tests, all green. * docs(codex-runtime): clarify the actual tool surface — shell covers terminal/read/write/find Previous docs and PR description undersold what codex's built-in toolset actually provides. apply_patch alone made it sound like the runtime could only edit files in patch format — implying you'd lose terminal use, read_file, write_file, search/find. That was wrong. Codex's 'shell' tool runs arbitrary shell commands inside the sandbox, which covers everything you'd do in bash: cat/head/tail (read), echo> or heredocs (write), find/rg/grep (search), ls/cd (navigate), build/ test/git/etc. apply_patch is for structured multi-file edits on top of that. update_plan is its in-runtime todo. view_image loads images. And codex has its own web_search built in (in addition to the Firecrawl-backed one Hermes exposes via MCP callback). Docs now have a 'What tools the model actually has' section right after Why, breaking the surface into three clearly-labeled buckets: 1. Codex's built-in toolset (always on) — shell, apply_patch, update_plan, view_image, web_search; covers everything terminal- adjacent. 2. Native Codex plugins (auto-migrated from your codex plugin install) — Linear, GitHub, Gmail, Calendar, Outlook, Canva, etc. 3. Hermes tool callback (MCP server in ~/.codex/config.toml) — web_search/web_extract via Firecrawl, browser_*, vision_analyze, image_generate, skill_view/skills_list, text_to_speech. Plus a 'What's NOT available' callout listing the four agent-loop tools (delegate_task, memory, session_search, todo) that need running AIAgent context and can't reach the codex runtime. Trade-offs table broken out: shell, apply_patch, update_plan, view_image, sandbox each get their own row with a one-line description so users can see at a glance what's available natively. Architecture diagram updated to list the codex built-ins by name instead of 'apply_patch + shell + sandbox'. No code changes — purely docs clarification. 167 codex-runtime tests still green. * fix(codex-runtime): _spawn_background_review signature + review fork api_mode downgrade Two real bugs in the self-improvement loop integration that the previous test mocked away. Bug 1: wrong call signature The codex helper was calling self._spawn_background_review() with no args after every turn. That function actually requires: messages_snapshot=list (positional or keyword) review_memory=bool (at least one trigger must be True) review_skills=bool So the call would have raised TypeError at runtime — except the only test that exercised this path mocked _spawn_background_review entirely and just asserted spawn.called, so the wrong-arg shape never surfaced. Bug 2: review fork inherits codex_app_server api_mode The review fork is constructed with: api_mode = _parent_runtime.get('api_mode') So when the parent is codex_app_server, the review fork ALSO runs as codex_app_server. But the review fork's whole job is to call agent-loop tools (memory, skill_manage) which require Hermes' own dispatch — they short-circuit with 'must be handled by the agent loop' on the codex runtime. So the review fork would have run, decided to save something, called memory or skill_manage, and silently no-op'd. Fixed in run_agent.py:_spawn_background_review() — when the parent api_mode is 'codex_app_server', the review fork is downgraded to 'codex_responses' (same OAuth credentials, same openai-codex provider, but talks to OpenAI's Responses API directly so Hermes owns the loop). Also rewrote the codex helper's review wiring to match the chat_completions path: - Computes _should_review_memory in the pre-loop block (was already being computed; now passed through to the helper as an arg). - Computes _should_review_skills AFTER the codex turn returns + counters tick (line ~15432 pattern in chat_completions). - Calls _spawn_background_review(messages_snapshot=, review_memory=, review_skills=) only when at least one trigger fires. - Adds the external memory provider sync (_sync_external_memory_for_turn) that the chat_completions path runs after every turn. Tests: Replaced the broken test_background_review_invoked (which only asserted spawn.called) with three sharper tests: - test_background_review_NOT_invoked_below_threshold: single turn at default thresholds → no review fires (would have caught the original 'every turn calls spawn with no args' bug) - test_background_review_skill_trigger_fires_above_threshold: 10 tool_iterations at threshold=10 → review fires with messages_snapshot=list, review_skills=True, counter resets - test_background_review_signature_never_breaks: regression guard asserting positional args are always empty and kwargs include messages_snapshot New TestReviewForkApiModeDowngrade class: - test_codex_app_server_parent_downgrades_review_fork: drives the real _spawn_background_review function (no mock at that level), asserts the review_agent gets api_mode='codex_responses' when the parent was codex_app_server. Live-validated against real run_conversation: - Counter ticked from 0 to 5 after a 5-tool-iteration turn - _spawn_background_review fired exactly once with kwargs-only signature - review_skills=True, review_memory=False - messages_snapshot was 12 entries (5 assistant tool_calls + 5 tool results + 1 final assistant + initial system/user) - Counter reset to 0 after fire 170 codex-runtime tests, all green. Docs: added a Self-improvement loop section to the codex runtime page explaining both how the trigger logic stays equivalent and that the review fork is auto-downgraded to codex_responses for the agent-loop tools. Also clarified that apply_patch and update_plan ARE codex's built-in tools (the previous version made it sound like they were separate from 'codex's stuff' — they're not, all five tools listed in 'What tools the model actually has' section 1 are codex built-ins). * feat(codex-runtime): expose kanban tools through Hermes MCP callback Kanban workers spawn as separate hermes chat -q subprocesses that read the user's config.yaml. If model.openai_runtime: codex_app_server is set globally (which is the whole point of opt-in), every dispatched worker ALSO comes up on the codex runtime. That mostly works — codex's built-in shell + apply_patch + update_plan do the actual task work fine — but it had one critical break: the worker handoff tools (kanban_complete, kanban_block, kanban_comment, kanban_heartbeat) are Hermes-registered tools, not codex built-ins. On the codex runtime, codex builds its own tool list and these never reach the model, so the worker would do the work but not be able to report back, hanging until the dispatcher's timeout escalates it as zombie. Fix: add all 9 kanban tools to the EXPOSED_TOOLS list in the Hermes MCP callback. They dispatch statelessly through handle_function_call() just like web_search and the others — they read HERMES_KANBAN_TASK from env (set by the dispatcher), gate correctly (worker tools require the env var, orchestrator tools require it unset), and write to ~/.hermes/kanban.db. Why kanban tools work via stateless dispatch when delegate_task/memory/ session_search/todo don't: those four are listed in _AGENT_LOOP_TOOLS (model_tools.py:493) and short-circuit in handle_function_call() with 'must be handled by the agent loop' — they need to mutate AIAgent's mid-loop state. Kanban tools have no such requirement; they're pure side-effect functions against the kanban.db plus state_meta. Tools exposed: Worker handoff (require HERMES_KANBAN_TASK): kanban_complete, kanban_block, kanban_comment, kanban_heartbeat Read-only board queries: kanban_show, kanban_list Orchestrator (require HERMES_KANBAN_TASK unset): kanban_create, kanban_unblock, kanban_link Tests: - test_kanban_worker_tools_exposed: complete/block/comment/heartbeat in EXPOSED_TOOLS (regression guard for the would-hang-worker bug) - test_kanban_orchestrator_tools_exposed: create/show/list/unblock/link Docs: - New 'Workflow features' section in the docs page covering /goal, kanban, and cron behavior on this runtime - /goal: works fully via run_conversation feedback; only caveat is approval-prompt noise on long writes-heavy goals (mitigated by the default :workspace permission profile) - Kanban: enumerated which tools are reachable via the callback and why the env var propagates correctly through the codex subprocess to the MCP server subprocess - Cron: documented as 'not specifically tested' — same rules as the CLI apply since cron runs through AIAgent.run_conversation - Trade-offs table gained rows for /goal, kanban worker, kanban orchestrator 172/172 codex-runtime tests green (+2 from kanban tests). * docs(codex-runtime): wire /codex-runtime into slash-commands ref + flag aux token cost Three docs gaps caught during a final audit: 1. /codex-runtime was only in the feature docs page, not in the slash-commands reference. Added rows to both the CLI section and the Messaging section so users discover it where they'd look for slash command syntax. 2. CODEX_HOME and HERMES_KANBAN_TASK weren't in environment-variables.md. CODEX_HOME lets users redirect Codex CLI's config dir (the migration honors it). HERMES_KANBAN_TASK is set by the kanban dispatcher and propagates to the codex subprocess + the hermes-tools MCP subprocess so kanban worker tools gate correctly — documented as 'don't set manually' since it's an internal handoff. 3. Aux client behavior on this runtime. When openai_runtime= codex_app_server is on with the openai-codex provider, every aux task (title generation, context compression, vision auto-detect, session search summarization, the background self-improvement review fork) flows through the user's ChatGPT subscription by default. This is true for the existing codex_responses path too, but it's more visible / important here because users explicitly opted in for subscription billing. Added a 'Auxiliary tasks and ChatGPT subscription token cost' section to the docs page with a YAML example showing how to override specific aux tasks to a cheaper model (typically google/gemini-3-flash-preview via OpenRouter). Also documents how the self-improvement review fork gets auto-downgraded from codex_app_server to codex_responses by the fix earlier in this PR. No code changes — pure docs. 172 codex-runtime tests still green. * docs+test(codex-runtime): pin HOME passthrough, document multi-profile + CODEX_HOME OpenClaw hit a real footgun in openclaw/openclaw#81562: when spawning codex app-server they were synthesizing a per-agent HOME alongside CODEX_HOME. That made every subprocess codex's shell tool launches (gh, git, aws, npm, gcloud, ...) see a fake $HOME and miss the user's real config files. They had to back it out in PR NousResearch#81562 — keep CODEX_HOME isolation, leave HOME alone. Audit confirms Hermes' codex spawn doesn't have this problem. We do os.environ.copy() and only overlay CODEX_HOME (when provided) and RUST_LOG. HOME passes through unchanged. But it was an emergent property without a test pinning it, so adding a regression guard: test_spawn_env_preserves_HOME — confirms parent HOME survives intact in the subprocess env test_spawn_env_sets_CODEX_HOME_when_provided — confirms codex_home arg still isolates codex state correctly Docs additions: 'HOME environment variable passthrough' section — calls out the contract explicitly: CODEX_HOME isolates codex's own state, HOME stays user-real so gh/git/aws/npm/etc. find their normal config. Cites openclaw#81562 as the cautionary tale. 'Multi-profile / multi-tenant setups' section — addresses the related concern: profiles share ~/.codex/ by default. For users who want per-profile codex isolation (separate auth, separate plugins), documents the manual CODEX_HOME=<profile-scoped-dir> approach. Explains why we DON'T auto-scope CODEX_HOME per profile: doing so would silently invalidate existing codex login state for anyone upgrading to this PR with tokens already at ~/.codex/auth.json. Opt-in is safer than surprising users. 174 codex-runtime tests (+2 from HOME guards), all green. * fix(codex-runtime): TOML control-char escapes + atomic config.toml write Two footguns caught in a final audit pass before merge. Bug 1: TOML control characters not escaped The _format_toml_value() helper escaped backslashes and double quotes but passed literal control characters (\n, \t, \r, \f, \b) through unchanged. TOML basic strings don't allow literal control characters — a path or env var containing a newline would produce invalid TOML that codex refuses to load. Realistic exposure: pathological cases like a HERMES_HOME with a trailing newline (env var concatenation accident), or a PYTHONPATH with a tab from a multi-line shell heredoc. Fix: escape all five TOML basic-string control sequences (\b \t \n \f \r) in addition to \\ and \" that we already did. Order matters — backslash must come first or the other escapes get re-escaped. Bug 2: config.toml write wasn't atomic If the python process crashed between target.mkdir() and the write_text() finishing, a half-written config.toml could be left behind. On NFS / Windows / some FUSE mounts this is a real concern; on ext4/APFS small writes are usually atomic in practice but not guaranteed. Fix: write to a tempfile.mkstemp() temp file in the same directory, then Path.replace() (atomic same-dir rename on POSIX, ReplaceFile on Windows). On rename failure, clean up the temp file so repeated failed migrations don't pile up .config.toml.* files. Tests: - test_string_with_newline_escaped — \n in value → \n in output - test_string_with_tab_escaped — \t in value → \t in output - test_string_with_other_controls_escaped — \r, \f, \b - test_windows_path_escaped_correctly — backslash doubling - test_atomic_write_no_temp_leak_on_success — no .config.toml.* left over after a successful write - test_atomic_write_cleanup_on_rename_failure — temp file removed when Path.replace raises (simulated disk full) 180 codex-runtime tests, all green (+6 from this commit). Footguns audited but NOT fixed (with rationale): - Concurrent migrations race. Two Hermes processes hitting /codex-runtime codex_app_server within seconds of each other could cause one writer to lose entries. Low probability (you'd have to enable from two surfaces simultaneously) and low impact (just re-run migration). Adding fcntl/msvcrt locking is more code than it's worth here. The atomic rename above means each individual write is consistent — only the merge step is racy. - Codex protocol version drift. We pin MIN_CODEX_VERSION=0.125 and check at runtime but don't reject too-new versions. Right call — the protocol has been stable through 0.125 → 0.130. If OpenAI breaks it later we'd see the error in test_codex_app_server_runtime on CI before users hit it.
jarvisxyz
pushed a commit
that referenced
this pull request
May 25, 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.
jarvisxyz
pushed a commit
that referenced
this pull request
Jun 1, 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>
jarvisxyz
pushed a commit
that referenced
this pull request
Jun 12, 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
jarvisxyz
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).
jarvisxyz
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>
jarvisxyz
pushed a commit
that referenced
this pull request
Aug 24, 2026
… loop When the Python interpreter begins teardown (user closes hermes, SIGTERM, OOM-kill), every executor-backed operation raises 'cannot schedule new futures after interpreter shutdown'. The outer except handler in run_conversation caught this error but did not recognize it as fatal — it kept retrying (API calls #4, #5, #6) until max_iterations, each time hitting the same dead executor and printing another traceback. The fix adds an early check: if sys.is_finalizing() or the error matches the 'cannot schedule new futures' pattern, break immediately with a clean interpreter_shutdown exit reason instead of retrying. The codebase already had this pattern in cron/scheduler.py and agent/tool_executor.py — the conversation loop just wasn't using it.
jarvisxyz
pushed a commit
that referenced
this pull request
Aug 24, 2026
…he shell When the TUI exits while the post-turn background review fork is still mid-request, every further API attempt raises 'cannot schedule new futures after interpreter shutdown'. The conversation loop treated this as a retryable API error: un-gated ❌ prints leaked onto the user's shell AFTER the TUI exited (call #4, #5, #6...) and the loop retried a doomed request until the interpreter froze the thread. Fix the class, not the site: - tools/interpreter_shutdown.py: single shared shutdown predicate (matches both CPython message variants + sys.is_finalizing()). - cron/scheduler.py, agent/tool_executor.py: existing per-site predicates now delegate to the shared home (tool_executor previously matched only the fuller variant). - agent/conversation_loop.py: inner retry handler recognizes the shutdown signal and abandons the turn — one log warning, no print, no traceback, no debug dump, no retry; outer handler gets the same guard for shutdown errors raised outside the API call. - The outer handler's bare print() now honors suppress_status_output (set by the background-review fork) instead of bypassing it. Refs NousResearch#55924 NousResearch#58720 (same class in cron delivery), adjacent to NousResearch#90683.
jarvisxyz
pushed a commit
that referenced
this pull request
Aug 26, 2026
…teway route Fixes NousResearch#92265 (proposed fix #2; #1 and #4 are separate follow-ups, see below). ensureGatewayForAgent() and ensureGatewayForProfile() both decided whether a secondary activation "succeeded" by checking Boolean(entry.connection) alone. entry.connection is set in openSecondary() BEFORE the WebSocket dial completes (`entry.connection = conn` happens ahead of `await entry.gateway.connect(wsUrl)`), so a transient first-dial failure -- caught by the surrounding try/catch and left for scheduleReconnect's backoff retry -- still left entry.connection truthy. Both functions then treated this as a successful activation: applyActive() switched g.activeKey and published $gateway to the closed socket, and publishActiveConnection() pushed the connection descriptor to the UI. The next chat RPC then failed with "Hermes gateway is not connected" against a route the user/desktop believed was live. Added an isOpen(entry.gateway) check alongside the existing Boolean(entry.connection) check in both functions' activation/publish conditions, gating BOTH applyActive() (which switches g.activeKey and publishes $gateway) and publishActiveConnection() (which pushes the connection descriptor) on the socket having actually reached 'open'. A failed first dial now correctly returns false / leaves the previous active route untouched, matching option 3 from the issue's own proposed fix ("if both bounded attempts fail, keep the existing active route") -- the existing scheduleReconnect backoff still owns recovery for that entry going forward. Not implemented in this PR (separate, lower-priority follow-ups): - Proposed fix #1 (one immediate bounded reconnect attempt before returning activation status) -- a larger behavioral change with its own retry/timing tradeoffs; left to a separate PR. - Proposed fix #4 (Bot Mode's own connection-ID-only guard in plugins/hermes-bots/plugin.js) -- host.ensureAgent() calls into the now-fixed gateway.ts functions, so this class of bug is already closed at the root; Bot Mode's own additional profile/state verification may still be worth adding but is a separate, narrower hardening pass on top of this fix. Found and fixed a genuine test-suite inconsistency while verifying: the existing "refreshes the active connection after a pooled profile reconnect succeeds" test in gateway-shared-remote.test.ts asserted setConnection was called once after a SINGLE ensureGatewayForProfile() call whose first dial failed -- i.e. it encoded the exact bug this issue reports as the EXPECTED, correct behavior. Rewrote it to assert the corrected contract: the failed first attempt does not call setConnection at all, and a realistic retry (calling ensureGatewayForProfile() again, since g.activeKey correctly never left the primary after the failed attempt -- ensureActiveGatewayOpen() is for reconnecting an already-active gateway that went stale, not retrying an activation that never succeeded) succeeds and publishes once the second dial goes through. Added a new test file (gateway-secondary-open-check.test.ts) following the established mocking pattern from gateway-agent-scope.test.ts, covering both ensureGatewayForAgent and ensureGatewayForProfile: a transient first-dial failure does not activate/publish (the exact reported symptom), and a successful dial still activates/publishes normally (sanity, no regression to the happy path). Verified as genuine regressions by reverting both isOpen() checks and confirming 2 of 4 new tests fail with exactly the reported symptom (activated resolves true / the primary gets replaced despite the failed dial). 44/44 pass across all 9 gateway-related test files (no regression). Dupe-swarm winner for issue NousResearch#92265; Biotrioo (PR NousResearch#92307) was the earliest submitter of the swarm and deserves first-report credit.
jarvisxyz
pushed a commit
that referenced
this pull request
Sep 8, 2026
…s (P5) (NousResearch#99220) * fix(relay): authorize send_message targets and surface egress declines P5 of the relay egress-authorization workstream. The relay path authenticated the SENDER but never authorized the DESTINATION, and the gateway compounded it from both ends. (a) send_message could silently name an arbitrary relay target. Its `target` parameter is free-form ('platform:chat_id'), so a model could name ANY chat id and the gateway would emit an outbound frame for it. gateway/relay/egress.py adds an attestation floor: a relay-routed destination must have a provenance this gateway can show -- the operator's home channel, the channel directory, or its own gateway session origins. Anything else is refused HERE, with a visible tool error naming the target, before a frame is written. Non-relay platforms and platforms served by a live native adapter in this process are untouched (same precedence resolve_delivery_transport applies). (b) Connector declines were swallowed into apparent successes. The connector's egress floor answers an unauthorized destination with a DEFINITE failure whose text is deliberately uniform (F-005). Several relay lanes degrade a *transport drop* by design and were degrading an *authorization refusal* the same way: - _send_media returned None, sending the caller into BasePlatformAdapter's text fallback -- a DIFFERENT op re-addressed at the very chat the connector had just refused. - _send_prompt returned None, so exec-approval / slash-confirm / clarify reported "relay prompt op unavailable" (a wrong reason) and ran their numbered-text fallbacks into the refused chat. - task_card_stop discarded the error entirely. - typing / delete / react / thread ops degraded silently at debug. is_egress_decline() classifies THAT a decline happened (never why -- the uniform text is not parsed for reasons) and requires a definite, non-ambiguous failure, so a lost-ack retry is still a transport outcome. Lanes with an error-carrying contract now report the decline verbatim; cosmetic bool/None lanes still degrade but log it at WARNING. Advisory progress drops that legitimately degrade are unchanged: the task_card send lane, the draft ambiguous/except branches, and every transport-exception path keep their existing fail-open behaviour. Tests: 21 mutations of the production source, all KILLED. * fix(relay): authorize the RESOLVED target; declines must not fall back Review round 1 (independently confirmed by a second reviewer) found three blockers. Two are fixed here; the third (B-2, Telegram @username) is a policy decision left open deliberately. B-1 — THE FIX CAUSED THE OUTAGE IT PREVENTED (tools/send_message_tool.py) The P5(a) guard ran ABOVE Slack user->DM resolution, so it authorized the internal pseudo-id `_parse_target_ref` emits (`user_name:ben`, `user:U...`). Provenances only ever hold RESOLVED conversation ids, so a fully attested DM was compared as a handle against a set of `D...` ids and refused: base slack:@ben SENT head(before) slack:@ben REFUSED Every Slack DM by handle was broken. Moved the guard below resolution; it now authorizes the destination that is actually sent to, and the refusal names the resolved id. Position is load-bearing, so it is commented as such and pinned: reverting the move turns exactly the four new cases red. B-3 — A DECLINE IS NOT A LANE FAILURE (gateway/run.py) `_approval_send_outcome` had only sent/failed/ambiguous, so a connector decline collapsed into `failed` — which is the cue to run the plain-text fallback into the chat the connector had just refused. The adapter fix in the previous commit improved the error STRING while user-visible behaviour stayed identical to base; the commit message overstated it. Fixed properly: - new `declined` verdict, recognised via the shared `is_egress_decline` contract (not string sniffing at the call site) - exec-approval returns without the text fallback - slash-confirm suppresses the text reply AND clears the registration, so a card that never rendered cannot capture the user's next message `send_clarify` was already correct (returns early inside the adapter). MUTATIONS (production source; both directions) classifier never returns 'declined' -> KILLED (4 cases) ALL failures classified as 'declined' -> KILLED (2 cases) guard moved back above Slack resolution -> KILLED (4 cases) decline CODE changed (review M05) -> KILLED marker match made case-sensitive (M10) -> KILLED M05 was a tautology: the test asserted the imported constant against itself, so changing the constant could not fail it. The wire contract is now pinned as a literal, because the connector stamps that exact string and a one-sided change is a silent cross-repo break. REGRESSION CHECK: the 12 failures + 1 collection error in this test selection are PRE-EXISTING cross-test contamination — the identical set fails at 7cf86188ac. Verified by diffing the failing sets: no new failures, 363 -> 374 passed. NOT FIXED (deliberate): B-2, Telegram `@username`. The Bot API resolves handles at send time, so there is no id to compare and no canonicalization exists yet. That is a policy decision, not a code move. * fix(relay): fail CLOSED on guard faults; classify the structured decline Third independent review. Two more blockers, both reproduced before fixing. 1. THE GUARD ITSELF FAILED OPEN (tools/send_message_tool.py:158) `_authorize_relay_target` wrapped BOTH the import and the call in one `except Exception: return None` — and None means AUTHORIZED at every call site. So any runtime bug inside the guard silently switched the entire P5(a) boundary off. Reproduced: with the guard raising, an unattested target sent. The docstring already stated the correct intent ("must not fail closed on its own IMPORT error") and the code did something broader. The two failures are not the same: a missing gateway package means there is no relay egress to authorize; a fault inside the guard means authorization did not happen. The import is tolerated, the call is not — a guard that cannot answer refuses. 2. THE STRUCTURED DECLINE WAS THROWN AWAY (gateway/run.py) The adapter preserves the connector's dict in `SendResult.raw_response`. My previous commit rebuilt a dict from the error STRING, which loses two contracts: * a decline carrying `code: egress_declined` and NO text renders as "relay egress declined" — no marker colon — so it classified as `failed`, which is exactly the cue to run the fallback into the refused chat; * `ambiguous: True` (lost ack) was flattened into a DEFINITE failure, re-sending a card that may already be on the user's screen. That is the duplicate-card bug the ambiguous verdict exists to prevent, reintroduced by the fix meant to harden the same path. Both call sites now classify `raw_response` when present, ambiguity first, and fall back to the wire sentence only for connectors that send no structured response. I had fixed the text-marker path and tested only the text-marker path. Worth naming: the review's probe was a shape my tests never produced. MUTATIONS (production source) guard fault returns None (fail open again) -> KILLED classifier ignores raw_response -> KILLED (3 cases) ambiguous treated as a definite failure -> KILLED (2 cases) 40 focused tests pass. Regression check vs be321faf27: identical 13-item failing set (pre-existing cross-test contamination), no new failures. STILL OPEN: B-2 / finding 3, Telegram `@username`. The reviewer is right that this is a REGRESSION of an existing contract (#53573 added Bot API username support), not merely an unspecified input, since relay provenance stores the numeric chat id. Fixing it means resolving the handle before authorization, or explicitly revoking the contract. That is a policy decision, not a code move, and it is Ben's call. * test(relay): pin M21 and M25, the survivors whose comments called them load-bearing Round-2 review reported six unpinned survivors from round 1. Two guard real behaviour and are now covered; the other four are cosmetic-lane warnings and fail-open branches I am leaving documented rather than pretending to close. M25 — thread-qualified session ids. `_session_ids` adds BOTH "chat:thread" and the bare chat, because the connector authorizes the CHAT. Without the split a gateway whose session origin is `-100999:77` cannot send to `-100999`, the chat it is demonstrably already talking in. KILLED. M21 — the generic `relay` plane must union every fronted platform, since a relay session is filed under its LOGICAL platform. KILLED. MY FIRST M21 TEST WAS THE DEFECT IT WAS TESTING FOR. I patched `_relay_fronted` — the very function the mutation empties — so emptying it changed nothing the test could see, and the mutation SURVIVED against a green test. Rewritten to drive the real `relay_fronted_platforms()` through its env source (`GATEWAY_RELAY_PLATFORMS`), which is how production learns it. That is the same "the test verifies my stand-in" failure I have spent this workstream removing from the connector harnesses, reproduced here in three lines of Python. The tell was identical: a mutation that survives a test written specifically to kill it. 334 tests pass. NOT PINNED, deliberately: M03 (success-guard on a malformed dict), M24 (empty-target allowance — the one fail-open branch, reachable only when the bare-platform path already resolved a home channel), M35/M36 (decline WARNINGs on cosmetic lanes). All four are observability or defence-in-depth rather than authorization, and the review agrees they are non-blocking. * fix(relay): defer Telegram @username authorization to the connector (B-2) Closes the last blocker. Two reviewers independently called this a REGRESSION of the public-channel username support added in #53573, not an unspecified input, and they were right: provenance stores RESOLVED numeric chat ids, so comparing `@channel` against them could only ever refuse. WHY THE GATEWAY CANNOT ANSWER IT. The guard fires only when there is no live native adapter — i.e. relay-fronted deployments — and on exactly those the CONNECTOR holds the bot token, not this process. There is no local way to turn a handle into the numeric id. Refusing here is not "fail closed", it is "fail always". WHY DEFERRING IS SAFE. The destination is still authorized one layer out: the connector's Telegram egress floor (gg#238, merged 743a7c2) classifies and refuses unauthorized destinations after ITS resolution — the layer that closed the reported vulnerability in the first place. Handles go from two guards to one, the authoritative one, not to zero. The carve-out is deliberately narrow and its EDGES are pinned, because the failure mode of an exemption is silent widening: telegram `@handle` -> deferred (the regression case) telegram numeric id -> still guarded matrix `@user:server` -> still guarded (telegram-only) bare name, no `@` -> still guarded attested handle -> normal path, attestation still consulted MUTATIONS carve-out widened to all platforms -> KILLED carve-out widened to every target -> KILLED carve-out removed (regression back) -> KILLED carve-out checked BEFORE attestation -> KILLED THE ORDERING MUTANT SURVIVED MY FIRST TEST. Both orderings return None, so asserting the verdict could not tell them apart — the test asserted the claim instead of the mechanism. Rewritten to observe that attestation is actually consulted. Same defect class as the M21 test earlier in this branch: a mutation surviving a test written specifically to kill it means the test is measuring the wrong thing. 341 tests pass. FOLLOW-UP (option 2, Ben's call, deliberately NOT done here): resolve the handle before authorizing so BOTH layers apply. That needs a resolution round-trip through the connector — new wire surface — so it belongs in its own phase rather than bolted onto this one. Recorded in the code comment at the carve-out, not just here. * fix(relay): close two fail-open boundaries; test the code-only decline for real Both blockers from review, each REPRODUCED before fixing. 1. STRUCTURED DECLINE HAD NO GUARD. Deleting `raw_response=result` from both `_send_prompt` return branches left all 34 tests green — a surviving, non-equivalent security mutant. The `code` field is the documented PREFERRED signal precisely because a connector may send no prose, and a caller rebuilding `{"success": False, "error": ...}` cannot see it. Cause: every existing case declines with marker TEXT. The evidence for the code-only path was a hand-built SimpleNamespace in a different file — a stand-in for the adapter, so it verified my fixture instead of production. Fixed with a CodeOnlyDecliningConnector driving the real `send_exec_approval` -> `_send_prompt`, feeding the REAL SendResult to the REAL `_approval_send_outcome`, plus the same shape on the media lane. drop raw_response SURVIVED (34 passed) -> KILLED 2. TWO FAIL-OPEN BOUNDARIES, both "absence" and "fault" sharing a return. `_relay_fronted` swallowed EVERY exception and returned an empty set, which `relay_routed_platform` reads as "not relay-routed" — skipping the guard. Probe, with a positive control in the same run: positive_control_denied = True discovery_fault_denied = False <- unattested target AUTHORIZED `_authorize_relay_target` caught every exception during IMPORT as "no gateway package". A module that exists and fails to initialize is a fault, not an absence, and returning None there means authorized. Now: ImportError alone is absence; anything else raises RelayRouteUnknown and `authorize_relay_target` converts it to a REFUSAL STRING (not a raised exception — every caller treats the return value as the verdict, so raising would trade a fail-open for a crash). Kept the converse under test so "fail closed" does not silently become "refuse everything in CLI/cron", which is the outage the broad except existed to prevent. discovery fault -> empty set KILLED RelayRouteUnknown -> authorized KILLED import fault -> authorized KILLED 397 passed (was 392, +5 new cases), zero failures. * fix(relay): close all seven review-round-3 blockers Every finding reproduced before fixing; every fix mutation-checked after. CONTENT LEAKS (the decline was laundered into a different op, same chat) #1 A declined DRAFT SEAL replayed as a plain send. On stream-is-the-message platforms the turn-final becomes draft(final=True); `_seal_open_draft` dropped the structured body, so `_absorb_into_open_draft` read a REFUSAL as a lane failure and fell through. Probe, Slack descriptor: before: draft(partial) -> draft(final,SECRET) -> send(SECRET) after: draft(partial) -> draft(final,SECRET) My first probe of this used a discord descriptor and showed no seal at all — the leak is real, my probe was wrong (streams only arm for Slack). #6 Task-card PROGRESS had the same defect one lane over: a bare failed SendResult reads as "card lane unavailable", and TurnRunner then sends the task text to the same chat. Both card methods now carry raw_response and the caller suppresses the fallback on a decline. AUTHORIZATION BYPASSES #2 `except ImportError` was NOT the fix I claimed last round. ImportError also covers a broken dependency inside an INSTALLED gateway; review probed `ImportError.name = "gateway.relay.dependency"` and got an authorized verdict. Now only a name identifying the gateway relay module itself is absence. An ImportError with NO name stays absence — refusing on a fault we cannot attribute would trade an unidentifiable bug for a real CLI/cron outage, and an existing test caught exactly that when I first got it wrong. #3 `relay_routed_platform` lowercases the requested platform; `_relay_fronted` returned configured names verbatim. A platform configured as "Discord" missed the membership test, looked native, and skipped the guard: 'discord' => refused 'Discord' => ALLOWED 'DISCORD' => ALLOWED An attestation bypass on a string comparison. UNDELIVERABLE PROMPTS THAT HUNG #4 `_clarify_send_disposition` handled `failed` and `ambiguous` but not `declined`, so a REFUSED clarify card fell through to wait_for_response and blocked until clarify_timeout — indefinitely when configured non-positive. A decline is more definitive than a failure, not less. #5 The exec-approval decline branch returned quietly, which suppressed the text fallback (right) but left the CENTRAL approval entry pending (wrong) — the dangerous command stayed blocked until the approval timeout. My comment claimed the registration was torn down; only RelayAdapter's private map was. It now raises `_ExecApprovalDeclined`, which propagates to `_await_gateway_decision`'s existing notify-failure path (drops the entry, unblocks the tool). A dedicated type, re-raised past the local `except Exception` that would otherwise have restored the leak. #7 THE GAP THAT LET ALL OF THIS SHIP. Both caller-level suppressions were unfalsifiable: deleting either branch left 36/38 tests green. The suites drove `_approval_send_outcome` and `RelayAdapter` but never the real TurnRunner / busy-session callers, so nothing observed whether a text send FOLLOWED a decline — which is the whole property. tests/gateway/test_decline_fallback_suppression.py drives both real callers and records every send. Each decline case is paired with an ordinary-FAILURE control, because without one a caller that never falls back would also pass. MUTATIONS (all on production source, anchors count-checked, restored after) #1 seal decline -> plain send KILLED #1b seal drops raw_response KILLED #2 nested ImportError -> authorized KILLED #3 fronted set not normalized KILLED #4 clarify declined branch removed KILLED #5 approval decline returns not raises KILLED #6 task_card drops raw_response KILLED #7 slash-confirm suppression removed KILLED #7's two were the reviewer's SURVIVORS (36/38 passing); both now die. 425 passed, zero failures. * fix(relay): close the three round-4 blockers Round 4 confirmed six of seven round-3 fixes and found three more. Each reproduced before fixing, each mutation-checked after. 1. A NAMELESS ImportError still authorized. Last round I admitted it as "absence" to protect the CLI/cron path. That reasoning was WRONG and the interpreter says so: import gateway.relay.nope -> ModuleNotFoundError, name="gateway.relay.nope" import totally_absent_pkg -> ModuleNotFoundError, name="totally_absent_pkg" Genuine absence is ALWAYS ModuleNotFoundError with `.name` set, so the CLI/cron path never produces a bare ImportError and nothing legitimate was being protected. A plain or nameless ImportError comes from an import hook or a module that failed while initializing — an unattributable FAULT. Now: absence is ModuleNotFoundError naming gateway / gateway.relay / gateway.relay.egress; everything else refuses. Two existing tests raised a bare ImportError to simulate absence and were corrected to the real shape. 2. SESSION ATTESTATION INVENTED IDS. `_session_ids` split every id on the first colon to recover "chat" from "chat:thread". Matrix ids contain a colon natively, so `!room:server.org` attested a bare `!room` — the guard vouching for a destination on its own fabrication. The split now applies only to platforms whose ids genuinely carry a `:thread` suffix (allow-list; unknown platforms are treated as un-splittable, which can only refuse more). Kept a Slack control: dropping the split entirely would refuse legitimate thread replies, which is the outage the split exists to prevent. 3. THE TASK-CARD FIX WAS UNFALSIFIABLE — my own round-3 mistake, and the same one round 3 caught me making. I added the production branch AND a test, but the test stopped at RelayAdapter: it proved `raw_response` is carried and never called `TurnRunner._task_card_publish`, which owns the property. Deleting the real branch left 30 tests green. Now driven through the real caller, with an ordinary-failure control. The lesson generalises: proving the DATA reaches the boundary is not proving the CALLER acts on it. Every one of these decline fixes has two halves and the second half is where the security lives. Also closed the round-4 non-blocking finding: `gateway/relay/egress.py` has its OWN import boundary, and the existing test intercepted the earlier import in tools/send_message_tool.py, so it was never exercised. Mutating that classifier to treat every ImportError as absence now dies. MUTATIONS (production source, anchors count-checked, restored after) R4-1 nameless ImportError -> authorized KILLED R4-2 session split unconditional KILLED R4-3 task-card caller branch removed KILLED (was SURVIVED) egress classifier: any ImportError = absence KILLED Also probed and found NOT a leak: a refused OPENING draft frame disarms the stream and the turn-final goes out via `send`. That send is itself guarded and the connector refuses it too, so no content is delivered — unlike the seal case (round 3, #1) where the seal was the only check on that path. 452 passed, zero failures. * fix(relay): recover the thread parent from thread_id, not a colon split Round 4 blocker 2 was closed with an allow-list of platforms whose ids have no native colon. Reviewing my own fix while round 5 ran, the allow-list is the wrong mechanism: it NARROWS a guess instead of removing it, and it still gets Matrix wrong the moment a Matrix session is thread-qualified (`!room:server.org:$thr` -> split yields `!room`). The structured field was there all along. `_session_entry_id` composes the id as f"{chat_id}:{thread_id}" and the entry still carries `thread_id` separately, so the parent is knowable EXACTLY: strip the known suffix, or add nothing. No platform list, no guessing, correct for ids that contain colons. Mutations: back to splitting on the first colon KILLED thread parent never recovered (over-refuse) KILLED Both directions matter: the first invents attestations, the second refuses legitimate thread replies. One existing test (M25) asserted the right PROPERTY with a fixture that omitted `thread_id` — a shape real entries never have. Fixture corrected, assertions untouched. 453 passed. * fix(relay): close the four round-5 blockers Each reproduced before fixing, each mutation-checked after. R5-1 A DISABLED NATIVE ADAPTER BYPASSED AUTHORIZATION. `_has_live_native_adapter` treated any entry in the adapter map as native; `resolve_delivery_transport` ignores a native adapter whose config is disabled and routes over Relay. Two independent routing classifiers, disagreeing: guard says native: True delivery routes relay: True So the guard skipped authorization for a send that went over the relay. The guard now applies the router's enabled-state rule; probed both configurations and they agree. R5-2 THREAD IDS WERE NEVER AUTHORIZED. The parser splits chat_id and thread_id; only chat_id reached the guard. On Discord the thread IS the destination — `POST /channels/{thread_id}/messages` — so an attested parent channel authorized an arbitrary caller-supplied thread. `authorize_relay_target` now takes thread_id and requires its own attestation (bare id or the `chat:thread` form a session origin produces); both call sites forward it. R5-3 A DECLINED **INITIAL** DRAFT WAS RETRIED AS A PLAIN SEND. Round 3 fixed the declined SEAL; the declined OPEN was a different path. `send_draft` returned a bare failure, so the stream consumer read "draft transport unusable", disabled drafts and fell through to `_first_send`. Measured through the real adapter and real StreamTransportMixin: before: ops ['draft', 'send'] after: ops ['draft'] send_draft now carries raw_response; a decline is terminal for the run and the guard sits in `_first_send`, where every fallback path converges. R5-4 MY ROUND-4 TASK-CARD FIX SUPPRESSED EXACTLY ONE UPDATE. It set `native_failed`, which the entry gate already uses for an ordinary broken lane, so the next progress event skipped the decline branch and went straight to the text fallback: after first publish: [] after second: ['send'] Terminal declines are now a separate `egress_declined` state checked at the entry gate. A refusal does not expire after one tick. MUTATIONS R5-1 disabled native counts as native KILLED R5-2 thread_id not authorized KILLED R5-2b tool does not forward thread_id KILLED (was SURVIVED) R5-3 initial-draft decline not terminal KILLED R5-3b _first_send guard removed KILLED R5-4 declined state not persistent KILLED R5-2b is the same gap that produced findings 3 and 4 of the last two rounds, a third time: every test called `authorize_relay_target` directly, so dropping the argument from the TOOL WRAPPER changed nothing. Testing the callee never proves the caller uses it — now pinned explicitly. Each fix ships with an ordinary-failure control, because every one of these makes the guard refuse MORE, and over-refusal is now the larger risk. 474 passed, zero failures. * refactor(relay): declare the terminal-decline state where it lives Both terminal-decline flags were set dynamically. They worked (neither class is frozen or slotted) but an undeclared attribute hides the state from anyone reading the class, and this one is security-relevant. _TaskCardState.egress_declined — declared dataclass field StreamConsumer._egress_declined — initialised in __init__ Lifetime verified while checking whether a refusal can leak ACROSS turns and mute a healthy destination: it cannot. _TaskCardState is constructed per progress-drain (run_turn_runner.py:420) and the consumer's flags per run (stream_consumer.py:163), so both are fresh each turn. Also verified the guard's blast radius after adding thread authorization: the ONLY callers of authorize_relay_target are the two model-facing send_message call sites. Gateway-internal sends — notably the handoff path, which creates a thread and immediately posts to it with no session provenance yet — go through transport.adapter directly and are unaffected. That was the most plausible over-refusal, and it does not reach this guard. 461 passed. * fix(relay): close the four round-6 blockers — the edit lane R6-1 MY OWN R5-1 FIX REINTRODUCED THE BYPASS IT CLOSED. I wrote `except Exception: return True` around the config lookup, so a config read fault declared the platform native while the ROUTER, reading the real config, sends over the relay: guard_has_live_native True guard_verdict None router relay Routing we cannot determine is UNKNOWN. It now raises RelayRouteUnknown, which the outer handler must re-raise rather than flatten to False, and `authorize_relay_target` turns into a refusal. This is the second time a convenience `except` in this function created a bypass; there is now no permissive return left in it. R6-2/3/4 THE NINTH LANE: `edit`. ONE dropped field, THREE leaks. `RelayAdapter.edit_message` discarded the connector response, and three independent callers read a bare edit failure as "editing is unavailable" and re-send the content as a NEW message to the same chat: stream edit fallback ['edit', 'edit', 'send'] the unseen tail queued reconciliation ['edit', 'send'] the WHOLE response task-card fallback ['edit', 'send'] the task text again Fixed at the source (edit_message carries raw_response) plus each caller: `_on_edit_failure` — the single funnel for stream edit failures — makes a decline terminal for the run, `_send_fallback_final` refuses to deliver a continuation after one, the queued reconciler returns instead of sending, and the task-card fallback sets the same terminal state R5-4 introduced. R5-4 fixed the native task-card op and I did not check its sibling fallback path. The pattern across rounds 3-6 is consistent: the fix goes where the decline is OBSERVED, and the leak lives wherever someone else later decides to retry. MUTATIONS R6-1 config fault -> assume native KILLED R6-1b RelayRouteUnknown swallowed as False KILLED R6-2 edit drops raw_response KILLED R6-2b edit-failure decline not terminal KILLED R6-3 queued reconcile falls back on decline KILLED R6-4 task-card fallback edit decline KILLED Each with an ordinary-failure control: a genuinely un-editable message must still be delivered, and a broken card lane must still reach the user. 481 passed, zero failures. * fix(relay): add a terminal-decline latch at the adapter choke point THE STRUCTURAL FIX, not a twelfth local check. Rounds 3-6 of review found ONE defect in eleven lanes: the connector refuses an op, and some caller downstream reads that as 'this lane is unavailable' and retries the same content through a DIFFERENT op against the SAME chat. Media, prompt, draft-open, draft-seal, native task card, task-card fallback edit, slash-confirm, exec-approval, clarify, stream edit, queued reconciliation. Each was closed by adding a check at one more call site. That approach cannot converge: gateway/ has ~60 outbound call sites, every one of them a place a future change can reintroduce this, and four consecutive review rounds each found another. The reviewer's own count of lanes is the argument against the per-site design. Every relay frame from every one of those callers passes through _transport.send_outbound. One latch there covers them all: once the connector refuses a chat, this adapter stops emitting CONTENT frames for that chat. Proven to subsume the local checks: with the stream-edit per-site check DISABLED, the leak probe still reports blocked=true — the frame never reaches the wire. The local checks stay as defence in depth and for their better error messages, but they are no longer the only thing standing between a decline and a re-addressed send. Scope is deliberately narrow, and each limit is mutation-pinned: per CHAT - a refusal must not mute other conversations CONTENT ops - typing/delete carry nothing; latching them would leave a stuck typing indicator for no security gain self-healing - cleared when the connector accepts that chat again, so a transient policy change does not need a restart Mutations: latch never set KILLED latch never consulted KILLED latch is global, not per-chat KILLED latch never clears KILLED 485 passed. * fix(relay): one route source; the latch already covered round 7's lanes Round 7 reviewed 573e41e294 — one commit BEFORE the terminal-decline latch — and independently reached the same conclusion I had: 'The per-call-site approach is structurally wrong. Use one turn-scoped choke point.' That is the latch in 6dbc004594. Its four 'still broken' lanes (tool-progress edit, progress-overflow edit, long-running heartbeat edit, stale streamed-final reconciliation) all share the shape edit_message->declined->adapter.send(same chat, same content), and NONE has a local check. Probed all four against the latch: tool_progress ops ['edit'] blocked progress_overflow ops ['edit'] blocked heartbeat ops ['edit'] blocked stale_final ops ['edit'] blocked That is the argument for the choke point, measured: lanes nobody patched are safe anyway. Pinned by a parametrized test named for those four lanes. R7-1 IS A REAL BYPASS THE LATCH DOES NOT COVER, and it is fixed here. The guard rebuilt routing from GATEWAY_RELAY_PLATFORMS while resolve_delivery_transport asks the CONNECTED adapter (fronts_platform, from the handshake identity set). Different snapshots: with env discovery stale or momentarily empty, the guard said 'native' and the router sent over the relay, skipping authorization. before: guard_relay_routed False / delivery relay after: guard_relay_routed True / delivery relay / unattested target refused The guard now asks the live adapter first and falls back to config only when there is no runner (CLI/cron) — pinned in both directions. R7-5 (non-blocking, and a fair hit): my stream-fallback test asserted _egress_declined and never drove _send_fallback_final, so removing that early return SURVIVED. The test now calls the real fallback and asserts the wire is untouched; the mutation dies. Mutations: R7-1 guard ignores the live adapter KILLED (was SURVIVED) R7-5 fallback early return removed KILLED (was SURVIVED) latch not consulted KILLED 491 passed. * fix(relay): close three holes found by attacking my own latch Round 8's brief told the reviewer to attack the latch. I did the same in parallel and found three real holes in it before the review returned. 1. send_for_platform BYPASSED THE LATCH ENTIRELY. It builds and posts its frame directly rather than through _outbound — and it is the delivery resolver's OWN entry point, so it is the single most important caller. before: ops ['edit', 'send'] after: ops ['edit'] gateway/AGENTS.md states the rule I had just broken: 'Seal-interception exists at BOTH egress doors (send() and send_for_platform()); a new egress door needs the same two checks.' The latch is a third such check and I had wired it to one door. 2. A COSMETIC SUCCESS CLEARED THE LATCH. Clearing on ANY success meant a typing indicator — routinely allowed for a chat whose content is refused — re-opened the door for the very next send: ops ['edit', 'typing', 'send'] Only a CONTENT op the connector accepted may clear it now. 3. A THREAD INSIDE A REFUSED CHAT WAS NOT COVERED. A thread lives inside its parent, so the same content reached the same conversation one level down: ops ['edit', 'send'] The latch key now strips the thread suffix. Also normalised int/str chat ids (callers pass both; a type mismatch would silently unlatch). MUTATIONS send_for_platform not latched KILLED cosmetic success clears the latch KILLED thread suffix not stripped KILLED draft-seal retry not latched SURVIVED — EQUIVALENT, proven: is unreachable while latched (a declined edit before the seal produces ZERO seal frames, measured). Kept as defence in depth because it posts directly, and documented at the site rather than covered by a test that could not fail. One self-inflicted bug on the way: a blanket replace put 1Password CLI brings 1Password to your terminal. Turn on the 1Password app integration and sign in to get started. Run 'op signin --help' to learn more. For more help, read our documentation: https://www.1password.dev/cli 1Password CLI is built using open-source software. View our credits and licenses: https://downloads.1password.com/op/credits/stable/credits.html Usage: op [command] [flags] Management Commands: account Manage your locally configured 1Password accounts connect Manage Connect server instances and tokens in your 1Password account document Perform CRUD operations on Document items in your vaults events-api Manage Events API integrations in your 1Password account group Manage the groups in your 1Password account item Perform CRUD operations on the 1Password items in your vaults plugin Manage the shell plugins you use to authenticate third-party CLIs service-account Manage service accounts user Manage users within this 1Password account vault Manage permissions and perform CRUD operations on your 1Password vaults Commands: completion Generate shell completion information inject Inject secrets into a config file read Read a secret reference run Pass secrets as environment variables to a process signin Sign in to a 1Password account signout Sign out of a 1Password account update Check for and download updates. whoami Get information about a signed-in account Global Flags: --account account Select the account to execute the command by account shorthand, sign-in address, account ID, or user ID. For a list of available accounts, run 'op account list'. Can be set as the OP_ACCOUNT environment variable. --cache Store and use cached information. Caching is enabled by default on UNIX-like systems. Caching is not available on Windows. Options: true, false. Can also be set with the OP_CACHE environment variable. (default true) --config directory Use this configuration directory. --debug Enable debug mode. Can also be enabled by setting the OP_DEBUG environment variable to true. --encoding type Use this character encoding type. Default: UTF-8. Supported: SHIFT_JIS, gbk. --format string Use this output format. Can be 'human-readable' or 'json'. Can be set as the OP_FORMAT environment variable. (default "human-readable") -h, --help Get help for op. --iso-timestamps Format timestamps according to ISO 8601 / RFC 3339. Can be set as the OP_ISO_TIMESTAMPS environment variable. --no-color Print output without color. --session token Authenticate with this session token. 1Password CLI outputs session tokens for successful 'op signin' commands when 1Password app integration is not enabled. -v, --version version for op Run 'op [command] --help' for more information on the command. into send_for_platform, which has no such variable. Two existing unfurl tests caught it — NameError at adapter.py:1407. 504 passed. * fix(relay): Telegram handle exemption + a turn boundary for the latch Round 8 blockers. Two of its four were already closed by 93750e351a (it reviewed the commit before it); these two are real and both are mine. B1 — THE TELEGRAM @HANDLE EXEMPTION COVERED A NATIVE SEND. _is_unresolved_handle exempts telegram @handles from attestation because "the connector resolves and authorizes it". That justification is FALSE whenever the gateway holds its own token: _send_to_platform calls _send_telegram(pconfig.token, ...) directly and no connector is involved. So an unattested @handle went out under the gateway's own credential while the numeric control was correctly refused. The exemption now requires that no native credential exists. A probe fault WITHDRAWS the exemption (falls back to the ordinary attestation check) rather than granting it. Shipped with the converse control: relay-only config still exempts @handles, and numeric targets stay guarded in both modes. B4 — THE LATCH HAD NO BOUNDARY, SO IT WAS AN OUTAGE MECHANISM. My own regression, and worse than reported. Removing "clear on cosmetic success" (correctly) removed the ONLY way the latch could ever clear: a content op can never reach the connector to succeed, because the latch blocks it locally first. A refusal at 09:00 muted that chat forever. A new inbound message for a chat is the generation marker — the natural teardown point. Suppression still holds for the whole turn. same_turn_blocked: true next_turn_delivered: true MUTATIONS (all killed) handle exemption ignores native credential native-credential fault GRANTS the exemption no turn boundary (latch never clears) teardown clears ALL chats not just this one teardown ignores the chat The last two SURVIVED first: I tested _clear_declined_for_turn directly and never proved _on_inbound calls it — the caller-level gap that has now produced four blockers on this branch. Added a test driving the real inbound entry point. One self-inflicted bug, caught by my own fault test: the probe imported load_config, which does not exist (it is load_gateway_config), so it always threw and returned the fault default. The test that pinned fault behaviour is what exposed it. 510 passed. * fix(relay): correct latch identity and boundary; one config snapshot Round 9, four blockers, all reproduced. B1+B4 — THE TEARDOWN WAS AT THE WRONG PLACE, twice over. It sat on the adapter's raw _on_inbound, which runs BEFORE profile routing, the ignored-channel guard, plugin hooks and user authorization. An unauthorized or dropped event could therefore clear a refusal belonging to an active turn, and stale content then went out as a different op. The same placement missed Discord interaction passthrough, which builds its own MessageEvent and calls handle_message directly, so slash commands and modal submits stayed muted after an earlier decline. Both are one mistake: I picked a lane instead of a boundary. Teardown now runs immediately after _hm_admit_event, the single admission gate every entry path shares. dropped event -> latch survives, stale send blocked admitted event -> latch clears B2 — THE LATCH KEY SPLIT ON ':', WHICH IS A MISTAKE I ALREADY FIXED ONCE. _latch_key did str(chat_id).split(":", 1)[0], so !room:tenant-a and !room:tenant-b both keyed !room: a decline in one Matrix room muted another, and inbound from one cleared the other's refusal. egress.py ::_session_ids stopped doing exactly this in round 4 and I reintroduced it three rounds later. Parent identity is never recoverable from identifier TEXT. Thread coverage is now structural: _thread_parent looks the relationship up in the recorded auto-thread map. B3 — AUTHORIZATION AND DISPATCH USED DIFFERENT CONFIG SNAPSHOTS. _handle_send retains one pconfig; the guard independently reloaded config. Across a transition the authorization snapshot could see a connector-only setup (exemption granted) while dispatch still held the native token and sent the unattested @handle itself. The guard now takes native_token from the SAME snapshot dispatch will use. A caller that omits it does not silently look like "no token". NB-1/2/3 also closed: real-object snapshot tests, an exception shield that faces a real exception, and send_follow_up no longer discards the connector's verdict (that discard is exactly how the edit lane laundered declines). MUTATIONS (all killed) latch key splits on colon again thread parent lookup disabled dispatch token ignored by guard tool drops the snapshot token admission teardown removed teardown moved BEFORE admission exception shield removed follow_up drops raw_response "admission teardown removed" SURVIVED first: I had tested the helper, not _handle_message. Added a test driving production _handle_message with admission stubbed both ways. Fifth caller-level gap on this branch. One self-inflicted bug caught before commit: I passed pconfig.token in _handle_react, which has no pconfig — a NameError on every reaction. 516 passed. * docs(relay): pin the latch's thread coverage limit as a deliberate trade _thread_parent only sees connector auto-threads, and that map is capped at 256 entries, so a user-created or evicted thread does not inherit its parent's latch. Documented at the site and asserted by a test, because the alternative - deriving parents from identifier text - is exactly what muted unrelated Matrix rooms in round 9. The primary control is unaffected: authorize_relay_target takes thread_id as part of the destination and attests it on every send (6 thread tests). * refactor(relay): one SendResult decline classifier for all 8 gateway lanes The extraction found a DEFECT, not just repetition. Eight gateway lanes each hand-rolled the unwrapping of a decline from a SendResult, and they did not agree. Six checked only raw_response. Two also checked the error text. A connector that answers with the uniform decline SENTENCE and no structured code - the documented contract for older connectors, per _approval_send_outcome - was therefore classified as an ordinary failure by those six lanes, so each treated a refusal as "editing unavailable" and retried through another op. Measured: text-only decline six-site check False two-site check True structured decline six-site check True two-site check True No content leaked, because the adapter latch classifies the transport dict directly and catches both shapes (verified: text-only decline still latches C1 and keeps SECRET off the wire). The cost was wrong verdicts and futile retries, not disclosure. declined_send(result) in gateway/relay/egress.py now owns this. It checks raw_response when structured, else the error text, and preserves the ambiguous exclusion - an ambiguous result is a transport outcome, so it must never read as a refusal. run.py keeps its own shape deliberately: that lane has three verdicts (ambiguous / declined / failed), so it checks ambiguous first and then delegates the boolean. MUTATIONS (all killed) helper drops the text-only branch helper drops the structured branch ambiguous no longer excluded draft lane decline check removed edit-failure lane decline check removed prompt verdict lane check removed slash-confirm lane check removed draft lane goes terminal on ANY failure (over-refusal direction) "draft lane decline check removed" SURVIVED first: _send_draft_frame had no test driving an unsuccessful send_draft at all. Added one, with an ordinary-failure control so the fix cannot silently become "one flaky frame mutes the chat". A non-unique anchor also masked the edit-failure lane on the first pass - the trap my own skill warns about. This closes the duplication that caused four of nine rounds of blockers: a new lane now calls one classifier instead of copying three lines. 519 passed. * fix(relay): latch identity, new-turn boundary, seal arming, ambiguity Round 10, four blockers, each reproduced before fixing. Two are my own regressions from the previous two rounds. B1 - ADMISSION IS NOT A NEW-TURN BOUNDARY. Round 9 moved teardown to just after _hm_admit_event. That is only an ADMISSION gate: an authorized message can be steered into a running session, answer a pending prompt, run a busy slash command, or be refused by the pause/drain gates - all without starting a turn. Each of those cleared the ACTIVE turn's refusal, and a later fallback from that turn reached the wire (probe: latch emptied, wire ops ['edit', 'send']). Teardown now runs after _claim_active_session_slot, the first point the runner OWNS a new turn. The new test drives production _handle_message through all four non-turn lanes plus the real new-turn path. B2 - LATCH IDENTITY OMITTED THE LOGICAL PLATFORM. One relay adapter fronts several platforms, so native ids collide. A Discord refusal for chat 42 was cleared by clear_egress_latch("telegram", "42") - the method took a platform and ignored it - and the Discord fallback then reached the connector. Keyed by normalized platform plus exact chat id; thread-parent expansion keeps the platform component. B3 - THE DIRECT DRAFT-SEAL PATH DID NOT ARM THE LATCH. _seal_open_draft posts through _attempt directly rather than _outbound, so a definite decline logged and returned but never latched. The immediate plain-send fallback was suppressed by the caller's own check; later same-turn sends were not (wire ['draft', 'draft', 'send'], the third frame carrying refused content). B4 - MY OWN REFACTOR MADE AMBIGUOUS RESULTS TERMINAL. send_draft's ambiguous projection discarded raw_response, so declined_send fell through to the error-text branch - and an ambiguous result whose text carries the decline marker ("... egress declined: ack lost") read as a DEFINITE refusal and terminated the run. Ambiguous means the frame may well have been delivered: a transport outcome, never an authorization one. Fixed on both layers: the projection carries the body (and the seal's ambiguous return is now explicit too), and declined_send's text-only branch - which cannot see the ambiguous flag - treats ack-lost text as transport ambiguity. Audited every SendResult projection in adapter.py for the same shape. MUTATIONS (all killed) latch key drops the platform clear_egress_latch ignores platform draft seal does not arm the latch ambiguous projection drops raw body declined_send infers decline from ack-lost text teardown back at admission 523 passed. * refactor(relay): split the terminal-decline latch out of the guard PR The latch moves to feat/p5-egress-decline-latch (pushed at 3cf45736d7, which retains the full history) for redesign. This PR keeps the authorization guard and the per-site decline checks. WHY. Across eleven review rounds the two halves behaved very differently. The guard is a PURE FUNCTION of the destination - its blockers were all "you asked the wrong question" (case sensitivity, nested ImportError, missing thread_id, config snapshot skew), each a one-line correction that then stayed fixed. Rounds 7-10 found nothing new in it. The latch is MUTABLE STATE WITH A LIFETIME living on RelayAdapter - an object registered once per process that holds the WebSocket and has no concept of a turn. Nine of its blockers reduce to three questions the adapter cannot answer: when does it end, who arms it, what is it keyed on. Every answer so far has been a proxy (a successful op, an inbound message, an admitted event, a claimed session slot) and every proxy was wrong in a lane found later. The per-site checks hold identical information on `st` - a PER-TURN object - and have produced zero blockers, because the state dies with the turn and nobody has to decide when it ends. The no-relaunder property does NOT depend on the latch. Measured on the real consumer path with the latch absent: a declined draft frame sets _egress_declined and puts nothing on the wire. Removal verified structurally rather than by eye: an AST diff of every symbol between HEAD and this tree reports only latch symbols gone, nothing added. That check caught two over-deletions my strip made - _on_inbound (consumed by a "next def" boundary) and _SEEN_INBOUND_MAX (a class constant inside the removed span). Both restored; 19 failures went to 0. ALSO: RESTORED A TEST I WRONGLY REPORTED AS PASSING. test_tool_guard_forwards_thread_id never made it into the repo - `git log -S` finds it in no commit - though round 5 recorded its mutant as killed. Dropping thread_id from the guard call therefore survived the entire tests/tools suite (146 passed). Written properly this time, driving the real _handle_send far enough to reach the guard. It now KILLS that mutant. MUTATIONS on this tree guard fault authorizes instead of refusing KILLED thread_id dropped from the guard call KILLED (was SURVIVED) handle exemption ignores native credential KILLED draft lane decline check removed KILLED prompt verdict lane check removed KILLED slash-confirm lane check removed KILLED 503 passed. * test(relay): close the phantom-coverage gaps the guard audit found The thread_id test that was reported as killing a round-5 mutant turned out never to have been committed. That is a reason to distrust the other claimed kills, so I re-ran every guard mutation against the COMMITTED tree instead of trusting the earlier reports. Result: 9 of 11 killed, and the two "SKIPPED" ones had non-unique anchors hiding SIX separate sites. Mutating those individually found three real survivors. CASE NORMALISATION (round 3, finding 3) WAS HALF-COVERED. test_relay_fronted_matching_is_case_insensitive varies the CONFIGURED name but always requests lowercase "discord", so it pins _relay_fronted's normalisation and nothing else. The REQUESTED name's `.lower()` was covered by nothing at all. Probe with it removed: relay_routed("Discord") -> False authorize("Discord", unattested) -> AUTHORIZED which is exactly the bypass round 3 reported, alive again and untested. Two further sites were untested in the OVER-REFUSAL direction: the attested store is keyed lowercase, so a mixed-case request missed its own attested set and refused legitimate traffic. attested_relay_targets' own normalisation was invisible to every existing test because they all monkeypatch that function away; it is now asserted against the real function with only its leaf sources stubbed. Three tests added. All six case sites now die when mutated. I also re-did the three fail-closed RelayRouteUnknown mutations properly. The first pass swapped whole lines and produced IndentationErrors, so "KILLED" there proved nothing but a syntax error. Neutralising each raise at correct indentation: all three genuinely KILLED. FINAL AUDIT ON THIS TREE — 17 mutations, zero survivors guard: thread_id dropped at the call site guard: react path unguarded guard: handle exemption ignores native credential guard: 3x fail-closed raise neutralised guard: 6x case-normalisation site classifier: ambiguous treated as a decline classifier: text-only decline branch removed lane: draft / stream-edit / prompt / slash-confirm checks removed 511 passed. * test(relay): make the stream-edit test fail for the right reason Review of 45835a282d raised one blocking issue and three non-blocking ones. All four are addressed; none was a production defect. BLOCKING — the stream-edit test failed on the double, not on a leak. test_declined_stream_edit_does_not_send_the_unseen_tail implemented only the GUARDED path in its consumer double. Removing either guard therefore raised AttributeError inside the fake before any send could be observed: guard 1 removed -> AttributeError: no attribute '_is_flood_error' guard 2 removed -> AttributeError: no attribute '_clean_for_display' Red, but for the wrong reason — the test could not have caught the leak it is named for. My own docstring claimed it drove the fallback and checked the wire; it did neither. The double now implements everything the UNGUARDED path reaches (_is_flood_error, _flood_strikes, _current_edit_interval, _last_edit_time, _notify_new_message, _try_strip_cursor, _clean_for_display, _fallback_prefix, _metadata_for_send). Both mutations now fail on real assertions: guard 1 removed -> assert consumer._egress_declined is True guard 2 removed -> AssertionError: the unseen tail reached the wire: ['send'] NON-BLOCKING 1 — a docstring claimed more than the test exercises. test_requested_platform_name_is_also_normalised described a mixed-case send_message(target="Discord:999") bypass. That entry point cannot reach it: _resolve_tool_target lowercases the platform at tools/send_message_tool.py:47 before the guard runs. The test still pins a real contract — the helpers must not assume a lowercased argument, for the gateway lanes and any future non-normalising caller — so the claim is narrowed to that rather than the test removed. NON-BLOCKING 2 — the module docstring said "every lane drives the REAL RelayAdapter". The stream tests drive mixin doubles by design, because the behaviour under test belongs to the adapter's CALLER. Docstring now distinguishes the two kinds. NON-BLOCKING 3 — latch-deletion residue in gateway/relay/adapter.py:418: return None return latched if surface_declines else None The second line was unreachable and referenced a name deleted with the latch. Removed, along with the 20-line comment block describing the latch as "the structural fix" — that mechanism now lives on feat/p5-egress-decline-latch, not here. The reviewer independently confirmed the large deletion: an AST census between 3cf45736d7 and f57a2298fa reports only latch symbols removed and nothing added. 511 passed. * docs(relay): correct three claims that outran the code Review of 41ce3cc765 found no new production defect but three overstated claims, one of them in my own commit message. 1. THE LATCH COMMENTARY WAS STILL THERE. My previous commit message said it removed "the 20-line comment block describing the latch as the structural fix". It removed only the unreachable statement. Twenty lines at adapter.py:361-380 still described a per-chat latch, a choke point and its scope rules - none of which exist on this branch. In a refusal-sensitive module that reads as coverage this branch does not have. Now removed for real. This is the same defect class as the tests: a claim that outran what the code does. I made it while fixing that class. 2. THE STREAM-TEST DOCSTRING OVERSTATED BOTH MUTANTS. It said the mutation "now fails on the assertion that a send reached the wire" - true of one guard, not both. Verified separately: remove the _on_edit_failure check -> dies on _egress_declined, never reaches the fallback remove the fallback early return -> dies on the wire: ['send'] Both are valid behavioural failures, which is what the blocker asked for; they are different observables and the docstring now says so. 3. Duplicate `from types import SimpleNamespace` from an earlier scripted insert; imports reordered. 112 tests pass in the four focused files. * fix(relay): close two authorization defects found in review Both were reproduced before fixing and both mutants are pinned. 1. A LIVE relay adapter whose fronts_platform() raised degraded into the config fallback. `_live_relay_fronted` returned None for every failure, and None means "no live adapter, use the config snapshot" — so a faulting adapter plus an empty/stale snapshot made the guard conclude "not relay-routed" and authorize an unattested destination, while resolve_delivery_transport asks that same adapter and still routes over the relay. Measured: relay_routed=False, verdict None for chat 999. Absence and fault now have separate return values: None only when there is no runner or no relay adapter; a live adapter that cannot answer raises RelayRouteUnknown. This is the third instance of this bug class in this file, and the first two were also mine. 2. An attested chat whose id equalled the requested THREAD id vouched for that thread. The `thread in attested` arm proved nothing about parentage. Measured: attested {"-100A", "7"} authorized (-100A, thread 7). Only the bound `parent:thread` form is accepted now. Nothing legitimate needed the bare arm — _session_entry_id records a threaded origin as f"{chat_id}:{thread_id}", and a thread addressed as its own channel arrives as chat_id and passes the parent check. The existing test blessed the bare form via parametrize, so it PINNED the defect. Corrected, plus negative controls for the sibling-chat and other-parent cases and a positive control proving genuine absence still takes the config path (otherwise fix 1 would break native-only deploys). Merged origin/main (was 22 behind). 428 passed via scripts/run_tests.sh; full 10-row mutation ledger re-killed on the merged tree, none dying on an exception rather than an assertion. * fix(relay): only a missing adapter is absence; everything else is a fault Reviewer BLOCKER, reproduced before fixing. Two more paths where a PRESENT relay adapter still degraded into the config snapshot: 1. `fronts_platform` may be a property or descriptor, so the ATTRIBUTE LOOKUP can raise — and the lookup sat inside the absence handler. Probed with a raising property plus an empty snapshot: live=None, routed=False, verdict=None, i.e. an unattested target authorized. The previous test made an already-retrieved METHOD raise, so it could not reach this. 2. A present adapter with no usable `fronts_platform` returned None for the same reason. An adapter that cannot say what it fronts is broken, not absent, so it now raises too. Also found by my own spot-check while the review ran: the nested imports of `gateway.config` / `gateway.run` inside the live probe shared the broad handler, so a broken installation degraded to the snapshot as well. Probed with a healthy-adapter positive control in the same run — healthy refused the unattested target, faulted authorized it. `_relay_fronted` one function below already drew this exact distinction for its own import. The boundary is now: `relay is None` is the ONLY absence. Everything about a present adapter — attribute access, callability, the call itself, and the imports needed to reach it — is a fault and raises RelayRouteUnknown. This is the fourth variant of absence-vs-fault in this file and all four were mine. The lesson is in the code as a comment rather than in a commit message nobody re-reads. Four controls keep genuine absence benign: no runner, no relay adapter in the runner, a real ModuleNotFoundError naming the gateway package, and the configured-attested-target-still-sends case. 434 passed via scripts/run_tests.sh; 9-row mutation ledger re-killed including both new guards, none dying on an exception. * fix(relay): invert the live probe to fail closed by default Reviewer BLOCKER round 2, reproduced: reading the adapter registry can also raise. A runner whose `adapters.get()` raised gave relay_present=True, live=None, routed=False, verdict=None — unattested discord:999 authorized. That was the FIFTH boundary in one function with the same defect: the call, the attribute lookup, a non-callable attribute, the nested imports, and now the registry lookup. Each round I patched the reported boundary and the defect moved one statement up. The cause was the shape, not the statements: the function asked "did something go wrong?" and answered None, and None MEANS "no live adapter, use the config snapshot" — so every statement was a new chance to fail open, and every new statement would have been too. Inverted rather than patched a sixth time. Each `return None` now sits behind an explicit narrow check that cannot itself be the fault (no runner, no adapters, no relay key, gateway package genuinely absent), and one outer handler turns anything else into RelayRouteUnknown. A statement added inside this function is now fail-CLOSED by default. Verified all six fault shapes raise (call, attribute, missing method, registry .get, .adapters property, runner ref) and all five absence shapes stay benign, plus a liveness control where the config snapshot disagrees with a healthy adapter and the adapter still wins. Four new tests, including the two absence controls that keep native-only and CLI deployments working. 438 passed via scripts/run_tests.sh. Mutation ledger: 8 killed. One survivor recorded as a proven equivalent mutant — widening `if not registry` to `or {}` is behaviourally identical because `{}.get()` returns None, i.e. the same absence; it is a readability guard.
jarvisxyz
pushed a commit
that referenced
this pull request
Sep 16, 2026
… fingerprint never authorizes a signal NousResearch#111617 review (andrexibiza P1 #3/#4, kvnloo nit): - worker_started_at persisted only gateway.status.get_process_start_time(): on Linux that is /proc/<pid>/stat field 22, clock ticks since THIS boot. The threat is a row surviving a reboot, and that counter does not, so an unrelated process on a later boot with the same PID and the same tick value passed _start_times_agree(). The fingerprint is now "<gateway.drain_control.current_instantiation_epoch()>|<start>" (boot_id + PID-1 start, the witness the drain marker already uses); both halves must match. Integer values on rows written before this change keep the start-time-only comparison. - A failed capture persisted NULL, which _pid_recycled treats as the legacy pre-fingerprint row and falls back to bare PID existence - a new spawn silently recreated the NousResearch#89614/ NousResearch#99558 kill authority. A failed capture now persists UNVERIFIED_WORKER_FINGERPRINT: the claim is held while the PID is live (never released beside it, never SIGTERM/SIGKILLed by timeout, stale-claim, manual reclaim, archive or the terminal reaper) and reclaimed once it is gone. NULL stays legacy-only. - Every tasks UPDATE that nulls worker_pid nulls worker_started_at too (archive_task and the reclaim/timeout/reopen paths): the fingerprint is part of the kill-authority tuple and must not outlive its pid. Live (real sleeper child): reboot-shaped row (same pid, same tick, other boot id) -> reclaimed to ready, child untouched; matching fingerprint -> SIGTERM delivered, exit -15. tests/hermes_cli/test_kanban_worker_pid_fingerprint.py: +2 hostile tests, both red on base. Not changed: the check-then-act window between _pid_recycled and kill (kvnloo P2) is real but needs pidfd_open/pidfd_send_signal (Linux 5.3+) to close atomically; left as the documented residual of "never kills a DETECTED recycled PID".
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
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.
Upstream PR: NousResearch#5868
Problem
The session seeding logic (lines 772-805) was copying the ENTIRE parent DM session history into new thread sessions. This caused messages from all threads in a DM to be mixed together — when a user was in thread B, they would see context from thread A.
Root Cause
When a bot reply in a Slack DM creates a thread and the user responds in it, the thread gets a new session keyed by
thread_ts. The seeding logic attempted to preserve context by copying the parent DM session's transcript, but this included messages from ALL threads, not just the relevant one.
Solution
Remove the seeding logic entirely. The Slack adapter already handles thread context correctly by fetching fresh thread history from Slack's API and injecting it into the message text (slack.py lines 890-898). The seeding was redundant and harmful.
Testing
Fixes: cross-thread context leakage in Slack DM threads