rearm_failed_platform takes the manager's discovery lock, which a reload sweep or a deferred load holds for its whole import, so call it through the scope-carrying executor hop like the lookup. The hung-load guard now matches the loader thread name through one _LOADER_THREAD_PREFIX constant instead of a copied literal. Gate r2 Lows (2c, efficiency, quality).
Gate finding (PL1 r1 efficiency, Medium regression): after a re-arm, the
next watcher tick ran the plugin's import + register() inside the sync
_create_adapter on the event loop, and run_with_load_deadline joins for up to
plugins.load_timeout_seconds, which stalls inbound messages, heartbeats and
the control socket.
The watcher now materializes the registry entry with platform_registry.get
in the gateway executor first, using _run_in_executor_with_context so the
profile ContextVars are preserved, and _create_adapter then finds the plugin
already resolved. The rest of _create_adapter stays on the loop, as it was
on main. Builtin adapter constructors ran there before this stack and may be
loop-affine, so moving the whole call into the executor would change
unrelated behaviour.
Gate finding (PL1 r1 efficiency, Medium): a permanently broken platform
plugin was re-armed and re-imported on every reconnect backoff tick
forever, re-logging its load failure each time.
The watcher now counts successful re-arms in the platform's reconnect info
dict and stops after 3 (_MAX_PLUGIN_LOAD_REARMS), so a transient startup
failure still heals while a broken plugin stops being re-imported. The
platform stays queued as adapter_unavailable, as before this stack. No new
config key.
Gate finding (PL1 r1 2c/efficiency, High): a plugin whose import or
register() hangs overruns its load deadline and leaves an abandoned daemon
loader. The reconnect watcher re-armed it on every backoff tick, adding one
more abandoned thread each time, and once _MAX_ABANDONED_LOADERS (8) were
live every plugin load in the process was refused (about 25 min at the
30-300s backoff).
rearm_failed_platform now skips the platform while its
plugin-load:<manifest_key> thread is still alive in _ABANDONED_LOADERS
(checked under _ABANDONED_LOADERS_LOCK). This was chosen over "never re-arm
after a timeout": a deadline overrun caused by transient startup I/O, where
the worker eventually finishes, still heals on a later tick, and at most one
abandoned thread per plugin can ever exist.
Gate finding (PL1 r1 2c/reuse/quality, High): rearm_failed_platform matched
any platform LoadedPlugin with an error and enabled=False. _gate_manifest
writes that same shape for its placeholders (disabled via config, not
enabled in config, removed from the catalog, legacy Relay refusal), and
the requires_hermes gate does too. The reconnect watcher therefore
imported plugins the user or the catalog had deliberately blocked.
The re-arm now re-runs gate_manifest against the current config and only
proceeds when the verdict is still defer/load, and it skips a
requires_hermes mismatch. The registry-snapshot alternative
(snapshot_registration showing a consumed loader) was probed and rejected:
a genuinely failed deferred load disposes its lease, so the snapshot shows
no loader (it would never re-arm the headline case), while a requires_hermes
skip leaves one behind (it would match the wrong case).
Gate finding (PL1 r1 quality, Low): _rearm_failed_platform_load was a
one-line forwarder in the handler-rewire mixin, unrelated to rewiring, and
its bool result was discarded at its only caller. Call the plugin manager
directly at the run_adapters call site instead.
A bundled platform plugin whose deferred load raises or overruns its load
deadline at startup (e.g. I/O contention from a state.db integrity check) has
its lease disposed, so the registry forgets the platform. The reconnect
watcher keeps it queued as adapter_unavailable, but nothing ever re-runs the
import: the platform only heals after a manual reload-plugins or a restart.
The watcher's adapter_unavailable branch now asks the current profile's
plugin manager to re-lease that platform's deferred loader (only when the
plugin is recorded as a failed platform load), once per backoff tick, so the
next pass retries the import. No new config key; the watcher already owns the
bounded backoff.
Refs #126356
Co-authored-by: Zheqing Zeng <26277217+zengzheqing@users.noreply.github.com>
Co-authored-by: JerryLiu369 <953639086@qq.com>
MessageEvent is imported at runtime in the new module, so the quoted forward references carried over from adapter.py are unnecessary. Final-head gate (quality Low).
Code-health ratchet: plugins/platforms/telegram/adapter.py was 7446 lines > its 7422 cap after the
held-inbound hand-over (#132829) added adopt_held_inbound/_adopt_held_event. The whole hold queue
(_schedule_held_inbound_redispatch, _hold_inbound_event, adopt_held_inbound, _adopt_held_event,
_rehold_from, _redispatch_held_inbound) moves verbatim into TelegramHeldInboundMixin; TelegramAdapter
inherits it, so call sites and instance-level patches are unchanged. The sibling logs under the
adapter's logger name so existing log filters still match.
Gate finding B05-3: once the reconnect entry stores the predecessor
adapter instead of a snapshot of its caches, carry_inbound_dedup is the
only caller of inbound_dedup_caches, and its "held by reference" docstring
described the old snapshot contract. Inline the isinstance filter into
the loop (fewer lines than rewriting the docstring; no tests reference
the helper) and return early on a missing predecessor instead of a
ternary.
Gate finding B05-2: adopt_held_inbound is only reached through
hand_over_held_inbound, which skips a None predecessor, and both runner
callers pass a candidate freshly built by _create_adapter for the same
platform as the replaced instance. So predecessor is never self and is
always a TelegramAdapter; the guard was dead code. Also use the file's
own `events = list(held); held.clear()` idiom.
Gate finding B05-1: the successor-forward branch of _hold_inbound_event
returned before self._accept_update(). The admission ContextVar is per
adapter instance, so the successor's own _accept_update() in its hold
path reads its own (empty) var and the predecessor's in-flight claim
stays unaccepted. _release_claim then skips _record_receipt, and the
update can be replayed after a restart even though it was handed off.
Accept on the retired adapter before forwarding, and deref the successor
weakref once.
Telegram holds inbound events that PTB has already acked (offset advanced),
so dropping them means losing the message for good. The hold queue lives on
the adapter instance and drains only on that instance's _mark_connected. When
a retryable fatal (network error, polling/ingress stall) makes the runner
build a NEW adapter, that drain never runs: the reconnect path carried the
inbound dedup caches across but not the queue. Held events, plus anything
teardown salvaged onto the retired instance later, were silently stranded.
The reconnect queue entry (primary) and the secondary reconnect task now
keep the replaced adapter as "predecessor". The existing carry_inbound_dedup
seam reads its dedup caches from it before connect. A new
hand_over_held_inbound seam moves its held events at publish time
(_install_reconnected_adapter / secondary profile_map install). A candidate
that fails to connect never owns the queue, so no hand-back is needed, and
delivery waits until the replacement is registered. TelegramAdapter.
adopt_held_inbound rebinds only source._transport_adapter_ref (profile and
role authorization stay as resolved) and leaves a successor weakref on the
predecessor. Late _hold_inbound_event calls there (teardown salvage after the
replacement drained) forward to the replacement.
Reported-by: j2h4u (Maxim Brashenko)
Fixes#132829
The only caller passes a positive budget and a zero remainder already yields an empty head via _head_within_budget, so the early break never changed the result; the boundary branch now binds content once. Final-head gate (quality Lows).
Gate r1 Lows: parametrize the whole current user item instead of a None
sentinel rewritten inside the test body; drop the per-part estimate sum,
which the exact-content equality already pins; drop the no-checkpoint row,
which passes on base and so does not pin the fix. Both tests stay red on base.
Gate r1 Low: _head_within_budget hard-coded the 4 chars/token ratio that
agent.model_metadata already names CHARS_PER_TOKEN, and bisected even for
all-ASCII heads, whose estimate is exactly ceil(len / CHARS_PER_TOKEN) so the
budget-sized slice always fits. Return that slice directly when it is ASCII;
clamp the budget at 0 so a non-positive budget still yields ''.
Equivalence vs the previous implementation: 3000 random cases (a/ж/中/é/space,
budgets -2..20), 0 mismatches.
The stack already carries its two invariant tests for #131788. Instead of a
new test function, the existing newest-first truncation test is
parametrized over ASCII/Cyrillic/CJK boundary text, which still goes red
on the old budget*4 char slice for the non-ASCII rows.
#132147 re-implemented the budget*4-capped estimator bisect inside
_truncate_input_text_parts, while FroRaut's #127458 introduced the same
search as _head_within_budget for the plain-string boundary. Two copies of
one search drift; the typed input_text path now calls the shared helper so
string and typed boundary items are cut by the identical rule.
Co-authored-by: FroRaut <79215537+Froraut@users.noreply.github.com>
When a native compaction checkpoint is replayed, the oldest
pre-checkpoint user message that only partly fits the retained-user
budget is cut at the front. The cut was content[: user_remaining * 4],
which counts characters, but the budget is charged with
estimate_tokens_rough, which counts UTF-8 bytes / 4 and one token per
CJK character. A Cyrillic boundary message kept about 2x the remaining
budget and a CJK one about 4x.
agent/native_compaction.py:prune_pre_checkpoint_items now cuts with a
new _head_within_budget helper, which binary-searches the longest
prefix whose _approx_tokens fits the remaining budget. ASCII text
still keeps user_remaining * 4 characters, and the str slice never
splits a UTF-8 character.
(cherry picked from commit 95b069263440372bec619bb195aa68ba42d213ca)
BN01 fold2 finding (2c Low): collapsing the over-budget branch in
d24e5875b7 read `item["content"]` unguarded. A user message whose text
is measured only via `output_text` (no `content` key) reaches that
branch when it exceeds the budget and raised KeyError, aborting the
prune. Use `item.get("content")` so it falls through to the
non-truncatable path (keep nothing, budget reset) as before the
refactor. The typed-boundary parametrize gains a row for it.
BN01 quality finding (Low): the over-budget user path had a string
branch and an else branch that each ended with their own
`user_remaining = 0`, and the else re-tested `parts_cost is not None`.
Fold them into one else that picks the head (string slice, typed-part
head, or nothing for a non-truncatable boundary), appends it when it
has non-blank text, and resets the budget once. Behaviour is unchanged:
a snapshot probe over string, blank, typed, blank-typed and text+image
boundaries prints identical output before and after.
BN01 efficiency finding (Low): the head search over an oversized
input_text part started with hi = len(text), so each of its ~20 probes
re-estimated slices up to half the whole part on every native-compaction
request. estimate_tokens_rough charges every char at least 1/4 token
(ASCII (n+3)//4, other non-CJK >= 1 UTF-8 byte, CJK-dense 1), so no
prefix longer than budget*4 chars can fit and the cap never changes the
result. Proven with a randomized real-import check (2000 cases across
ASCII, Cyrillic, CJK, emoji and mixed text: 0 mismatches); 1M Cyrillic
chars at budget 64k: 40.8ms -> 28.0ms.
BN01 quality finding (Low): _truncate_input_text_parts copied each
fully-fitting part with {**part} although it never mutates it. The
fully-fitting item path already appends the original item uncopied,
and the clipped boundary part is still a new dict. No caller mutates
the returned parts (the result is only placed into the retained
message), so the copy was dead work.
Keep the converter->pruner checkpoint/no-checkpoint case (the issue's
headline: an older completed ask must not survive while the newest
oversized typed ask is dropped) and one boundary case covering multipart
clipping, the token-estimator budget for CJK text, and an oversized
mixed text+image boundary that now retains no older ask (behaviour change:
previously the oversized boundary was skipped and an older ask kept).
Also drop the provenance-footer disclaimer from the docstring.
Parallel fix in #131819 by liuhao1024 credited here.
Co-authored-by: liuhao1024 <sunsky.lau@gmail.com>
Code-health ratchet: plugins/platforms/matrix/adapter.py was 3308 > 3278 and
tests/gateway/test_matrix.py 3310 > 3245 (both over the 2,000-line target, so
they may only shrink vs main).
- Move the existing _CryptoStateStore OlmMachine state-store shim (byte-identical)
into the new topical sibling plugins/platforms/matrix/adapter_crypto.py; the
one call site in _connect_setup_e2ee late-imports it. No facade re-export.
- Move this stack's encrypted-drop warning tests (and their _cryptoless_client
helper) into tests/gateway/test_matrix_encrypted_drop_warning.py.
Gate finding B08-2: `((room_data or {}).get("timeline", {}) or {}).get("events", ()) or ()`
guarded against shapes a /sync join entry never has. rooms_join comes from
`sync_data.get("rooms", {}).get("join", {})` (same unguarded style a few lines
up) and mautrix's own handle_sync reads the identical payload as
`room_data.get("timeline", {}).get("events", [])`, so any malformed entry would
already fail there. Match that form; keep the per-event isinstance guard.
Gate finding B08-1: when MATRIX_E2EE_MODE is optional/required but the
decryptor failed to set up at connect (degraded mode), the warning still
told the user to pip install the E2EE deps and set MATRIX_E2EE_MODE=optional
-- advice that is wrong (the mode is already on) and hides the real cause.
Keep the install hint only for mode=off; otherwise name the active mode and
point at the earlier Matrix E2EE startup warning. The warn-once test is
parametrized over off/optional to pin both messages.
Keep two invariant tests: the warn-once test now also carries a plaintext
room in the same sync (must not warn) and asserts the MATRIX_E2EE_MODE
hint; the crypto-attached test stays. Drops the standalone plaintext test.
The default MATRIX_E2EE_MODE is off, so installing the E2EE deps alone
never attaches a decryptor; an operator following the old hint would
stay deaf. Point at the mode setting too.
Under hermes acp/gateway, a Matrix connection with E2EE off or degraded
(optional mode with missing deps or a failed setup) drops every
m.room.encrypted timeline event silently: mautrix dispatches them to an
empty ROOM_ENCRYPTED handler set, so the room looks connected but deaf —
syncs succeed with no errors and no warnings (#131778).
_absorb_sync now checks each joined room's timeline and logs one warning
per room when encrypted events arrive while client.crypto is unset,
pointing at the E2EE install hint. With a decryptor attached, mautrix's
own DecryptionDispatcher already reports decryption failures.
(cherry picked from commit 8cf08825296b88ab57ed0b83371840bb5fc7c7c0)
Gate finding B01 #1 (2c + quality): since api_server (/v1) runs now ride
the cron drain floor, the docstrings and comments for
parse_cron_drain_timeout, _load_cron_drain_timeout,
_get_cron_drain_timeout, the DEFAULT_GATEWAY_CRON_DRAIN_TIMEOUT comment,
cli-config.yaml.example and the messaging guide still called it a
cron-only floor. Say it covers /v1 runs too, that 0 opts them out as
well, and why: an interrupted /v1 run fails its waiting caller.
Sibling sites of the api drain-floor fix: the 'waiting up to Ns' shutdown
log gated on cron only, so a longer wait held open for api_server runs
was silent; and the cron_drain_timeout default comment still described a
cron-only floor (with a duplicated #82161 paragraph). Log both counts and
describe the floor as covering cron jobs and api_server runs.
stop()'s drain grouped api_server (/v1) runs with chat turns on the
restart_drain_timeout budget, which defaults to 0. A default-config stop
therefore never entered the drain loop and hard-interrupted every
in-flight /v1 run after 0.00s, even though a caller is blocked on its
result and nothing resumes it. #82161 split only cron off onto its own
watchdog-clamped floor (agent.cron_drain_timeout).
Let api_server runs ride that same clamped floor (no new config key);
chat turns and deferred post-turn workers stay on the chat budget, and
cron_drain_timeout: 0 still opts out. Reuses the existing
_active_api_run_count counter via _drain_work_counts.
Fixes#132989. Diagnosis and the 'ride the cron deadline' approach by
the reporter, @ItoDonno.
Code-health ratchet: hermes_cli/config.py was 4268 lines > its 4249 cap after the bare-model
container refusal grew the section guard. Move the guard verbatim into a topical sibling;
set_config_value late-imports it, and the sibling late-imports _exit_invalid, so there is no
module-level facade<->sibling cycle. No shim left behind; no test patches the old path.
#131435 gate reuse finding: _guard_section_overwrite computed the
mapping/list label twice and carried a second copy of the
"Replacing entire 'model' section" notice for forced containers. Past
the refusal, a container reaching the model branch implies --force, so
one `if force:` branch prints the notice with the right noun. Output
is unchanged. The docstring now names the container refusal, and the
inline comment is cut to one line.
The coerce exception compared `key.strip().lower() == "model"` while
_guard_section_overwrite, which gates the parsed value, compares
`key == "model"` (#131435 gate finding). set_config_value already
rejects keys with surrounding whitespace, and a differently-cased key
has no string default in DEFAULT_CONFIG so it never reaches the
exception. Use the same exact compare as the guard; behaviour is
unchanged.
The container refusal for the bare `model` shorthand only ran when the
existing `model` value was a mapping. With `model` absent or a string
shorthand, `hermes config set model '[a, b]'` wrote `model: [a, b]`,
a shape no model reader accepts (#131435 gate finding).
Hoist the refusal ahead of the mapping-section early-out: a list is
refused whenever --force is not given; a mapping is still refused only
over an existing section, since a mapping over an absent/string model
is a valid section write. Extends the kept refusal test with
absent/string-model list rows.
The salvaged fix re-ran yaml.safe_load in set_config_value with its own
copy of the invalid-literal error, because _coerce_config_set_value
returns string-default keys verbatim and bare `model` has a string
default. Exempt bare `model` structured literals from that early return
instead, so one parse site (and one error message, including the
quoting hint) serves every key. Behaviour is unchanged: the section
guard still refuses a mapping/list under bare `model` without --force.
`hermes config set model '<json>'` printed a success line while
redirecting the raw JSON text into `model.default` as the model id
(#131435, item 6): `model` is seeded as a string default, so the
generic coerce kept the literal verbatim, the bare-name shorthand rode
it into `model.default`, and every runtime reader then failed to
resolve the route while `config get` echoed it back.
Parse the literal first so the section guard sees the real shape:
- bare `model` + a mapping/list literal without --force is refused with
a pointer to `model.provider` / `model.default` (the same fail-closed
shape the container-slot guard took in #114471)
- with --force the validated mapping replaces the whole section, which
is the documented force meaning for section writes
(cherry picked from commit c08983182745c3eae395834a1fd5081a3b0267e7)
auxiliary_client.py is over the 2,000-line target and may only shrink; the normalizer this PR rewrites now lives in agent/auxiliary_codex_response.py and the adapter late-imports it. No behaviour change. Fixes the code-health ratchet (8426 > 8407).
Compatible hosts may return Responses output items as plain dicts; the SimpleNamespace coercion in _parse_codex_final_response had no test (gate 2ab mutation stayed green).
An incomplete response with incomplete_details.reason=content_filter kept
finish_reason 'content_filter'. The aux consumers (context compressor
truncation guard, micro_compaction, MCP sampling) only reject 'length', so a
filtered partial summary was committed as if it were complete. No aux
consumer reads 'content_filter', so map every provider-incomplete response to
'length' (completed tool calls still stay 'tool_calls').
The aux leak rescue re-derived the normalizer's completeness gates and got the
phase gate wrong: a completed response with a commentary item plus unphased
tool-call-shaped text came back as (text, 'stop') while the same mix with
plain text came back as 'length' (jonpol01 review on #132163).
Add recover_leaked_tool_call to _normalize_codex_response; aux passes False
because it has no continuation to re-elicit a real call. The text then stays
content and the normalizer's own phase/completion rule decides stop vs
incomplete, so the rescue block, its four private imports and the false
"continuation path will re-elicit" WARNING per aux call all go away.
The leak-like-answer rescue fell back to response.output_text whenever no
final-phase message carried text, even when the output had a commentary or
analysis item. The shared normalizer never treats output_text as the answer in
that shape (it is the streamed narration), so the aux path promoted mid-turn
commentary to a final "stop" answer. Apply the same gate: use output_text only
when no message item has a commentary/analysis phase.
Gate review r3 (2c Low #1). New matrix row: empty commentary item plus a
leak-like output_text must stay (None, "length").
The leak-like-answer rescue only refused responses with an unfinished *message*
item. A completed response whose function_call is still in_progress is equally a
partial turn (the shared normalizer flags every non-server-side item status in
_INCOMPLETE_STATUSES), yet the aux path rescued its text as a final "stop".
Mirror the normalizer's gate: any item whose type is not in
_SERVER_SIDE_TOOL_CALL_TYPES and whose status is unfinished blocks the rescue.
xAI's in_progress web_search_call on completed responses keeps working.
Gate review r3 (2c Low #2). The partial-item matrix row now uses an in_progress
function_call next to the leak-like message.
The completed-answer rescue copied the normalizer's message-item text
selection but not its output_text fallback, so a completed answer that
resembles a tool call and arrives only through output_text (empty output,
or an empty message item, from Codex-compatible hosts that return a
Responses object) was still dropped as empty "length".
Fall back to output_text when no message text is found. Commentary-only
responses keep codex_message_items set and never reach this branch.
Found by BN02 r2 quality review.
