Both primary recovery paths fenced the route lookup but not the WebSocket
ticket mint that follows it. When a connection apply (boot hook) or a
foreground activation (request recovery) landed while the ticket was in
flight and the newer socket had already dropped, the stale attempt dialed the
old primary route. connect() no-ops on an open socket, so the gap only shows
once the applied socket is closed, but then it re-attached the window to the
gateway the user had just left.
The boot reconnect now compares the route revision it recorded against the
current one again before dialing; request recovery re-checks ownsForeground().
useGatewayRequest's primary recovery resolves the connection profile-less, so
main answers through the window's CURRENT route. When the user activated
another source while that lookup was in flight, the late answer was published
with setConnection() over the newer foreground descriptor (and a failed
lookup cleared it), so REST and filesystem helpers routed to the old backend
while the newer source was in front.
Recovery now snapshots the gateway activation epoch and publishes, clears, or
dials only while the primary still owns the foreground. The boot hook's
own-route reconnect loop keeps restoring the primary socket, and the replay
guard already refuses to resend the original request.
Reported-by: dokterdok
Address review on #132237: an in-flight primary reconnect whose route lookup
resolved after a connection apply recorded its old descriptor over the newly
applied primary. The next socket drop then dialed the old gateway by its
explicit route, where the profile-less lookup on base followed the applied
window route.
Every primary route record now bumps a revision; a reconnect snapshots it
before the lookup and drops the result if a boot or apply recorded a newer
route meanwhile, so it no longer records, publishes or dials the old route.
The source-qualified primary lookup is unchanged.
Address the ordinary-switch draft-intent race from review on #132237: a
draft started on another gateway while a source switch was still doing its
bookkeeping (rememberConnection) was cleared by the switch's late re-home,
so the next session.create went to the switched-to source instead.
Every new-chat owner intent goes through captureNewChatSource, which now
bumps an intent revision. selectConnection snapshots it when the switch
starts and only re-homes the draft if no newer intent arrived, so a newer
intent wins even when it repeats the same route values. Completed switches
with no newer intent re-home as before.
Address review on #132237:
- Project-tree rows go through the canonical stampRowsWithOwningConnection:
a row that already names an owner is never re-owned, and local stays bare
on the primary path so the legacy profile door (#94166) is unchanged. local
is stamped only when it is an active registry secondary.
- Resume remembers an ambient owner hint only for a row that actually
resolved; an unresolved id no longer gets pinned to the foreground source.
- The silent boot restore keeps an explicit new-chat route pinned while it
was in flight; the route clear lives in one helper.
- Drop the untested pre-recovery route check in useGatewayRequest (the
post-recovery check covers it) and note why the two primary reconnect
paths resolve differently.
- Tests: move the primary-reconnect cases into the reconnect describe with an
inline resolver fake, assert the explicit primary profile exactly, and drop
an overlapping parametrized case.
Primary reconnect called profile-less getConnection(), which main resolves from
the window's foreground route. A drop while a secondary was foregrounded
re-dialed the primary socket to that secondary's backend. Reconnect by the
primary's recorded registry route (or legacy profile); boot and connection
apply keep following the foreground.
useGatewayRequest recovered a transport failure through the current active
route and retried the original payload there. A gateway switch between
dispatch and the delayed rejection replayed the request on the other source.
Retry only while the dispatch route is still active; otherwise surface the
original error.
Resume kept the ambient connection only for remote mode, so an untagged row
while This device was an active registry secondary reduced to a bare profile
and routed to the window primary. Keep the legacy primary-local profile door
(#94166) unchanged, and remember the resolved owner for later prompts.
projects.tree rows were published without connection_id, unlike the
Electron-tagged REST lists. Opening a tree-only session fell back to the bare
profile, which resolves to the window primary, so resume/prompt ran on Home
while This device was active. Tag rows with the same registry connection REST
requests are pinned to, and drop late responses after a source change.
A draft started from a gateway-group + pins $newChatRoute. selectConnection
captured the new source but left that route, and resolveNewChatOwnerRoute lets
an explicit route win when profile names match, so the next session.create
landed on the previous gateway while the UI showed the new one. Superseded or
failed switches leave a newer draft untouched.
Allowlists (*_ALLOWED_USERS, LINE groups/rooms, SimpleX groups) rendered as a
single empty input with a redacted "«redacted:…»" placeholder, so a saved list
looked like a hidden secret and could only be replaced wholesale.
- Backend: messaging env rows carry is_list, and list rows carry their plain
value (IDs are not secrets; password fields still only leave redacted).
- Desktop + dashboard: one visible text box per entry, x to remove, "+ Add
another" (Enter also adds); a pasted comma list splits into boxes. Saved as
the same comma-separated value the gateway already parses; removing every
entry and saving clears the key.
- Desktop help copy for the allowlist fields says "one per box" in all locales.
Review findings on the onboarding wizard:
- A token pasted with curly quotes reached the HTTP header and crashed with
UnicodeEncodeError; it now goes through the same non-ASCII stripping
save_env_value applies, before the Discord check.
- Reconfiguring replaced DISCORD_ALLOWED_USERS with the owner alone; the
wizard now starts from the existing entries and only adds.
- Hermes always requests Message Content, so a disabled intent makes Discord
refuse the connection (close 4014), not deliver blank messages. Wizard,
.env template and docs now say so.
The setup test records every write, so a rejected token appearing in any
write fails it, and uses ASCII fixture tokens now that the wizard sanitizes.
Discord was the hardest gateway to set up: the .env template had no Discord
section at all, the wizard saved whatever was pasted, and users had to find
the Message Content Intent toggle, build an OAuth2 invite URL by hand and turn
on Developer Mode just to learn their own user ID.
The wizard now reads GET /applications/@me with the pasted token: a rejected
token is re-prompted instead of saved, an intent that is off gets a direct
link to the toggle and a re-check, the invite link is printed with the
permissions the adapter uses, and the allowlist defaults to the application's
owner (or accepted team members). Offline, the token is still saved with the
manual intent reminder. The wizard moves from the 7k-line adapter into
plugins/platforms/discord/onboarding.py.
r2 quality Low: _infer_like_recovery was a hand copy of pass 1 of
map_lost_and_found_rows (classify -> LayoutEvidence.add ->
infer_physical_layouts). The test therefore checked a copy of the
production loop, and would stay green if pass 1 changed. Write the
sessions rows into a schema-less lost_and_found table, the way
test_session_recovery_lost_and_found._map_salvage_rows does, and call the
real map_lost_and_found_rows. Then assert every row is mapped, none hit
unrecognized_layout_rows, and the inferred layout is the physical column
order. Still red with the base prod files (29 of 38 sessions mapped).
r2 quality Low: the store fixture returned an open SessionDB and never
closed it, although SessionDB.__enter__ asks owners to release it
deterministically. It also returned the db as a tuple element the test
threw away, and set HERMES_HOME, which tests/conftest.py already sandboxes.
Yield from inside `with SessionDB(...)` and drop the unused db element and
the setenv.
r2 reuse Low: _physical_columns re-implemented
hermes_cli.session_recovery._table_columns (same PRAGMA, same values) and
_sessions_rows hand-rolled the quoted column list that _quoted_columns
builds byte for byte. Use the production helpers so the test cannot drift
from how recovery reads the table.
Gate r1 (2c Low): the comment above SESSION_ID_RECOGNIZERS narrated the
incident and its measurement, and _is_session_id repeated it. Keep the
contract in two lines at the tuple and point the predicate at it; the
measurement belongs in the PR body, not the source.
Keep one parametrized recognize/reject table over every minted shape plus
near-misses, and the end-to-end check that each shape as a parent id leaves
layout inference resolving every column. The dropped cases duplicated these
or re-asserted the existing wrong-layout veto.
session_lost_and_found now classifies through is_known_session_id, so the
noqa re-export only existed to satisfy an identity assertion in
test_hermes_state_ids. Import the predicate it uses and drop the pin.
The cherry-picked recognizers cover cron_, the CLI/gateway /bg shape and
room_, but a census of every site that hands an id to SessionDB on base
finds more shapes that salvage still drops (and that, as a
parent_session_id, still veto layout inference for the whole table):
tui_gateway/methods_prompt.py:1289 f"bg_{uuid4().hex[:6]}" (TUI/Desktop /bg)
gateway/platforms/api_server.py:3156/3341 f"api_{epoch}_{hex8}" (create/fork)
gateway/platforms/api_server.py:1084 f"api-{sha256[:16]}" (chat completions)
gateway/platforms/api_server_runs.py:689/703 run_{uuid4().hex} fallback
acp_adapter/session.py:179/203, api_server_openai_routes.py:1127 str(uuid4())
Each pattern stays anchored on exactly what its mint site fixes.
session recovery's salvage classifier decided "is this cell a session id?"
with SESSION_ID_PATTERN alone (^\d{8}_\d{6}_), which only covers ids minted
by new_session_id. Several first-class surfaces DERIVE an id instead:
cron/scheduler.py f"cron_{job_id}_{now:%Y%m%d_%H%M%S}"
hermes_cli/cli_commands_mixin.py f"bg_{now:%H%M%S}_{uuid4().hex[:6]}" (/bg)
gateway/.../api_server_room_dispatch.py f"room_{sha256(seed)[:32]}"
Measured against a copy of a real 3,373-session store: 379 ids (11.2%)
failed the pattern -- 375 cron, 1 bg, plus 3 non-session test artifacts.
job_id is normally uuid4().hex[:12] (cron/jobs.py) but legacy/user ids like
"job-1" are real and still present in live stores, so the cron recognizer
accepts a general job-id token.
Two distinct consequences, both fixed:
1. DROPPED SESSIONS. classify_lost_and_found_row() returned None for those
rows, so recovery silently discarded every cron/bg/room session. On the
real store: 2994 of 3373 rows kept before, 3370 after (the 3 still
dropped are 's'/'session-key'/'pre-compress-key' -- not sessions,
correctly rejected).
2. POISONED LAYOUT INFERENCE. parent_session_id is a _TEXT_SHAPE_RULES
column checked with the same predicate. A cron job's CHILD session has an
ordinary timestamp id, so it classifies fine and becomes layout evidence
-- while carrying the unrecognised cron id in parent_session_id. Since one
bad sampled value vetoes a candidate layout, a single such row collapsed
inference for the whole sessions table, i.e. the positional-guessing path
#101409's fix exists to prevent. Proven on a synthetic store: 20 clean +
cron parent + child-of-cron -> EMPTY before, 59/59 columns named and 0
wrong after.
Fix: hermes_state_ids gains SESSION_ID_RECOGNIZERS + is_known_session_id(),
the recognition side of the minting contract; _is_session_id() consults it.
Each pattern is anchored on exactly what its minting site fixes -- these are
layout sentinels, so a loose pattern would buy recall at the cost of
wrong-column mappings.
Tests: tests/hermes_state/test_session_id_recognition_recovery.py (new, 43
cases) covers recognition, non-dropping, the parent_session_id poisoning
path, and adversarial wrong-layout rejection (rotated layouts still vetoed;
near-miss and arbitrary text still rejected). Mutation-checked: with the
added recognizers removed, 12 of 43 fail -- including the end-to-end
poisoning test.
(cherry picked from commit f82b7182f03e1eee55c832a3f90fffbe580d1f5c)
Gate r2 (reuse Low): the long-request test rebuilt the fixture by
overwriting the request and appending groups 10..39, which
_oversized_active_turn(request, groups) already does. The
"user text after the last summary boundary" join was copied at three
sites; fold it into a test-local _actionable_user_text(). Behaviour is
unchanged: the long-request test is still red with 97183e2be1's
production change reverted (cut 2 > 3 fails) and green on HEAD.
Gate r2 (quality Low): two docstrings still described the split gate
measuring the request against the active-task cap. 97183e2be1 removed
that measurement; the cap now only shapes the task snapshot. Reword both
to state what the tests actually pin. Docstring-only.
Gate r1 (2c Low): the comment in _latest_user_task_snapshot claimed the
split-turn gate measures the same authored text, which no longer holds.
Keep only the reason the reply quote is dropped first. No net line growth.
The oversized-active-turn split (#80449) refuses to fire when the
authored turn-opening request exceeds 1,400 characters. A long /goal
continuation prompt therefore pins its entire autonomous run in the
protected tail: every automatic pass finds an empty compress window,
records no_progress, arms the 300 s structural backoff, and the session
grows into the provider context limit. /compress hits the same empty
window.
The token soft ceiling already bounds the request row, and the split
request is restated verbatim after the handoff by
_reappend_inflight_user_task, so the character cap only blocks
progress. Drop it from the split gate. The historical task snapshot
keeps its own cap and reply-quote handling.
(cherry picked from commit 0081611ff1f088ae0cca86d916ee802e74fe465a)
r2 quality Low: exact equality with [{"thread_id": "t1"}] already rules out a
notify key, so the extra `"notify" not in` clause could never fail alone.
Keep the equality and note why notify must be absent.
Gate finding (2ab Medium): dfba37255c dropped `notify` from both approval
sends because A2A reads it as the turn-final reply, but c484e47fe2 trimmed
the only test that pinned it, so reverting the runner change stayed green.
Extend the existing tests instead of adding a function: the card test
asserts send_exec_approval gets the verbatim thread metadata, and the
fallback-notice test gains a text-fallback row (_PlainAdapter) asserting
the prompt metadata is exactly thread_id + is_approval_prompt +
_interim_send. Both fail with dfba37255c's run_turn_runner.py reverted.
Add the clarify text-fallback invariant. It drives the real
TelegramAdapter send through text_fallback_coro and fails on base, where
the send carries disable_notification=True. Drop the runner metadata
test: it only pins that the metadata did not change from base. The
_notification_kwargs parametrize already fails on base for
is_approval_prompt, which keeps the stack at two invariant tests.
This is the same bug class as #132516. When a native clarify card fails
definitively, the runner retries with the base numbered-text
send_clarify, and that goes out through adapter.send(). In Telegram's
default "important" mode the retry carried only the thread metadata, so
_notification_kwargs sent the question with disable_notification=True.
The user got no push and the clarify wait timed out unanswered.
Tag the fallback with is_approval_prompt, the human-decision marker
_notification_kwargs now honours. text_fallback_coro only runs for
adapters that override send_clarify natively (Telegram, Discord, Slack,
...). The only other reader of the marker is WeCom, and WeCom keeps the
base send_clarify, so no other adapter changes.
The salvaged change also put notify=True on both approval sends.
On A2A, `notify` marks the turn-final reply, and A2A has no native
approval renderer. A prompt that needs a human decision would therefore
complete the caller's task with the prompt text while the turn is still
waiting (@ehz0ah's review on #132527). Telegram already pushes the
prompt through the approval-aware _notification_kwargs, so only the
is_approval_prompt marker is needed. The card send goes back to the
verbatim thread metadata: Telegram's control-message path never silences
cards.
Tests: the runner test pins that the fallback metadata stays interim and
does not carry notify. The adapter tests become a single parametrize.
Fold the _notification_kwargs change back into the original one-line
shape, so adapter.py grows by one line instead of six.
In the default Telegram notification mode ("important") the plain-text
approval fallback marked only is_approval_prompt, which no notification
decision consults, so _notification_kwargs sent the prompt with
disable_notification=True: the phone got no push, the user never learned
a dangerous command was waiting, and the tool sat the full approvals.timeout
(default 300s) before failing closed (#132516).
Mirror the cron-delivery convention on both approval sends in
_approval_notify_sync: notify=True on the button card and the text
fallback, and make _notification_kwargs itself approval-aware
(is_approval_prompt defeats disable_notification) so every surface that
marks the prompt is covered in one place. Loosens no guard and
auto-approves nothing.
(cherry picked from commit 576fec2ab6a6cdca08f96968574ba741c47568c0)
r2 quality Low: the cron-fire loop-liveness block made a real loopback TCP
connect to the discard port and relied on nothing listening there for
_forward_cron_fire_to_gateway to return None; anything bound to :9 would
fail the final assertion for a reason unrelated to the off-loop fix.
Monkeypatch httpx.AsyncClient with a stub whose post raises ConnectError,
keeping the None outcome without a socket dial. Still red with the
asyncio.to_thread wrap reverted (loop blocked ~3 s).
r2 2c Low: the timed-out own-gateway probe was raised to WARNING with no
dedupe, so a permanently wedged pipe peer logged it on every 30 s watcher
cycle and every control-socket rescan, indefinitely. Carry timed-out names
across passes in _profile_probe_timeout_warned (like the sibling
_profile_own_gateway_warned) and log repeats at DEBUG; a profile whose
probe recovers drops out of the set, so a later stall warns again.
9b85ac8795 moved _gateway_fire_endpoint (config/.env/multiplex reads) behind
asyncio.to_thread but nothing failed when the wrap was reverted (gate r1 2ab,
toothless). Extend the existing default-port test: block the resolver on an
Event and assert a concurrent asyncio.sleep heartbeat completes while the
forward is still pending. Red with the to_thread wrap reverted.
The stalled-probe branch set a -1 sentinel that fell into the "own gateway"
path: it warned "Profile '%s' still runs its own gateway" (sending the
operator after a process that may not exist — the stall can be a busy
housekeeping pool) and recorded the name in _profile_own_gateway_warned,
which then suppressed the warning if a real own gateway appeared next cycle.
A timeout now logs its own warning, skips the profile this cycle and stays
out of the blocked/warned set. Gate r1 2c finding (Low).
Same class as #132547 one process over: two async dashboard routes reach
`live_gateway_pid_for_home` (-> runtime record -> host-gateway control
dial) synchronously through `default_gateway_multiplexes()`:
- PUT /api/messaging/platforms/{id} via `_multiplex_port_binding_conflict`
- the Chronos cron-fire forward via `_gateway_fire_endpoint`
On Windows that dial is a pipe read, so a wedged gateway froze every
dashboard request for the length of the dial. Move both calls onto
asyncio.to_thread, the shape the sibling /api/gateway/start|stop routes
already use for `multiplexed_profile_refusal`.
Refs #132547.
`_query_windows_pipe` reads a synchronous pipe handle that has no settimeout,
so `_read_response_line`'s deadline is only checked between chunks: a peer
that accepts and never answers (a wedged gateway, or this gateway dialling its
own pipe from its own loop thread, #132547) blocks the caller forever. The
POSIX twin is already bounded by sock.settimeout and degrades to None.
Run the pipe exchange through the existing agent.deadline.run_bounded_sync so
every Windows caller of query_gateway_control (identify, pause, rescan, serve,
...) gets the same `timeout` bound and falls back to the scan layer on expiry.
A timed-out exchange abandons one daemon thread until the pipe read returns;
that is bounded by the number of stalled dials, not per-item hot loops.
Refs #132547.
reconcile_served_profiles called the own-gateway liveness probe inline on
the gateway event loop; the probe can end in a control-socket read with
no timeout of its own (a Windows named pipe stalls there until its peer
answers), parking shutdown_watchdog liveness probes until the multiplexer
is hard-killed with exit 75. The probe now runs on the bounded
housekeeping pool under a 5s bound; an unprovable profile reads as
blocked for the cycle (the multiplexer never races a possibly-live own
gateway) and the next cycle probes again.
(cherry picked from commit 84209e2f1a3bcddbc8b1e0123df70b02448d3f7a)
r2 quality Low: bundled_skill_texts skipped any file that failed a strict
UTF-8 decode, but skill_view's _read_skill_text reads with
encoding="utf-8-sig", errors="replace" and never raises, so a file with one
stray byte is still served to the model while the guard skipped it. Read
every file the same way and drop the try/except; the helper becomes a list
comprehension.
fba20028f4 also braced the placeholder in two tool-error strings that are
returned to the model as tool results (tool_call bridge unknown-name hint and
the malformed connector-name error), but nothing guarded them: reverting
either change kept every test green. Run the same ROLE_TAG pattern over both
rendered messages so a regression fails here instead of 403ing a session.
The model_tools.py connector-name string stays review-only.
Gate finding: SKILLTAG r1 2ab Low (toothless tool-error half).
skill_view serves any file under a skill root that decodes as UTF-8 text;
the extension set only drives the file listing. The role-tag guard filtered
by suffix, so 56 servable text files (.html, .sty, .bib, .tmpl, extensionless,
plugins/ assets) were never scanned, and its comment claimed otherwise.
Scan skills/, optional-skills/ and plugins/ in full, skipping only files that
fail the same strict UTF-8 decode, and fix the comment.
Gate finding: SKILLTAG r1 2c Low (scan scope).
website/scripts/generate-skill-docs.py output for the four skills whose
SKILL.md changed in this stack; docs-site-checks fails on generator drift.
Refs #132504
Widen the bundled-skill guard from the single tool placeholder in *.md to
the role-tag family (tool/system/assistant, open or close) in every file
type skill_view serves (md/py/yaml/json/sh/tex plus asset scripts), so the
next sibling - like the one in a system-atlas .mjs asset - fails CI instead
of a user's OpenRouter session.
Refs #132504
The same angle-bracketed chat role name that OpenRouter's gateway 403s as
role_tag_injection (#132504) also ships in:
- optional-skills/creative/system-atlas (SKILL.md + assets/data.example.mjs,
both served verbatim by skill_view) as a docs path placeholder;
- three tool-error strings returned to the model as tool results
(tool_call bridge "not a known tool name" hint, and the malformed
connector-name errors in model_tools and the connector gateway).
A tool result enters history and is re-sent every turn, so one of these
errors poisons the session exactly like a skill_view load does. Switch all
of them to the brace form already used for the skill docs. Comments,
docstrings and log-only strings never reach the provider and are left as is.
Refs #132504
The guard previously scanned only SKILL.md files, but skill_view serves
references/, templates/ and scripts/ content verbatim too, so a token inside
a referenced doc travels into later requests just the same. Scan every
bundled *.md under skills/ and optional-skills/ instead (the .hub cache
stays excluded), and move the test to tests/skills/ beside the other
bundled-skill doc contract tests.
Mutation-checked: injecting the token into a references/*.md turns the suite
red; restoring turns it green.
(cherry picked from commit 52d2e4e3dbda9d99151696b889e79cd8ca44a73c)
