The local faster-whisper model singleton (_local_model) is loaded once
and never released — the 'base' model holds ~370 MB of RAM/VRAM for
the entire lifetime of the process, even when no voice messages arrive
for hours or days. On long-running gateway processes (especially with
local LLMs competing for the same GPU) this is wasteful.
Add a config-driven idle unload: after stt.local.unload_after_idle_seconds
(default 0 = never) of no transcription activity, a lightweight daemon
thread sets _local_model = None so the Python GC can reclaim the
ctranslate2 objects. The next voice message reloads the model
transparently (the existing lazy-load path handles it).
The watcher:
- Checks every 30s whether idle time exceeds the configured threshold
- Acquires _local_model_lock before unloading (prevents races with
concurrent transcriptions that are mid-load)
- Exits immediately if the model is already None (unloaded by another
path, e.g. the CUDA fallback eviction)
- Is restarted by each transcription with the current config value,
so changing stt.local.unload_after_idle_seconds in config.yaml takes
effect on the next voice message without a process restart
Default is 0 (never unload) — zero behavior change for existing users.
Recommended value for gateway processes: 300 (5 minutes).
15 tests: config resolution (garbage/negative/None fallbacks), unload
safety (already-None, lock acquisition), touch timestamp, watcher
lifecycle (unload after timeout, no unload within timeout, exits when
model already None, stopped on new start). Existing STT test suite
unchanged.
Three-reviewer pass (reuse / quality / efficiency) on the trim diff;
four findings folded:
1. Short-clip input gate (efficiency, HIGH): the trim previously paid
the full ffmpeg encode before the <10%-saving discard check — every
dense conversational voice note burned 3 subprocess spawns + a
complete re-encode on the synchronous response path for nothing.
New _CLOUD_TRIM_MIN_INPUT_SECONDS=12 gate: below it, savings can't
matter (a >=10% saving is ~1s of audio, and several providers bill
a per-request minimum anyway — Groq bills 10s minimum), so the
whole pipeline is skipped using the duration we already probed.
Typical 5-10s voice notes now pay 1 ffprobe (~50ms), not 3 spawns +
encode (~0.3-1s; multi-second on small-VPS gateway hosts).
2. Shared encode profile (reuse, HIGH): the trim's ffmpeg command
duplicated _transcode_audio_for_stt's encode byte-for-byte (same
16kHz/mono/AAC-32k/faststart args, same subprocess.run kwargs).
Extracted _STT_M4A_ENCODE_ARGS + _run_ffmpeg_stt_encode(ffmpeg,
in, out, audio_filter=None); both call sites now share one owner,
so codec/bitrate/timeout changes can't drift between the paths.
3. is_truthy_value for the enable flag (quality, MEDIUM): raw
bool(cfg.get(...)) treated a YAML string "false" as enabled — the
exact bug class utils.is_truthy_value (already imported, already
used by is_stt_enabled and the xai/elevenlabs flags) exists for.
4. All-silence guard scales with keep_ms (quality, LOW): the fixed
0.3s floor equals the default keep window, so an output consisting
solely of one kept pause could pass as "speech"; now
max(0.3, 2*keep_seconds).
Also: _probe_audio_duration docstring documents it as the canonical
sync seconds-probe (gateway/run.py and the Telegram adapter carry
local variants of the same ffprobe invocation).
Tests: 24 now — YAML-string-false disables; short clips skip the
encode entirely (encoder mock asserted not-called); E2E fixtures
moved past the input gate. E2E re-verified: 13.2s note -> 6.2s
(-53%), 8s clip skipped with 1 probe.
Local faster-whisper gets Silero VAD (bf8004e3a) so silence never
reaches the model. Cloud providers got no such protection: the raw
file uploads untouched, so every second of silence in a voice note is
paid for twice — upload time and per-audio-minute billing — and cloud
Whisper hallucinates junk tokens on silent stretches exactly like
local Whisper did before the VAD hardening. A 13s voice note with two
long pauses is billed as 13s of audio to transcribe ~6s of speech.
Close the gap client-side: before uploading to a built-in cloud
provider (groq/openai/mistral/xai/elevenlabs/deepinfra), collapse long
pauses with ffmpeg's silenceremove filter, keeping
stt.cloud_trim_keep_ms (default 300) of every pause so word boundaries
and natural pacing survive. Uses ffmpeg, already a dependency of this
exact path via _transcode_audio_for_stt — no new dependency.
The trim is strictly best-effort — ALL of these upload the original
untouched, transcription never fails because of the trim:
- stt.cloud_trim_silence: false
- ffmpeg/ffprobe missing, trim failure, or timeout
- trimmed result ~empty (mostly-silence clip: the provider, not a
client-side dB heuristic, decides whether it contains speech)
- trim saves <10% (re-encoding for nothing)
Command-type and plugin providers are deliberately NOT trimmed: they
may wrap local CLIs that want the original bytes or run their own VAD.
E2E (real ffmpeg + faster-whisper): 13.2s voice note with 7s pause ->
6.2s upload (-53%); transcript of trimmed audio matches the original
on both utterances. Dense-speech and all-silence WAVs correctly fall
back to the original. 22 unit+E2E tests; STT/voice suite failures
identical to upstream/main baseline (all pre-existing).
_download_image() and _download_video() both used client.get() +
response.content, buffering the entire media body into memory before
checking the size cap. A server that omits Content-Length could send
an arbitrarily large payload, causing OOM.
Extract _stream_download_to_file() shared helper: streams via
client.stream() + aiter_bytes(), writes chunks to a temp file, enforces
the running byte count against the cap after each chunk, and atomically
replaces onto the destination on success. Cleans up the temp file on
failure. Uses utils.atomic_replace() for cross-device/symlink safety.
Malformed Content-Length values are now caught and ignored instead of
crashing with ValueError; the streaming cap is the authoritative guard.
Approach adapted from PR #10440 by @WuKongAI-CMU (closed as stale —
14923 commits behind, reverted 32 commits of vision_tools.py evolution
including SSRF-safe client, retry classification, and lazy imports).
Closes#10440
_fts_teardown_trash_step deleted rows via 'WHERE key IN (SELECT key
LIMIT N)' — each chunk's subquery re-scanned from the start of the
table, so chunk k skipped past (k-1)xN already-deleted rows: O(n²)
total row visits. On a v22 shadow table with ~230K rows that is on the
order of 10^8 row visits, turning optimize-storage teardown into a
multi-hour grind on slow disks, with a write lock held per chunk.
Single-column INTEGER-PK trash tables now drain via a fts_teardown_<tbl>_progress
high-water marker mirroring fts_rebuild_step: each chunk claims rows
past the marker (SELECT ... WHERE key > ? ORDER BY key LIMIT N), deletes
the claimed range, and publishes the new marker in the same transaction.
Per-chunk work is bounded → O(n) total.
TEXT-PK tables (the FTS config shadow table, pk like 'version') and
compound-key tables fall back to the legacy chunked delete — those are
small by construction.
Fixes#79324
Follow-up to the #80376 salvage:
- TurnLeaseToken.degraded is dead code since acquire() started raising
TurnLeaseTimeoutError: the only constructor site passes the default and
repo-wide there are zero external readers. Remove the field, its ctor
param, the repr segment, and the always-False guards in rebind()/release();
the class docstring's 'retained for compatibility' claim described
consumers that do not exist.
- Revert pure black-rewrap churn in test_config_env_bridge_authority.py and
test_turn_lease.py (hunks on functions this change does not touch), keeping
the functional Windows-env/encoding additions.
- Turn the default-value test into an invariant: config default must equal
gateway.turn_lease.DEFAULT_LEASE_WAIT instead of pinning the 1800 literal.
- Rewrap the dangling 'Released' comment line in gateway/run.py.
Verified: 20/20 targeted gateway tests, ruff clean; mutation check — with
gateway/turn_lease.py reverted to pre-fix main the module fails, restored it
passes.
prompt.submit honored truncate_before_user_ordinal on every request. A
client that carried a leftover ordinal into an ordinary send therefore
issued something the gateway could not tell apart from a real rewind —
same method, same shape, an in-range target — and the cut was applied
with replace_messages(), which DELETEs the durable rows. One report lost
244 messages (296 -> 52) with no prompt and nothing to restore from.
The existing guard only covered ordinal 0, where the cut empties the
transcript; a mid-session ordinal sailed straight through. Only the
client knows whether a submit is a rewind, an edit, or a regenerate, so
require it to say so: an ordinal without confirm_truncate is refused on
4029 and neither memory nor the DB is touched. Desktop sends the flag
from the one place that builds these params, so every rewind path is
covered and a stale build fails closed with an actionable error instead
of quietly deleting a conversation.
Sibling of the chat_completions zero-byte-args fix (previous commits):
a clean SSE close after content_block_start(tool_use) but before any
input_json_delta / message_delta yields an SDK final-message snapshot
whose content is NON-empty (the tool_use block is present, input={})
and whose stop_reason is None. That shape sailed past both
empty-stream guards (they only fire on empty content) and executed the
tool with empty input — no retry, no error: the same silent-data-loss
class as #80498, one provider transport over.
A legitimate completion always carries a stop_reason, so a
tool_use-bearing message without one is a mid-tool-call stream drop.
Raise EmptyStreamError for it, riding the same bounded stream-retry
(HERMES_STREAM_RETRIES) the eventless-stream case already uses.
Gate checked on both return paths (raw SDK snapshot and
accumulator-modified message). Regression tests cover the dropped
shape (mutation-verified: disabling the gate fails exactly that test),
the legitimate tool_use completion, and the text-only no-stop_reason
shape (pre-existing behavior preserved).
Locks in two gaps left by 015a114a2 (#80623): a mixed response where one
tool call completes validly while a sibling has zero argument bytes still
gets discarded whole via the shared partial-stream-stub path, and the
zero-byte trigger now has an end-to-end test through run_conversation's
retry loop, not just at the chat_completion_helpers unit level.
When the stream closes right after a tool call's name arrives but
before any argument bytes are delivered, has_truncated_tool_args
was never set (the existing check required a non-empty, whitespace-
stripped arguments buffer). The call fell through to a normal "stop"
finish_reason, later coerced to "{}" at dispatch and executed
silently with no arguments and no retry.
Route this case through the same dropped-mid-tool-call stub/retry
path already used for partially-truncated JSON.
Post-merge review of aecb9ca89 found the join guard over-broad: skipping
the join whenever ANY fragment self-matches _PREFIX_RE reopened a leak
for non-newline splits — sk-<15 chars>ESC<25 chars> masked only the
self-matching head and left the 25-char tail in cleartext (fully masked
before the guard; main never masked this shape at all, so the merged
state was still >= main, but the salvage's own coverage regressed).
Skip the join only when the span crosses a line boundary (\n / \r) —
that is the shape where adjacent legitimate text gets swallowed
(ghp_<token>-then-'button [ref=e3]' annotation bug). ESC/zero-width
controls never legitimately separate a token from prose, so joining
there is safe and restores full-tail masking.
Both legs mutation-checked: reverting to the unconditional skip fails
the new tail-mask test; removing the guard fails the annotation test.
Follow-up to the salvaged registration contract:
- share one _raise_if_cron_registration_error() helper for the two
byte-identical dashboard 424 except-blocks (web_server + cron router,
via the existing late() seam)
- add endpoint-level 424 coverage for /api/cron/blueprints/instantiate
(previously only the sync worker was tested)
- give chat/CLI surfaces a human-facing user_message() (job name, no
exception class name) and add a recovery hint (pause/resume or update
re-registers via provider reconcile) to the model/REST message
- consolidate five inline provider test doubles into one ABC-subclassing
make_cron_provider conftest factory; the web_server test double now
subclasses CronScheduler so an ABC rename fails loudly
- narrow the wrapper facade to keyword-only (**kwargs) and route the
tool's partial-failure return through tool_error()
Follow-up hardening on the salvaged #80687 shrink-merge guard, folding in
the best part of the competing #80703 (credit: @JoaoMarcos44):
- Stat-stamp fast path: load_jobs() inside a _jobs_lock() section records
jobs.json's (mtime_ns, size, ino) BEFORE reading; the save-path merge
and the post-stage verify skip their full read+parse when the stamp
still matches. The healthy no-race save (every mark_job_run /
claim_dispatch / heartbeat / advance_next_runs tick persist) now costs
one stat() instead of up to two full JSON parses.
- Fail-safe stamp discipline: the stamp is captured pre-read (a sibling
racing the load leaves it older than disk -> mismatch -> merge runs),
includes st_ino (mkstemp+rename always allocates a new inode, so
coarse-mtime filesystems cannot false-match), resets on section
entry/exit, and is INVALIDATED - never refreshed - after any write in
the section (a refresh would let a nested create_job be clobbered by
an outer caller's stale payload; probe-verified both directions).
- _merge_unexpected_disk_jobs no longer mutates the caller's list in
place - it returns a new list when anything was recovered, and logs the
recovered ids.
- The tolerant read cascade (utf-8-sig + strict=False fallback) is
factored into one shared _parse_jobs_file used by both load_jobs and
_peek_jobs_unlocked, so future encoding/shape fixes land once. The
peek's repair-free re-entrancy contract is now documented - a repairing
read on the save path would recurse through _save_jobs_unlocked (the
exact defect the stamp-reconcile approach in #80703 had).
4 new regression tests (fast path, no-mutation, corrupt-file save,
nested-create-vs-stale-outer-save), each verified to fail against the
implementation it guards.
Co-authored-by: joaomarcos <joaomarcosdias444@gmail.com>
CI slice 1/12 caught a regression in _mask_control_split_tokens: a
COMPLETE prefix token at end-of-line followed by ordinary text (browser
accessibility annotations: 'ghp_<tok>\nbutton [ref=e3]: Copy') was
joined across the newline into one stripped-copy match, and the mask
swallowed the adjacent line ('button' disappeared).
Join only when no fragment inside the span matches _PREFIX_RE on its
own — a self-matching fragment is already handled by the ordinary
prefix pass, so joining can only cause damage. All smuggling shapes
(ESC/ZWSP/newline splits with under-length fragments) still mask;
regression test added and mutation-checked (fails without the guard).
_write_checkpoint persisted s.command verbatim to ~/.hermes/processes.json.
Recovery only uses command for display/logging (the process is already
running; adoption re-validates PID + start time, never re-runs the
command), so masking is lossless.
A masked secret's visible head/tail could carry control bytes (newline,
NUL, DEL, C1 0x80-0x9F, zero-width) into config/status/dump output.
Strip every control incl. \n/\t (display differs from redact_sensitive_text,
which preserves \n/\t as line structure) before slicing; all-control values
return the configured empty fallback.
Consolidates the previously-closed #58079 approach (strip controls before
masking) - supersedes it.
- Import file_safety._BLOCKED_PROJECT_ENV_BASENAMES instead of copying
it (comment-enforced parallel lists drift); lookup is now
case-insensitive to match file_safety's .lower() semantics (cat .ENV
on macOS/Windows case-insensitive filesystems reads the same secrets).
- Strip shell quotes plain split() leaves attached (cat ".env").
- Drop the dead _ENV_FILE_EXCLUDE_SUFFIXES logic (exact-basename
membership already excludes templates) and the stray blank-line noise.
- Document the defense-in-depth limits (sudo/full-path/substitution
readers) mirroring is_env_dump_command's precedent, and correct the
docstring overclaim about name-independence.
- Annotate command as str | None (tests pass None).
Terminal output from file-read commands (cat, head, tail, ...) uses
code_file=True, which skips the generic ENV-assignment redaction pass.
Reading a .env file through the terminal therefore leaked any key whose
value has no recognized vendor prefix (Mistral, Gemini AQ.*, tvly-dev-,
bu_, Spotify client secrets).
Detect file-read commands targeting .env-style basenames (mirroring
agent/file_safety's blocked list) and route them to code_file=False so
the existing ENV pass masks opaque values. Templates (.env.example,
.env.sample, ...) are excluded.
Salvaged from #61352 (145 commits of drift; conflict with the test-prune
wave resolved by NOT resurrecting pruned tests). Authored by @ShaoRou459.
Closes#61352
The api_messages build used a shallow msg.copy(), decoupling only
top-level fields. Every nested container (tool_calls entries and their
function dicts, multimodal content-part lists, reasoning_details) stayed
aliased to the persisted history, so ANY in-place transform on the send
copy silently rewrote the stored transcript.
Probed every send-path transform against that aliasing shape on main:
content strip loop safe (top-level reassign)
_canonicalize_api_tool_calls (repair) LEAKED <- #80616's fix
_sanitize_messages_surrogates LEAKED (multimodal parts,
tc ids/args, reasoning)
_sanitize_messages_non_ascii LEAKED (multimodal parts)
_sanitize_api_messages safe
_drop_thinking_only_and_merge_users safe
The retry loop already believed the copies were independent - it
sanitizes messages AND api_messages separately (~L3555) - so the
aliasing was accidental everywhere.
Fix at the chokepoint: _clone_message_for_send clones every container
(dict/list) recursively while sharing immutable leaves, so every
downstream in-place transform - current and future - is safe by
construction. Cost is container-count, not string-bytes: 100KB argument
strings and base64 payloads are shared (measured ~0.5ms vs ~0.1ms per
1500-message build; noise next to one json round-trip). Same clone
applied to the prefill-message insert (same class, same pipeline).
The class-wide invariant test runs the full send-path transform
pipeline over an adversarial fixture (malformed args, surrogates,
non-ASCII, multimodal parts, reasoning fields) and asserts the history
stays byte-identical; an AST contract pins the build-site wiring so the
shallow copy can't quietly return. Both mutation-verified: reverting
the clone to shallow fails 4 isolation tests, unwiring the build site
fails the AST contract.
0xGr1mm's branch fix (previous commit) remains as defense in depth at
the exact site the #80498 incident hit; his regression tests and the
class-wide invariant give layered coverage.
`_canonicalize_api_tool_calls` promises copy-on-write in its own docstring
— "the persisted history is untouched" — and the call site repeats it:
"Operates on api_messages (the API copy) so the original conversation
history in `messages` is untouched."
The canonicalize branch keeps that promise (`tc = {**tc, "function": {...}}`).
The repair branch does not:
except Exception:
tc["function"]["arguments"] = _repair_tool_call_arguments(...)
`api_messages` is built with `msg.copy()` — a SHALLOW per-message copy — so
every `tool_calls` entry is the same dict object the persisted history
holds. Assigning into `tc["function"]` therefore writes through to the
stored turn. The sibling loop two lines above only touches `am["content"]`,
one level deep, which is why the aliasing never showed up there.
On the unrepairable path `_repair_tool_call_arguments` returns "{}", so
that write replaces the model's real arguments with an empty object in the
transcript. A stream that dies mid `write_file` loses the file content it
had already streamed — the reported symptom in #80498, where a chapter
draft was silently reduced to `{}` and only a WARNING remained:
Unrepairable tool_call arguments for write_file — replaced with empty
object (was: {"content": "# 骨架-第25章\n> 承接...)
Mirror the canonicalize branch: build a new tool-call dict instead of
assigning into the shared one. The API copy still carries "{}" — the
repair's whole purpose is to never ship broken JSON — but the history keeps
what the model actually sent, so the transcript, session persistence and
any later retry still have it.
The in-place write was not an oversight in isolation: it predates the memo
refactor, which preserved it deliberately for byte-parity. The existing
`test_history_not_mutated` asserts exactly this invariant but restricts
itself to valid arguments, and its docstring records the gap — "(Malformed
args take the in-place repair path — pre-existing behavior)". That is why
a test file whose header already claims "the persisted history is never
mutated (copy-on-write preserved)" stayed green through the bug.
Four tests close it: history keeps the original bytes, the send copy is
still repaired, a broken call does not disturb its siblings, and repeated
sends stay lossless. On unpatched main three of them fail; the parity and
complexity tests are unaffected because the difference is only observable
when the history list is separate from the send copy — which is the shape
production uses.
Refs #80498
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The PR renamed _resolve_auto to _resolve_auto_route (3-tuple return).
This test patches the resolver to mock the auto-detection chain, but was
still patching the old _resolve_auto name. The patch never fired, so the
test fell through to the real auto-detection (which finds no providers
in CI's hermetic env). Update patch target to _resolve_auto_route with
the 3-tuple return value.
- state_db fixture: drop the HERMES_HOME setenv + sys.modules purge
(SessionDB takes an explicit path; tests/conftest.py already sandboxes
HERMES_HOME; the purge risks split-class identity for other modules
holding the old hermes_state reference) — matches the plain
tests/hermes_state/ sibling fixture pattern.
- probe guard asserts on the CALL ('has_archived_messages(') instead of a
local-variable name — a reintroduced probe under any rename now trips
it (mutation-checked: renamed-probe reintroduction fails the guard;
restored stack green).
- _archived_count uses the public get_messages(include_inactive=True)
instead of poking db._lock/_conn.
Follow-up hardening on top of the salvaged #80809 watchdog, porting the
locked state-machine design from #75301 (credit: @Zeraphim):
- All lifecycle transitions (stale/cancelled/done) now happen under
request_client_lock. A user or monitor interrupt marks the request
'cancelled' so a racing stale timer can no longer misclassify the kill
as provider staleness and feed a false +1 into the #58962 cross-turn
circuit breaker.
- 'done' is set under the lock on completion, so a late timer callback
that lost the race to a successful response is inert instead of
leaving a spurious streak=1 behind the reset.
- Registration race closed: if the budget expires while the client is
still being constructed, _make_client aborts the freshly-registered
client and fails the call with a retryable TimeoutError instead of
opening a brand-new socket after the only watchdog already fired.
- _resolve_direct_stale_timeout now fails closed: a raising resolver
propagates (same as the worker path) instead of being swallowed into
an infinite budget that would silently disarm the watchdog and
reinstate the very hang #80759 is about.
4 new regression tests, each verified to fail against the pre-fix
watchdog implementation.
Co-authored-by: Zeraphim <diamantejc87@gmail.com>
Cron turns and delegated children are routed onto direct_api_call, which
ran the request inline with no stale detector. The abort plumbing was
registered but nothing ever invoked it, so a provider that accepted the
request and then went silent — connection held open, zero bytes, no
error — hung the run until an external actor killed it, which also
orphaned the execution row. The httpx read timeout is not a usable bound
(1800s default, and this failure mode never trips it), and the job-level
inactivity monitor was observed not to fire.
Arm a watchdog timer on the same budget the interrupt worker's poll loop
uses, so these turns get exactly the patience every other non-streaming
request already gets. On expiry it only aborts the in-flight sockets
through the already-registered hook — it never issues a request, so the
inline / no-worker property that fixes the nested-pool deadlock is
preserved — bumps the cross-turn stale circuit breaker, and surfaces a
retryable TimeoutError so the outer loop reconnects on a fresh pool.
Fixes#80759
#80216 fixed /retry (and a follow-up fixed yuanbao recall) destroying
soft-archived active=0/compacted=1 in-place-compaction rows via the
destructive replace_messages default. Two sibling sites still carried the
same class:
- acp_adapter/session.py _persist (non-owned-agent branch): probed
has_archived_messages and FAILED OPEN into the destructive full replace
on any probe error; the probe can also race a concurrent
archive_and_compact. Now passes active_only=True unconditionally — on a
fresh create/fork every row is active=1 so behavior is identical, and
the probe (its only production caller) is deleted.
- tui_gateway/methods_prompt.py edit/regenerate truncation: bare
replace_messages() deleted the archived transcript of a compacted
session on every edit/regenerate. Now active_only=True.
hermes_state.has_archived_messages docstring updated (probe is now
test/diagnostic-only). Test stubs in test_tui_gateway_server.py accept the
new kwarg. New regression tests: real-SQLite archive-survival for both
write shapes, fresh-session equivalence (the claim the unconditional
switch rests on), and source-level guards pinning that neither site
re-grows the fail-open probe (both mutation-checked: revert either fix and
its guard fails).
- is_tts_echo sliding window: return True immediately when ratio >= threshold
instead of scanning all remaining windows. Common echo case drops from
~4s to <1ms for long spoken text (found by /simplify-code efficiency review).
- Clear _voice_barge_phase in _voice_submit_barge_utterance finally block
alongside _voice_barge_capture, preventing stale phase from a previous
trip affecting a future call.
- Add _voice_last_tts_text and _voice_barge_phase to _make_voice_cli test
helper so it matches __init__ state.
Reviewer feedback on the fragment-echo fallback added in 24730b10c:
- A short playback-phase transcript (e.g. a genuine one-word "yes")
could trivially match a same-length window of a longer spoken reply
at ratio 1.0 and be wrongly dropped as a self-capture. The fallback
now requires the transcript to be at least
MIN_FRAGMENT_LENGTH_FOR_ECHO characters before it runs.
- The fallback split on whitespace, so it never engaged for
no-whitespace languages (both transcript and spoken text collapse to
a single "word"). Switched the sliding window to be character-based
instead of word-based, matching the function's tokenization-independent
contract.
Adds regression tests for both cases.
is_tts_echo() compared the captured transcript against the *entire*
spoken text with a whole-string similarity ratio, which only scores high
when the two strings are close in length. But a playback-phase barge
capture is cut immediately when the trigger fires and only spans the
pre-roll buffer plus time-to-silence, so a genuine self-capture is
typically a short fragment of a longer reply, not a near-verbatim repeat
of the whole thing -- for any response longer than a clause, the
length-diluted ratio fell below threshold and the echo sailed through
ungated.
When the whole-string check misses, also slide a window sized to the
transcript's word count across the spoken text and compare against each
window, so a short fragment echoed from within a much longer
multi-sentence reply is still caught. Add regression tests for a short
fragment matched at the start and in the middle of a longer reply.
The full-duplex barge-in listener added in 5081551f0 stays active during
TTS playback with no acoustic echo cancellation. On some speaker/mic
combinations, TTS bleed alone crosses the barge threshold, gets
transcribed, and is queued as the next user turn -- whose reply is then
spoken, captured, and queued again, producing an unbounded TTS -> STT ->
TTS feedback loop (#75780).
Add a fail-closed transcript-level guard: when a barge trip happens during
the playback phase, compare the captured transcript against the TTS text
Hermes just spoke (tools/voice_mode.is_tts_echo, a language-agnostic
character-level similarity ratio). A close match is dropped instead of
queued, and the mic is handed back to the normal continuous-listening
loop. Generation-phase trips (no TTS playing, so no bleed is possible)
are unaffected.
_handle_media_message had the same gap as _handle_text_message: it didn't
set user_id/user_name or any reply_to_* fields on MessageEvent. A user
replying to a photo/video/audio in Matrix got null sender metadata and
null reply context — the exact bug PR #80293 fixes for text messages.
Mirrors the text handler's reply fallback parsing + sender propagation
into the media handler, and adds a test covering a media reply event.
Also fixes test env mutation: _make_adapter now accepts monkeypatch
and uses monkeypatch.setenv() instead of bare os.environ assignment,
preventing env var leakage across tests in the same xdist worker.
The Matrix adapter built MessageEvent from inbound room events but dropped
the sender's MXID and display name on the event itself -- only 'source'
carried them. Other adapters (signal/slack/telegram/discord/mattermost/irc)
have the same gap; this PR fixes matrix and adds the supporting
top-level MessageEvent fields so the rest can follow.
Downstream effects for matrix specifically:
- gateway prompt assembly can now read event.user_name (or source)
without having to dig into source per platform
- reply context (reply_to_text / reply_to_author_id /
reply_to_author_name) is parsed from the inline > <@user:server> ...
Matrix fallback format before stripping, instead of discarded
- the gateway's existing [Replying to: "..."] renderer can now show
who the user was replying to (was always anonymous for matrix)
MessageEvent gains two optional top-level fields (user_id, user_name,
both default None) so non-IM producers (cron/webhook/autonomous) remain
unaffected. Source still carries the same values for callers that
already read from there.
Tests cover:
- non-reply message carries sender user_id/user_name on MessageEvent
- different senders (alice, bob) both propagate
- reply message carries reply_to_message_id + reply_to_text +
reply_to_author_id + reply_to_author_name, parsed from the
> <@carol:example.org> original question\n\nactual reply shape
- non-reply message does NOT spuriously set reply_to_* fields
Sibling matrix tests (148 across test_matrix*.py) remain green.
Authored by WintleChoung <cwt@users.noreply.github.com>
Salvaged from PR #80293.
Widen #79293's trailing-in-flight guard from 'last message is assistant'
to 'last non-tool message is assistant': a multi-call batch snapshotted
between the executor's per-result appends looks like
[..., assistant(c1,c2,c3), tool(c1)] — c2/c3 are pending, not orphaned,
but the tail-only guard missed that shape and stripped them (same silent
result loss as the original bug, via concurrent /compress or the gateway
hygiene pass).
Preserving is safe on both shapes: the pre-API chokepoint
(sanitize_api_messages step 2) injects stub results for any call that
genuinely never gets an answer, while stripping a live call silently
loses its late result.
test_sanitizer_strips_orphaned_keeps_valid's mixed valid/orphan shape
moves mid-list — at the tail it is byte-identical to a live partial
batch and the sanitizer now correctly presumes in-flight there.
New regression test fails without the walk-back (c2/c3 stripped),
passes with it.
Tool_executor.py appends role=tool results AFTER running each call. When
context compression fires mid-chain, the trailing assistant(tool_calls)
message is a pending request whose result has not yet been appended.
_sanitize_tool_pairs previously stripped it as an 'orphan', so when the
executor later appended the real result, repair_message_sequence dropped
it as unmatched and the completed side effect (and final synthesis) was
lost. Preserve the trailing in-flight call verbatim; only genuinely
orphaned calls in the discarded region are stripped.
Adds regression tests: three unit tests for _sanitize_tool_pairs plus an
end-to-end test reproducing compression -> side-effect completion ->
result-returned flow. Confirmed failing on pre-fix code, passing with
the fix.
Review follow-up for the salvaged #79881/#63963 stack: the shutdown
interrupt fires exactly once, but work can materialize AFTER that one
shot on BOTH sibling paths:
- a /v1/runs task admitted before the drain populates
_active_run_agents only once _create_agent returns
(queued-before-agent window);
- a _running_agents entry claimed as _AGENT_PENDING_SENTINEL is
promoted to the real agent by track_agent() on its own schedule,
after the one-shot walk skipped the sentinel.
Either way the settle loop waited on work nothing signaled, and the
turn went straight to the post-interrupt tool-subprocess kill — the
exact amputation the fix exists to avoid, in a rarer window.
If any work is still live when the settle loop exits, re-invoke
_interrupt_running_agents (which already skips sentinels and folds in
the API-server helper) so late-materializing agents on either path get
the cooperative interrupt. Regression test drives the real stop() path
with an accelerated loop clock and asserts exactly two interrupt
signals.
The shutdown drain ACCOUNTS for API-server work but never INTERRUPTS it.
`_drain_active_agents()` folds `_active_api_run_count()` into both its wait
loop and its `timed_out` verdict, while `_interrupt_running_agents()` iterates
`self._running_agents` only -- a dict no API turn ever enters, because the
API server owns its own agent lifecycle. `gateway/run.py` states the gap
against itself: "API-server / desk sessions have the same structural gap
(#63529)."
The user-visible result is that every gateway restart with a live API or
desktop turn burns the full drain timeout and then runs
`_kill_tool_subprocesses("post-interrupt")`, which amputates the turn's tool
subprocesses with no cooperative interrupt and no resume marker.
There are seven API agent-entry points. Six funnel through `_run_agent()`
(both session-chat routes, and `/v1/chat/completions` + `/v1/responses` in
streaming and non-streaming form) and are counted by `_inflight_agent_runs`;
the seventh, `/v1/runs`, runs its own lifecycle and is counted through
`_active_run_tasks`. None of the six has a run_id, so the run_id-keyed
`_active_run_agents` cannot reach them, and only two pass `agent_ref` -- which
lands in a caller-local list, not a registry.
So register once at the single unconditional creation site inside
`_run_agent`, beside the existing `_publish_turn_process_ownership()` call,
and unregister in the same `finally` that already clears it. That one
symmetric pair covers all six callers. The registry is adapter-owned and
keyed by object identity, kept separate from `_active_run_agents` because
that dict is run_id-keyed and scoped to the public `/v1/runs` stop API.
`interrupt_active_runs()` then walks both registries, deduped by identity, so
the interrupt set matches the set the drain waits on. The settle window after
the interrupt now polls API work as well: the interrupt is cooperative, and
without this the window closes the instant `_running_agents` is empty -- which
it always is for API turns -- and the tool kill lands on a turn that was asked
to stop microseconds earlier.
kshitij
kshitij
Chen Jin
kyssta-exe
HexLab98
poisdahl
texasich
dsad
joaomarcos
Gille
Soheil Fakour
Peter
Gr1mmJ4w
chelsealong
WintleChoung
Shotflame
briandevans