With capture:auto the desktop always preferred client streaming, so a local
desktop with a working backend mic silently switched from PortAudio to
getUserMedia default-device — dropping wake_word.input_device selection
(#74363). A ready backend input now wins under auto; client capture is the
fallback for a preferring surface on a mic-less backend, and capture:client
still forces streaming.
Also removes the dead auto branch (both arms returned local) and lets the
client-feed test skip cleanly when numpy is absent.
- wake.status reports effective capture from the armed detector (client vs
local), plus frame_length/sample_rate; GUI status probes prefer client
- Gateway test doubles accept external_audio on start_listening
- Desktop PCM feeder uses a bounded ordered queue instead of dropping frames
while a wake.feed RPC is in flight
- /wake on and status/re-arm paths pass client_capture so remote reattach works
Remote headless backends have no PortAudio mic, so "hey hermes" fails even
when openWakeWord is installed. Let the desktop stream 16 kHz int16 PCM via
wake.feed while detection stays server-side.
- wake_word.capture: auto|local|client (+ GUI client_capture prefer)
- WakeWordDetector external_audio queue + feed_audio API
- wake.feed RPC; wake.start/status report capture + frame_length
- Desktop getUserMedia feeder; stop on wake.detected, re-arm after voice
- Docs + unit tests (26 pass in tests/tools/test_wake_word.py)
The artifact download URL returns a 302 redirect to a signed blob URL.
urllib sent the Authorization header to the blob, and the blob rejected it
with a 401 error. The download now has two hops. The first hop authenticates
to the API. The second hop follows the redirect without the auth header.
The query runs?event=workflow_call returns nothing for this repository.
GitHub flattens reusable-workflow jobs and their artifacts into the caller
run. The fetch now lists the artifacts on the orchestrator run only. The
dead sub-run enumeration is gone. Two API calls per cycle are gone with it.
The 'artifact statuses updated' reason never appeared. The code updated the
count before the comparison. Now the code compares first and updates after.
The code rejects zip members that contain '..' or start with '/'.
tests/ci/test_live_comment.py is deleted. This repository does not keep
tests for CI infrastructure.
The live comment poller got its review statuses from two sources. The first
was the REVIEW_STATUSES environment variable, fixed at the start of the
comment-live job. The second was one ci-timings artifact, downloaded at the
end of the run. Status details (error messages, action_required items)
appeared only after all jobs finished. The job pass/fail results were visible
as each job completed.
Now every status-producing workflow_call uploads a small review-status
artifact when it completes. The poller lists all review-status-* artifacts
from the orchestrator run and its workflow_call runs every cycle. It
downloads each artifact and merges the statuses into the comment. A status
appears as soon as its job finishes.
Changes:
- live_comment.py: _fetch_artifact_statuses became fetch_all_review_statuses.
The new function lists the artifacts via the API, downloads each one, and
parses it. Removed the review_statuses_json parameter, the
--review-statuses-file argument, and the subprocess import.
- ci.yml: removed the REVIEW_STATUSES environment variable, the inline Python
merger, and the --review-statuses-file argument. Renamed the
ci-timings-review-status artifact to review-status-ci-timings.
- Eight workflow_call files: added a step that writes review-status.json and
uploads it as an artifact after each review_status output.
- test_live_comment.py: added tests for _parse_status_file and
_merge_statuses.
The gateway terminal guard crashed with 'ValueError: embedded null byte'
(command never ran, exit_code -1) when a command invoked an ELF binary by
full path. _read_referenced_script correctly rejects the binary locally
(NUL in first chunk), but the read_remote_script fallback
(_read_script_in_env) then re-read the SAME file's bytes without a NUL
guard, decoded them, and fed machine code back into the scanner, which
re-tokenized it into a bogus NUL-bearing path and crashed at os.open.
- _read_script_in_env: skip content containing a NUL byte on both the
local-read and remote-cat branches (mirrors _read_referenced_script:
a binary is nothing to scan), so binary never re-enters the guard.
- _read_referenced_script: tolerate ValueError from os.open on a
NUL-in-path, alongside the existing OSError guard, so the guard can
never crash the terminal tool regardless of input.
Extends the #76762 NUL-safety fix (local path only) to the gateway's
remote-read fallback path.
Add activity_hb.join(timeout=2.0) after activity_hb_stop.set() in
direct_api_call's finally block so the heartbeat thread is deterministically
stopped before client teardown. Add test verifying no stray _touch_activity
fires after direct_api_call raises an exception.
Follow-up to PR #78548 by @xxxigm.
Top-level delegate_task runs in the background, and the 450s progress-stall
monitor only sees api_call_count / tool / last_activity_ts. Subagents use
non-streaming direct_api_call, which previously touched activity once and then
went silent — so a healthy local GGUF / long-prefill wait looked frozen and
was interrupted around ~450s as "Operation interrupted: waiting for model
response", even when child_timeout_seconds was raised. Refresh activity while
the inline request is open, and treat last_activity_ts advances as sync
heartbeat progress too.
get_compression_lineage's forward walk accepted any non-branch child as
the compression continuation. Delegate subagent rows (_delegate_from)
and tool-tagged rows (source=tool) created before the real continuation
were picked as the lineage successor, so the lineage — and session .md
export built on it — followed a subagent's transcript instead of the
actual conversation continuation.
Rename _is_branch_child_row to _is_explicit_fork_child_row, treat
_delegate_from and source=tool rows as explicit forks alongside
_branched_from, and require _is_compression_child_row in the forward
walk instead of merely excluding branches.
Sliced from PR #79024 by @RyderFreeman4Logos (the cache-scope portion
of that PR is tracked separately in #79017).
Cherry-picked from PR #78959 by @JoaoMarcos44 with authorship preserved.
Follow-up: hoist _cache_scope_from_session_id(session_id) to a local in
build_kwargs so it's computed once instead of 4 times per call.
Closes#78941. Closes#79012. Closes#79013. Closes#79014. Closes#79015.
Co-authored-by: JoaoMarcos44 <joaomarcosdias444@gmail.com>
_skin_set has no try/except around yaml.safe_load, so invalid YAML raises
and aborts the command. The {} fallback comes only from safe_load()
returning None on a zero-length file — which is exactly the state a torn,
unsynced write leaves behind, so the data-loss chain is unchanged.
`write_profile_meta` and `hermes skin set` are both read-modify-write
helpers that rewrite a user-visible YAML file with a bare truncating
write, bypassing `utils.atomic_yaml_write` — the shared helper whose
docstring states that "every destructive file rewrite in the codebase
shares one implementation".
Both read halves swallow a parse error and fall back to `{}`, so a
truncated file is not transient corruption. The next call reads `{}` and
silently, permanently drops every field the caller did not explicitly
pass:
* `write_profile_meta` promises "unspecified fields preserve existing
values". After an interrupted write, a follow-up call that only sets
`description_auto` erases the profile's `description` — it vanishes
from `hermes profile list` and never comes back.
* `_skin_set` exists so that "changing one token never disturbs the rest
of the look". `path.write_text(...)` neither fsyncs nor swaps
atomically, so a crash or power loss can leave `<skin>.yaml`
zero-length; the next tweak then rewrites from empty and the whole
palette is gone. The gateway's skin watcher repaints live surfaces
from this file within ~1s, so a half-written file is observable.
Routing both through `atomic_yaml_write` gives temp file + fsync +
`atomic_replace`, which also preserves a symlinked target (GitHub
#16743) and restores owner/mode, and emits emoji descriptions as real
UTF-8 instead of `\UXXXXXXXX` escapes (GitHub #51356).
Supersedes #51808, which fixed the unicode-escaping symptom alone by
adding `allow_unicode=True` to the same `yaml.safe_dump` call.
A dispatched console handler that calls sys.exit("message") or
raise SystemExit("message") sets exc.code to a string. int(exc.code or 0)
then raises ValueError, which is not a ConsoleCommandError, so it escapes
execute()'s handler and crashes the local REPL on an ordinary user mistake
(e.g. removing a credential that does not exist). Treat a string exit code
as a status-1 failure carrying that message.
Timing out a bounded git probe must not leave helper descendants
(credential helpers, git-remote-https, hook children) running after the
probe fails open. bounded_git_probe now spawns the child in its own
process group on POSIX (process_group=0), and _kill_git_process_tree
signals the whole group with os.killpg — gated on the child actually
leading its own group (pgid == pid), so a shared-group spawn can never
take down unrelated processes. Windows keeps the existing taskkill /T /F
tree kill.
Proven live on main: a fake git that forks a 300s descendant left the
descendant running after the probe timeout; with the fix the descendant
dies with the launcher. Fast path and fail-open contract unchanged.
Port of openai/codex#36793 (Terminate timed-out Git process trees).
Nothing covered the update path, which is the worst thing to break: a broken
updater strands users on the version that cannot fix itself. `hermes update`
alone is ~2000 lines (hermes_cli/update_cmd.py) and had no end-to-end test.
tests/install/install-update-e2e.sh installs a genuine earlier Hermes through
the real one-liner (curl -fsSL https://…/install.sh | bash, served by
dev-sandbox's MITM proxy at the canonical URL, cloning "github.com" through the
upload-pack shim), which really installs uv, a managed Python, Node and the
venv. It then applies ONE update route and requires the checkout to land on this
commit with `hermes --version` still working -- so a pass means the venv and
entry point survived, not merely that git moved.
One route per run, each on a sandbox built from scratch. Sharing one install
across routes -- or rewinding with `git reset --hard` between them -- leaves the
second route running against a tree the first already updated (same venv, same
console script, same __pycache__), which is not the state any real user is in: a
route could pass only because its predecessor did the work, and a failure in the
first left the second exercising something undefined.
--install-ref chooses what to install first, so this covers "update from an
older release", not just from the tip. Installer flags are probed against the
target rather than assumed, because releases from months back predate flags
current Hermes takes for granted: --skip-browser is read out of that ref's own
install.sh, and `--yes` is asked of the installed `hermes update --help` (the
update subcommand has lived in main.py, subcommands/update.py and update_cmd.py
across the tags we sample, so a static parse rots silently -- and did). Without
those probes, old releases die on "Unknown option: --skip-browser" and
"unrecognized arguments: --yes" before doing any work.
Installer output is streamed through tee rather than captured: a real install of
uv, Python, Node and the venv IS the substance of this test, so it belongs in
the job log, not only in an artifact. pipefail keeps the installer's exit status
rather than tee's, so a failed install cannot look like a pass. The sandbox's
own proxy log is printed in full on failure, since a rejected TLS handshake
explains a failure that otherwise reads as a bare `curl: (35)`.
Deliberately reuses dev-sandbox rather than adding a second harness. An earlier
draft rewrote install.sh's hardcoded URLs with insteadOf and ran it against the
host; that tested the installer LESS faithfully (bash install.sh instead of the
real one-liner, host libs instead of a clean machine, ssh disabled to keep a
failed rewrite from reaching real GitHub) while duplicating a fake Internet we
already have.
Shell, not pytest, so scripts/run_tests.sh and run_tests_parallel.py stay
untouched: a pytest version needed an entry in the former's `env -i` credential
allowlist and a _SKIP_PARTS exclusion in the latter, and every meaningful line
was a command run inside the sandbox anyway.
Two guards, both earned during bring-up. It prefers the `sandbox` wrapper and
falls back to the raw script only when bwrap is on PATH (under Nix the wrapper
supplies the PATH and DEV_SANDBOX_* vars, so the bare script exits 127). And it
refuses to run on a dirty worktree: every dev-sandbox invocation re-derives fake
main from the working copy, so uncommitted changes move the update target
between the call that installs and the call that verifies -- a failure that
looks like a broken updater but is a moving reference.
scripts/tests/ has held three PowerShell suites that no workflow ever
invoked -- there is no Windows runner in CI, so they have been inert since
they landed. A regression test nothing executes is worse than none: it
reads as coverage.
Adds a windows-latest job, gated on a new `installer` lane so it only fires
for PRs touching install.ps1 or its tests. The 8.3 suite runs under both
pwsh 7 and Windows PowerShell 5.1, since install.ps1 arrives via `irm | iex`
into whichever shell the user already has and 5.1 is what ships with Windows.
Only the 8.3 suite is wired up. The other two fail on main today for
unrelated reasons; they can join once they are fixed.
Selecting an NVIDIA NIM model whose id reached config without the nvidia/
prefix produced a bare "HTTP 404: 404 page not found" — retried three times,
never naming the model. It reads exactly like an outage or an auth failure,
which is where the Discord thread spent its time before the id was spotted.
normalize_model_for_provider() had no branch for nvidia, so a bare id passed
straight through to the API. Repair it from the provider's curated catalogue:
a bare name that matches exactly one entry modulo the prefix gets it back.
That's a lookup, not a guess — build.nvidia.com also fronts local NIM
containers and third-party models, and anything absent from the catalogue is
left alone. Because the repair runs on every runtime setup, an already-broken
config self-heals on the next turn and prints what it changed.
If a bare id still reaches the wire, the 404 now explains itself. The
classifier consults the same catalogue: a prefix-less id the provider only
serves as vendor/model is a deterministic failure, so it classifies as
model_not_found instead of burning three retries on a retryable "unknown",
and the error trace names the id to use.
Fixes#78796
Adds a last_read_at watermark to the sessions table so surfaces (CLI,
TUI, desktop) can badge unread conversations. Read state derives from
the watermark vs latest activity, so new messages flip a conversation
back to unread with zero writes on the message path. NULL means never
tracked, so shipping the column doesn't badge pre-existing history.
set_session_read() stamps the whole compression lineage, matching the
archive/pin semantics; list_sessions_rich() rows carry a derived
`unread` key. DB layer only — no surface exposes it yet.
* fix(discord): reject empty outbound messages
* test(discord): cover empty final reply backfill state
Missed-message backfill decides what to replay from discord_messages, so
a dropped final reply must be recorded as failed by the new guard the
same way the exception path records one — otherwise the reply is both
never sent and never retried.
Co-authored-by: Jony <619963502@qq.com>
* chore: map 619963502@qq.com to zyz619963502zyz for PR #73449 salvage
---------
Co-authored-by: Jony <619963502@qq.com>
Three fakes pin mark_exhausted_and_rotate's signature explicitly and broke on
the new argument. They now assert it rather than just tolerate it — the xAI
spending-limit case is exactly the billing-403 this fixes, so it should be
pinning `failure_reason == "billing"`.
The sole-credential cooldown sized the bench from the raw HTTP status, but
403 is overloaded: error_classifier maps OpenRouter's "key limit exceeded"
and xAI's spending-limit block to FailoverReason.billing, while an edge
throttle with the same status is transient. Only 402 was excluded from the
short cooldown, so a spent account on a single key retried every 60 seconds
and re-failed forever.
Thread the classified reason from recover_with_credential_pool through
mark_exhausted_and_rotate to _exhausted_ttl. Billing keeps the full bench
regardless of status; everything else transient still recovers in 60s. The
verdict is stored on the entry (_EXTRA_KEYS, so it persists to auth.json) —
without that a restart would re-read a bare 403 and downgrade the bench.
Tests: sole billing-403 stays benched, survives reload, unclassified 403
still recovers; call-site coverage that the reason actually reaches the pool.
Three existing kwargs assertions updated for the new argument.
next_available_at() was computing the full 1-hour TTL for a sole
credential on a 429, contradicting the 60s cooldown in _available_entries.
The fallback restore gate (agent_runtime_helpers) uses next_available_at
to decide when to switch back from fallback to primary — so the agent
stayed on fallback for an hour instead of ~60s.
Add sole_credential computation in next_available_at mirroring
_available_entries, and a test verifying the short cooldown propagates.
A pool with only one usable (non-DEAD) credential has nothing to rotate to.
On a transient throttle (429 rate-limit, 403 edge-throttle, 5xx) the offending
key was benched for a full hour (EXHAUSTED_TTL_429/DEFAULT), so single-key /
no-fallback setups got an hour of hard failures for a throttle that resets in
seconds. The pool already special-cases 401 to recover quickly for single-key
setups; extend that to transient throttles when the credential is the sole
non-DEAD entry. 402 (billing/quota) keeps the full bench — a quick retry can't
help. Provider-supplied reset_at still overrides.
Adds tests covering sole 429/403 recovery, 402 full-bench, and multi-key
(no early recovery).
hermes debug share runs on the backend. A desktop app connected to a
remote, docker, or SSH backend writes desktop.log on the client machine,
so the bundle can never contain it — and the report rendered that as a
bare "(file not found)", which reads as "the app logged nothing" and
sends triage after a client-side bug it cannot see.
Name the writer and the path to collect by hand. Backend-written logs
are unchanged, a present desktop.log is still captured, and an empty one
still reports "(file empty)" — the app ran and logged nothing is a
different fact from the file being on another host.
* fix(agent): adopt .env credential/base-url edits at the turn boundary
A Settings save (desktop PUT /api/env, hermes setup) updates .env and
the saving process's os.environ, but a live session worker keeps the
base_url/api_key captured at agent init until restart — an open chat
silently kept calling the old endpoint (e.g. a local-server key sent to
api.openai.com, failing with an opaque 401).
Add AIAgent._try_refresh_env_client_credentials(), called at the start
of each conversation turn: re-resolve the provider's env-sourced
credentials (load_env() is mtime-memoized, so an unchanged file costs
one stat()) and rebuild the client via the existing
_replace_primary_openai_client machinery when the user edited them.
The refresh reacts only to env edits — resolved values changed since
the last look — never to mere divergence from the agent's current
values: credential-pool rotation and failover legitimately move the
session off the env credential, and stomping those back would flap.
Config model.base_url / pool custom endpoints keep precedence: edits
are only adopted while the session still runs on the registry default
or the previously-seen env value.
Lift _get_env_prefer_dotenv out of _seed_from_env to module level
(get_env_prefer_dotenv) so both the pool seeder and the per-turn
refresh share the same .env-over-os.environ resolution, including the
op:// indirection handling.
Fixes#67821
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(agent): address sweeper review on env credential refresh
- Cover named custom providers (#67935): provider="custom" has no
PROVIDER_REGISTRY entry, so resolve the config block's key_env through
the same lookup the runtime resolver uses.
- Make the edit baseline transactional: a failed client rebuild rolls the
agent back and leaves _env_creds_seen un-advanced so the unchanged edit
is retried next turn.
- Recompute route-derived TLS material and default headers on a base-url
change, via a _reapply_route_client_config helper shared with
credential-pool rotation so the two paths cannot drift.
- Rebase onto main: get_env_prefer_dotenv keeps the scoped _get_secret
semantics from the profile-isolation fix (no raw os.environ reads).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* chore: map jskang@lablup.com to rapsealk
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Brooklyn Nicholson <brooklyn.bb.nicholson@gmail.com>
* fix(credential-pool): clear exhaustion state on key rotation
When a user rotates an API key (e.g. via `hermes setup` after hitting a
rate limit), _upsert_entry updates the access_token on the existing pool
entry but preserves the stale last_status=exhausted from the old key.
On the next session the pool finds the entry, sees it exhausted, and
returns no usable credentials — even though the new key is valid.
Fix: when access_token changes on an existing entry, reset last_status,
last_error_code, last_error_reason, last_error_message, and
last_error_reset_at. The exhaustion state belongs to the old key, not
the new one.
* chore: add pasevin@gmail.com to AUTHOR_MAP
* fix: clear last_status_at on key rotation, remove unused pytest import
Address review feedback from teknium1 on PR #22622:
- Add last_status_at=None to the reset block (matches all other
token-sync reset paths in credential_pool.py)
- Assert last_status_at is None in the regression test
- Remove unused pytest import flagged by ruff + ty
Drop the manual web.search_backend / web.backend config-reading block
that duplicated _read_config_key in web_search_registry.py. The function
now delegates directly to get_active_search_provider() (which reads the
same config keys via the registry's canonical resolver) and falls back
to _get_search_backend() only when the registry has no providers loaded.
Also updates the TestXaiWebSearchBackendPreference tests to monkeypatch
the registry instead of load_config_readonly, and adds two new tests for
the legacy fallback path (no provider registered -> _get_search_backend).
Lock in backend preference, wire-name aliasing, and normalize mapping
so configured non-xai search providers stay on the Hermes client path.
Also init conflict-recovery generation on the telegram bare-adapter
helper so CI polling progress tests do not AttributeError.
#75017: Telegram polling conflict retry used drop_pending_updates=False,
starting a new getUpdates session that immediately got 409'd by the
previous still-expiring session — creating the very conflict it was
trying to recover from. Switch to drop_pending_updates=True so Telegram
terminates stale sessions. Also add a recovery-generation guard so the
first transient getUpdates success after a retry doesn't reset the
conflict counter back to 0 (defense-in-depth from PR #75096).
#75153: The WAL-reset warning always said 'hermes update can repair'
even for git/pip/system Python installs where it can't. Now uses
detect_install_method() + recommended_update_command_for_method() to
give a context-appropriate hint (hermes update for git, docker pull for
docker, nix message for nix, generic install hint as fallback).
- write_file: encode content once, share bytes between bytes_written and
the sha256 verification (drops a second full-content encode per write)
- patch_parser: replace the except-TypeError retry around
write_file(pre_content=...) with signature-based feature detection so a
TypeError raised inside a capable implementation propagates instead of
triggering a duplicate write; tests for both duck-typing contracts
- tests: real-ops V4A BOM round-trip + _file_has_bom disk-probe guard
(the teknium1-review regression previously only covered by a fake)
- comment: document dirs_created's long-standing "parent ensured" meaning
Bug 1 (UTF-8 BOM loss on V4A UPDATE):
_file_has_bom() trusted pre_content for BOM detection, but the most
common pre_content provider — read_file_raw() — deliberately strips
BOMs so the agent never sees U+FEFF glyphs. Passing BOM-stripped
content through pre_content caused a false-negative: the method
returned False and write_file() silently removed the marker on rewrite.
Fix: _file_has_bom() now always probes the first 3 bytes on disk
(head -c 3), ignoring pre_content for BOM purposes. pre_content is
still used by two other consumers — line-ending detection and lint/LSP
delta computation — neither of which is affected by BOM stripping.
Bug 2 (backward compatibility):
_apply_update() called write_file(path, content, pre_content=...) as a
keyword argument. Duck-typed file_ops implementations that only
implement the two-argument write_file(path, content) contract would
raise TypeError.
Fix: wrap the call in try/except TypeError, falling back to the
two-argument form when the keyword is not accepted.
Also declare tomli in pyproject.toml (pre-existing conditional import
for pre-3.11 Python, caught by the pre-commit dep scan after staging
file_operations.py).
Tests:
Add TestV4ABomRoundTrip with two cases:
- UPDATE on BOM-bearing file preserves the marker
- UPDATE on plain file does not inject a BOM
Addresses teknium1 review on PR #55661.
write_file currently spawns up to 6 subprocesses per call:
1. mkdir -p (separate call before atomic write)
2. cat (to read pre-content for lint/BOM/line-ending detection)
3. _atomic_write (mktemp + write + mv — the essential one)
4. wc -c (to measure bytes written)
5. _check_lint_delta (post-write lint — also essential)
6. LSP snapshot (also essential)
This PR removes three of them without changing any observable behavior:
1. Fold mkdir -p into _atomic_write shell script (−1 subprocess/write)
The atomic write script already runs a single shell; adding mkdir -p
to it costs zero extra processes.
2. Add optional pre_content parameter to write_file (−1 subprocess/patch)
patch_replace and V4A _apply_update already read the file for fuzzy
matching. Passing that content as pre_content skips the redundant cat
inside write_file. Fully backward-compatible: callers that don't pass
pre_content still read from disk as before.
3. Replace wc -c with len(content.encode('utf-8')) (−1 subprocess/write)
We already have the content in memory; encoding it to get the byte count
is equivalent to wc -c for UTF-8 text.
4. Remove redundant _check_lint loop in apply_v4a_operations (−N subprocesses/V4A)
write_file already runs _check_lint_delta internally. The old code ran a
bare _check_lint(f) loop over all modified files — a re-read + re-lint
without post_content context. Now lint results propagate from write_file
via a four-tuple return, zeroing out the extra subprocesses.
Net effect:
- write_file: 6 → 3 subprocesses per call (new files)
- patch_replace: 6 → 5 subprocesses per call (pre_content skips cat)
- V4A multi-file patches: saves 1 subprocess per modified file
- A typical 4-file V4A patch drops from ~28 to ~16 subprocess calls
#71775 moved deferred single-use-token refreshes outside the pool lock
(correct — they hold a cross-process flock plus network I/O). But
_refresh_entry_impl's three terminal-auth-failure quarantine paths do a
bare read-modify-write of self._entries. Those used to run with the
caller holding self._lock; on the deferred path they run unlocked, so a
concurrent mutation between the read and the write is silently lost.
Wrap all three in 'with self._lock' (an RLock, so locked callers
re-enter safely) and correct the _refresh_pending_entries docstring,
which claimed the mutations were already self-locking.
Post-merge gate-sweep finding on the #71775 salvage (#77714).
Sibling to the acquire_lease re-select fix.
select() re-selects once deferred single-use-token refreshes complete;
acquire_lease() performed the refresh but returned its pre-refresh
answer. Since _acquire_lease_under_lock returns early exactly when a
refresh is pending (if not available: return None, pending_refresh),
a pool whose entries all needed a refresh always returned None — the
caller failed an answerable request right after the refresh succeeded.
Retry once, only when the first pass was empty and a refresh ran.
Post-merge gate-sweep finding on the #71775 salvage (#77714).
CI caught a missed caller-shape update. Both PRODUCTION callers of
_write_sse_chat_completion / _write_sse_responses were converted to
ThreadSafeAsyncQueue, but two pre-existing tests in
tests/gateway/test_api_server.py construct the writer's queue
themselves and still passed a stdlib queue.Queue.
The consumer now does 'await asyncio.wait_for(stream_q.get(), ...)',
which on a queue.Queue blocks the thread forever:
test_stream_cancelled_persists_incomplete_snapshot hung until
pytest-timeout killed it (CI reported the whole file as 'no tests
ran (timeout before collection)'). The sibling disconnect test only
survived because it pre-fills before the first await.
tests/gateway/test_api_server.py: 99 passed (was 1 failed + a 60s
hang); with the SSE/api_server suites: 147 passed.
Gate finding (/simplify-code pass): both cross-thread tests passed
loop=loop explicitly, but no production caller does — all six
(_on_delta, _on_tool_*) rely on the queue resolving its own
_loop_ref in __init__. The kwarg made the tests vacuous: a broken
_loop_ref still passed them.
Dropping the kwarg exercises the real path. Verified by mutation:
with self._loop_ref = asyncio.new_event_loop() (wrong loop), both
tests now FAIL; they passed before this change.
The conflict-marker strip fused test_agent_task_raises with the body of
test_failed_result_dict — restore both as separate tests (content from
the PR head, verified verbatim).
Addresses teknium1 sweeper review (2026-07-30) requiring coverage of:
1. ThreadSafeAsyncQueue.put_threadsafe() off-loop boundary: a real
daemon thread pushes into the queue from outside the owning event loop
while the consumer awaits get(), mirroring the run_conversation
worker-thread producer path. Includes a 20-concurrent-thread
no-drop regression.
2. Long-reasoning bound stability for thinkingPreview: 100k-char input
plus empty/collapsed cases must not crash and must retain the visible
tail marker inside the bounded 24k clean window.
The session event stream (api_server.py:~2236) was the one genuinely
unicode-distinct SSE writer — json.dumps(payload, ensure_ascii=False) +
.encode('utf-8'). Every other writer uses plain json.dumps. Route it
through _sse_frame(..., ensure_ascii=False) so _sse_frame is now the single
source of truth for ALL SSE frame serialization in the module (chat-
completion, responses._write_event, /v1/runs, and the session stream).
Byte-identical for non-ASCII payloads: verified against the historical
inline encoder (raw bytes preserved). The ensure_ascii=False path is now
exercised by test_sse_frame_ensure_ascii_false_reproduces_session_event_stream.
_extend _sse_frame with an explicit ensure_ascii param (default True,
byte-identical to a bare json.dumps) and route the two sibling writers
through it: _write_sse_responses._write_event and the /v1/runs event
stream. This completes the dedup PR #65009 — previously only the five
_write_sse_chat_completion sites used the helper, leaving the other two
writers on inline json.dumps with no shared shape.
No behavior change: every writer's emitted bytes are unchanged (verified
byte-for-byte, including non-ASCII payloads where the default
ensure_ascii=True matches the original inline encoders). The ensure_ascii
option is exposed so a future writer can opt into raw non-ASCII bytes
without fractalizing the format again.
Adds tests/gateway/test_sse_frame.py asserting the byte-contract
invariant between _sse_frame and the historical inline encoders.
_write_sse_chat_completion and _write_sse_responses bridged their
stream_delta_callback queue into the event loop via
`await loop.run_in_executor(None, lambda: stream_q.get(timeout=0.5))`
in a while-True poll — a thread-pool round trip on every 0.5s tick even
when idle, plus up to 500ms of tail latency between a delta landing in
the queue and it reaching the SSE response.
Add ThreadSafeAsyncQueue (asyncio.Queue + a put_threadsafe() that wraps
call_soon_threadsafe), used by both streaming producer closures
(_on_delta, tool start/complete callbacks — all invoked from the worker
thread running run_conversation via loop.run_in_executor). Consumers
now do a plain `await asyncio.wait_for(stream_q.get(), timeout=0.5)` —
woken immediately when a delta arrives, no executor hop, no poll
interval.
Updated tests/gateway/test_sse_agent_cancel.py's 7 call sites to
construct ThreadSafeAsyncQueue inside the running loop (required, since
it captures asyncio.get_running_loop() at construction) instead of a
bare queue.Queue() at test-method scope.
Brooklyn Nicholson