2971 Commits

Author	SHA1	Message	Date
Teknium	580322ef1e	fix(mcp): stdio MCP children get the routed profile's vault secrets, not the default's ... Under a multiplexed gateway, `_build_safe_env` forwarded `os.environ[name]` for every name tagged in the process-global `_SECRET_SOURCES` map. That map is filled by EVERY served profile's secret-source hydration, while `os.environ` only ever holds the LAUNCH (default) profile's values — so once any profile's 1Password/Bitwarden source supplied e.g. GITHUB_TOKEN, every profile's stdio MCP server was started with the default profile's token. Resolve those names through the active profile's secret scope (`get_secret`) instead: the routed profile's value, or omitted when that profile has none. Under multiplex `get_secret` never falls through to environ; single-profile runs keep the .env overlay + environ behaviour, so the existing "vault vars reach MCP subprocesses" contract still holds there. `secret_source_names()` exposes the tagged NAMES only — values are never read from the shared map. Docs: the multi-profile guide's "MCP subprocesses only see their own profile's secrets" claim is now true for source-injected names too; say so explicitly.	2026-09-10 18:12:39 -07:00
Teknium	1bd4e33d36	fix(vault): make browser_vault_fill work on the default Browser Use backend ... On the default backend (browser.backend unset → browser_exec) the vault tools were advertised but could never fill: the CDP supervisor that carries the secret-bearing eval is started only by the built-in browser_* session path, so _eval_js_secret failed closed with supervisor_required and the origin pre-check fell back to an agent-browser CLI eval against a browser browser_exec never touched. - browser_exec now attaches SUPERVISOR_REGISTRY to the CDP endpoint it just routed the harness to (BU_CDP_WS/BU_CDP_URL), so the fill talks to the SAME browser over the same secret-capable WebSocket. BU direct-cloud (BU_AUTOSPAWN) exposes no endpoint and keeps the supervisor_required refusal. - CDPSupervisor.focus_page(origin, accept=<js>) (used by browser_vault_fill, next commits) re-attaches the page session to the open tab on the item's origin whose DOM holds the form being filled (browser_exec opens its own tabs; the supervisor's initial attach picks the first page target, which is chrome://new-tab-page). browser_vault_fill uses it before the origin pre-check with a per-kind probe (password input / card fields / address fields). Live: evals/vault_fill_live_e2e.py drives the real browser_exec tool against Hermes' packaged Chromium with the login page in the third tab; A/B with the attach line disabled fails at "did not attach a supervisor", enabled fills the password into the /login tab and card fields into the /checkout tab with every model-facing read scrubbed. Also: browser_vault_list/fill described the workflow as "type the identifier with fill_input", a helper that exists only inside browser_exec code (toolset browser-use) and is a ghost on the built-in stack. model_tools._rewrite_browser_vault substitutes the concrete name from the session's actual tool set (`fill_input` inside browser_exec, or browser_type), the same dynamic cross-reference pattern browser_navigate uses for web_search.	2026-09-10 10:35:07 -07:00
Teknium	4140901d15	fix(browser): stop refusing credential-named query params on cloud browser/extract backends ... browser_navigate / browser_exec / web_extract refused any URL whose query carried a credential-NAMED parameter (token, signature, access_token, ...) when the backend was a cloud provider. That is exactly the shape of magic links, OAuth callbacks and signed CDN assets, so on Browserbase/Browser Use the agent could not finish a sign-in flow or open an X video asset ("Blocked: URL contains a credential-like query parameter"). The floor protected nothing: the cloud browser already sees every cookie and typed password of the session, and with the credential vault it receives the real password at fill time. Hermes' own secrets leaking into a URL stay blocked by the value-shaped _PREFIX_RE check (_secret_url_error), which is backend-independent. IMDS and private-address floors are unchanged.	2026-09-10 10:35:07 -07:00
Teknium	b51da65258	fix: adapt execute_code cell authority to the widened prompt-callback table ... _callback_api() now yields (getter, setter) pairs for every per-thread prompt (approval, sudo, vault unlock); the kernel cell captured and restored the old fixed 4-tuple. Iterate the table so a cell carries every callback and a future addition needs no change here. Test recorder unpacks the new shape. Also: perfectionist import order in ui-tui interfaces.ts (CI lint).	2026-09-10 10:35:07 -07:00
Teknium	ff90eb28ff	test(bot-mode): trim the author and loop-guard suites to their invariants ... Keep one or two behaviour tests per seam (author reset on a cached agent, forged _turn_author refused, guard trips and cools, single charge on the busy path) and drop the parser/setting enumerations. a2a_key goes with them: nothing in this PR reads it; the honcho follow-up that does can bring it back with its consumer.	2026-09-10 10:27:07 -07:00
Erosika	091ac34ba9	fix(bot-mode): qualify the peer-dm author id with the sender's hostname ... message_agent sent a bare bot:<profile> id through hermes peer dm, so a remote coder and the recipient's own coder shared one author id. The peer branch now sends bot:<hostname>/<profile>, with the hostname cleaned like any author field and slashes dropped. The direct local path keeps the bare id.	2026-09-10 10:27:07 -07:00
Erosika	3db4defcc1	fix(bot-mode): qualify a relayed author from the local connection too ... delivery_turn_author kept the bare bot:<profile> id when the sender's connection was the Desktop's own "local", so a DM relayed from that machine collided with the recipient's profile of the same name. A relayed DM always crosses gateways, so the connection id is now part of the id whenever the Desktop sends one, and only the direct message_agent path in tools/bot_mode_dm.py stays bare. The relay.ts and session_auto_continue.py comments added earlier are cut to one line each.	2026-09-10 10:27:07 -07:00
Erosika	d3c8bacbe3	fix(bot-mode): qualify relayed authors with the sender's connection id ... A relayed DM stamped bot:<profile> on the recipient turn, so an ops profile on another machine and the local ops profile shared one author id. The Desktop now forwards from_connection with each bot_relay.deliver, and delivery_turn_author builds bot:<connection>/<profile> for it while the Desktop's own gateway ("local") keeps the bare id. An api author object accepts an optional origin string that yields the same shape.	2026-09-10 10:27:07 -07:00
Erosika	6881e4d3fc	fix(bot-mode): carry the relay sender into a live Bot Chat turn ... When the target Bot Chat is already open on this gateway, the relay handler delivers through `prompt.submit` with `queued: true`, and that branch dropped the envelope's sender. The model still saw the text prefix, but the turn reached the agent unattributed, the exact case the subprocess branch fixes. The relay handler now stamps the author on the submit as a `DeliveryAuthor`, an in-process object a JSON client cannot build, so `prompt.submit` accepts it the way it accepts a hosted-room callback and refuses a dict with error 4124. The busy queue keeps an authored envelope in its own slot, the drain hands the author to the turn runner, and the runner passes it to an agent that declares the keyword. A plain prompt after an authored dm carries no author. Local deliveries to a desktop-owned Bot Chat take the live-owner mailbox instead. The admission intent and the mailbox record now carry the author, a retry under the same id with a different author is refused, and the owner gateway hands the author to the turn it runs. Isolated compute turns still run unattributed, because the compute-host frame has no author field.	2026-09-10 10:27:07 -07:00
Erosika	55b3ea0b11	fix(gateway): count each bot message once in the loop guard and consume the author variable ... The Telegram adapter asks the authorization check before dispatch, the ingress gate asks it again, and the busy path asks a third time. Each call counted one loop-guard event, so a Telegram bot tripped the budget after a third of the configured messages. The verdict now only refuses a chat that is cooling down. The ingress gate counts an admitted bot message once. `parse_turn_author` treats only booleans, integers and the strings true/1/yes as a bot flag, and returns None for an author with neither id nor name. Names keep format characters and non-breaking spaces so emoji sequences survive. The quiet one-shot pops HERMES_TURN_AUTHOR before the turn so tool subprocesses do not inherit it. `max_events` must be a whole positive number. Issue numbers move out of code comments.	2026-09-10 10:27:07 -07:00
Erosika	16d15869ba	feat(bot-mode): carry the sender through local and relay deliveries ... a bot dm arrived as an ordinary user message. the only trace of the sender was the "Message from" text prefix, which the model reads and nothing else does. the recipient's memory provider saw its own configured user. message_agent now passes the sender as {"id": "bot:<profile>", "name": <handle>, "is_bot": true} to the delivery runner (--author <json>), which sets HERMES_TURN_AUTHOR on the recipient one-shot only. the -Q turn reads it and passes turn_author into run_conversation. the desktop relay forwards the envelope's from_profile/from_handle to bot_relay.deliver, which sets the same variable on its delivery turn. the runner drops any inherited author first so a delivery without one stays unattributed. the text prefix is unchanged.	2026-09-10 10:27:07 -07:00
Teknium	ac07e20407	fix: resolve subagent control authority from the live session slot ... Subagent list/tail/steer/interrupt authorized against a per-record copy of the owning session's transport (`owner_transport`). That copy had to be re-synced at every reattach site; `_rebind_live_transport` did it for session.resume/activate but prompt.submit and the queued-prompt drain still attached bare, so a client that reconnected through a prompt (the common path on a remote gateway / Bot Mode switch) streamed fine while `subagent.list` returned [] and controls rejected. Read `owner_session_record["transport"]` at check time instead: the slot is already mutated by every attach/detach/viewer-failover path, so no site can forget the sync. `owner_transport` stays as the capture-time "commissioned by a gateway session" marker (None = no RPC authority ever); non-dict owners keep the exact-object rule. Drops the registration-time re-read and the attach-time registry loop. Diagnosis credit: nftpoetrist (#106663) — their prompt.submit / drain regression tests pass against this change with no call-site edits.	2026-09-10 10:23:12 -07:00
Erosika	37eb6e1b05	test(bot-mode): pin the waiter budget from Python constants, not relay.ts text ... The waiter budget test read relay.ts with a regex, which AGENTS.md bans and which the Python CI lane would not rerun on an apps/-only PR. It now checks DESKTOP_DELIVER_TIMEOUT_SECONDS against the module's own constants and that REPLY_WAIT_SECONDS exceeds it. relay-deliver-budget.test.ts still pins the TS mirrors against those Python constants from the Desktop side.	2026-09-10 10:18:27 -07:00
Erosika	b1915cb02d	fix(bot-mode): keep the relay waiter watching past the Desktop deliver deadline ... The sender-side waiter gave up at 900s while the Desktop held bot_relay.deliver open for 1500s, so a turn finishing between minute 15 and minute 25 wrote a reply nobody read. REPLY_WAIT_SECONDS now rebuilds the Desktop budget from the same numbers and waits 60s past it. The two turn constants move into tools/bot_relay.py so the gateway handler and the waiter share one definition.	2026-09-10 10:18:27 -07:00
Victor Kyriazakos	49ef015ca3	fix: join delegated work before finite chat exits	2026-09-10 05:02:41 -07:00
Siddharth Balyan	b4d04eb8fd	Connector tools (Gmail, Linear, Notion, ...) are searchable and callable through tool_search for signed-in Nous users (#106842) ... * feat: add session-scoped connector access for onboarding * fix(connectors): availability is the config flag AND the portal entitlement — no free-tier leg The port carried a third availability leg from hermes-magic: a stored guest (free-tier) identity short-circuits the managed-tool entitlement check. That leg reads hermes_cli.anon_auth, which does not exist on hermes-agent main, so connectors_available() raised ImportError inside its fail-closed try and the whole connector surface was silently dark on a plain upstream checkout. On this tree availability is the two-leg AND the design started with: tools.connectors.enabled AND managed_nous_tools_enabled(). The free-tier leg is a hermes-magic concern and belongs in hermes-magic's own delta over this branch, next to the identity it depends on. Its integration test goes with it. * docs(tool-search): connectors section — remote tools through the bridge The squashed port carried the code but not the user-facing docs. Restores the Connectors section of the Tool Search page and the connector-gateway host / CONNECTOR_GATEWAY_URL override on the Tool Gateway page, updated for the manage_connections tool and the pure-connector batch rule. * fix(tool-search): connector tools rank with local tools in one pass instead of taking leftover slots dispatch_tool_search ran BM25 over the local catalog, filled `limit` slots, then appended connector hits only into slots left empty. On a 300-tool catalog no slot was ever empty, so with Gmail and Google Calendar connected "send gmail email" returned five betterstack tools and zero connector tools. The gateway's hits for a query now become catalog entries (connector name, slug words, description as the search text) and join the local catalog for that query's BM25 pass. One ranking, one rarest-token admission rule for both sources, `limit` as the total per query. The merge loop and the separate record builder for connector hits are gone; `_shared_tool_record` serves both sources. The gateway search timeout rises from 8 s to 30 s. One request with six use_cases measured 7 s, so 8 s sat on the edge and cut real answers off; the failure path is unchanged (local-only results, no error to the model). Live, 311 local tools + gateway, before -> after: "send gmail email": 5 betterstack tools -> gmail SEND_EMAIL, CREATE_EMAIL_DRAFT "read google calendar events": 5 betterstack tools -> googlecalendar EVENTS_LIST_ALL_CALENDARS "linear create issue", "betterstack incident": unchanged Benchmark (25 labelled queries): connector recall 0.09 -> 0.82, precision@5 0.18 -> 0.59, false positives on absent intents 17 -> 2. * refactor(tool-search): connector leg into tools/connector_search.py tools/tool_search.py is a facade. The connector leg (gateway hits as catalog entries for tool_search, remote schemas for tool_describe, the connections_in_scope gate) was appended to it by the port. It now lives in its own sibling, tools/connector_search.py, and the facade imports the three entry points: connections_in_scope, connector_entries_by_group, remote_schemas_for. No behaviour change. The tool_describe remote block became remote_schemas_for(names, current_tool_defs, connector_describe) with the same inputs, the same silent-degradation contract and the same injection seam the tests already use. * fix(tool-search): at most 7 queries per call, the gateway's search limit One tool_search call sends all its queries to the connector gateway as one search request. The gateway answers 7 use_cases per request and returns HTTP 502 for 8 or more (measured 2026-09-09, re-measured with one-word use_cases: it is a count limit, not a size limit). With the client cap at 10, a model sending 8 to 10 queries lost every connector hit for that call and saw local-only results with no error. The shared constant splits: _MAX_QUERIES_PER_CALL = 7 for search, _MAX_DESCRIBE_NAMES_PER_CALL = 10 for describe, which has no remote count limit. Eight or more queries now get the existing "too many queries" retry hint before any request is made. No chunking: one call, one request. * fix(tool-search): the model is told that connectors__ names are manage_connections accounts tool_search results carry names like connectors__gmail__CREATE_EMAIL_DRAFT and manage_connections is the tool that checks and connects those accounts, but nothing told the model the two are the same thing. A model that hit CONNECTION_REQUIRED had to infer the fix on its own. The tool_search description gains one sentence making the link, added at assembly only when manage_connections is in the session's tools. Signed out or with connectors off the tool is absent and the description is unchanged, so it never names a tool the model cannot call. This follows the existing rule for cross-tool references (tools/AGENTS.md): they are added dynamically from the session's actual tool set, never hardcoded in a schema. Tool defs are fixed for the life of a conversation, so the description is byte-stable per conversation; this is a one-time prefix change. Live, real get_tool_definitions() against a signed-in home: sentence present. Same home with auth.json removed: manage_connections absent, sentence absent. * fix(connectors): /stop halts a connector batch before the next remote call dispatch_connector_batch runs every remote entry of a tool_call batch in sequence. The executor only checks the interrupt flag between tools, and the whole batch is one tool to it, so a /stop landing during entry 1 of 20 still sent the other 19 to the gateway. The loop now reads tools.interrupt.is_interrupted before each dispatch. Once set, it stops calling handle_function_call and fills every unstarted slot with the loop's existing error-slot shape, code INTERRUPTED and the message "Stopped by the user before this call was made.", so the result envelope stays valid and the counts stay honest. Entries already dispatched keep their real results. Test: three connector calls where the fake client sets the interrupt on the first execute. The client sees exactly one call and slots 2 and 3 carry INTERRUPTED. Red on the base branch, green with the fix. * test(connections): schema assertions become dispatch contracts test_schema_documents_wait_and_its_timeout froze description fragments ("REQUIRED", "can NOT disconnect", "Nous Portal"). A wording edit fails it while a real regression (a disconnect that reaches the gateway) does not. That is a snapshot of prose, not a behaviour contract. Delete it. The requirement that wait needs connectors is already covered by test_wait_requires_connectors. The user-only disconnect boundary is now asserted as behaviour: action disconnect with a connector returns an error and the fake client records no call. That replaces the earlier de-authenticate test, which only checked that the word "dashboard" appeared in the error text. Test count in the file goes from 26 to 25. * docs(tool-search): connector batches are one gateway request per entry The user guide said a connector batch travels as one gateway request. It does not: model_tools_connectors.dispatch_connector_batch re-enters core dispatch per entry, and each entry becomes its own execute request in bridge._run_remote (plus at most one literal-slug retry when the gateway reports TOOL_NOT_FOUND under the conventional slug). The docstrings in tools/tool_gateway/bridge.py and tools/tool_gateway/__init__.py still described the abandoned V1 plan and claimed nothing outside the package imports it. Rewrite those sentences to match the code: one request per entry, in input order, dispatched from model_tools_connectors.py, with the per-entry approval and interrupt behaviour that motivated the split. The guide also still showed the single-call shape tool_call(name, arguments); both places now show the `calls: [{name, arguments}]` array the schema advertises and note that a single local call is an array of one. Docs only, no test. * fix(tools): the between-turns refresh never rewrites the bridge tools The per-turn MCP refresh folds a fresh tool snapshot into the live array with preserve_prefix: order and membership stay, but a name present in both takes the fresh schema. That is right for ordinary tools, whose schema is a constant. tool_search is the one tool whose description is derived from the session: the deferred-tool count, the embedded listing, and, on this branch, whether manage_connections was present. A late MCP server or one failed portal lookup (manage_connections' check_fn fails closed) changed those bytes on the next turn, and every byte after tool_search in the cached prefix was re-prefilled. The array also contradicted itself in that case: the flapping manage_connections was carried forward while the description lost its hint. The bridge entries now keep the bytes they were built with for the life of the conversation. Nothing is lost: tool_search reads the live catalog at dispatch, so tools that arrived late are still found; connector availability is checked at dispatch too. The compaction-boundary rebuild (content_aware, the one sanctioned cache break) still refreshes the description. Consequence: connector exposure in the prompt is decided once, at agent build, by whether the user was signed in then. That is the intended contract. * refactor(tool-search): normalize_tool_call_entries lives with the other argument validation The port appended the tool_call argument parser to the tool_search facade. The family already has tools/tool_search_validation.py for exactly this work (schema validation of deferred call arguments), so the parser moves there and the facade imports it. No behaviour change; the one test that imported it now imports from the defining module. * refactor(connectors): delete the unused batch dispatcher; _run_remote becomes run_remote bridge.dispatch_calls and its helpers (_dispatch_calls_inner, _run_pre_dispatch, _run_local, _error_slot, _maybe_parse_json) and the LocalDispatch / PreDispatch seams had no production caller. Connector dispatch runs through model_tools_connectors: dispatch_connector_batch re-enters handle_function_call once per entry, so scope, hook, approval and middleware policy fire against each composed name inside core dispatch, and dispatch_connector_call hands the single planned entry to the bridge's transport function. Only tests called the batch dispatcher, and they exercised policy seams that production never wires. The transport function is the module's real entry point, so it drops the underscore: _run_remote becomes run_remote, body unchanged. The module docstring now describes the two legs that exist (availability with D32 silent degradation, and run_remote) instead of the injected seams. Imports that only the deleted code used are gone; merge.py is untouched because every export still has a caller. Tests that drove dispatch_calls are deleted where they covered the removed seams (pre_dispatch blocks and rewrites, local_dispatch classification, mixed batches). The literal-slug fallback, the per-entry transport failure, and the hook rewrite reaching the gateway request body are re-targeted at handle_function_call('tool_call', ...) with the fake client swapped in at bridge._default_client_factory, the same seam test_connector_dispatch_policy uses. Each re-targeted test fails when the retry is disabled in run_remote. * fix(connectors): search keeps the twin a colliding name reaches, and says so format_connector_name strips the toolkit prefix, so GMAIL_FETCH_PROFILE and a literal FETCH_PROFILE on gmail both compose to connectors__gmail__FETCH_PROFILE. describe and execute decode that name to the prefixed slug first, so the literal twin is unreachable under it. If a vendor ever shipped both, search could describe the literal under a name that runs the prefixed tool. Search is the one place that sees both twins in one response. It now keeps the twin the name reaches and drops the other with a WARNING that names both slugs, whichever the gateway listed first. Short names stay; no marker, no per-process map, no change to describe or execute. No such pair exists in the live catalog today; the guard turns a silent alias into a logged one.	2026-09-10 02:21:16 +05:30
Siddharth Balyan	cf4b78e91f	fix(tool-search): a query no tool answers returns nothing, not five tools sharing one word (#106676) ... search_catalog admitted every document with BM25 score > 0 and then padded to `limit`. BM25 sums over the tokens a document shares with the query, so on a 300-tool catalog "send gmail email" returned five incident tools that shared only "email", and the discriminating word ("gmail", in no document) had no say. The model read those as the answer. Admission is now the query's rarest token: a document is a result only if it contains the query token with the highest IDF, the one that names the intent. Common verbs ("send", "read", "create") sit in dozens of documents and never gate; vendor and object words ("gmail", "github", "incident") do. A token no document carries admits nothing, and the existing empty-group hint tells the model to retry without it. The name-substring fallback is deleted: it admitted tools that matched no query token at all. Result descriptions are clipped at 500 characters instead of 400. Over 353 vendor tool descriptions, 500 keeps 91% whole and every first sentence (first-sentence max 329); 400 kept 82%. Measured on the live 311-tool catalog with 25 hand-labelled queries: precision@5 0.18 -> 0.43, wrong names returned 102 -> 66, false positives on absent intents 17 -> 13. Live before/after: "send gmail email" went from five betterstack tools to an empty group with the retry hint; "linear create issue" and "betterstack incident" are unchanged.	2026-09-10 02:21:15 +05:30
Teknium	b7bef04861	fix(kanban): an explicit scratch workspace never inherits the board's project ... Move the "explicit scratch means no project" decision into the one resolver every surface funnels through, `kanban_db.create_task`: board-project inheritance now runs only when the caller left `workspace_kind` open (`None`), and `workspace_kind` defaults to scratch after that check. The tool handler keeps the #106347 fix for `project=""` (no `or` collapse) and `board=` scoping but drops its handler-local sentinel logic, since the resolver now owns the rule; the `self_task` project inheritance for dispatcher-owned workers is unchanged. Sibling surfaces had the same bug through the same line and are fixed by the same change: - CLI `hermes kanban create --workspace scratch` on a project-scoped board produced a project worktree; `--workspace` no longer defaults in argparse so the resolver can tell "omitted" from "scratch". - Dashboard `POST /tasks` with `workspace_kind: "scratch"` did the same; `CreateTaskBody.workspace_kind` defaults to `None` for the same reason. - `kanban_swarm.create_swarm` threads `None` through for consistency. Tests: one resolver invariant in test_kanban_board_project.py (explicit scratch stays scratch, omitted still inherits) and the salvaged tool test folded into a single parametrized matrix over scoped/unscoped target boards.	2026-09-09 12:18:58 -07:00
KoNit-K	dc5c87c6c1	fix(kanban): honor explicit scratch/project on MCP kanban_create ... Pass the caller's board into create_task and treat explicit workspace_kind=scratch or empty project as no-project so ambient board project_id cannot override MCP args. Co-authored-by: Cursor <cursoragent@cursor.com>	2026-09-09 12:18:58 -07:00
Teknium	85e423482a	fix(tools): kill the browser_exec CLI tree on timeout on Windows too ... The salvaged fix only ran the CLI in its own session on POSIX and kept plain subprocess.run on Windows — the one platform where the wedge in #106244 is actually reproducible: CPython's run() retries an unbounded communicate() after kill() there, so a grandchild holding the capture pipes blocks the worker forever. On POSIX run() wait()s the PID and returns promptly; the grandchild merely leaks (live-reproduced on Linux). One code path for both platforms: - _group_popen_kwargs: start_new_session=True on POSIX, CREATE_NEW_PROCESS_GROUP + hide flags on Windows (replaces the hide-only _windows_popen_kwargs). - _kill_cli_process_group: os.killpg SIGKILL on POSIX, taskkill /T /F on Windows (same kwarg set as the sibling taskkill sites). - The Popen decodes with encoding="utf-8", errors="replace" like every other subprocess call in this file (windows footgun rule). - Drain test patches the kill helper instead of os.killpg so it runs on every host. Windows behaviour is not live-verifiable on this Linux host.	2026-09-09 12:11:13 -07:00
Teknium	6788c3224f	test(tools): trim the browser_exec group-kill tests to two invariants ... Drop test_gone_group_still_surfaces_timeout: the killpg-race branch is a contextlib.suppress and the test only pinned the exact communicate() timeout sequence (a change-detector). The real-grandchild test and the bounded-drain test remain — the two behaviour contracts of the fix.	2026-09-09 12:11:13 -07:00
liuhao1024	60debff28d	fix(tools): kill the whole browser-use CLI process group on browser_exec timeout ... subprocess.run only kills the direct CLI child on TimeoutExpired; a browser_harness daemon / Chrome helper grandchild inherits the stdout/ stderr pipes and keeps them open, so the internal communicate() blocks on pipe EOF forever. The wedged tool call never returns, its activity heartbeat keeps stamping last_activity_at every 30s, and the session is pinned at "now" in the desktop sidebar indefinitely (#106244). On POSIX, run the CLI in its own session (start_new_session=True) and SIGKILL the whole process group on timeout, then drain the pipes under a bounded deadline. Windows keeps subprocess.run.	2026-09-09 12:11:13 -07:00
teknium1	acf9177c70	test(fal): trim the billing-409 salvage to two invariant tests ... Keep the two tests that fail on main without the fix: - test_fal_common: a keyed managed submit makes exactly one POST (plus the negative arm: an unkeyed submit still goes through the SDK retry ladder) - test_image_generation: the 409 BILLING_ERROR body surfaces `unsupported_pricing_meter` instead of the generic "not yet enabled" text Dropped from #106484: the duplicate video-plugin billing test (same helper, same assertion), the `_fal_client = fake` / `import_fal_client` stub churn and the `tools.lazy_deps` stub — fal-client is installed in CI (`--extra fal`) so those fixtures were not needed; the `_load_fal_client` no-op fixture on TestManagedGatewayErrorTranslation for the same reason. Also drop the redundant `retry_request is None` re-check in `_ManagedFalSyncClient.submit` — `__init__` already raises when the helper is missing.	2026-09-09 11:46:36 -07:00
Matt Earls	0c6b94e499	fix(image-gen): preserve managed FAL billing errors ... Avoid retrying idempotent managed FAL submissions because the retry can mask the initial billing failure. Surface structured Nous billing diagnostics consistently for image and video paths, with hermetic regression coverage. (cherry picked from commit 289ce039e9a522dc8016ae4a512214c05d0a8bc0)	2026-09-09 11:46:36 -07:00
teknium1	9678d59c23	test(mcp): stdio arm of the transport-rebuild test now expects no replay ... The parametrized lifecycle test drove both transports through a ClosedResourceError on the first call_tool and asserted a replay. For the HTTP arm that is still the session-expired contract; for the stdio arm a pipe closing after dispatch is now the ambiguous mid-call death and must surface outcome_uncertain with exactly one invocation. The transport is still rebuilt on both arms.	2026-09-09 11:27:29 -07:00
teknium1	cff103a8b7	fix(mcp): classify an SDK-first transport close after dispatch as an ambiguous stdio death ... The child watcher polls every 250 ms, so in practice the MCP SDK sees the closed pipe first and `call_tool` raises ClosedResourceError / "Connection closed" before the watcher fires. That exception is not a _StdioChildExited, so it fell past the stdio recoverer into _handle_session_expired_and_retry, which reconnects and replays the call -- the exact duplicated-side-effect path the previous two commits close. Live repro against a real stdio child that applies an effect then exits without replying: 2 effects with the contributor's commits alone, 1 effect + outcome_uncertain after this. On a stdio server, a session-expired-class error raised by an RPC that was already dispatched is re-raised as _StdioChildExited(in_flight=True) so the stdio recoverer owns it. HTTP servers are untouched: their session-expired retry is still the right recovery. Tests trimmed to the salvage bar: the contributor's test_precall_respawn_retry_dying_midcall_is_uncertain_without_replay (a variant of the watcher-race case) is replaced by the SDK-first regression the review on #106440 asked for; the existing pre-call retry test still pins that a never-sent call is retried once.	2026-09-09 11:27:29 -07:00
Eva	73c104ee35	fix(mcp): preserve uncertainty after retry exit ... (cherry picked from commit 775dd2c4d7eb28cd82cb8d943c8ea11640474227)	2026-09-09 11:27:29 -07:00
Eva	144b86ef48	fix(mcp): avoid replay after mid-call stdio exit ... (cherry picked from commit b29e68d0b020c805d1fccfc55f3a7c6ee6589ba9)	2026-09-09 11:27:29 -07:00
Teknium	d5926b2494	fix: persist API delegation units once without waking the model	2026-09-09 10:55:32 -07:00
Teknium	179790df6f	test: reproduce API delegation provenance and replay loss	2026-09-09 10:55:32 -07:00
teknium1	e2763baf1c	refactor(kanban): route the reviewer guard through _check and tighten its tests ... Use the module's `_check`/`_Reject` idiom instead of an inline `return tool_error(...)` so every kanban_request_review validation failure renders through the same path, and drop the unreachable `or "none"` (list_profile_names() always contains "default"). Tests: compare the task's (status, assignee, run) tuple and the event log before/after instead of the unordered 6-assert block, use the context-managed kanban_db_connect.connect (the kb.connect alias is a plugin-compat pointer — scripts/check_compat_pointers.py flagged it), and reference #106163 in the invariant's docstring. Salvage note vs #106214 (@gaoanze888): that PR guards the same condition inside hermes_cli/kanban_db.py::request_review, but the DB primitive is also the chokepoint for `hermes kanban request-review` and the dashboard's drag-to-review, both operator surfaces where a non-profile assignee (external/human review lane) is a documented board shape (website/docs/user-guide/features/kanban-worker-lanes.md) — and it forced five unrelated test fixtures to monkeypatch profile_exists to True. The model-facing tool wrapper is the layer where a typo'd string is a bug, so the guard lives there.	2026-09-09 09:45:13 -07:00
auroracapital	1d89286b36	fix(kanban): reject phantom worker reviewers ... kanban_request_review(reviewer=<name>) reassigned the task to whatever string the model supplied. A non-profile value (e.g. the literal "reviewer") parked the card in `review` on an assignee the dispatcher can never spawn, with no error to the worker — the chain stalled silently (#106163). Validate the explicit reviewer against installed profiles before touching the board and return a tool error listing the installed profiles so the model can self-correct. Salvage of #97429: the kanban_diagnostics `review_reopened` hunk and its tests were dropped (main already replaced that loop with `_latest_event_ts`; the tests targeted the PR's pre-refactor base). Re-authored from the placeholder identity `regen <regen@local>` to the PR author's GitHub noreply address (misconfigured local git, not malice).	2026-09-09 09:45:13 -07:00
Teknium	bf53ff00a7	fix(config): one bounded backups/config/ dir replaces four config.yaml.bak schemes ... Four writers each dropped their own uniquely-named copy of config.yaml next to the real file and none of them ever deleted anything: hermes setup (config.yaml.bak.YYYYMMDD_HHMMSS, one per run even with no change), the corrupt-YAML snapshot (config.yaml.corrupt.<ts>.bak), hermes migrate xai (config.yaml.bak-pre-migrate-xai-<ts>) and the Docker boot migration (config.yaml.bak-<ts>, .env.bak-<ts>). A home dir accumulated a dozen variants with no way to tell which mattered. hermes_cli/config_backups.py::backup_config is now the single writer: backups/config/config.yaml.<reason>.<YYYYMMDD-HHMMSS>, skipped when the newest copy for that reason is byte-identical, rotated to the newest five per reason. backups/ is already excluded from full backups so nothing nests. Legacy siblings written by the old schemes are moved into the dir on first use; hand-named copies (config.yaml.bak-my-note) are left alone. Live: three `hermes setup --non-interactive` runs against an unchanged config went from three .bak files in HERMES_HOME to one pre-setup copy under backups/config/; repeated loads of broken YAML produce one corrupt copy instead of one per process (deduped by content).	2026-09-09 02:36:00 -07:00
kshitijk4poor	e333113871	fix(review): keep /refine under the background_review origin; attendedness is its own flag ... The salvaged commit forked an explicit /refine under a new "refine_review" origin so the memory delete gate would not treat it as unattended. But is_background_review() is the key for every other review guard — skill_manager_guards (curator-owned-only, read-before-write), skill_manager_tool (archive instead of rmtree), skill_ledger actor, write_approval staging, the [auto] tag — so a /refine fork silently escaped all of them. Carry attendedness separately: the fork keeps origin "background_review" and sets _review_attended; turn_context binds it beside the origin ContextVar; the memory gate keys on the new is_unattended_review(). Also run the gate AFTER _validate_single_op / the operations list check, as memory_tool's own docstring requires, so an invalid replace is rejected now rather than staged and failed at approve time.	2026-09-09 12:19:13 +05:30
liuhao1024	c0714575c3	fix(review): distinguish explicit /refine from unattended reviews and surface staged consolidations ... Review follow-up on #105944 (#105921): - explicit /refine forks now run under the refine_review write origin (explicit flows from the CLI/gateway handlers through _spawn_background_review_now and spawn_background_review_thread down to build_cache_parity_fork), so a user-requested review keeps the full memory operation set; only automatic reviews stay behind the unattended delete gate. - the unattended delete gate now stages the denied replace/remove (or whole batch) into the pending store instead of dropping it: the fork's own review summary is never published, so a plain denial lost the consolidation request with no surfacing path. The staged proposal carries a proposal_staged marker that summarize surfaces as an action line, and a staging failure still fails closed to a plain denial. - regression tests: explicit-path origin pass-through, refine_review keeping replace working, near-limit denial end to end (add rejected by budget -> replace staged -> proposal surfaces, store unchanged).	2026-09-09 12:19:13 +05:30
liuhao1024	1571f502a9	fix(agent): scope background review memory access to its trigger (#105921) ... The review fork's tool whitelist granted the whole memory toolset whenever the profile had memory enabled, regardless of which nudge fired, so a skill-nudge fork held remove/replace on MEMORY.md it was never asked to use; combined with the memory tool's near-limit 'consolidate now' hint, an unattended fork deleted standing rules with no user in the loop. - Pass review_memory from spawn_background_review_thread through _run_review_in_thread/_run_review_fork into _review_tool_whitelist; a skill-only review no longer gets the memory tool at all. - Fail-closed operation gate in memory_tool: a background-review fork may add, never replace/remove (single or in a batch) — consolidation decisions reach a human via the review summary instead. - Keep the deny/prompt wording in sync with the whitelist so a memory-less review doesn't advertise memory.	2026-09-09 12:19:13 +05:30
Phil Mossman	ac10770894	fix(cron): don't stamp the next occurrence on an off-tick manual run ... claim_job_for_fire() derives the occurrence identity from next_run_at before the same function advances it. On a scheduler tick next_run_at is the occurrence being run, which is correct; on an off-tick manual run it is the NEXT occurrence, so the execution is stamped with the identity of a slot that has not happened yet. _job_is_due() then finds a completed execution carrying that identity and skips the real slot, returning before the last_dispatch write — no error, no log line, no dispatch record. The manual flag already guards this and both _job_is_due() and claim_job_for_fire() honour it; the agent-facing run-now path never declared itself. Add a keyword-only manual= parameter and pass it from _claim_for_manual_run(). Deliberately not force=True: force also calls _activate_job_record(), which would resume a paused or disabled job, and the run-now tool depends on continuing to refuse those. The local flag is renamed to manual_fire so the new parameter is not shadowed inside the apply closure, which would raise UnboundLocalError. Three existing tests in tests/tools/ pinned the old call signature via assert_called_once_with; they now pin manual=True, so dropping the flag again fails loudly rather than silently reintroducing the skip. Restores the intent stated in #104790 — the column records the scheduled instant an execution was claimed for, and an off-tick manual run was claimed for none. Fixes #105690 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-09-09 12:17:13 +05:30
kshitijk4poor	0e9fc2cc15	test(process-registry): exercise the portable probe payload ... Execute the selected no-op rather than freeze its spelling, while rejecting /bin/true to model the NixOS failure. Mark the regression Linux-only and retain the current user-bus environment handling. Consolidates the earlier NixOS scope-probe report and fix in #102587 with the PATH-independent payload from #105436. The fallback resolver is not needed when /bin/sh is used directly. Co-authored-by: Scott Garrand <sgarrand@gmail.com>	2026-09-09 10:56:37 +05:30
Cesar-Azeredo	37ce016dc6	test(process-registry): mark systemd probe tests linux_only	2026-09-09 10:56:37 +05:30
webtecnica	7a7ead8179	fix(process-registry): use portable /bin/sh probe for systemd-run scope availability (#105365)	2026-09-09 10:56:37 +05:30
Teknium	9d865810b6	fix(mcp-oauth): keep refresh_token when a refresh response omits it (#62333) ... HermesProviderMixin._handle_refresh_response overrides the SDK's handler (to accept any 2xx and keep token bodies out of logs) but dropped the SDK's RFC 6749 section 6 carry-forward. An authorization server that does not rotate refresh tokens (TinyFish, Google, Zoho, Asana, Futu) answers the refresh grant without a refresh_token; we then stored the response verbatim, erasing the only refresh token we had, so the next expiry had nothing to refresh with and forced a browser re-auth roughly one TTL after every login. Carry the prior refresh_token (and scope, per section 5.1) forward on the OAuthToken before _store_tokens, so both the live provider and the on-disk token file keep it. A rotating AS still wins: only None fields are filled. Tests: two invariants on the real HermesMCPOAuthProvider + HermesTokenStorage (omitted -> preserved in memory and on disk; provided -> rotated). The carry-forward test is red on main.	2026-09-08 17:44:51 -07:00
Teknium	b1f003e186	feat: add FAL GPT Image 2.5 generation and editing selections	2026-09-08 14:57:15 -07:00
Teknium	aa83c6d614	fix: hide inactive grouping options from delegation schema	2026-09-08 13:38:18 -07:00
ckomma	be9c2bd19c	fix(cron): derive the user bus env per probe and scoped spawn ... A system-level gateway unit has no ordering against user@<uid>.service and linger may be enabled after boot, so the bus can appear after the one-shot adoption in run_gateway() ran. Derive XDG_RUNTIME_DIR/DBUS_SESSION_BUS_ADDRESS fresh for the availability probe and every scoped spawn (cron worker, Kanban worker, PTY/pipe terminal spawns, scope cleanup) so the 60s failure TTL can actually recover. Refs #104893.	2026-09-08 22:35:39 +05:30
Teknium	13fb5e1ece	test(browser): migrate pinned-profile fixtures to real SQLite	2026-09-08 05:10:57 -07:00
Teknium	58d6d5223a	fix(browser): bound auth backups without replacing live SQLite files ... Preserve Ben Barclay's diagnosis and replace the staging-file approach with SQLite-coordinated writes and a five-second backup callback deadline. A main-file replacement can replay an abandoned destination WAL; immutable source reads can miss committed source WAL. Neither raw copy nor replacement is safe when the destination is locked. Refuse unavailable auth databases without raw-copy fallback, retaining the existing close-browser-and-retry flow. Keep two invariant tests for lock refusal/recovery and source-versus-destination WAL contents. Convert existing text masquerading as database fixtures into real SQLite fixtures. Related: #105754 Related: #96659	2026-09-08 05:10:57 -07:00
Ben Barclay	b4d7cf735d	fix(browser): real-profile auth mirror hangs forever on a locked destination ... A `browser_exec` call could park a thread in `sqlite3_sleep` permanently while mirroring Chrome's auth DBs, holding the agent's turn open. The turn never reaches its `finally`, so no `session.info running=false` settle is emitted and the Desktop composer latches busy — every later message queues and never sends. Captured live: one thread stuck 24+ minutes across two dumps, turn accepted at 15:03 with no `tui turn finished` 46 minutes later. Root cause is the DESTINATION, not the source. `Connection.backup()` retries a busy destination internally and ignores the connection's busy timeout, so `sqlite3.connect(dst, timeout=5)` cannot bound it. A destination left locked by an earlier hung mirror therefore blocks the next mirror forever — and because the tool-level 420s timeout abandons the thread without interrupting a C-level lock wait, the lock is never released and every subsequent launch re-hangs the same way. Self-perpetuating. Two changes: - Back up into a fresh `<dst>.new` and `os.replace()` it into place. No other process can hold a file we just created, so there is nothing to contend on, and the swap stays atomic. Measured against a live Chrome with a deliberately locked destination: 0.0006s vs an indefinite hang. - Drop the `mode=ro` (no `immutable=1`) source fallback. 8e746668ba added `immutable=1` to fix exactly this hang but left `mode=ro` as a fallback, keeping the unbounded path one exception away; sqlite's busy timeout does not cover lock negotiation, so nothing bounds it. `immutable=1` is also the semantically correct mode — a committed snapshot of a file another process owns. The bounded plain-copy fallback is unchanged. Tests: three regressions, all mutation-checked (fail on base, pass here). The locked-destination test runs the copy on a worker with a join deadline so the unfixed behaviour fails fast instead of hanging the suite. 199 passing across the browser real-profile and CLI suites.	2026-09-08 05:10:57 -07:00
Ben Barclay	866332bfb5	fix(relay): authorize send_message targets and surface egress declines (P5) (#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.	2026-09-08 17:51:35 +10:00
Teknium	5280fe9987	fix: cron and local DMs reach an open Desktop Bot Chat ... Route local producers to durable owner ingress before attempting the unowned CLI lane. Preserve per-run/per-message IDs and receipt-first retry handling; never fall back after ambiguous admission. Report cron admission as queued, not completed or failed, in job status, the execution ledger and CLI/tool UX. Native isolated Electron validation reproduces SESSION_NOT_OWNED on main for both idle and busy owners. Fixed owner consumes idle cron, busy cron, local DM and mounted-chat cron exactly once, keeps its lease, yields to queued human input, and preserves the prior model-request prefix and tool schema. Inference alone used a deterministic loopback wire stub; no paid model call.	2026-09-07 16:48:29 -07:00
FalconOrtiz	db96c8ced7	fix: admit Bot Chat deliveries through the live session owner ... Adapt FalconOrtiz's owner-mailbox proposal from #101564 onto the current notification poller and topical modules. The durable mailbox is cross-process ingress only: the existing owner admits its normal prompt turn after the current turn and human FIFO clear. Retain immutable receipts, stable admission identities, capability and lease fencing, compression lineage, and disable blind recovery replay of imported turns. The original stale server hunks and expiring receipt protocol were rebuilt rather than cherry-picked: the current facade decomposition and durable busy admission contract differ. Credit the earlier owner-mailbox work in #100544 and durable producer work in #100319. Co-authored-by: fangliquanflq <fangliquan@qq.com> Co-authored-by: 686f6c61 <github@00b.tech>	2026-09-07 16:48:29 -07:00