Review follow-ups on the identity binding:
- The sentinel's `start_time` is `time.time()` at `record_startup`, seconds
after the process was born once imports finish, so comparing it with
psutil's create_time within 2 s would have read every real gateway as
undecidable and silently stopped the #109538 cold-start. `record_startup`
now stamps `create_time` (psutil birth via the existing
`process_identity._process_create_time`), `mark_exited` carries it, and
the attestation compares birth to birth. A sentinel from a gateway older
than the stamp falls back to the PID-only rule.
- A resume token written by pre-generation code and resumed by this code
probes the marker again instead of skipping the spawn.
- Horizon allows a 60 s backwards clock step; the unused `now` parameter is
gone; the create-time tolerance is a named constant; the read-then-unlink
in `_consume_start_attestation` is documented as best-effort.
`_attested_pid_exited_cleanly` matched the lifecycle sentinel by numeric PID
only, so a stale marker for PID 111 flipped from "clean exit" to "crash" once
an unrelated PID 222 lifecycle overwrote the sentinel, and a reused PID's clean
exit could vouch for a different life (#110020 review, gateway_windows.py:937).
`_write_start_attestation` now records `create_times: {pid: create_time}` via
the existing `process_identity._process_create_time`; `mark_exited` carries the
running sentinel's `start_time` onto the exited sentinel; the attested probes
fail closed for a bound PID whenever the sentinel cannot be shown to describe
that incarnation (other PID, start time off by > 2s, or no start time) —
"unknown" never reads as "dead". A missing sentinel still reads as dead, and
markers without `create_times` keep the PID-only rule.
Tests: two attestation tests (stale marker vs. moved-on sentinel → no
authority; own incarnation keeps authority / clean exit / legacy marker) and a
ledger test for the carried `start_time`. Mutation: with HEAD's prod files the
no-authority test and the ledger test fail.
`attested_death_generation` matched the clean-exit sentinel by PID only and never looked at
the marker's `ts`, so a historical marker (e.g. one left behind before a Desktop-owned era)
could later override Desktop ownership and authorize a duplicate gateway (#76129).
The read-only probe now treats a marker whose `ts` is missing, unparsable, in the future or
older than `START_ATTESTATION_MAX_AGE_S` (24h — the marker only bridges the seconds between a
✓ and the next CLI invocation) as no authority: `None`, fail closed, marker left unconsumed.
Partial follow-up to #110020 review thread (d); binding to per-PID process create_time is left
for a follow-up (needs `mark_exited` to carry `start_time` into the exited sentinel).
The resume token only recorded `cold_start_if_installed: bool` and execution re-read the
mutable one-shot start-attestation marker to decide whether a Desktop-owned install still owed
a cold-start. A concurrent `hermes gateway status`/`start` (`check_start_attestation`) consumes
that marker between plan and execution, so the spawn was skipped and the token cleared with no
gateway running.
The marker now carries a `generation` nonce. The plan records the generation whose unclean
death authorized the cold-start on the token (`attested_generation`); execution authorizes the
spawn from the token, still re-checks live gateway PIDs, and consumes the marker only while it
is still that generation — a newer marker written by a concurrent start keeps its own report.
`attested_gateway_died` becomes `attested_death_generation` (`None` = undecidable, fail closed).
Follow-up to #110020 review thread (a).
_cold_start_windows_gateway_after_update cleared the dead attestation as soon
as _spawn_detached() returned a PID, before _wait_for_gateway_ready() proved
the gateway survived. When readiness failed, the RuntimeError registered the
retry, but the retry then saw Desktop lifecycle ownership with no marker and
returned success without spawning anything — the silent outage of #109538
came back through the retry path (#110020 review). The marker is now
consumed after readiness is confirmed, so a failed spawn leaves the retry
its recovery obligation.
_attested_pids_from kept any `isinstance(p, int)` item, so `[true]`, `[0]`
and `[-1]` each survived as a "PID" and made attested_gateway_died([]) read
True — a malformed marker could override Desktop gateway-lifecycle ownership
and spawn a duplicate gateway (#110020 review). PIDs are now `type(p) is int
and p > 0`, and one malformed item fails the whole list closed: the writer
never emits such values, so a partially-bad list is not trustworthy either.
Review follow-ups on the refresh transaction:
- `_provider_state_transaction` takes a `timeout_seconds` applied to BOTH the
active and the root lock. The refresh passes max(default, refresh timeout
+ 5 s); before, only the profile lock used that budget and root's lock kept
the 15 s default, so the waiting profile raised TimeoutError instead of
adopting whenever the peer's POST ran long. The regression test now holds
the endpoint past the lock floor and fails without the passthrough.
- Peer adoption requires a stored access token as well as a rotated refresh
token; an incomplete stored pair falls through to the refresh.
- `_save_codex_tokens` keeps its body: the per-path lock is reentrant, so the
refresh calls it inside the open transaction (as the CLI-recovery path
already did) instead of a split-out helper.
Follow-up to #110024 (ehz0ah's review thread). `_refresh_codex_auth_tokens` POSTed the
single-use refresh token to OpenAI first and only then entered
`_provider_state_transaction("openai-codex")` for the write-back, so root's lock covered the
save alone. `resolve_codex_runtime_credentials` holds only the caller's own profile lock, so
two profiles borrowing the same ROOT grant could both submit `old-rt`; last root save won and
OpenAI answered `refresh_token_reused` / revoked the family — the failure #87503 exists to
prevent.
The transaction now spans re-read -> endpoint refresh -> write-back:
- enter `_provider_state_transaction` first; the yielded state is root's, re-read under root's
lock. If its refresh token already differs from the one we were about to submit, a peer
rotated it: adopt the stored pair and return without touching the endpoint.
- otherwise POST and write back through `_store_codex_tokens_in`, the body of
`_save_codex_tokens` split out so it can run inside an already-open transaction.
`_save_codex_tokens` keeps its signature for the login/import/CLI-recovery callers.
Holding the advisory flock across the network call is safe here and already the established
shape: `resolve_codex_runtime_credentials` holds the active-store lock across the same POST,
and every waiter's timeout is `max(AUTH_LOCK_TIMEOUT_SECONDS, refresh_timeout + 5)`, i.e. it
outlives one full endpoint timeout. `_load_auth_store` readers never take the lock, so
readers are not blocked; `_file_lock` is reentrant per thread per path, so the nested
transaction inside the caller's lock and the CLI-recovery save inside the transaction both
re-enter cleanly. A release-POST-retake variant would reopen the window it is meant to close.
Test: two refreshers with the same stale pre-read pair against a rotate-once endpoint that
rejects any replay — the endpoint sees `old-rt` exactly once, both callers end with the
rotated pair, root holds it, the profile store stays unshadowed. Red on origin/main
(`refresh_token_reused` surfaces for the second caller).
The summary path had grown a verbatim copy of turn_api_request's 3-line
surrogate/ASCII chokepoint — the same drift class this PR removes for the
hand-rolled kwargs builder. Move the two lines and the #50959 rationale into
`message_sanitization.sanitize_outbound_kwargs` and call it from both sites,
so the next sanitizer step added to the main loop cannot miss the summary.
Tighten two comments: "same kwargs builder" (cache_control redecoration is
not re-applied here) and a `_summary_text` note that is true for all three
summary branches, not just the chat one.
`_summary_text_with_scrub` was the old helper plus a warning — `.content`
never carried tool calls, so nothing was being discarded that was not
already ignored. Restore the original name, keep the warning with the WHY
(the request now carries tools, this path never executes a call), and make
the tool-only test assert the log line so it actually binds the new code
path instead of passing on the pre-fix retry behaviour. Trim the builder
comment to the WHY.
Building the summary through `_build_api_kwargs` means it now carries
`tools`, but it still skipped the `_sanitize_structure_surrogates` /
`_force_ascii_payload` chokepoint the main loop applies after building
(turn_api_request). On cache-planned routes the main loop scrubs a deep
copy of the tools, so `agent.tools` can still hold a lone surrogate that
providers reject with a non-retryable 400 (#50959 class) — a request the
tool-less summary never used to make. Apply the same two passes here.
The destination preflight / refusal path suppressed the whole progress lane
for a flat DM regardless of mode, so an operator who WROTE `tool_progress:
all` got nothing there (before #108668 they got text bubbles via the
fallback). Silence is right only for Slack's tier default, where no text
lane was asked for; explicit new/all now routes through the editable text
fallback instead. Also hoists resolve_tool_progress into the existing
display_config import in _run_agent_display_settings.
Test proven red on the salvaged head (adapter.sent == [] with `all`).
Review findings (Salt, adversarial pass on the two preceding commits):
- BLOCKING: a `tool_progress: null` (global, platform, or legacy overrides)
counted as an explicit mode because the gate tested key presence, while
the display resolver skips None and inherits. Null resolved to Slack's
tier default `off` and disabled cards, which is the default-off trap the
change exists to avoid. Explicit intent is now a non-None value (or the
env bridge). Tests cover null at each level plus null-over-global-all;
mutation to key-presence turns the three null cases red.
- TASTE: `_TaskCardState.egress_declined` now also latched on unsupported
destinations, so the name no longer described the field. Renamed to
`publication_suppressed` with both causes documented; readers unchanged.
- SHOULD-FIX: slack.md still promised an unconditional text fallback and
described the opt-in as independent of tool_progress. Rewritten: cards
follow an operator-written off (including /verbose), null inherits, an
un-threaded chat with the card lane active shows no tool progress, other
native failures keep the editable fallback.
In flat Slack DMs (reply_in_thread false) the connector refuses task cards
("slack task_card requires a thread anchor"; native Slack: "No Slack thread
target"). The card lane treated that like a transient native failure and
fell back to an editable text message, so every tool event re-rendered
"Hermes is working / - tool - running" in the DM: text tool progress on a
platform whose default is off, for an operator who never enabled it.
Treat unsupported-destination refusals as terminal for the turn (same
latch as an egress decline) and log at info; transient native failures
keep the text fallback.
Slack task cards are tool progress rendered natively, but the card lane
ignored the operator's tool_progress mode. Slack's built-in display tier
sets tool_progress off, so the lane was decoupled on purpose (#29483) to
keep cards on for unconfigured installs. The side effect: an operator who
wrote `display.platforms.slack.tool_progress: off` to silence tool updates
still got cards, and on relay-fronted Slack (where the connector always
advertises task_card) there was no setting that could turn them off.
Gate the card lane on operator intent, not the tier default: cards stay on
when nothing is configured, and go off only when tool_progress was written
as `off` (global, platform override, legacy overrides, or the env bridge).
`new`/`all` keep cards.
Tests assert the wire contract: no native card send, no stop, no fallback
text for an explicit off; card lane engaged for `new` and for the
unconfigured tier default (regression guard for #29483). The duplicate-tools
fixture now mirrors production's _safe_callback null-guard.
Three inline context-manager classes shared the same __enter__/__exit__
boilerplate and differed only in the events yielded before the raise.
Also drop the unreachable 'or agent.base_url' fallback: every
anthropic_messages init path sets _anthropic_base_url, and the two
sibling call sites read it bare.
A tool_use block name recorded by a stream attempt that died before any
visible text survived into the next attempt: only the deltas_were_sent
mid-tool branch cleared result["partial_tool_names"]. When the retry then
streamed plain text and dropped, the partial stub blamed the stale tool
("Stream stalled mid tool-call (old_tool)") and the stale name could make
a later attempt look mid-tool-call when deciding whether the drop is
retryable. Reset it in _start_stream_attempt alongside
provider_tool_in_flight, which already has attempt-local semantics.
Regression: two-attempt stream (tool_use start + parse error, then text +
drop) — stub content and emitted deltas carry no stale tool name.
Follow-up to the cherry-picked fix: keep the classifier widening
("expected value at line" is now a transient stream parse error on the
main turn) but replace the messages.create() fallback with a retry on the
same stream wire.
Why not create(): the fallback ran outside Relay (lost request rewrites),
outside _handle_stream_error (could replace text already shown to the
user with a different generation), ticked no liveness events for the
whole buffered payload, and a bare identical retry still re-emits the same
malformed JSON.
Why not drop the fine-grained-tool-streaming beta (#108583/#109056): live
probe on claude-sonnet-4-5, ~500-line tool call - beta on: max inter-event
gap 1.6 s; beta off: 139 s zero-event gap while Anthropic buffers the
args, which the 180/240 s stale-stream detector kills on larger payloads
(the regression 80a899a8e2 fixed).
Instead, on a parse error the retry sets `eager_input_streaming: false`
on every tool for that request only (the SDK/API per-tool field overrides
the legacy beta header), so Anthropic returns buffered, server-validated
args while the happy path keeps fine-grained streaming. A tool_use that
started streaming is registered in partial_tool_names so the mid-tool
transient retry fires the same way it does on the chat_completions wire.
Under a loaded runner the primary stall plus the fallback retry overran the
0.2s total ceiling, so the retry never started and attempts==1 failed
intermittently (seen once in a 40-worker tests/agent run). Idle stays 0.05s;
the ceiling moves to 2s per the >=2s wall-clock rule in AGENTS.md.
The "screen unchanged" result points the model at its previous capture. After
context compression that capture may be summarized away, so the note would refer
to pixels no longer in context. Mirror read_file's reset_file_dedup: the
compaction boundary (both the summary path and the codex app-server path) now
clears the session's screenshot digest, and the first capture afterwards delivers
the image again even when the screen is byte-identical.
Key the screenshot-dedup state by the same profile-scoped session id the
backend cache uses, so two multiplexed profiles sharing a session id (or a
DISPLAY) never dedup against each other's frames, and forget the state in
release_computer_use_session so a re-created session's first capture
always delivers pixels. Reword the unchanged note to cover the aux-vision
path (where the prior result was an analysis, not an image). Tests: the
dispatch path (explicit capture + capture_after) honours the streak cap;
release forgets state.
Port from openclaw/openclaw#129924: a capture whose pixels are
byte-identical to the previous capture of the same target in the same
session returns its full text metadata (element index included) plus an
explicit 'screen unchanged' note instead of the multimodal image block.
Adapted for Hermes: openclaw gates dedup on per-frame context-epoch
tracking; Hermes bounds staleness with a consecutive-omission streak cap
(2) so full pixels are re-delivered before compaction could evict the
referenced image. Dedup state is per-session (no cross-session leaks),
append-only (no history rewrites — prompt cache prefixes untouched),
and skipped entirely when no session_id is present.
Following the grant's source on every save made a fresh device-code
login (or `hermes auth import`) under a profile that had been borrowing
root's Codex grant overwrite root's account instead of creating the
profile's own. Redirecting a save into another file is the exception, so
it is opt-in: the refresh path passes write_through=True; login, import
and recovery keep saving locally. The two save branches collapse into one
(store, path, set_active) triple.
Test: root discovery on Windows comes from LOCALAPPDATA — set it so the
fixture's root is the resolved root on every host.
The picked tests monkeypatched _auth_file_path/_global_auth_file_path
directly and leaned on a HOME override to dodge the pytest seat belt.
Isolate the way the rest of tests/hermes_cli does instead: Path.home ->
tmp_path and HERMES_HOME -> <root>/profiles/<name>, so the fixture drives
the same get_default_hermes_root() resolution production uses. Drop the
classic-mode test (no new behaviour: source == active store is the
pre-existing save path). Two invariants remain: root-borrowed refresh
lands in root (singleton + pool) with no profile shadow; profile-owned
grant stays local with root untouched.
Codex refresh tokens are single-use with rotation-family reuse
detection. _save_codex_tokens resolved the state via the profile's
root fallback but always persisted into the ACTIVE (profile) store, so
a profile-scoped refresh left the global store holding the consumed
refresh token — the next process to read it replayed it and OpenAI
revoked the whole rotation family, forcing a manual device-code
re-auth (#87503; observed four times on one multi-profile deployment).
Mirror the xAI source-aware save (#43589/#74339): resolve the state
with _load_provider_state_with_source; when the grant came from the
global root, write the rotated chain back to root only — singleton AND
credential_pool entries, under the root store's own lock, without
creating a shadowing profile key. Best-effort, with the same pytest
seat belt as the xAI path.
Fixes#87503
The launcher guard rejected every python shebang, so a pip/uv console
script pinned to the running venv (#!<venv>/bin/python) was also
discarded in favour of `python -m hermes_cli.main`. Reuse
linux_desktop_entry._shebang_escapes_running_env, which already knows
that `env` shebangs escape and a shebang inside the running
interpreter's directory does not; only the escaping launcher loses the
venv. Also drops the second shebang classifier the fix had introduced.
attested_gateway_died() re-ran find_gateway_pids() (current profile only)
although both callers had just proven the process table empty with
all_profiles=True, and it re-implemented check_start_attestation's
liveness rule. Callers now pass the liveness they hold (current_pids=[])
and both probes share _attested_dead(), so the consuming and read-only
twins cannot drift.
Four new tests overlapped: plan-time and spawn-time attested-death overrides
both exercised the same predicate via monkeypatched lambdas. Collapse to:
- one end-to-end test using a real attestation marker in a tmp home: dead
attested gateway keeps the plan under Desktop ownership, survives the
spawn-time re-check, and the marker is consumed by the spawn;
- one probe test: no marker / null or non-list pids / non-dict / non-JSON all
read False (fail closed), alive and clean-exit read False, read-only when
it does read True.
Existing #76129 tests keep their attested_gateway_died=False pins unchanged.
A Desktop self-update hand-off exits the app before the updater runs and can
kill the messaging gateway in those same seconds (#109538), so the updater's
discovery finds no live PID while the one-shot start attestation still
vouches for the dead one. Both Desktop-ownership checks then read "nothing
running" as "nothing to restore" and the bot stayed down until a manual
start.
Consult the attestation non-destructively before Desktop-owned lifecycle
suppresses a cold-start: a vouched-for PID gone without a clean ledger exit
keeps the plan and is restored; no attested death preserves the #76129 skip
unchanged.
Trimmed salvage of #110045 (deltas 1 + 2 only), stacked on the #110009 scope inheritance:
- `declared_conversation_scope` treats an inherited value as a DECLARED scope only when it
carries the `gwk_` prefix. A rotated CLI parent publishes no affinity scope (None → sticky
key falls back to the conversation root); the fork now publishes exactly the same instead
of an explicit physical lineage root. `resolve_prompt_cache_scope` honors any inherited
value directly, so the body `prompt_cache_key` still matches.
- `build_cache_parity_fork` snapshots `parent._conversation_root_id()` as
`_cached_conversation_root`; with `_session_db=None` the fork's own walk fell back to the
parent's PHYSICAL id, so after a compression rotation the review's Portal
`conversation=` tag fragmented usage attribution across one logical conversation.
Dropped from the original: copying `_gateway_session_key` onto the persistence-detached
fork (no cache-identity consumer reads it there; the compression-boundary hooks were
deliberately severed by `_detach_fork_compression`), and the defensive
hasattr/callable/try wrapper around `_conversation_root_id()`.
build_cache_parity_fork gives the same-model fork the parent's session_id,
cached system prompt, tools[] and session_start — but with
_persist_disabled=True and _session_db=None, BOTH cache-identity resolvers
diverged from the parent on their own: declared_conversation_scope failed
closed on _persist_disabled, and the lineage walk skipped on the missing
DB. The fork's affinity header (set_affinity_scope) and body
prompt_cache_key (cache_scope_id on the OpenAI-wire transports) therefore
keyed a different bucket than the gateway parent, costing one cold
~full-context request per review. Not gateway-only: any parent whose
lineage root != current physical id diverges too (teknium1's triage table).
Fix, per the triage's suggested direction: on the not-routed branch only,
the fork stamps _inherited_cache_scope = resolve_prompt_cache_scope_safe
(parent) — the parent's ALREADY-RESOLVED scope, no DB access from the fork,
persistence fully detached. Both declared_conversation_scope and
resolve_prompt_cache_scope return the inherited scope first when set, so
the header path and the body path are fixed together (fixing only one
leaves the other divergent — Vivamisu's header/body split observation).
Routed (different-model) forks, /branch children, delegate/tool children
and fresh sessions set nothing; the fail-closed default stands untouched.
/btw shares build_cache_parity_fork and gets the repair for free.
Three gaps in the #110544 guard, all reported in its review and reproduced:
- A writer reopened by _reopen_after_close_locked (teardown/worker race,
#94736) came back with no guard: the next stray close + foreign close
deleted its WAL again.
- _try_wal_checkpoint refreshed the guard outside self._lock; landing after
close() it pinned an OFD lock with no connection behind it, so a foreign
`PRAGMA journal_mode=DELETE` saw `database is locked` forever.
- Refcounts keyed on (fd, inode) treated a recycled fd number as a surviving
lock: A+B live, close A, C reuses A's fd, close B left C recorded as guarded
while a foreign EXCLUSIVE succeeded.
The guard now counts handles per inode, re-locks every matching descriptor on
each hold (OFD re-lock is idempotent), and unlocks on the last handle only;
the reopen path holds it; the checkpoint refresh runs under self._lock and
skips a closed handle. The macOS holder scan folds case so a case-only alias
of the sidecar path on APFS still matches.
Clean-room port of the approach in zed-industries/zed#63342. The native
Gemini adapter previously down-translated every tool schema into the
restricted FunctionDeclaration.parameters subset, which was lossy: anyOf
unions without an outer type, bare arrays, $ref/$defs indirection and
additionalProperties had to be stripped or repaired, and one
unrepresentable construct could 400 the entire request (live repro:
INVALID_ARGUMENT ...properties[bare_array].items: missing field).
Google now accepts plain JSON Schema in parametersJsonSchema on all
current models. The adapter sends full schemas through that field; the
old subset translator is replaced by a light normalizer that deep-copies,
strips root $schema, inlines same-document $refs (MCP pydantic / zod
emit them; unresolvable or circular refs pass through untouched with the
reason logged), and guarantees an object root.
Live-verified against the real API: the union+bare-array+$ref schema
that 400s through the legacy parameters field is accepted with 200 via
parametersJsonSchema on gemini-3.7-flash and gemini-2.5-flash, and
gemini-2.5-flash returns a correct functionCall against it.
tests/contracts -> tests/tui_gateway/contracts (tree-layout rule: tests mirror a source
package). test_rpc_params_cannot_spoof_runtime_artifacts: forged owner_transport /
owner_session_record / owner_token keys are now refused at the wire (4000 + key path)
instead of silently dropped before the handler; the invariant (no steer reaches the
agent) is unchanged and asserted directly.
apps/shared/src/gateway-events.ts is now a thin layer over
gateway-contract.generated.ts (client-local synthetic events + the
GatewayEvent envelope); gateway-events.json, its two rendezvous tests and
the duplicated BillingBlock / SessionInfo / ProjectInfo hand copies are
gone. Desktop, TUI, web and shared typecheck against the generated
RpcMethods / ServerRequestMap / BackendGatewayEventMap.
What tsc found once the types were honest: three phantom fields the
backend never sent (tool.start.todos, error.reason,
voice.transcript.voice_stopped) - the TUI todo tests were driving the
list through the phantom and are retargeted to tool.complete, where the
wire actually carries it; nullable fields (`None` on the wire) were typed
as plain optionals in eight places and now coerce at the boundary;
SessionResumeResult had a stale generic.
Contract fixes from the consumer pass: TranscriptMessage is the gateway
projection (text/row_id/context/args), not the stored row; SkinPayload
matches HermesSkin (empty-string defaults, never null); SessionLiveInfo
model/tools/skills are required (always emitted); BillingBlock.billing_url
is required-nullable (dataclass asdict).
tui_gateway/AGENTS.md documents the declare -> regenerate -> tsc loop.
Handlers own their documented domain codes (4006 missing session_id, 4015 bad
url, 4009 orphan claim); the contract's job on the way in is the one check no
handler performs — an unknown key (4000 with the key path). Missing/mistyped
fields are re-checked AFTER a successful handler answer under the strict
test policy, so a contract narrower than the wire still fails the suite.
Two models widened from the suite: SeedMessage (clients forward stored rows
verbatim), tool.complete.args (mirrored child rows omit it). Tests that
drove session.activate with prompt params (and vice versa) or stubbed
_live_session_payload with a bare {session_id} now send the real shapes.
215 methods, 13 server→client requests and 67 notifications now have Pydantic
contracts under tui_gateway/contracts/<topic>.py, rendered to
apps/shared/src/gateway-contract.generated.ts (616 types) and
gateway-contract.openrpc.json. tests/contracts/test_generated.py pins both
files to an in-memory regeneration and asserts catalog completeness from the
CODE side (every registered handler / emitted event / sent request has a
contract, nothing orphaned). scripts/ci/classify_changes.py runs the Python
lane when either generated file changes.
Phantom fields the hand-typed TS carried and no emitter ever set:
tool.start.todos, error.reason, voice.transcript.voice_stopped.
Under the 40-worker file runner, service.stop(timeout=1.0) / runtime.stop(timeout=0.5)
and the 2s _wait_for lost the race to scheduler latency (a different test each run,
green on retry). Bounds move to the 5s the other 25 sites already use; the bounded-stop
invariant keeps a 2s ceiling, still far inside the join timeout.
The gateway asked the user questions (approval, clarify, sudo, secret,
vault, MCP setup, the desktop read/act bridges) by emitting a
`<x>.request` EVENT carrying a hand-minted request_id, blocking the
agent thread on a module dict keyed by that id, and exposing a paired
`<x>.respond` METHOD per kind — thirteen pairs, four registries
(`_pending`, `_answers`, `_batch_clarify`, `_EXPIRING_REQUESTS`) and a
per-kind reconnect snapshot (`pending_clarify` / `pending_approval`)
that only two of the thirteen kinds ever got. JSON-RPC already has the
primitive: the server sends a request frame with an id and the client
answers with a response frame bearing the same id.
`tui_gateway/server_requests.py` owns the one mechanism:
send() block the agent thread until the response frame
(`srq-<n>` ids; ints belong to the client)
send_async() fire-and-callback variant (bot relay)
cancel*() withdraw with ONE `request.cancel {id, method, reason}`
event (timeout / interrupt / process exit /
answered elsewhere) instead of per-kind *.expire
open_requests() the still-open frames, replayed by session.resume,
session.activate and session.events.since so a
reconnecting client re-renders every kind, not two
clarify.lock stays a real client→server RPC (locks one batch
answer early); locked answers merge into the final
set even when the closing response carries only the
tail the user answered last
A client that does not implement a method answers -32601 and the agent
fails fast (the old fixed-timeout "unavailable" probes for tour/preview
still work — a wire error IS an answer). Approval: the queue entry's
settle hook withdraws the request when `/approve` from another surface,
a timeout or an interrupt resolves it first, so no window keeps a dead
card. Compute-host children own their waits; the parent mirrors their
open frames for replay and relays `clarify.lock` + response frames.
Clients: `JsonRpcRequestChannel` gains `onRequest` (unhandled → -32601,
dedup by id) and `JsonRpcGatewayClient` re-delivers `open_requests`
from the replay result. Desktop gets `gateway-event/server-requests.ts`
(one handler per method, replacing the request branches of
`input-requests.ts` / `desktop-bridge.ts`) and a `store/server-requests`
registry so every answer site calls `respondToServerRequest(id, result)`
synchronously; the TUI gets `createServerRequestHandler.ts` +
`serverRequestStore.ts`. `gateway-events.json` now pins both halves
(events + server request methods); the two contract tests check both.
Live (real stdio gateway, real `clarify_callback` on the agent thread):
before, `clarify.request` event + `clarify.respond` RPC, batch final
answers lost ('' returned); after, `{"id":"srq-…","method":"clarify"}`
frame, `session.events.since.open_requests` replays it, response frame
`{"answer":"yes"}` reaches the agent, batch lock + final response
merge to `{"q0":"1","q1":"free text"}`.
The backend never sent a JSON-RPC request; when it needed an answer from the
renderer it hand-correlated a `*.request` notification with a later `*.respond`
method through four module-level dicts, a timeout thread and 13 derived
`*.expire` names, plus a separate reconnect snapshot per prompt kind. That is a
second request/response layer built on a protocol that already has one.
`tui_gateway/server_requests.py` sends `{id: "srq-…", method, params}` and
blocks on the response frame with that id (string ids never collide with the
clients' integer ids). One `request.cancel {id, method, reason}` notification
withdraws a request on timeout / interrupt / session close. `open_requests` on
`session.resume` / `session.activate` / `session.events.since` re-delivers
unanswered requests after a reconnect; the shared TypeScript channel does that
itself before the caller sees the result. Batch clarify keeps its per-question
locks as a normal `clarify.lock` RPC (the last lock resolves the request).
Approvals stay queue-backed (`tools.approval` owns the timeout, `/approve all`,
coalescing): the request resolves the queue entry and the entry's own
resolution withdraws the request through `register_gateway_settle`.
Deleted: `_block`, `_respond`, `_pending`, `_answers`,
`_pending_prompt_payloads`, `_batch_clarify`, `_EXPIRING_REQUESTS`, the
`*.respond` methods, every `*.request` / `*.expire` event, `pending_clarify`.
Compute-host (turn isolation) mirrors the child's open request and relays the
response frame / lock to it. Desktop, TUI and shared clients register
`onRequest` handlers where they used to switch on `*.request` events; answers
are response frames over the socket the request arrived on, so #91684's
owner-routing class cannot recur for prompts.
Two remaining halves of #89184 (Desktop Settings saves rewriting unrelated
config):
- The `fallback_providers` structured editor normalized every entry down to
`{provider, model}`, so any edit (remove a row, pick a model) re-emitted a
hand-written local-gateway chain without its `base_url` / `api_key` /
`key_env` / `api_mode` — the next autosave persisted bare pairs and the
fallbacks silently routed to the public provider. Entries now carry every
key through; the editor only owns the two selects.
- `PUT /api/model/moa` did `cfg = load_config(); cfg["moa"].update(...);
save_config(cfg)`: the whole default-expanded snapshot went back to disk,
so a Desktop MoA autosave re-persisted every other section too (the
2026-09-10 repro: `fallback_providers: []` written alongside the MoA block
the user had just edited). It now saves `{"moa": ...}` with
`merge_existing=True`, the same section-scoped write every other sparse
writer uses since #110535. Hand-edited moa keys (#58819) still survive.
The `model.default not persisted / base_url cleared` symptom from the 0.20.4
report no longer reproduces on main through the real REST path (Config page
diffs against a baseline since 5361867c6d32; `_denormalize_config_from_web`
keeps the on-disk `model:` block).
SessionDB(read_only=True) cannot create a missing store, so a fresh install
running hermes insights / /insights errored instead of reporting no data
(reported by @ehz0ah on #110718; guard shape from @kshitijk4poor's #110026).
Co-authored-by: kshitijk4poor <kshitijk4poor@users.noreply.github.com>
emozilla