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.
#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).
Cherry-picked from PR #58560 by @itsflownium, adapted to current main
(_getenv instead of os.getenv). Moves ANTHROPIC_API_KEY check ahead of
Claude Code credential file and credential_pool auto-discovery so an
explicitly configured key is never shadowed by auto-discovered OAuth.
Fixes#58546
The skills index is runtime-mutable: the agent adds and patches skills mid-session, so it is not byte-stable. Keeping it in the stable band breaks that band prefix-cache contract, because every skill edit changes the stable band and invalidates the entire cached prefix in front of it. Move it to the front of the volatile band so the stable scaffold (identity, tool guidance, model guidance) stays cacheable across skill edits.
A successful compaction frees the largest allocation a long session ever
drops (the compressed-away message dicts), but Python's arena allocator
keeps those pages in the heap — RSS retains the pre-compaction
high-water mark until exit. #76905's trim_memory lifecycle covers the
gateway/TUI housekeeping loops but not the CLI compression path.
Call trim_memory(reason='post-compression') at the compression-success
point in ContextCompressor.compress(), following the house pattern
(lazy import in try, debug-level log on failure). The helper is
glibc-gated, config-gated and rate-limited, so it is a safe no-op on
other platforms and cannot fail compression.
Re-expresses the intent of #70782 (JonthanaHanh), which reached for a
bare gc.collect(); trim_memory is the house mechanism and already
wraps a collect.
Salvage of #71282 (Fixes#71281): a routable-but-dead endpoint (corp
LAN address while off-VPN) blackholes TCP SYNs, so every probe in the
model-metadata waterfall waits out its full connect timeout — 20+
seconds of stall per startup across detect_local_server_type,
fetch_endpoint_model_metadata, and the per-model probes.
A module-level blackhole cache keyed on host:port is populated when
any probe observes a ConnectTimeout (httpx or requests; read timeouts
deliberately excluded — an accepted connection is not a blackhole) and
consulted at the top of each guarded function. 30s TTL: long enough to
collapse one startup burst, short enough that VPN recovery is picked
up without a restart. Guard ordering: blackhole check -> disk L2 ->
HTTP waterfall, and a blackholed leg aborts the remaining legs instead
of letting each stall in turn.
Squash of the PR's two real commits (the branch's merge commits made
it un-rebase-merge-able; content verified identical via merge-tree).
fetch_endpoint_model_metadata's generic (non-LM-Studio) /models fetch and
its llama.cpp /v1/props context-length follow-up built request URLs
straight from the unrewritten candidate, unlike every other local-probe
site. Both retained the multi-second dual-stack IPv6 connect penalty
that _localhost_to_ipv4() exists to skip (measured on macOS: localhost
32.9ms vs 127.0.0.1 0.1ms on a dead port; ~2s on Windows). normalized
stays the cache key so caching behavior is unchanged; only the outbound
request target is rewritten.
Re-derived from PR #61528 onto current main (original no longer applied
cleanly).
CI-caught: test_verification_stop_caching and test_tui_gateway_server::test_native_vision_turn_persists_a_renderable_image_ref both assert on append_message.call_args, but the flush loop now calls append_messages_batch. Same class of test-fake fallout fixed in 5 other files — these two were missed.
Re-derivation of #23254 (@devsart95) on today's flush loop. The turn
flush in _flush_messages_to_session_db wrote one BEGIN IMMEDIATE
transaction per message row; a typical agent turn (user + assistant +
tool results) paid 3-8 transactions -- and, off WAL (the default on
macOS while the WAL-reset guard is active), 3-8 fsyncs -- per turn.
Adds SessionDB.append_messages_batch: same row shape as append_message
(shared _prepare_message_row serializer + _MESSAGE_INSERT_SQL column
list, so the two writers cannot drift), same compression-lock and
compression-closed guards, one aggregated session-counter UPDATE, one
transaction for the whole batch. Row serialization stays outside the
write lock.
The flush loop now collects the turn's new rows and writes them in one
call. All-or-nothing pairs exactly with the persisted-marker stamping:
on failure no rows landed and no markers were stamped, so the next
flush re-writes the whole tail (same recovery contract as before,
minus the partial-prefix case that could double-count).
Measured (same harness, 5-message turn, journal_mode=DELETE,
synchronous=FULL): 2.32ms -> 0.83ms median per turn flush (64% faster,
5 fsyncs -> 1). On WAL the win is smaller but the atomicity fix holds.
SubdirectoryHintTracker re-injected identical context files whenever the same
AGENTS.md was reachable through more than one path. Symlinked shared
workspaces, hardlinks, and timestamped backup copies all alias a single file,
so a normal session could ship the same 8KB of instructions two or three
times. Nothing deduped it and nothing excluded directories that only ever
hold copies.
Two changes:
* Track a sha256 of every injected hint body. Repeat content is skipped, and
the working directory's own context file is seeded at construction so the
copy prompt_builder already loaded at startup is never sent again.
* Skip directories that hold copies rather than authoritative context
(backups, node_modules, venv, site-packages, .git, .Trash, vendor, caches).
Screening is relative to working_dir, so a project that legitimately lives
under vendor/ keeps discovering its own subdirectory hints.
Measured on a real session that touched a symlinked shared workspace:
3 injections / ~24,000 chars before, 1 injection / 8,112 chars after.
14 new tests cover symlink aliasing, byte-identical copies, working-dir
seeding, distinct content still being injected, each excluded directory name,
excluded ancestors, and the working-dir-inside-excluded-name case.
Review folds on the #71775 salvage (dossier findings 1+2):
- self._lock becomes an RLock and the mutation primitives
(_replace_entry, _persist) are now self-locking, so the deferred
single-use-token refresh path — which deliberately runs its
cross-process flock + OAuth network I/O OUTSIDE the pool lock —
still serializes its pool mutations against concurrent
select()/rotation. In-lock callers re-acquire reentrantly.
- Dropped _refresh_pending_entries' redundant second _replace_entry:
_refresh_entry already merges the refreshed entry internally.
Adds tests/agent/test_credential_pool_deferred_refresh.py pinning both
invariants: select() must NOT hold the lock during the refresh window
(the PR's whole point), and the post-refresh mutations MUST contend on
the lock (blocking-thread probe).
select() and acquire_lease() held self._lock during the entire
_available_entries() loop, which for openai-codex and xai-oauth providers
includes a cross-process file lock (_auth_store_lock) plus OAuth token
refresh HTTP POST. The lock timeout can exceed 20 seconds, blocking all
credential pool consumers across every gateway thread and subagent.
Collect single-use-token refresh entries under the lock, then execute the
refreshes outside it. On success the refreshed entry is merged back into
the pool and re-selected. Non-single-use providers (anthropic, nous)
continue refreshing inside the lock since their refresh is a simple HTTP
POST with no cross-process coordination.
The 2-line alias had zero production consumers (web_server calls
get_usage_breakdown directly). Tests rewired onto the real API; the
contracts they pin are unchanged. Stale test docstring fixed.
The curator LLM review loop (_run_llm_review) built its AIAgent without
enabled_toolsets, so it advertised the full default catalog (~30 tools plus the
context_engine lcm_* family) on every call. The fork uses only four tools, fixed
by its own system prompt, with no dispatch path to the rest, so ~26 tool schemas
shipped on every request as dead weight: ~7K input tokens per call on a loop that
makes 50-100 calls per consolidation pass.
Restrict the fork to enabled_toolsets=["skills", "terminal"], the same tools the
prompt already names. Behavior-neutral: the prompt held the model to these tools
and nothing routed calls to the others. Mirrors the background_review fork
(background_review.py:788-794). Call-site only; AIAgent already forwards the kwarg.
Adds test_review_fork_restricts_toolsets_to_skills_and_terminal (captures the
constructor kwarg) and test_review_fork_toolset_surface_is_skills_plus_terminal
(pins the resolved surface).
Rebase fold on the salvaged gate:
- is_trivial_prompt/TRIVIAL_PROMPT_RE move to agent/memory_provider (the
ABC both the core gate and providers already import) — one source of
truth; honcho's _TRIVIAL_PROMPT_RE now aliases it, turn_context and the
queue_prefetch_all warm path (a sibling site main grew after the PR's
base) both use it
- tests: gate tests at the prefetch call site (mutation-checked), shared
classifier tests incl. prefix-collision guards (k8s/yolo/note/supper),
and honcho dialectic-machinery tests re-driven with a substantive prompt
("hello" became trivial by design — those tests exercise thread cadence,
not the classifier)
The INDEXED BY pin is a hard dependency -- SQLite raises 'no such
index' when the named index is missing. That happens in production:
the web dashboard's usage analytics (_get_usage_analytics,
_get_models_analytics) open state.db read_only=True, which skips
_init_schema, so a DB last written by a pre-index version has no
idx_messages_assistant_calls_by_session and every insights call
crashes with OperationalError (reproduced E2E).
Probe sqlite_master once in __init__ and strip the pin from the four
prepared statements when absent -- identical rows, optimizer-chosen
plan, no crash. Replaces the change-detector test that froze the
crash as intended behavior with a fallback-equivalence test.
Review follow-up (#67341): on a freshly initialized state.db (before
ANALYZE has run) the source-filtered branches of _get_tool_usage /
_get_skill_usage did not select idx_messages_assistant_calls_by_session
— the optimizer drove from idx_sessions_source_id and probed each
session's messages via idx_messages_session_active, scanning non
tool-call rows. Pin the index with INDEXED BY on all four fixed-predicate
branches so the plan is deterministic for both the unfiltered and
source-filtered scopes without depending on statistics.
Safe because the index is declared in SCHEMA_SQL (created by every
read-write SessionDB._init_schema) and every InsightsEngine caller opens
a read-write SessionDB; read-only attachments (which skip schema init)
are never used for insights.
Extract the four queries into class constants and add tests: query-plan
coverage for both scopes without ANALYZE, row-level equivalence between
pinned and un-pinned forms, and an assertion that INDEXED BY fails loudly
if the index is absent.
InsightsEngine._get_tool_usage and _get_skill_usage scan messages for
role='assistant' AND tool_calls IS NOT NULL, but no index aligns with
that predicate, so SQLite scans the full messages table on a large
state.db. Add a partial index over exactly those rows.
role and tool_calls are base columns in the messages table, so the index
lives in SCHEMA_SQL (created on both fresh and existing databases via the
executescript on every open) rather than DEFERRED_INDEX_SQL.
Adds schema regression coverage (fresh + reopened DB, plan uses the index)
and an Insights regression test proving tool/skill output is identical with
and without the index present.
Fixes#67341
Review follow-up on the #56798 salvage: the gate shipped fully dormant
(no provider profile sets supports_prompt_cache_key, no production
caller passes it, and no plain 'openai' profile exists to set it on) —
AGENTS.md rejects dead code wired in without E2E proof.
Activate the one endpoint where the field is first-class: exact-host
api.openai.com (OpenAI documents prompt_cache_key; GPT-5.6+ docs
recommend it for cache routing). Deliberately NOT substring matching —
Azure/OpenAI-compat endpoints may reject unknown fields and stay
opt-in via the flag. 4 new tests (imply + 3 spoof/proxy/Azure
negatives); mutation-checked (substring-weakened host check fails the
spoof tests).
Review fold on the #76341 salvage: the substring test ('gh' in
source.lower()) classified GH_TOKEN and GITHUB_TOKEN as gh_cli, so a
user's env-var-specific suppression was silently bypassed (and
suppressing gh_cli silently dropped env tokens). Pre-existing bug on
main, but the PR's early gate makes the classification decide whether
the exchange runs at all. Match resolve_copilot_token's exact
'gh auth token' sentinel instead.
Adds 3 regression tests: env-var suppression gates the exchange,
gh_cli suppression doesn't swallow env tokens, all-sources suppression
skips the resolve subprocess entirely. Also corrects the ~13s comment
(actual worst case ~35s: 3x10s timeouts + 4.5s backoff).
The copilot branch of _seed_from_singletons ran the suppression gate
_after get_copilot_api_token(), which retries the network exchange 3x
with backoff (~13s worst case). A source the user already suppressed
(hermes auth remove copilot gh_cli) still burned the full exchange dead
time on every pool load — model picker open, /model, agent startup —
only to have the entry discarded afterwards.
Move the _is_suppressed() gate ahead of the network call, matching the
early-gate pattern every other singleton branch uses. Suppressed copilot
sources now skip the exchange entirely. Measured: model.options payload
build drops from ~13s to ~0.2-0.4s for a user with copilot suppressed.
Add regression test test_load_pool_skips_exchange_for_suppressed_copilot
asserting the exchange is never invoked for a suppressed source.
Root cause: two independent bugs in convert_messages_to_anthropic()
(agent/anthropic_adapter.py), the final conversion step before every
Anthropic messages.create() call, both producing HTTP 400 "text content
blocks must contain non-whitespace text":
1. _ensure_leading_user_turn() synthesized a filler user turn with
content [{"type": "text", "text": " "}] (a single space) whenever the
built payload didn't start with role=user (e.g. after context
compaction leaves a leading assistant summary). The space is itself
whitespace-only, so the guard traded a "leading assistant turn" 400
for the "text content blocks" 400 it now hits. Fixed to reuse the
existing non-blank _EMPTY_TEXT_PLACEHOLDER ("(empty)").
2. _convert_user_message() filtered blank text blocks from list-type
user content with an all-or-nothing check:
all(blank for b in blocks if b.type == "text"). This is vacuously
true when a message has zero text-type blocks (silently destroying
valid non-text blocks like images/documents it never inspected), and
false as soon as any single text block is non-blank — which let a
*sibling* blank text block sit untouched next to valid content and
reach Anthropic as-is. Replaced with per-block filtering (mirroring
the assistant-side logic already in _convert_assistant_message),
preserving all non-blank/non-text blocks and relocating any
cache_control marker carried by a dropped block.
Also added _scrub_blank_text_blocks(), a final defense-in-depth pass run
as the last step of convert_messages_to_anthropic() (after every other
transform, including nested tool_result content lists) so a blank text
block from any current or future producer never reaches the wire. It
logs only structural metadata (message index, role, content location,
block index/type) — never message text, tool arguments, tokens, or
credentials.
An earlier local patch to sanitize_api_messages() (agent_runtime_
helpers.py) attempted to fix this by rewriting blank assistant content
before the OpenAI->Anthropic conversion step, but the real leaks were
introduced downstream of that sanitizer, inside the Anthropic-specific
converter itself — the patch never touched the actual defect and has
been fully reverted (agent_runtime_helpers.py is back to its committed
state; verified via `git diff` showing no changes).
Verified against a real Telegram message end-to-end: the gateway no
longer produces the "text content blocks must contain non-whitespace
text" error on a fresh conversation turn.
Testing:
- 9 new end-to-end regression tests in test_anthropic_adapter.py
(TestFinalPayloadHasNoBlankTextBlocks) covering content="",
content=" ", content=[{"type":"text","text":""}], mixed blank+valid
text, blank text next to a valid tool block, an assistant tool-call
message with blank content, the leading-synthesized-user-turn case,
and a blank text block nested inside a tool_result's own content list.
- Fixed one pre-existing test that had asserted the broken " " filler
behavior as correct.
- Full tests/agent/ + tests/run_agent/ suite (4671 tests) run against
both the patched tree and a stashed pre-fix baseline: identical 148
pre-existing failures in both runs (unrelated subsystems — codex
app-server integration, credential-pool interrupt handling, OpenAI
client lifecycle), zero failures unique to either side.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- tests/agent/test_session_activity.py asserts against
ACTIVITY_DESCRIPTION_MAX instead of the literal 120.
- The session-stall WARNING log line names its config knob
(agent.session_stall_timeout) so operators can find the setting.
- hermes_state.py: collapse the triple blank line near line 191.
- hermes_cli/status.py no longer imports the private
hermes_cli.main._relative_time: the helper moved to a public home
(hermes_cli.timefmt.relative_time); main._relative_time stays as a
thin back-compat wrapper (sessions_cmd and external patchers keep
working).
test_progress_extends_idle_budget_until_success raced wall-clock: the
0.1s-idle/0.04s-tick shape left ~60ms of slack per tick, so one slow
scheduler pass on a loaded CI box lapsed the idle budget mid-loop.
Widened to 0.5s idle / 0.1s ticks (5x per-tick margin, total runtime
still <1s) per the FLAKY policy's minimum-margin guidance.
The fail-fast admission path (bounded compress pool, F6) only logged a
WARNING; in the compression-attempt telemetry stream a wedged pool
looked like compression simply stopped being attempted. Emit the
existing attempt telemetry with failure_class='pool_saturated'
(commit_status=aborted, split_status=aborted) on refusal, following
_emit_compression_attempt_telemetry's existing call shape. Regression
extends the F6 saturation test (sabotage-verified).
The compression heartbeat's terminal 'context compression completed'
stamp force-persists against the PARENT session id (agent.session_id at
stamp time). After the out-of-place rotation the parent is archived but
kept advertising a fresh last_activity_at + terminal label forever.
Clear the parent row's activity labels best-effort after a committed
rotation (keeps last_activity_at so idle clocks stay continuous; the
child carries live labels). Regression asserts the archived parent's
labels are cleared while the child's lineage is intact
(sabotage-verified).
revoke_commit_admission() used to invoke the holder-qualified lease
release unconditionally — including while an admitted commit was still
mutating SessionDB — letting a second compressor acquire the durable
lock mid-commit and interleave with the first commit's writes.
The admission_revoked flag store stays lock-free, but the lease-release
decision now coordinates with the fence lock:
- revoke acquires the fence lock non-blocking; on success no commit can
be in flight (an admitted commit retains the lock until finish_commit)
and the release runs immediately, still under the lock so a racing
begin_commit cannot slip between the check and the release.
- on failure the release is deferred: finish_commit() re-checks
_admission_revoked and performs it AFTER the commit completes (prompt
even if the worker thread is later parked), and the begin_commit
refusal path does the same for a revoke that lost the race to a
transient lock-setup/cancel boundary. All paths are idempotent with
the worker's own outer cleanup (DB release is holder-qualified).
Invariant encoded + tested: no second compressor can acquire the durable
lock while an admitted commit is still mutating; after a post-revoke
commit finishes the lease is released promptly. Both regressions
(revoke-during-commit deferral, revoke-before-commit immediate release +
refused begin_commit) are sabotage-verified.
Rebase onto origin/main brought in 'let explicit interrupts cancel safely',
which predates this branch's pooled progress-timeout + F1-F6 fence rework.
Reconcile the two:
- begin_commit(cancel_event) re-checks the hard-cancel Event under the
fence lock again (lost in the mechanical rebase).
- compress_context: restore aux_interrupt_protection around the summary
call, the post-return frozen-cause AuxiliaryExplicitCancellation check,
and the full rollback/telemetry handler for explicit interrupts.
- run_agent._compress_context: recreate the per-attempt fence registration
(_active_compression_commit_fence) that hard_interrupt() uses to
serialize cancel admission, and thread that exact fence through both
the direct and pooled paths (run_compress_context_with_progress_timeout
now accepts an external fence).
- test: the pooled worker isolates the live transcript (F3), so the
hard-interrupt rollback regression mutates the engine's input snapshot
rather than reaching around it to the caller's list.
Same hermetic-CI trap the concurrent-fork suite already guards against:
without credentials the one-time feasibility probe aborts compression
before the stubbed engine starts, so the F3/F4 blocked-state assertions
went vacuous-false in CI while passing on credentialed dev machines.
Both progress-aware waits (sync compress wrapper and gateway session
hygiene) slept a FULL idle interval and only then compared progress, so
progress early in an interval let silence approach 2x the configured
idle timeout before the waiter noticed. Compute each wait slice as
idle_timeout - elapsed_since_last_progress instead.
Regression: a worker that reports progress early and then goes silent is
timed out in ~1x the idle budget, not ~2x.
PR #76354 review, 'idle timeout can allow nearly twice that silence'.
The process-wide 4-worker pool retained the stdlib executor's unbounded
queue: four hung summaries wedged every slot, a fifth compression queued
silently, waited out its whole budget without starting, and remained
eligible to run later as an expensive stale job whose fence was already
cancelled (the first fence check used to sit AFTER the summary call).
- Bounded admission: submission fails fast (messages returned unchanged,
loud warning) when all pool slots are occupied; slots are freed by a
future done-callback. Recovery contract documented at the constant: new
work fails fast while wedged, wedged workers are fence-cancelled and
restore service when they return; a worker that never returns costs its
slot — bounded, observable degradation instead of unbounded queueing.
- Not-yet-started futures are cancel()ed on timeout.
- The cancelled fence is checked BEFORE any expensive summary work, both
in the pooled wrapper (stale queued job) and inside compress_context
(pre-summary gate), so a stale job never burns an LLM call or acquires
session state.
Saturation regression: 4 event-blocked summaries wedge the pool, a 5th
submission fails fast (asserted while the four are provably still
blocked), the refused job never runs after worker recovery, and a fresh
submission after recovery succeeds.
PR #76354 review, blocking finding 6 / merge gate 7.
Session rotation runs on the pooled worker thread, whose copied context
gets the child id — the CALLER's ContextVar still holds the parent, and
get_session_env() prefers a bound ContextVar over os.environ. Tools and
subprocesses invoked on the caller thread after a compression.in_place=false
rotation therefore saw the STALE parent HERMES_SESSION_ID.
After the pooled wrapper returns, rebind the session id in the caller's
own context (set_current_session_id) alongside the existing logging
repair; idempotent when no rotation happened.
Behavioral regression: with the gateway-style bound session context, a
post-compression get_session_env("HERMES_SESSION_ID") read on the caller
thread now returns the child id.
PR #76354 review, blocking finding 5 / merge gate 6.
A host timeout previously left the timed-out worker holding the durable
per-session compression lock AND refreshing its lease indefinitely, so a
truly hung summary blocked every later compression attempt; and a LATE
successful summary could clear the failure cooldown the host had just
recorded.
Transplant the lease-cancellation invariants from PR #71569
(@ciabata-git): the worker publishes an idempotent, holder-scoped release
hook on the fence once it owns the durable lock (begin_lock_setup /
register_cancelled_lock_release close the acquire→publish race), the
refresher start is serialized against the release path, and the host
invokes the hook on idle timeout, hygiene timeout, and every unwind
(revoke_commit_admission now also releases). ABA safety: the SessionDB
release is holder-qualified (DELETE ... WHERE holder = ?), so a stale
release can never free a replacement holder's lease.
State ordering: the compressor consults a fence-cancellation check BEFORE
clearing the failure cooldown, so a late worker cannot undo the host's
timeout cooldown; the check is installed only for the fenced call and
removed in a finally.
Regression implements the reviewer's exact 5-step scenario: summary
blocked indefinitely → host timeout → a NEW compressor acquires the
durable lock while the old summary is STILL blocked → old worker released
→ it cannot clear cooldown, release the new holder's lease, or publish
stale state.
PR #76354 review, blocking finding 4 / merge gates 4 + 5.
Co-authored-by: ciabata-git <ciabata-git@users.noreply.github.com>
The pooled worker closure captured the caller's live `messages` list and
compress_context explicitly supports plugin/legacy context engines that
mutate that list in place — so after a host timeout, a late engine could
rewrite the live conversation (roles, ordering, persisted content)
concurrently with the resumed turn.
The worker now deep-snapshots the transcript on the worker thread before
any engine code runs; the caller's list object is never handed to pooled
code. Results reach caller-visible state only through the returned value
of an ADMITTED commit (the host discards results on timeout/cancel), and
durable SessionDB mutation was already gated behind the commit fence.
No-op passes map the unchanged snapshot back to the caller's original
list so identity-based no-op detection and flush dedup keep working.
Document the thread-safety contract for context-engine and
memory-provider extension points (they now run on pooled threads) in the
module docstring and the context-engine plugin guide.
Regression: an in-place-mutating engine plus host timeout proves the
caller's live transcript is byte-identical WHILE the worker is still
blocked inside the engine (released only after the assertions).
PR #76354 review, blocking finding 3 / merge gate 3.
The sync compress wrapper only handled concurrent.futures.TimeoutError;
KeyboardInterrupt, task cancellation, or any other exception while
waiting let the host unwind while the detached worker kept full commit
authority — it could later enter the commit fence and mutate durable
state (in-place archival, session rotation) behind the caller's back.
Wrap the whole host wait in try/finally: any exit that did not settle the
worker (returned result or won the fence race) revokes future commit
admission via a new lock-free CompressionCommitFence.revoke_commit_admission()
(begin_commit re-checks the flag under the fence lock, so no admitted
commit is ever abandoned mid-mutation). The gateway hygiene wait gets the
same guarantee via a BaseException handler that revokes admission and
defers helper cleanup until the worker actually returns.
Reconciliation with PR #74449 (suparious): that PR routes EXPLICIT host
interrupts into auxiliary-call cancellation; this change is the
complementary host-side guarantee that no unwind — explicit or not —
leaves an unfenced worker. The two compose (fence revocation here is the
outer safety net; #74449's aux cancellation remains the fast path) rather
than duplicating one another.
Regressions: KeyboardInterrupt and generic-exception unwinds assert the
fence is revoked WHILE the worker is still blocked pre-commit, then
release the worker and prove begin_commit() is refused.
PR #76354 review, blocking finding 2 / merge gate 2.
begin_commit() retains the fence lock until finish_commit(), so a hung
SessionDB commit made try_cancel_before_commit() return None forever and
the host spun ahead of the overrun-warning loop — a genuinely hung commit
stayed unbounded AND silent. Add a lock-free phase marker (threading.Event
set inside begin_commit while the lock is held, readable without it) and
break the host spin on commit_in_flight so the bounded overrun loop — and
its WARNING + on_commit_overrun surfacing — is reachable WHILE the commit
is still blocked. Applies to both the sync compress wrapper and the
gateway session-hygiene wait.
Regression asserts the warning and callback fire while the event-gated
fake commit is still blocked; the test releases the worker only after
those assertions (addresses helix4u's released-before-asserting callout).
PR #76354 review, blocking finding 1 / merge gate 1.
The stubbed-compressor fixture still let the one-time compression-model
feasibility probe run inside the first _compress_context call. On machines
with real credentials configured, that probe resolves a live auxiliary
provider (credential pool seeding, Copilot token exchange over HTTPS),
which nondeterministically exceeds the 2s event-timing budget in
test_fence_cancelled_compression_leaves_lock_reacquirable (reproduced on
PR #73031's own head). Mark the probe done in the shared fixture: these
tests exercise locking/fencing/rotation, never aux feasibility. Suite
runtime drops from ~90s to ~4s.
Jeffrey Quesnelle
kshitij
xxxigm
kshitijk4poor
Jordan
marzukia
konsisumer
Ahmett101
rlaope
pierrenode
devsart95
BK Bot
dsad
Rod Boev
VittoriaLanzo
PRATHAMESH75
Georgio Constantinou
wangyunyou
Pooyan6
Coffee☕️
Alex Fournier
Teknium