* refactor(prompt): diet the memory/skills guidance block — schema-taught curricula removed, form rule + pruning contract kept (537 -> 223 tok in the combined block)
* polish: literal check-glyphs in source; memory capacity posture — save proactively, replace/consolidate when full
* refactor: single spine for memory/profile guidance — form rule + capacity posture written once, variants differ only in opening frame
* refactor: ONE memory-guidance builder — frame adapts to enabled stores, body written once, positive posture leads (maintainer direction)
* fix wording: memory is loaded per SESSION, not injected per turn (maintainer correction)
Review follow-ups on the salvaged #90845:
- hygiene_max_turn_hold_seconds registered in config_defaults next to
its sibling hygiene knobs (run.py already read it; the key was
undiscoverable).
- Turn-hold abandonment now records a flat 60s retry-after via the
existing cooldown column. Without it, sustained traffic re-spawned,
held, and cancelled a fresh compressor on EVERY turn — a per-turn
summary-model token burn that never commits. Deliberately outside the
x1/x3/x9 failure ladder: the compressor is healthy, so the failure
streak must not advance (witness updated to assert exactly that
boundary: no streak increment, flat <=120s spacing, turn-hold reason).
Session hygiene auto-compression runs inline on the incoming-message path
and awaits the summary worker with a progress-aware inactivity budget
(hygiene_timeout_seconds) that extends up to hygiene_total_ceiling_seconds
(default 600s). A summary model that keeps streaming tokens keeps resetting
the inactivity slice, so the wait can stretch toward the ceiling while zero
bytes reach the user — chat transports (Telegram ~30s idle-timeout) drop the
connection and the turn appears frozen, even though the gateway is healthy.
Add hygiene_max_turn_hold_seconds (default 10), a turn-hold budget that caps
the wall-clock the incoming message waits on hygiene compression. The wait
slice is additionally capped at the remaining budget so the budget is
re-evaluated even when the worker keeps the inactivity slice large. On
exceeding the budget the gateway abandons the inline wait and proceeds on
the uncompressed transcript via the existing timeout path, which revokes the
worker's commit admission (CompressionCommitFence) and defers cleanup — so a
stale compression finishing later can never overwrite the turns appended
after the wait was abandoned.
Well under the typical transport idle-timeout, this guarantees the message
is answered promptly while the detached compression completes in the
background. Configurable via compression.hygiene_max_turn_hold_seconds.
Adds a regression test: a worker that streams progress continuously (so the
inactivity slice never fires) must be abandoned once it exceeds the
turn-hold budget, the turn proceeds uncompressed, and the stale commit is
fenced (no session mutation, role alternation intact).
Final /simplify-code pass on the full diff: after the content-digest
rework, _creds_cache_key had become a wrapper with zero production
callers — get_vertex_credentials inlined the same read/except dance —
kept alive only by its own tests. One helper now owns the
(bytes, cache_key) resolution for all three cases (ADC sentinel,
readable file, unreadable fallback); prod calls it, tests target it.
No behavior change: 10/10 green, same mutation-check results.
test_every_marker_emitting_call_site_goes_through_the_central_clamp read
production .py files with a regex and pinned an exact call-site count --
the change-detector / reads-source-in-tests antipattern AGENTS.md bans
outright. Replaced with a test that drives the real
plan_cache_sections_for_destination fan-in and asserts the emitted wire
markers: ttl=1h on the measured opencode-go route, ttl stripped on the
unmeasured opencode route. Mutation-checked: emptying
MEASURED_1H_PROVIDERS turns it red.
`effective_cache_ttl` evaluates the generic `is_qwen_model` clamp before any
route-level allowance, so a configured `prompt_caching.cache_ttl: 1h` is
silently rewritten to `5m` for every Qwen model on opencode-go. Reproduced on
this commit's parent, no provider traffic:
effective_cache_ttl('1h', 'opencode-go', 'qwen3.7-plus') -> '5m'
_build_marker('5m') -> {'type': 'ephemeral'}
A repair for this exists historically -- payload d6b33faae1, merged as
a43fe4918d -- but neither commit is reachable from current main
(`git merge-base --is-ancestor` returns non-zero for both), so the regression
is live on this lineage.
Restores the precedence fix: MEASURED_1H_PROVIDERS (an allow-list holding only
opencode-go, the one route measured with a delayed read past five minutes) is
consulted ahead of the generic Qwen clamp, with NO_1H_TIER_MODELS nested inside
it for models measured to ignore the tier even on a capable route.
opencode-go stays in ALIBABA_FAMILY_PROVIDERS. That set is also the
cache-marker-layout opt-in read by
agent_runtime_helpers.anthropic_prompt_cache_policy, so narrowing it would turn
working five-minute caching into *no* caching rather than extending the window.
The two sets are kept separate on purpose and a test pins the separation.
One deliberate divergence from the historical payload, found by independent
review: that patch checked NO_1H_TIER_MODELS globally, ahead of the provider
gate. MiniMax on its own Anthropic-compatible endpoint is a separate and
genuinely cache-eligible route, and the global check regressed its configured
1h to 5m off the back of an opencode-go observation -- an unrelated-provider
change this repair must not make. Measured:
base effective_cache_ttl('1h', 'minimax', 'MiniMax-M2.5') -> '1h'
payload -> '5m'
here -> '1h'
Scope note on the evidence: the delayed-read run covered qwen3.8-max and
glm-5.2. The rule is keyed on the route, not the model, so the deployed
qwen3.7-plus is covered by it but has never itself been measured. This change
restores *sending* the requested 1h marker; it does not establish that the
provider honours 1h retention. Those stay separate claims, and the provider
labels every write `ephemeral_5m_input_tokens` whatever ttl was requested, so
only a delayed read past five minutes with no intervening call can settle it.
Tests: 11 new cases covering the deployed model, marker shape, precedence
against the generic clamp, cache eligibility surviving the repair, negative
controls for unrelated routes, provider case normalization, and a closed-set
audit of every marker-emitting call site. Ten mutations were applied to scratch
copies and each turned the suite red, including hoisting the generic clamp back
above the allowance, dropping opencode-go from ALIBABA_FAMILY_PROVIDERS,
un-nesting the model denial, and adding a new unclamped sender.
Known risk, not discharged here: this marker shape has never been sent on the
Go relay. #77217 records the sibling Zen relay returning HTTP 400 on an
unexpected marker shape, so field validation must check HTTP status before it
looks at any cache counter.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QV9Ld4f5ZTqdZWncpfZcQh
Security review on #97701 (unsupportedpastels) found and reproduced a
metadata-signature collision: atomic replacement that preserves size
and mtime (deployment tools that restore metadata; equal-length JSON)
rotates the private key under an IDENTICAL (path, mtime_ns, size)
signature, so the cache kept serving the old identity — exactly the
failure the PR set out to close.
The stat idiom is right for config caches (guards a parse, mtime
collisions are harmless). It is wrong for a credential cache (guards
an identity). The key is now (path, sha256(content)):
- _read_sa_file() reads the file ONCE per call, returning both the
bytes and the digest key; on a miss the credentials are built from
that same snapshot via from_service_account_info — closing the
stat->read TOCTOU the reviewer also flagged (key and credentials can
never describe different bytes).
- Read failure degrades to the bare-path key and the SDK's own file
read: byte-for-byte the pre-signature behavior.
- Cost: one read + sha256 of a ~2KB JSON per cache probe, noise next
to the OAuth token mint the cache exists to avoid.
New regression test per the review: equal-length content swap with
mtime restored and atomic replace — asserts a NEW cache key and a NEW
Credentials object (fails on the stat-keyed version, reproducing the
reviewer's cache_keys_equal=True). Fake google-auth harness gains
from_service_account_info.
Suite 10/10; ruff green.
The /simplify-code reviewer caught a real regression in the signature-
keyed cache commit: the ADC-failure fallback still compared
`cache_key == "__adc__"` (string), but keys are tuples now — the
comparison is always False, silently disabling the retry that picks up
a service-account file added after startup. The lead's pre-verification
had checked sentinel collision, failure-path pop, and TOCTOU, but
missed this consumer of the OLD key shape.
Guard now tests the actual condition (`not resolved_path` — this
attempt was ADC) instead of a key literal, so it can't rot again if
the key shape changes. New regression test drives the full path: ADC
raises, SA file appears on re-resolution, retry succeeds — fails on
the tuple-comparison version AND on any future key-shape change that
breaks the guard.
Pattern-D fix (stale cache after out-of-band change): the Vertex
credentials cache was keyed on the service-account file PATH alone, so
rotating the file on disk (key revoked and re-issued, new identity)
kept serving tokens minted from the OLD Credentials object for the
life of the process. Operators rotate compromised keys precisely when
they most need the new identity to take effect.
The cache key is now the file's (path, mtime_ns, size) signature — the
established idiom (agent/skill_utils.py:414, hermes_cli/config.py:3343,
and the shape #89792 applies to model overrides). Rotation bumps the
signature, forcing one re-read; the superseded entry for the same path
is evicted on insert so the cache stays bounded at one Credentials per
file. ADC keeps a stable sentinel key ("__adc__",) and its existing
expiry/refresh handling; a stat failure degrades to the bare-path key,
i.e. exactly the pre-signature behavior.
Tests (tests/agent/test_vertex_adapter.py):
- rotation invalidates: rewrite + mtime bump -> new key, new
Credentials object, old entry evicted (fails on main: main serves
the first identity's object after rotation)
- stat failure falls back to bare-path key, never raises
- ADC sentinel stable across None/empty resolved paths
Suite 8/8 green; ruff green.
LiteLLM OpenAI->Anthropic translation copies tool-message content parts
verbatim, so the envelope-layout part-level cache_control landed at
tool_result.content[0] - a placement the Anthropic Messages schema rejects
with a non-retryable HTTP 400 that killed the whole turn (any tool-using
cron/session on a LiteLLM-fronted Anthropic route).
New envelope_tool_part_cache_markers_supported() predicate (keyed on the
existing _is_litellm_route token matcher) threads a tool_part_markers flag
through build_prompt_cache_plan / apply_anthropic_cache_control and all
four decoration sites (main loop x2, destination replan, MoA). On LiteLLM
routes role:tool messages carry no markers and the breakpoint budget
reallocates to the nearest eligible message; OpenRouter/Nous Portal keep
the part-level form they honor, native Anthropic layout unchanged.
Asserting an error string contains "read" proves the wording, not that a
model obeying it reaches a working patch. TestPatchRecoveryLoop walks the
loop through the public tool — half-formed patch, read the error, follow it,
patch succeeds — and pins the invariants that centralizing validation could
have broken: ordering ahead of the skill lookup, new_string='' still
deleting, missing new_string still rejected, and a rejected patch leaving
the file byte-identical.
The two recovery tests fail against the pre-fix dispatcher; the rest pass
either way as regression guards.
`skill_manage(action='patch')` rejected a missing `old_string` with:
old_string is required for 'patch'. Provide the text to find.
That is a dead end. The model cannot tell whether it omitted the argument or
supplied text that did not match, so it retries blindly — and then escapes to
the neighbour that always works: `action='write_file'`, which rewrites the
entire skill file and destroys unrelated content. `skill_manage`'s own action
enum puts that destructive path one token away from the failing one.
The error now names the recovery route: `old_string` must be the EXACT text
currently in the file, read the target first (the skill's SKILL.md, or the
file named by `file_path`), copy the snippet verbatim, and do not fall back
to `action='write_file'`.
Validation lives in `_patch_skill` rather than the dispatcher. `skill_manage()`
previously returned its own bare missing-argument error before ever calling
the helper, which would leave the new guidance unreachable through the public
tool. Removing that duplicate makes the helper the single source of truth; its
{"success": False, "error": ...} flows through the same json.dumps path, so
the serialized shape is unchanged, and validation still precedes the skill
lookup — a missing old_string on an unknown skill still reports the argument
error rather than "skill not found".
Fixes#33064
- Preserve streamed assistant text in Desktop UI when message.complete delivers empty text.
- Prevent destructive hydration in Desktop useMessageStream over rendered text on empty completion.
- Recover stream buffer in finalize_turn when final_response is empty on healthy turns.
- Unify in-place blank assistant repair, watermark clone resolution, non-blank concurrent winner adoption, and batch row appends into a single atomic guarded SessionDB transaction.
- Synchronize canonical committed content to live in-memory messages dicts and preserve all-or-nothing rollback semantics on persistence failure.
Squash of the three commits on PR #96768 (net diff is tests-only: the
mid-series production hunk in agent/transports/codex.py was reverted
within the PR after review). Pins the cache-scope isolation invariant
for hosts that mint one physical session per response, plus the
system-prompt write-path lifecycle under a per-response session (#96570).
Bedrock's cachePoint rules are per-model-family AND per-field. Amazon Nova
accepts a cachePoint block in `system` and `messages` but rejects it inside
`toolConfig.tools`, failing the whole request with
ValidationException: Malformed input request: #/toolConfig/tools/18:
extraneous key [cachePoint] is not permitted
so every tool-enabled Nova turn fails, with no retry path and no way for the
user to turn cache markers off (#97281).
The adapter decided placement from one static allowlist that answers only
"does this model cache at all", never "in which section". Any family whose
placement rules differ breaks 100% of turns until someone edits the table and
ships a release — the same maintenance trap the `_NON_TOOL_CALLING_PATTERNS`
comment already admits to ("if a model fails with a tool-related
ValidationException, add it here").
Make Bedrock's own verdict authoritative alongside the table: classify the
rejection by the JSON pointer AWS returns, drop the marker for that one
section, retry the request once, and remember the verdict for the rest of the
process so later turns are built clean. The other sections keep their cache
markers, so Nova still gets system/messages caching instead of losing prompt
caching wholesale. This mirrors the module's existing self-heal idiom
(`is_streaming_access_denied_error` → non-streaming `converse()`).
Applied at all four boto3 call sites: `call_converse`, `call_converse_stream`,
and both Bedrock dispatch sites in `chat_completion_helpers` (the streaming
one is the path in the report). A rejection with no marker to strip returns
None so the caller re-raises instead of looping.
Fixes#97281
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gcoy6nLTg5R6FHHhjcLZEC
Port from anomalyco/opencode#44571: OpenAI caps prompt_cache_key at 64
chars (DeepSeek and Zai inherit the same limit via their OpenAI-compatible
APIs) and rejects longer values with HTTP 400. The Responses transport
already bounds keys via _bounded_prompt_cache_key, but the Chat
Completions transport passed caller-supplied keys (request_overrides,
top-level or extra_body) through unmodified on both the profile and
legacy kwargs paths.
Bound caller keys with the same pck_<sha256[:24]> hash shape codex.py
uses so both transports behave identically; blank keys are dropped
instead of sent empty. Hermes-generated keys were already safe
(content-addressed pck_ hashes).
- Anthropic classifier counts signature_delta and citations_delta payloads
(content-bearing delta types the transport emits — relay_llm.py handles
both) so signed-thinking/cited-text generation keeps ticking the fence.
- Fix the stale 'per streamed event' comment above the Anthropic
on_stream_event lambda (left over from the conflict resolution).
- Rename test_completed_response_without_stream_payload_does_not_tick to
test_completed_response_ticks_only_terminal_signals — the old name
contradicted its own assertion (dispatch + shim ticks are expected).
Follow-up for the salvaged substantive-progress fix, adapted to main's
three-hook architecture:
- Keep the dispatch tick and the completed-response shim tick (both
deliberate on main; one-shot terminal events that cannot defeat an
inactivity timeout) — update the two PR assertions accordingly.
- Pin the end-to-end bug: keepalive/empty-role chunks must leave
CompressionCommitFence stale, a substantive token must refresh it.
- Pin the fast-lane telemetry contract (#96945/#96963):
time_to_first_progress_ms records on the first frame of any kind via
the split-out _notify_aux_timing_response helper.
Second follow-up for salvaged PR #94547, folding in review findings from the
duplicate-PR cluster (#47015, #55054, #58476, #72977, #73685 all fix the
same 401) and the sweeper review of #73685:
- Replace the dot-anchored suffix predicate with exact-match against the
existing _ALLOWED_TEAMS_SERVICE_HOSTS allowlist (two of the five
duplicate PRs converged on this independently). Any Azure customer can
register <name>.trafficmanager.net profiles, so suffix matching was not
safe. Also requires https on the default port — :444 on an allowlisted
host no longer receives the bearer (sweeper finding on #73685).
- Stream _fetch_attachment_bytes through _read_httpx_body_with_limit
instead of buffering response.content — the shared inbound media cap now
applies to authenticated downloads too (sweeper finding: a lying
Content-Length must not OOM the gateway).
- Serialize token refresh with a lazily-bound asyncio.Lock so concurrent
attachments share one STS POST (review finding on #94547).
- Token expiry now uses time.monotonic() (from #55054) — wall-clock jumps
can't extend a stale token.
- Tests updated: exact-allowlist predicate (lookalike/subdomain/port/scheme
negatives), streaming fake client, and a concurrent-cold-cache lock test.
Mutation-checked: suffix match, silent drop, no-lock, and unbounded
buffer each fail a test.
Follow-up for salvaged PR #94547 (Sibbern's Bot Framework attachment auth):
- The host check used bare endswith('trafficmanager.net') /
endswith('botframework.com'), which matched attacker lookalike hosts
(evil-trafficmanager.net) — sending the bot's bearer token off-platform.
Same threat model _ALLOWED_TEAMS_SERVICE_HOSTS already documents. Now a
single dot-anchored predicate, _is_botframework_attachment_host(),
used by both _fetch_attachment_bytes and the _on_message image branch
(was copy-pasted in two places).
- BF images whose bytes fail image validation were silently dropped with
no log (cache_media_bytes returns None; old path logged). Add the
missing else-warning, mirroring the document branch.
- Record cached_m.media_type instead of the raw content_type so the
MessageEvent MIME matches what was actually cached.
- Init _bf_token_cache in __init__ (was masked by getattr).
- Tests: host predicate dot-anchoring (attacker lookalikes blocked),
BF routing vs generic helper, bearer attach + attacker-host block end
to end, token acquire + cache reuse, token-failure degradation, and
the silent-drop regression guard. Mutation-checked: reverting the
dot-anchor, the else-warning, or the token cache each fails a test.
Reviewer findings from the formal /simplify-code pass, all verified:
1. Scanner blind spots (HIGH): five more locked pure readers were
invisible to the v1 gate — get_compression_fallback_streak and
get_compression_ineffective_count hid behind `conn = self._conn`
aliasing; list_gateway_sessions, find_session_by_origin and
search_sessions hid behind SQL held in variables/f-strings (the
scanner required unknown == 0 to flag). All five converted to
_read_ctx(); the scanner now (a) tracks self._conn aliases and
(b) flags lock blocks with NO proven write instead of silently
skipping unprovable SQL. Sabotage self-check extended to pin all
three detection classes (literal, alias, variable-SQL) plus a
mixed variable-SQL writer that must NOT fire.
2. get_meta reverted to the writer lock: its inline comment (present
on main) documents a real read-your-writes dependency —
fts_rebuild_step reads rebuild progress that a pooled WAL reader
cannot see mid-transaction. The blanket conversion had overridden
a documented design decision; it is now the single justified
_ALLOWED_LOCKED_READERS entry, replacing the dead
_enter_fts_fail_open entry (whose lock block counts 3 writes and
never needed allowlisting).
Strengthened scanner on pre-conversion main: 43 violations
(39 pure-read + 4 no-proven-write). This branch: zero.
Suites: gate 2/2; tests/test_hermes_state.py + tests/state/ 331
passed (same 3 pre-existing FTS-rebuild reds as clean main); 433
passed across the 12 consumer suites of the five newly-converted
methods (compression anti-thrash, session search, status, scheduler).
Pattern-C architectural fix (write-lock contention), completing what
#90734 started: that PR fixed the four UNLOCKED readers racing the
writer connection; this one fixes the 39 LOCKED pure readers convoying
every concurrent turn's persistence behind the global writer lock, and
adds the gate that stops the class from re-entering.
The gateway shares ONE SessionDB across every agent. A read-only query
under `with self._lock:` blocks all concurrent writers for its
duration; under WAL, _read_ctx() serves the same read from a pooled
read-only connection with no lock at all (non-WAL falls back to the
locked writer byte-for-byte, so DELETE-journal installs are unchanged).
Converted (SELECT-only bodies, mechanical `with self._lock:` →
`with self._read_ctx() as conn:`): gateway routing loaders, session/
message counters, titles, compression tip/lineage/cooldown readers,
telegram topic bindings, prune candidate scans, meta readers, resume
resolution — 39 methods, verified per-method that every statement is a
SELECT and every conn use stays inside the with-block. Read-modify-
write methods and checkpoint/maintenance PRAGMAs stay on the writer
lock (their read is ordered against their own write).
Gate: tests/state/test_no_locked_readers_gate.py — an AST scanner over
SessionDB that fails CI when a pure-read method body takes the writer
lock, with a sabotage self-check proving the scanner fires. On
pre-conversion main it reports 38 violations; on this branch, zero.
Measured (2 writer threads + 3 reader threads, 3s, WAL):
before: 78.5k reads, p99 2.49ms, max 160.7ms (readers convoy)
after: 200.4k reads, p99 0.53ms, max 25.5ms (2.6x throughput,
4.7x better p99, 6x better tail; writes unchanged)
Honest caveat: on runtimes where WAL is refused (the currently-bundled
SQLite 3.46 trips the WAL-reset-vulnerability gate → journal=DELETE),
_read_ctx() falls back to the identical locked-writer path and this
change is behavior-neutral by construction; the win applies to WAL
installs (legacy WAL DBs, fixed runtimes, wal-configured operators).
Suites: tests/test_hermes_state.py 243 passed; combined state sweep
540 passed — the only 3 reds are the pre-existing
test_fts_runtime_rebuild failures, verified failing on clean
origin/main before this change.
Reuse kitty Unicode placeholders plus after_render write_raw so
prompt_toolkit's screen-diff can host the same crisp sprite as the TUI.
Re-queue the transmit after Ctrl+L / resize so the image comes back.
Co-authored-by: Sam Foreman <saforem2@gmail.com>
POST /boards/{slug}/export and POST /boards/import, so the desktop and
dashboard can drive board transfer. Both exchange filesystem paths
rather than bytes, the same contract profile export/import uses and for
the same reason: the client runs its native save/open dialog on the
machine that hosts the backend, so a path is all either side needs, and
a board carrying a few hundred megabytes of attachments never has to
cross the renderer heap.
Also covers the pre-existing rename (PATCH) and delete endpoints, which
had no tests — including that delete archives to a restorable directory
and refuses to touch `default`.
`hermes kanban boards export|import` moves a board between machines:
tasks, comments, links, history, and attachments in one .tar.gz.
Two things make this more than a tar of the board directory. The
database is live — kanban runs in WAL mode, so a filesystem copy loses
whatever still sits in the -wal sidecar and tears if the dispatcher
commits mid-copy; export goes through SQLite's online-backup API
instead. And rows carry machine-local state: claims, worker PIDs,
absolute workspace and attachment paths, session ids, and the gateway
chat ids subscribed to task events. Shipping those verbatim is how an
imported board arrives holding a claim owned by a process on someone
else's laptop, or starts pushing task events into a stranger's Telegram
thread. Everything machine-local is stripped on export and re-stripped
on import, since an archive is untrusted input.
Imports always land as a NEW board, auto-suffixing the slug on
collision, so an import can never merge into or overwrite a board that
is already there. Tasks whose workspace was a directory or git worktree
on the source machine are parked in triage rather than left for the
dispatcher to claim and burn into the failure breaker.
* feat(a2a): outbound client tools are config-gated — served only when a2a_agents configured, inbound platform enabled, or A2A_PORT set (-561 tok/call on unconfigured installs)
* ci: retrigger after runner startup_failure on rerun attempt
The locked dependency tree now carries @babel/* 8.x, which requires
node ^22.18.0 || >=24.11.0. Our engines.node arm said ^24.0.0 and the
installer gates (node_satisfies_build / Test-NodeVersionOk) accepted any
Node 24 — so a system Node 24.0–24.10 cleared every gate we own and then
failed 'npm install' with EBADENGINE under engine-strict=true.
- Raise the 24 arm to ^24.11.0 in root + desktop package.json and the
package-lock.json mirrors
- Tighten node_satisfies_build (install.sh) and Test-NodeVersionOk
(install.ps1) to 24.11+; update user-facing wording
- Add invariant tests: every engines.node arm floor must satisfy every
locked dependency's engines.node, and the installer gates must encode
the same floors as the manifest — so the next babel-style floor bump
turns into a CI red instead of a user install outage
- docs: correct stale 'Node.js v22' provisioning claim
* feat(skill_manage): operations[] batch — several ops on one skill, atomic with rollback (memory-tool pattern); staged as ONE pending write under the approval gate
* refactor(skill_manage): operations[] IS the interface — single op = list of one (maintainer-directed); flat fields unadvertised handler compat; delete = sole-op routing
* guard(skill_manage): reject intra-batch same-file clobbers — double write/remove per path, full rewrite after an earlier SKILL.md edit; patch chains stay legal
* refactor(skill_manage): name-per-op — the call IS the operations array; cross-skill batches with all-touched-skills rollback
* guard(skill_manage): unify the intra-batch conflict guard — any destructive op on an already-touched file is rejected, with path normalization
Aggressive live testing found three holes in the two-part guard:
patch-then-write and patch-then-remove on the same supporting file
silently discarded the patch, and './references/x.md' //-style path
spellings slipped past the duplicate-write check. One rule now covers
the class: a destructive op (write_file/remove_file/full rewrite) on a
(skill, normalized-path) any earlier op touched is rejected pre-effect;
additive patches stay legal, so patch chains and write-then-patch still
work. Tests cover all three holes plus the pre-effect assertion.
`/handoff <platform>` never completes on a multiplexed gateway, and when it
does complete it can deliver through the wrong profile's bot. Three distinct
faults, all the same family: multi-profile code paths that assume a single
store / a single adapter map.
1. The watcher polls only the ROOT store.
`_handoff_watcher` resolves `self._session_db` with no profile scope, which
always yields the root `state.db`. But `/handoff` run under
`hermes -p <profile>` writes `handoff_state='pending'` into THAT profile's
store. Nothing ever reads it, so the CLI times out after 60s while the
gateway is alive and connected. The watcher now iterates
`[(None, None), *secondary_profiles]` and polls each inside
`_profile_runtime_scope`.
2. The destination session key is built without the profile namespace.
`_process_handoff` called `build_session_key()` with no `profile=`,
producing `agent:main:...` while that profile's own adapter routes organic
inbound messages on `agent:<profile>:...`. The handoff bound a key nobody
reads.
3. Delivery uses the PRIMARY profile's adapter and config.
`self.adapters` holds only the default profile's adapters (secondaries live
in `self._profile_adapters[name]`) and `self.config` only the default's home
channel. A secondary profile's handoff was therefore sent by the wrong bot,
to the wrong chat, while persisting the right session key and reporting
`handoff_state='completed'` — a false positive that looks correct in the
database and is wrong on the wire.
Two robustness fixes in the same path:
4. Head-of-line blocking between profiles. `_process_handoff` runs a full agent
turn plus delivery; awaiting it inline meant one slow handoff stopped the
watcher from even polling the other profiles. With the CLI's 60s deadline, a
valid handoff could time out purely because another profile's was ahead of
it. Dispatch is now fire-and-forget, with an in-flight guard so a row is
never claimed twice, and a bounded drain on shutdown.
5. Rows stranded in `running`. Only the watcher sets `running`, for the span of
one in-process dispatch, so a row still in that state at startup belongs to
a gateway that died mid-dispatch. It can never reach a terminal state, and
`request_handoff` refuses new requests unless the state is
NULL/completed/failed — that session could never hand off again, silently.
`reclaim_stale_running_handoffs()` now fails those rows once per store at
watcher startup. Failing (not re-queueing) is deliberate: the dead gateway
may already have switched the session key and dispatched, so a blind retry
risks double delivery.
Behaviour on single-profile installs is unchanged: the scope list degrades to
the unscoped root poll, `_resolve_profile_for_key` returns None when
multiplexing is off (byte-identical keys), and config/adapters fall back to
`self.config`/`self.adapters`.
Tests: 13 new across three files. Each was verified to FAIL against the
unpatched code (the fix was reverted and the suite re-run) so they are real
guards rather than decorative assertions. Verified end-to-end on a live
4-profile gateway: `handoff_state` goes failed -> completed, and a planted
stranded `running` row is reclaimed at startup with the reason recorded.
Review follow-up on #97052 (helix4u): a fork with no upstream remote whose HEAD matches origin/main used to print plain "Already up to date!" under --yes even though official main was never consulted, so an unattended stale fork looked current. _sync_with_upstream_if_needed now returns whether the official upstream was actually checked, and the commit_count == 0 completion line says "Up to date with your fork (official repo not checked)." when it was not. Skip-as-decline semantics are unchanged: no prompt, no remote mutation, no decline marker. Caller-level regression test added for the fork + no-upstream + --yes + HEAD==origin/main path; helper tests now pin the return contract.
_sync_with_upstream_if_needed called bare input() with no assume_yes parameter and no tty check, so a fork checkout without an upstream remote wedged hermes update forever in any non-interactive context (CI, cron, the desktop updater hand-off): stdin stays open, EOFError never fires. Thread assume_yes and the gateway input_fn into the helper and skip the prompt as a decline under assume_yes or a non-tty stdio pair, without writing the decline marker or touching git remotes, so interactive runs still get asked later. Both call sites forward the interaction state; the config-migration and stash-restore prompts already carry this gate.
Closes#60240 (prompt half). Supersedes #78678, #92448, #92410.
Co-authored-by: BlackishGreen33 <BlackishGreen33@users.noreply.github.com>
Co-authored-by: salch-cred <salch-cred@users.noreply.github.com>
Co-authored-by: jackulau <jackulau@users.noreply.github.com>
* refactor(read_file): capability-gate the anydoc format list; PDF coverage teaching lives in the response-time warning (426 -> 244/291 tok/call)
* feat(read_file): bundle firecrawl-anydoc 0.2.4 in core, typed NeedsOcrError handling, config-gated hosted OCR with local-OCR-first guidance
* refine(read_file): NEEDS-OCR warning hints at checking for an OCR skill without naming one; hosted_ocr knob unadvertised (maintainer-directed)
* simplify(read_file): drop the anydoc schema gate — bundled core dep makes absence a broken install, not a variant; formats stated unconditionally (263 tok/call)
* feat(read_file): PDF wording upgrades to 'scanned or text' when a trusted hosted-OCR route exists (direct key or explicit config; nous gateway excluded until Parse proxy works)
* simplify(read_file): FIRECRAWL_API_KEY is the ONLY hosted-OCR gate — nous gateway route removed (Parse proxy broken), config true no longer unlocks; false still disables
The usage anchor (#97206) now trusts provider-reported usage. These two
tests simulated a tool-heavy near-overflow turn while the shared fixture
reported a 12-token prompt — the anchored pressure check honestly
concluded no pressure. Give the scenario 18K anchored prompt tokens so
the tests pin the same compaction decision they always did.
Every provider response carries usage.prompt_tokens — exact ground truth
for the full request (system prompt + tool schemas + history). Context-size
checks now anchor on the last main-loop response's usage and estimate only
the messages appended since, instead of re-estimating the whole history
with chars/4 heuristics and flat 1500-token image costs. The estimate error
window shrinks from the entire conversation to one turn and self-corrects
at every response.
- agent/model_metadata.py: capture_usage_anchor() / anchored_context_tokens()
with a structural base-message identity check that fails closed on any
transcript rewrite.
- agent/conversation_loop.py: anchor captured at the single main-loop usage
site (MoA uses pre-fold aggregator usage; advisor/aux calls never anchor);
pre-API pressure check prefers the anchor.
- agent/turn_context.py: preflight compression estimate prefers the anchor.
- agent/context_breakdown.py: /context display prefers the anchor.
- Invalidation: compaction rewrite (conversation_compression), codex native
compaction (codex_runtime), session reset/switch (run_agent), plus the
fail-closed structural check for splices/micro-compaction.
- Usage-less responses keep the previous anchor; no anchor -> pure
estimation fallback (first request of a session).
Pass 2 of repair_message_sequence matched results only by id/call_id,
pruning calls answered through response_item_id or composite bridge
ids. Use the shared variant helpers (tool_call_id_variants /
tool_result_id_variants) so the unified alias policy applies
(#55626/#63000/#93251).
The positional sanitizer pass changes the crash/resume duplicate shape:
an interrupted first occurrence is now stubbed instead of deduped, so
the replayed call survives with its own immediate result. Update the
#64335 empty-key test to the new semantics and add regression tests for
the #94704 acceptance shape (historical-result + replayed-call +
fresh-call) and the production interrupted-turn shape (session
7d57a602b83d).
DeepSeek v4 rejects a payload where an assistant message carries a
tool_call whose tool result does not follow it immediately (HTTP 400
"An assistant message with 'tool_calls' must be followed by tool
messages responding to each 'tool_call_id'"). Context compression can
displace a tool result past a user turn; the result then lands ~100
messages away from its declaring assistant message.
Two gaps let the poisoned shape reach the wire (reproduced from the
production request dump of session 4d8727cbcf04, replayed through both
functions):
1. repair_message_sequence Pass 1 drops the displaced tool RESULT as
stray but leaves the declaring assistant message carrying the now
unanswered tool_call (with empty content) in the durable history.
2. sanitize_api_messages stubbed only globally-absent result ids: the
displaced result still exists in the transcript, so the id survives
the set-subtraction, no stub is injected, and the payload 400s.
Fix both layers so every path is order-independent:
- repair_message_sequence: new Pass 2 prunes tool_calls that have no
result in the immediately-following tool run (matching on id or
call_id, same superset rule as Pass 1). If pruning empties the turn
(no content/reasoning left), the whole message is dropped rather than
sending an empty assistant message. Codex interim turns are exempt,
as in Pass 0.
- sanitize_api_messages: the orphan/stub logic is rewritten as a single
rolling positional walk that drops results not immediately following
their declaring assistant (including results appearing BEFORE their
call) and injects stub results for positionally-uncovered calls even
when a mispositioned result exists elsewhere.
Adds six regression tests: repair pruning, whole-turn drop when pruned
calls were the only payload, valid-pair negative control, positional
stub injection, result-before-call orphan drop, and a fully-paired
transcript negative control.
Teknium