AST-driven pass over every Path.read_text()/write_text() without an
explicit encoding= across non-test code: 71 sites in 34 files
(skills_hub, hermes_cli/main+profiles+service_manager+container_boot,
mem0/hindsight/honcho plugins, achievements dashboard, release/CI
scripts, productivity+comfyui skill helpers, agent/*). Verified zero
positional-encoding collisions before insertion; per-file compile()
check after.
Adds a check-windows-footguns rule flagging bare single-line
read_text/write_text (multi-line forms stay covered by the AST guard
test from #38985). Together with the salvaged contributor commits this
retires the ~169-site bare file-I/O class (#37423's long tail).
Path.read_text() without an explicit encoding uses the platform's
default encoding. On Windows this is typically cp1252 or mbcs, which
causes UnicodeDecodeError or silent data corruption when reading
UTF-8 content (JSON files, user text, config with non-ASCII chars).
This is the read-side companion to the write_text() encoding fix.
Fixed the most critical locations that read JSON data, user content,
and config files across 14 files with 31 call sites.
Pattern: .read_text() → .read_text(encoding='utf-8')
json.loads(path.read_text()) → json.loads(path.read_text(encoding='utf-8'))
Several file-I/O call sites still use open() / Path.read_text() /
Path.write_text() without an explicit encoding, so they fall back to
the platform default. On Windows CN/JP/KR locales (GBK/CP932/CP949)
any non-ASCII byte in a config/state/user-content file raises
UnicodeDecodeError or UnicodeEncodeError and crashes the caller.
to the remaining hot paths:
- agent/copilot_acp_client.py: fs/read_text_file and fs/write_text_file
(Copilot's read_file / write_file tools,
directly reported in #18637 bug 2)
- agent/model_metadata.py: context-length YAML cache load + two
save sites (context probing is on the
call path of every model invocation)
- agent/nous_rate_guard.py: cross-session rate-limit JSON state
(read + atomic write via os.fdopen)
- cron/scheduler.py: user config.yaml read in run_job
- gateway/delivery.py: cron output writes for AI-generated
content, very likely non-ASCII
yaml.dump call sites also gain allow_unicode=True so the emitted
YAML preserves non-ASCII chars as-is instead of emitting \u escape
sequences.
Adds regression tests that monkeypatch builtins.open / Path.read_text
/ Path.write_text to simulate a GBK locale: each test raises
UnicodeDecodeError / UnicodeEncodeError unless the caller explicitly
passes encoding='utf-8'. Verified that the tests fail on main and
pass with this change, on Linux as well as on Windows.
Refs #18637
Widen the #70773 fix beyond the three in-request cleanup sites removed in
the cherry-picked commit: every remaining path that swaps out the shared
OpenAI client could still hard-close its pool from a thread that doesn't
own the in-flight sockets (credential rotation/refresh on the turn thread,
dead-connection cleanup, gateway cache eviction, transport recovery) —
the same FD-recycle corruption vector, just rarer.
Add AIAgent._retire_shared_openai_client(): shutdown(SHUT_RDWR) all pooled
sockets (FD-safe from any thread, unblocks in-flight readers) but never
call client.close() — FD release is deferred to GC, which cannot run until
every borrowing thread has unwound its SSL BIO. Refcounting is the
ownership handshake; with no borrowers the FDs are released immediately.
Wired into:
- _replace_primary_openai_client (rotation/refresh/dead-conn cleanup)
- try_recover_primary_transport (primary_recovery)
- release_clients (gateway cache_evict)
agent.close() keeps the hard close: full teardown is a real session
boundary where no request may be in flight.
Tests: new tests/run_agent/test_70773_shared_client_fd_corruption.py
covers the three watchdog/retry sites plus retire semantics; existing
close-assertions updated to pin retire-not-close.
The streaming stale watchdog was calling
_replace_primary_openai_client() from its polling thread, which closes
the shared client's connection pool. Worker threads from previous
stale-killed attempts may still be unwinding their SSL BIOs, causing
TLS application-data to overwrite SQLite file headers via FD reuse.
This is the same corruption vector documented in #67142 for Anthropic,
where the fix was to never close the shared client from a non-owner
thread. Apply the same pattern to the OpenAI-wire path:
- Stale stream watchdog: skip shared client replacement
- Mid-tool-retry cleanup: skip shared client replacement
- Stream retry cleanup: skip shared client replacement
The request-local client is already closed via _close_request_client_once.
The shared client is replaced lazily by _ensure_primary_openai_client
on the next request, which runs on the owning thread.
Closes#70773.
Follow-up to the cherry-picked #68258 base: the cross-session-stable
prefix (_cached_system_prompt_static) was only recorded on fresh
builds, so two paths silently degraded to the legacy single-breakpoint
layout (flagged in review of #68258/#69341/#69704):
- Session restore: gateway surfaces build a fresh AIAgent per turn and
restore the persisted prompt verbatim from the session DB; the static
prefix stayed None from turn 2 onward, flip-flopping the wire layout.
- Post-compression cached-prompt reuse: _invalidate_system_prompt()
clears the static prefix, and the keep-cached-prompt branch never
restored it.
Both sites now reconstruct the stable tier and adopt it ONLY when the
authoritative prompt string literally startswith() it — stable-tier
drift (skills edited, identity changed) falls back to the legacy layout
with the stored bytes untouched. Fail-open on any builder error. The
restore-path rebuild is gated on _use_prompt_caching so non-Anthropic
routes skip it entirely.
Refs #68191
Co-authored-by: JonthanaHanh <92574114+JonthanaHanh@users.noreply.github.com>
Co-authored-by: joaomarcos <joaomarcosdias444@gmail.com>
Co-authored-by: webtecnica <webtecnica@users.noreply.github.com>
The conversation loop normalizes message text right before the API call so
the request prefix is byte-identical across turns -- the stated reason is
KV cache reuse on local inference servers and better cache hit rates on
cloud providers. Cache breakpoints were injected *before* that pass, which
defeats it.
`_apply_cache_marker` rewrites a plain-string `content` into a
`[{"type": "text", ...}]` block. The normalization pass is guarded on
`isinstance(content, str)`, so every message that just got marked is
silently skipped by it and keeps its raw leading/trailing whitespace. A
message is only marked while it sits in the last-3 window, so:
turn N in the window -> marked, content "file1\nfile2\n"
turn N+1 rolled out -> plain, content "file1\nfile2"
The same logical message is sent with different bytes on consecutive
turns. The prefix stops matching at that position -- which is inside the
span the breakpoints were placed to protect -- so the reusable prefix
collapses back toward the system breakpoint on every turn. Tool results
carry a trailing newline almost by default (any shell command output), so
this is the common case, not an edge case.
Move the injection below every message mutation. Besides fixing the
whitespace divergence this stops breakpoints from being spent on messages
that the orphan sweep or the thinking-only drop is about to remove or
merge away -- a marker on a dropped message is a wasted breakpoint out of
the four available.
Nothing between the old and new call sites reads `cache_control`, and the
mutators now see the plain-string shapes they were written against.
The salvaged drift check compared durable rows to the in-memory snapshot
by content and ran in both modes. Two problems:
1. In-place compaction (the default) archives non-destructively — drift
cannot lose data there, and the strict-prefix content comparison
failed against seeded histories, aborting every in-place compaction
(5 test failures in test_in_place_compaction.py).
2. Content equality wedges on sessions with legal in-memory mutation of
past turns (multimodal compression, retry replacement) — the same
permanent-abort shape as #14694.
Now rotation-only and length-based: abort only when the durable parent
has MORE rows than the snapshot (a writer committed in the lease window).
Dead helper _durable_history_matches_snapshot removed.
When two consecutive compactions each failed to clear the threshold, the
anti-thrashing breaker blocked automatic compaction PERMANENTLY for the
life of the session: nothing decremented _ineffective_compression_count
(or _fallback_compression_streak) while blocked, so a session whose
middle region was briefly too small to compact never auto-compacted
again — it grew unbounded until the provider's hard context limit, and
only /new or /reset recovered it.
Recovery is a probation probe, not amnesty: after
_ANTI_THRASH_RECOVERY_SECONDS (300s) of continuous block the gate grants
exactly ONE attempt by dropping tripped counters to 1 strike (persisted,
so sibling agents on the same session row — gateway hygiene — unblock
too). An ineffective probe re-trips the guard on the next real-usage
verdict and the next recovery waits a full fresh window, so the worst
case in a truly incompressible session is one compaction attempt per
window — bounded, not thrash.
The recovery clock is armed lazily on the first BLOCKED evaluation and
is deliberately not durable: a restart that loads a durable tripped
counter (#69872) starts a full fresh window blocked, preserving the
restart-must-never-disarm contract (#54923).
Fixes#14694
Review follow-up for the dropped tool-call recovery (#69630): the
re-prompt pair was tagged _dropped_toolcall_nudge, but that marker was
not part of the ephemeral-scaffolding contract. _persist_session /
_flush_messages_to_session_db would therefore write the synthetic
'issue the actual tool call now' user message (and the narration-only
interim assistant turn) as real transcript rows — a resumed session
could replay the internal retry instruction as user-authored context
and prompt unsolicited tool use.
- Add _dropped_toolcall_nudge to _EPHEMERAL_SCAFFOLDING_FLAGS
(run_agent.py) so both SQLite and JSON persistence skip the pair.
- Add it to _SYNTHETIC_USER_FLAGS (conversation_compression.py) so the
compressor never treats the nudge as human intent.
- Flag the interim assistant half of the pair too, and include the
marker in the finalization scaffolding pop so a genuine turn end
strips the pair from the live transcript (mirrors the
_empty_recovery_synthetic pattern).
- Regression tests: flagged messages classify as ephemeral, the
returned transcript contains no scaffolding, and the turn tail stays
on the real assistant answer.
The initial fix guarded the no-tool-calls else branch, but that branch only
SETS final_response — the turn actually finalizes later, in a separate block
after final_msg is built. Runs that reached finalization via that path exited
without the guard ever running (observed live: a scheduled PR reviewer stalled
at tool_turns=1-2 with zero recovery nudges).
Move the recovery to the finalization chokepoint (right after final_msg is
built), so it catches every path that ends a turn. Single guard now:
- increments a consecutive-stall counter and re-prompts (bounded to 3),
- resets on any successful tool round, and
- resets on a genuine (non-mismatch) turn end,
so it guards each stall independently without capping the whole run and
without looping forever.
Verified live: the scheduled reviewer now recovers through the stalls and
submits real reviews — PR 57800 APPROVED, PR 54826 COMMENTED — one PR per run.
Some providers (observed: claude-opus-4.8 / claude-sonnet-4.5 on GitHub
Copilot, ~2026-07) return finish_reason="tool_calls" while the parsed
tool_calls array is empty — the model signalled it wanted to act but the
payload shipped no call. The conversation loop took the no-tool-calls
else branch, treated the turn's narration as the final answer, and exited
with the task unstarted.
On unattended multi-step jobs this is silent failure: a scheduled PR
reviewer, for example, would narrate "Let me verify the PR..." and stop
at tool_turns=0 every run, never submitting a review, while the job still
reported success.
Fix: in the no-tool-calls branch, detect the provider contract violation
(finish_reason == "tool_calls" with zero tool_calls) and re-prompt the
model to emit the call instead of exiting. Bounded to 3 consecutive
stalls; the budget resets after any successful tool round so it guards
each stall independently rather than capping the whole run. The narration
may live in content or only in the reasoning field (empty content) — the
guard keys on the finish_reason/tool_calls mismatch, so both are covered.
A genuine finish_reason="stop" text turn is unaffected.
Verified live: a scheduled reviewer that died at tool_turns=0 every run
now recovers through 20+ stalls per run and submits real reviews.
Tests: tests/run_agent/test_dropped_tool_call_recovery.py covers the
re-prompt, the empty-content case, the clean-stop control, and the
bounded-loop guard.
The last_activity field showed 'tool completed: read_file (120.7s)'
identically whether the tool succeeded or failed, making post-mortem
analysis of silent-hang reports (#69131) unnecessarily hard.
Append ' (error)' to the activity description when the tool result is
classified as a failure, in both the concurrent and sequential
execution paths.
(Salvaged from PR #69467; its conversation_loop.py activity-touch hunk
is the same fix as PR #69577 and lands in the preceding commit.)
After a tool call completes, the conversation loop does to
start the next API call. Between the last (from tool
completion in tool_executor.py) and the next one (at the start of the
next API call), there is a gap. If anything during this gap takes time
— context compression, slow provider prefill, or other post-tool
processing — the combined inactivity window can exceed the gateway's
inactivity_timeout (default 120s). The gateway kills the session and the
user sees 'agent never returns a final response', even though the tool
call itself succeeded in 0.1-0.2s.
Fix: call right before so the gateway
sees a fresh timestamp immediately after tool results are posted,
regardless of how long the follow-up API call takes.
Fixes#69559
Three durable ledgers used `with _connect() as conn:` where the sqlite3
connection context manager commits/rolls back but never closes, leaking the
db/-wal/-shm file descriptors on every call. On a long-running gateway this
exhausts RLIMIT_NOFILE and fails unrelated components with
`[Errno 24] Too many open files`. Same bug class as the cron execution ledger
(#69567 / PR #69594), which the connection helpers here are modeled on.
Fix: route every ledger operation through a `_transaction()` context manager
that guarantees `conn.close()` on exit. `_connect()` keeps its
schema-on-connect contract (several tests call it directly) and now self-closes
if schema init fails.
Adds per-module regression tests asserting every opened connection is closed,
including the no-op-update and exception-mid-transaction paths.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Follow-up to the cherry-picked #68633 commits, closing the final open
review point (egilewski): _relocated_replay_cache_control was applied
only inside `if replayed:`. When anthropic_content_blocks contained
only a blank cache-marked text block, `replayed` came out empty, the
function fell through to the main path's placeholder, and the cache
marker was lost; signed thinking + a blank marked text block likewise
returned with no cacheable carrier for the relocated marker.
The replay branch now appends the non-whitespace "(empty)" placeholder
when no cacheable (text/tool_use) block survives the blank filter and a
blank text block was dropped (or a marker needs a carrier) — so replay
stays schema-valid on Bedrock/strict endpoints and the breakpoint
survives on the placeholder.
Also reconciles the block-level tests from #69517 with the new
drop-then-fallback contract (blank blocks are dropped at the block
level; the message-level result is still always non-blank).
Refs #69512
Co-authored-by: ygd58 <buraysandro9@gmail.com>
Follow-up per independent review of #68633 (GPT-5.6-sol-xhigh in Codex,
reviewer egilewski) on this PR.
Two real bugs in the blank-text-block filtering added by that fix:
1. `effective = blocks or content` fell back to the RAW, unfiltered
`content` variable whenever every block was filtered out as blank --
which happens precisely when the entire message content WAS the
blank/whitespace payload the filter exists to remove (a sole blank
text block, a sole cache-marked blank block, or standalone
whitespace scalar content with no tool_calls). The fallback silently
restored the exact invalid content the filtering just stripped,
leaving the message provider-invalid.
Fixed: `effective = blocks if blocks else [{"type": "text", "text":
"(empty)"}]` -- never falls back to raw `content`. Also moved the
cache_control application (both the relocated-from-a-dropped-block
marker and the message-level marker) to run against `effective`
instead of the pre-fallback `blocks`, so a cache marker on a block
that was the ONLY content still lands on the (empty) placeholder
rather than being silently lost when `blocks` was empty at the
point it would otherwise have been applied.
2. The normal-path blank-text check used `(blk.get("text") or "").strip()`,
which is not type-safe for a truthy NON-string, non-None text value
(e.g. an int or dict from an invalid upstream payload) -- `or`
doesn't substitute for a truthy value, so `(7 or "").strip()` still
raises AttributeError. Now checks `isinstance(text, str)` first,
matching the replay path's `_sanitize_replay_block()`, which the
reviewer confirmed was already correctly type-safe.
Added regression tests for: sole blank list block, sole whitespace
scalar content, sole cache-marked blank block (marker relocation to
the placeholder), a truthy non-string (int) text value both mixed with
a surviving tool_use and as the sole content, and a dict-valued text
field. 7/7 new tests pass; 193/193 in the full
tests/agent/test_anthropic_adapter.py file; 23/23 in
tests/agent/test_prompt_caching.py (unaffected, confirmed).
Ports #63228 forward onto current main per teknium1's review.
Bedrock and strict Anthropic-compatible endpoints reject text blocks
where text is empty or whitespace-only with HTTP 400. The normal
list-content path extended blocks without filtering, and the
ordered-replay fast path (_sanitize_replay_block) returned blank text
blocks unfiltered.
Per review, fixes three gaps in the original port:
1. Type safety: the normal-path filter used blk.get('text', '').strip(),
which crashes with AttributeError when text is explicitly None (not
absent) -- .get()'s default only applies when the key is missing.
_convert_content_part_to_anthropic() can preserve None from an
invalid upstream input text block. Now uses
(blk.get('text') or '').strip() on both paths.
2. Cache marker loss: prompt_caching.py's _apply_cache_marker() sets
cache_control directly on content[-1] for list content. If that last
part happens to be blank text, dropping it without relocating
cache_control silently loses the breakpoint. Both the normal and
replay paths now capture a dropped block's cache_control and reapply
it to the new last surviving cacheable block via the existing
_apply_assistant_cache_control_to_last_cacheable_block() helper
(setdefault semantics, so it never clobbers a legitimately-placed
marker).
3. Scalar whitespace: the non-list content branch
(blocks.append({'type': 'text', 'text': str(content)})) accepted a
truthy whitespace-only string unfiltered. Now filtered the same way
as list-content blocks.
8/8 new tests pass in TestBlankTextBlockFiltering (including None-safety,
scalar-whitespace, and cache_control-relocation regressions on both
paths); 186/186 in the full tests/agent/test_anthropic_adapter.py file.
OpenAI documents GPT-5.5 / GPT-5.5 Pro as extended-cache-only: in-memory
prompt cache retention is not available for them, and only
prompt_cache_retention: "24h" is supported. Responses requests that omit
the field see near-zero cached_tokens even with a stable prompt_cache_key
and identical prefixes (observed on an OpenAI-compatible Responses relay:
0 cached across repeated identical calls before; 97% cache reads after).
Send the field for the gpt-5.5 model family (bare and namespaced ids like
openai.gpt-5.5) on OpenAI-compatible Responses routes, mirrored in the
auxiliary Codex adapter, and pass it through preflight normalization.
Skipped for xAI, GitHub/Copilot, and the chatgpt.com Codex backend, which
reject or ignore body-level cache fields.
A brew Python upgrade (original report) or an interrupted venv rebuild
(v0.19.0 report in the same thread) can leave certifi importable while
its bundled cacert.pem is missing or a dangling symlink. Every TLS
connection then fails — Feishu/Telegram/WeChat/DingTalk all down —
with an opaque 'Could not find a suitable TLS CA certificate bundle'
from deep inside httpx/requests.
The existing repair infrastructure only probed
`hasattr(certifi, 'contents')`, which PASSES in exactly this failure
state, so neither the early venv self-heal nor `hermes update`'s
import-probe repair ever classified certifi as broken. Extended, not
replaced:
- hermes_cli/_early_recovery.py: the in-process probe now also
validates that certifi.where() exists and is a plausible bundle
(>=1KiB), so the pre-import self-heal repairs it like any other
wiped core package.
- hermes_cli/main.py (_detect_broken_lazy_refresh_imports): the
subprocess probe script used by `hermes update`'s venv repair
applies the same bundle-file check inside the target venv.
- hermes_cli/doctor.py: `hermes doctor` already failed the cert
check; `hermes doctor --fix` now repairs it (pip force-reinstall
certifi + module-cache invalidation + re-verify), covering
brew/manual venvs where no update marker exists. Failures funnel
into the manual-action list with the exact command.
- agent/ssl_guard.py: the startup SSLConfigurationError hint now leads
with `hermes doctor --fix` instead of only the raw pip command.
Fixes#29866
When api_key_hint from a 401 response doesn't match any pool entry
(common with OAuth tokens where runtime_api_key rotates), the pool
rotated without marking anything exhausted and handed back a fresh
selection. Because nothing was ever marked, the pool could never reach
the "no available entries" state — the caller retried the same dead
token forever (~6 attempts/sec), starving the event loop so /stop was
never processed; only killing the gateway ended it.
Rebased onto the identity-tracking rework that landed on main
(73c4b5a045): the single-entry escape from that commit already stops
the most common OAuth case, so this fix bounds the REMAINING gap —
multi-entry pools ping-ponging A->B->A with an unmatched hint. Cap
consecutive no-mark rotations at one full lap of the available
entries, then return None so the error surfaces / fallback activates.
Deliberately does NOT mark innocent entries exhausted (the original
PR's approach): that would quarantine a healthy key for the full
cooldown TTL on a hint that provably matches nothing. No cooldown is
written by the escape, so healthy keys stay available next turn --
bounded without hammering.
The streak resets when a rotation identifies a real entry and on any
successful normal select(), so only genuinely consecutive unmatched
rotations trip the bound.
Fixes#70401
Hardening follow-up to the #69619 review fix. The previous regression
byte-pinned only the rescued pre-#69619 generation; older frozen entries
were covered solely by fragment assertions and a self-matching loop that
cannot detect a frozen entry mutating (the loop tests each entry against
itself).
- Pin all four _HISTORICAL_SUMMARY_PREFIXES generations as literals in
_FROZEN_PREFIX_GENERATIONS and assert order-sensitive tuple equality
plus detect/strip for each
- State the prepend-only contract explicitly on the tuple: never mutate
or reorder existing entries
Negative controls verified: mutating, dropping, or reordering a frozen
entry each fail the new test, while the legacy self-matching loop still
passes under mutation — confirming the closed coverage gap.
Address review on #69619: the previous commit mutated the newest frozen
entry in _HISTORICAL_SUMMARY_PREFIXES and never froze the live prefix it
retired (the generation with both the four-heading discard clause and
the tools-active clause). A summary persisted immediately before
upgrading was therefore treated as an ordinary message on
resume/re-compaction, keeping the old handoff text embedded in the body.
- Prepend the exact pre-change live prefix as a new frozen entry
(newest-first), leaving all existing frozen entries byte-identical
- Restore the Jul 2026 (#65848 class) frozen entry to its original
four-heading text
- Pin the retired generation as a literal in
test_summary_prefix_semantics.py so mutating or dropping it fails CI
- Make the #65848 tool-use regression position-agnostic (match the
pre-clause generation by content, not tuple index)
Verified byte-identity of both rescued generations against the parent
commit. 233 focused prefix/resume/compressor tests pass.
Remove three directive-heavy section headers from both the LLM
and deterministic summary templates that caused the agent to
resume stale tasks after context compression:
- Historical In-Progress State
- Historical Pending User Asks
- Historical Remaining Work
These sections read as actionable instructions even within a
REFERENCE-ONLY wrapper, hijacking the user's latest message.
The remaining sections are purely descriptive/past-tense.
Frozen prefix copies in _HISTORICAL_SUMMARY_PREFIXES updated
to match. Test 8/8 passed.
Anthropic released Claude Opus 5 (+ -fast variant) — both are live on
OpenRouter and the Nous Portal /models endpoint (verified against both
live APIs). Opus 4.8 entries are kept.
- hermes_cli/models.py: opus-5 + opus-5-fast in OPENROUTER_MODELS;
opus-5 in _PROVIDER_MODELS[nous] (Portal serves both, curated list
carries the base model like the rest of the Nous Anthropic block).
Ordering: below fable-5 flagship, above opus-4.8.
- agent/model_metadata.py: claude-opus-5 -> 1M context (matches live
OpenRouter metadata).
- agent/reasoning_timeouts.py: claude-opus-5 -> 240s stale-timeout
floor (same as the opus-4.x thinking family).
- website/static/api/model-catalog.json: regenerated via
scripts/build_model_catalog.py.
Both providers bill via official_models_api (live pricing), so no
_OFFICIAL_DOCS_PRICING snapshot entry is needed for these routes.
AST-driven pass over every subprocess.run/Popen/check_output/check_call/call
with text=True (or universal_newlines=True) and no explicit encoding=:
append encoding='utf-8', errors='replace' at the kwarg site. 136 call
sites across 28 files (cli.py, hermes_cli/main.py, tools_config.py,
environments, computer_use, gateway, scripts, skills helpers, agent/*).
Together with the salvaged #55339/#60741 commits this closes out issue
#53428's bug class; the salvaged #60751 linter rule in
check-windows-footguns.py now enforces it repo-wide (verified: 807 files
scanned, zero findings).
On Windows with Chinese locale (GBK), subprocess.run(text=True) without
explicit encoding causes UnicodeDecodeError crashes. This fix adds
encoding='utf-8', errors='replace' to all subprocess.run() and
subprocess.Popen() calls that use text=True across 76 non-test Python files.
Fixes#53428 (master tracker for Windows GBK locale crash).
Note: credential_pool.py and electron changes excluded per reviewer request —
those will be submitted as separate focused PRs.
Base-url+model dedup was meant for custom shim aliases, but it also
skipped first-class providers that share an inference host while using
different credentials. That stranded xai-oauth spending-limit failover
to the xai API-key provider when both used the same model slug.
Replace 3 duplicated entry_id resolution blocks (try/except +
entry_id_for_api_key + fallback to None) in agent_init.py,
chat_completion_helpers.py, and switch_model with a single
sync_credential_pool_entry_id(agent) function in agent_runtime_helpers.
Follow-up to #70323.
Track the selected credential by stable pool entry ID so token refreshes and shared cursor movement cannot detach failures from the entry that issued them. Stop unmatched single-entry pools from reporting a no-op rotation as successful recovery.
Co-authored-by: Maxim Esipov <maksesipov@gmail.com>
Registers `ar` in the supported-language set and alias table and ships
locales/ar.yaml at full key and placeholder parity with en.yaml, covering
approval prompts and gateway slash-command replies. Identifiers, commands,
paths, config keys, model/provider names, and {placeholder} tokens are kept
verbatim.
Co-authored-by: Da7-Tech <286182457+Da7-Tech@users.noreply.github.com>
profile_name was only written on the agent's initial lazy create
(e8b7ce8c1); every parented child row — compression rotation, TUI
/branch, desktop branch first-persist — was created without it. A
non-default profile's lineage therefore turned NULL on its first
compression or branch and aggregated as "default" in unified session
lists, completing the cross-profile session-jump.
Fix the class at the DB layer: _insert_session_row's parent backfill now
COALESCEs profile_name from the parent alongside cwd/git_* (#64709
pattern), so any parented child inherits its lineage's owning profile.
Stamp it explicitly at the three create sites as well — compression
rotation (mirroring _ensure_db_session), TUI session.branch, and the
TUI first-prompt row persist — so rows are self-describing even when the
parent row predates the profile_name column.
Flips the default fan-out cadence from per_iteration (advisors re-run on
every tool iteration, multiplying advisor spend by tool-loop depth) to
user_turn (advisors run once on the first message of each user turn; the
acting aggregator works the rest of the tool loop with that turn's
advice). Until per-mode benchmarks justify a costlier default, MoA
defaults to the cheapest, lowest-impact cadence (#67199).
One default for everyone — no split legacy/new-preset semantics; presets
that want per-step advising set fanout: per_iteration explicitly. All
three modes (user_turn / per_iteration / every_n:N) remain selectable;
every_n:1 still collapses to per_iteration (semantic identity), while
unparseable values now fall to user_turn (the default).
Docs updated with a default-change note; the per-iteration rerun test
pins its mode explicitly.
Co-authored-by: skyer-flyyy <188930297+skyer-flyyy@users.noreply.github.com>
The skill-authoring guide and curator prompt both reference
descriptions as the primary discovery mechanism but never mentioned
the 57-char system prompt truncation. Add explicit guidance:
- Authoring guide: frontmatter docs, template comment, size limits,
pitfall #3 with good/bad examples, verification checklist
- Curator prompt: parenthetical noting the 57-char window when
writing umbrella skill descriptions
When a skill is created or edited with a description longer than
SKILL_PROMPT_DESC_LIMIT (60 chars), the tool response now includes a
system_prompt_preview field showing exactly what the system prompt
skill index will display. This gives the agent immediate feedback to
self-correct truncated trigger phrases.
Also adds tool schema guidance about the 57-char window and fixes a
stale docstring in skill_commands.py that incorrectly claimed the
system prompt renders the full description.
The system prompt skill index truncates long descriptions to 57 chars,
but this limit was a hardcoded magic number. Extract it as a named
constant and factor the normalization logic into a shared private
helper so the extraction function and the new truncation predicate
cannot drift.
No behaviour change — pure refactor.
Verification follow-up for the #51226 salvage: the host call site guarded
select_context with hasattr(), but the ABC defines a default on every
engine, so the built-in ContextCompressor (and any non-implementing
engine) still paid per-request shallow copies of the conversation
history plus a hook call on every provider request. Identity-check the
bound method against ContextEngine.select_context and return the
request untouched — mirroring the existing base-method short-circuit in
_notify_context_engine_turn_complete — so the default path does zero
work, not just produces an identical result.
Adds two pins: the base no-op is never invoked (patched-to-raise base
stays silent), and ContextCompressor.__dict__ contains neither new verb.
Also registers the contributor email mapping for @chaos-xxl.
Addresses the hermes-sweeper review on #51226:
- _apply_context_engine_selection now passes shallow copies of the read-only
conversation_messages / incoming_message to the hook, so an engine mutating
them in place cannot corrupt persisted transcript state (enforces the
request-only contract, not just documents it). Adds a mutation-regression
test asserting persisted history + incoming message are untouched.
- on_turn_complete docstring: scope the coverage claim to the standard
finalization seam. Some abnormal early-return paths (content-policy block,
provider terminal failure) currently persist+return without finalization and
don't emit the hook; documented as best-effort with a shared-seam follow-up,
rather than over-promising a guaranteed callback for every early exit.
- _apply_context_engine_selection: reject an empty list. all([]) is True, so
a [] returned by a failing/buggy engine previously replaced a valid request
with an empty message list the downstream sanitizers can't restore; now it
falls open to the unmodified request (honors the fail-open contract).
Thanks @johnnykor82 for catching this on #41918's review.
- test: empty list keeps the original request (fail-open regression).
- docs: document select_context()/on_turn_complete() in the public
context-engine plugin guide (were still describing only the old contract).
- context_engine.py: document that select_context() runs before cache-control
and all request sanitizers, so (a) replacements still pass host validation
and (b) the no-op default keeps the request byte-stable (AGENTS.md prompt-
cache invariant). Note the hook is evaluated per provider request.
- tests: no-op path is byte-stable for cache-control; a role-unusual
replacement is passed through for the existing downstream sanitizers to
normalize (select_context does structural validation only).
The on_turn_complete() observation hook is the engine's post-turn signal,
so it should receive the completed turn's canonical token usage when the
host has it, not a hardcoded None. Per @johnnykor82's #41918 contract: the
engine uses prompt/completion + cache_read/write/reasoning buckets to judge
how large/expensive the selected context was before the next select_context().
- conversation_loop.py: stash the most recent provider response's usage_dict
(the same canonical shape fed to update_from_response) on the agent as
_last_turn_usage; reset to None at turn start so turns that never reach a
provider response (early failure / interrupt) forward None, not a stale
prior turn's usage.
- turn_finalizer.py: forward agent._last_turn_usage instead of usage=None.
- context_engine.py: document the usage param contract on the ABC hook.
- tests: cover both ends through the real finalize_turn path — completed turn
forwards the full canonical bucket set intact; no-response turn forwards None.
Co-authored-by: johnnykor82 <johnnykor82@users.noreply.github.com>
