Follow-up to the salvaged #99912 rescue. The original helper left the
rescued orphan IN the adapter slot while the caller also swapped it in as
the current turn, so the post-turn _dequeue_pending_event ran the same
follow-up a second time (live repro: TURNS=['Sent','C','C','D']). The
helper now pops the oldest orphan and returns it to run as this turn,
stages the NEXT orphan in the slot so the drain continues the chain in
arrival order, and the call site parks the incoming message behind the
chain via _enqueue_fifo (slot when free, overflow otherwise) instead of
always appending to overflow. The rescued event's own source drives the
turn so reply anchors point at the message actually being answered.
Tests: contract updated for the new return type; added the 2-orphan chain
case and the single-orphan-then-new-message slot case (both fail against
the original helper shape).
Review note on #99912: rescued = 1 followed by if rescued: is a constant
conditional — the log block runs unconditionally now that staging is
single-orphan by design.
When a follow-up is demoted to /queue during compression-in-flight,
it lands in SessionState.conversation.queued_events (overflow) with
the slot event in adapter._pending_messages. After the slot's turn
completes, _promote_queued_event should move the overflow head into
the slot for the recursive drain. When that drain never runs — the
#99882 shape: busy window ended through an exit that skipped the
promotion site — the overflow is silently orphaned: never dispatched,
never persisted, never logged. A 170-char Telegram follow-up vanished
without a trace; its re-send also vanished for the same reason.
Fix: _rescue_orphaned_overflow stages one orphan into the empty slot
on the next idle arrival, and the new message is enqueued behind it
so FIFO order (#28503) holds — oldest orphan runs as this turn, the
rest drain in order, the new message last. The helper is best-effort
(slot occupied or no overflow → no-op) and logs at WARNING when it
fires so a future drain regression is visible.
Tests (tests/gateway/test_fifo_overflow_rescue.py, 4 cases on the real
GatewayRunner FIFO):
- moves overflow head to empty slot
- no-op when slot occupied
- no-op when no overflow
- FIFO preserved: orphan-1, orphan-2, new-msg in exact arrival order
Existing queue suites pass unchanged (test_queue_consumption — 5 passed).
Fixes#99882
Before turning hard stops on for unattended platforms, make sure they cannot
cut off normal work:
- Edit -> re-run is progress. A successful mutating call (write_file/patch,
a green terminal/execute_code, browser actions, job/message/cron/memory/
skill mutations) marks progress for every failing signature still being
counted this turn; the next identical retry restarts its streak instead
of accumulating toward exact_failure_block_after. A pure replay never
mutates anything between attempts, so it is still blocked at 5.
- Distinct red commands are diagnosis. For FAILURE_TOLERANT_TOOL_NAMES
(terminal, execute_code, process pollers, browser_navigate, web_extract)
same_tool_failure_halt_after warns but never halts.
- subagent and api_server keep the warn-only default: both are supervised
task loops with a live parent/client and do real edit -> re-run work.
Live A/B (real AIAgent platform=telegram, real patch+terminal, 8 rounds of
patch -> red check -> patch ...):
unmitigated branch: HALTED at round 6 (repeated_exact_failure_block)
this commit: COMPLETED all 8 rounds, final answer delivered
Loop shapes still stopped: identical failing read_file 8 calls,
identical successful terminal 5 calls (vs 602 on main).
Six new tests pin these flows; all fail on the unmitigated version.
Widen the salvaged #49189 hard-stop default so it covers the loop shape in
the #100849 debug bundle and #89069: a model replaying the same SUCCESSFUL
call (terminal, skill_view, memory) with a byte-identical result. The
per-turn idempotent_no_progress block only tracks IDEMPOTENT_TOOL_NAMES, so
those loops ran until the iteration budget (600 calls, ~40 min) with only a
notice appended.
- agent/tool_guardrails.py: observe_call's tool-agnostic consecutive-identical
streak raises a halt (identical_call_streak_halt) at
hard_stop_after.idempotent_no_progress when hard stops are active. Pollers
stay exempt; a changed result resets the streak; warning-only sessions are
unchanged.
- run_agent.py: surface that halt from _append_guardrail_observation like
every other guardrail halt (appends guidance, ends the turn).
- hermes_cli/config_defaults.py: declare non_interactive_hard_stop_enabled.
- docs: configuration.md describes the streak hard-stop.
- tests: streak halts terminal under hard_stop; never under soft mode,
for pollers, or when results change.
Live A/B (real AIAgent platform=telegram, mocked client replaying one call):
identical failing read_file main: 602 API calls, budget exhausted
branch: 8 calls, repeated_exact_failure_block
identical successful terminal main: 602 API calls, budget exhausted
branch: 5 calls, identical_call_streak_halt
Treat skill_view and skills_list as idempotent read-only tools so the existing no-progress guardrail can warn or block repeated identical skill loads. This prevents large skill outputs from being re-added to the context in tool loops.
Add regression coverage for repeated skill_view results under hard-stop guardrails.
BasePlatformAdapter._acquire_platform_lock emits `{scope}_lock` with
retryable=True on purpose (#54167): a MID-RUN reconnect must be able to
recover once the live holder exits or a stale record is cleared. The
startup router keyed solely off that flag, so a live foreign holder of the
bot token at zero-connected startup landed in `_failed_platforms` with
gateway_state=running — alive, deaf, and retry-storming the token every
backoff — instead of the exit-78 (EX_CONFIG / startup_failed) contract
that #51228 established for single-writer conflicts.
Minimal class fix, salvaged from #83183 (@alexgunsberg) against current
main:
- gateway/restart.py: `is_global_startup_conflict(error_code)` — matches
the `*_lock` / `lock_conflict` code families every adapter emits for
scoped-lock and identity conflicts. Code only, never message text.
- gateway/run.py primary startup routing: a lock-conflict failure is
routed as non-retryable (parked `fatal`, not queued). Nothing else
connected → exit 78; alongside a transient peer → NS-609 mixed mode,
gateway stays alive and only the peer retries.
- gateway/run.py `_schedule_secondary_profile_startup_reconnect`: the same
contract for multiplex secondaries — park `<profile>:<platform>` fatal
like `duplicate_credential` instead of scheduling a reconnect storm.
- Mid-run behavior is untouched: `_handle_adapter_fatal_error_impl` and
the reconnect watcher still treat `*_lock` as retryable (#54167).
Not carried over from #83183 (superseded on main or out of scope): the
`degraded` lifecycle write only fires on the all-retryable path and the
runner immediately overwrites it with `running` (so busy/drain already
see `running`); the secondary retry bridge landed separately in
96489f3c1b (#92064); Buzz/IRC/LINE lock-tuple unpack and the reconnect
ownership registry are separate class fixes.
Live repro (real GatewayRunner.start(), isolated HERMES_HOME + lock dir,
live holder subprocess owning the lock via production
acquire_scoped_lock): before — exit_code=None, gateway_state=running,
telegram `retrying`, queued in _failed_platforms; after — exit_code=78,
gateway_state=startup_failed, telegram `fatal`, _failed_platforms={}.
Co-authored-by: alexgunsberg <alex@gunsberg.fi>
Widen the salvaged guard from a hand-maintained four-package floor to the
class it stands for: every `dependencies` + `devDependencies` entry in the
desktop workspace manifest. Live probe on this box: a tree holding vite,
katex, electron and electron-builder but missing `@rolldown/plugin-babel`
still passed the floor-only guard, and `vite build` died loading
`vite.config.ts` after `prebuild` had already run. The floor stays as an
unconditional fallback for an unreadable manifest; optionalDependencies
are skipped because npm legitimately omits them (get-windows).
Five new vitest cases (12 total); the two class tests fail when the
manifest union is removed. Refs #86443.
Follow-up to the salvaged #87980: the test kept its own copy of the
build-critical package list (drift hazard) and the module's default
export had no consumer.
Refs #86443
assert-root-install.mjs exists to turn an incomplete root install into one
actionable line instead of a failure deep inside the build. It only ever
checked that vite resolved, so an install covering part of the workspace
graph passed the guard and died later on something else. That is the shape
reported in #86443: the updater's npm install brought in 521 of the 769
packages a full install gives, root node_modules had vite but not katex, and
the build failed on an unresolved katex/dist/katex.min.css with nothing
pointing at the install as the cause. apps/desktop/src/styles.css imports
that stylesheet, so katex is as load-bearing for the renderer bundle as vite
is, and electron / electron-builder are the same for packaging.
Check all four and name every missing one, so a partial install is reported
once and completely rather than one package per build attempt.
Resolution walks node_modules upward the way Node's own lookup does, rather
than going through require.resolve: a package whose exports map does not
expose ./package.json is not resolvable by path even when correctly
installed, and that must not read as missing. It also keeps a dependency
that landed in the app workspace instead of the hoisted root passing.
The guard now runs from prebuild, ahead of npm run clean, so a tree that
cannot build is rejected before the build deletes its own outputs. On this
checkout clean removes build/electron-types and the tsbuildinfo files, not
release/, so this ordering is not by itself what saves a packaged app; it is
the narrow correctness point that a doomed build should not destroy anything
first. build keeps its own call for anyone invoking the build steps directly,
and the check is pure filesystem lookups, so running it twice costs nothing.
The check is extracted as a pure checkRootInstall() returning {ok, error},
matching assert-dist-built.mjs, so it is unit testable without spawning a
process.
All seven TestLoopTickWitness cases that need real UNIX-domain sockets
(socket.AF_UNIX socket nodes or asyncio.start_unix_server producers)
fail on native Windows, where neither primitive exists. Mark exactly
those cases with a shared skipif so a Windows run reports SKIPPED
instead of erroring, while the platform-independent witness-absent
contracts (mocked probes, file-only heartbeats) keep running there.
Split the legacy two-witness-contract test in two: its stale-file arm
is file-only and keeps running on Windows; its dead-listener-node arm
needs a real socket node and is skipped with the rest.
Follow-up to @zoser69's #78111 cherry-pick:
- lift the redirect into _redirect_platform_display_key() and apply it
BEFORE _validate_config_key / type coercion, so the unknown-key hint and
the string-vs-bool coercion both see the canonical path
- widen to the sibling surfaces: config get resolves the canonical key
(previously echoed the dead top-level value — the misleading half of the
report) and config unset removes the canonical leaf
- regression tests: get mirrors gateway resolve_display_setting, unset
removes the redirected leaf, note printed, helper touches ONLY
OVERRIDEABLE_KEYS (connection keys / 4-segment / already-canonical
paths untouched)
- docs: configuration.md per-platform section names the canonical CLI
path and the accepted shorthand
Problem A of #71047: 'hermes config set platforms.telegram.streaming false'
wrote to a key the gateway never reads. The connection config
(gateway/config.py) reads only token/extra/overrides from the top-level
platforms.<name> block, while per-platform display settings (streaming,
show_reasoning, tool_progress, ...) are resolved from
display.platforms.<name>.<setting> (gateway/display_config.py).
Redirect a platforms.<name>.<setting> key to
display.platforms.<name>.<setting> only when <setting> is a known per-platform
display setting (gateway.display_config.OVERRIDEABLE_KEYS), leaving real
connection keys (token, extra, channel_overrides, ...) untouched. The
gateway.display_config import is lazy/try-guarded to avoid a circular import
and to keep the CLI working where gateway is not importable.
Adds tests/hermes_cli/test_config_set_platforms_redirect.py covering the
redirect, connection-key non-redirect, and the no-stray-top-level-platforms
case.
Follow-up for salvaged #88472 (+ #100055 / #99995 / #88998 intent):
- add_contributor.py compares filenames with str.casefold(), the same key
scripts/check-case-collisions.py uses repo-wide, so non-ASCII folds
(ß ~ ss) are caught the way macOS/Windows fold them.
- The KNOWN_CASE_CONFLICTS allowlist is gone: the historical
agent@Agents-Mac-mini.local pair was removed on main (fcdae2cf0b), so the
repo-wide test asserts zero collisions.
- New tests: same-login different-spelling is still refused (the filename
pair is the problem, not the login) and the exact spelling stays
idempotent; casefold vs lower coverage.
Co-authored-by: alfred-amanda <288490622+alfred-amanda@users.noreply.github.com>
contributors/emails/ uses the email as the FILENAME, so two mappings differing
only in case are the same file on Windows and on default macOS. The tree has
such a pair today:
contributors/emails/agent@Agents-Mac-mini.local -> skip-agent
contributors/emails/agent@agents-Mac-mini.local -> momomojo
git writes one and then reports the other as modified in a FRESH clone, forever.
The repo cannot be checked out clean on those platforms, which breaks any tool
that gates on a clean tree -- our own Windows Desktop rebuild refuses with
"fresh clone is NOT clean" and never gets to build.
add_contributor() now refuses a mapping that case-collides with an existing one,
for the same reason it already refuses a conflicting login: the tool exists so a
typo cannot silently reassign commits, and a collision does exactly that on half
the platforms it lands on.
Two tests: the guard, and a directory-wide check that no NEW collision appears.
The existing pair is pinned in KNOWN_CASE_CONFLICTS rather than resolved here --
the two files name DIFFERENT logins, so picking one reassigns a contributor
commit history, and that is a maintainer call. Please resolve it; the pin keeps
the breakage visible and stops it spreading meanwhile.
Verified: pytest tests/scripts/test_contributor_map.py -- 9 passed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two guards for the reconnect-queue eviction fix:
- a complete Matrix password config (homeserver + user_id + password)
counts as credentialed and stays retryable;
- every incomplete variant is still dropped, which is what keeps the
#64674 empty-primary multiplex eviction intact.
The incomplete cases pin the "read extra, not the environment" property,
and they set a fully-populated MATRIX_* environment explicitly to do it.
That setenv is load-bearing: tests/conftest.py sandboxes HERMES_HOME to a
tempdir and scrubs MATRIX_* from the environment, so production .env is
never loaded under pytest. Without the explicit setenv these cases pass
against an os.getenv-reading implementation and guard nothing.
Verified both directions against throwaway worktrees, live checkout
untouched:
- pre-fix implementation: the positive case fails (1 failed, 5 passed);
- os.getenv-fallback implementation: 4 of the 5 incomplete cases fail.
The "blank" case cannot discriminate by construction -- whitespace is
truthy, so `extra.get(k) or os.getenv(...)` never consults the
environment -- it guards strip()-emptiness instead.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
_platform_has_bot_credential() decides whether a failed platform may be
retried. It only inspected PlatformConfig.token / .api_key, but Matrix
supports password login (MATRIX_USER_ID + MATRIX_PASSWORD, no
MATRIX_ACCESS_TOKEN), and build_config() puts those on extra{} rather
than .token.
So a password-auth Matrix config read as credential-less, and the
reconnect watcher deleted it from the retry queue on the first transient
failure. A momentary DNS failure at boot therefore took Matrix down
permanently: the homeserver was healthy, but nothing ever retried and
recovery required a manual gateway restart. Observed live as a ~13h
outage after a boot-time "Temporary failure in name resolution".
Mirror the adapter's own gate (homeserver + user_id + password).
Read ONLY from extra, never os.getenv: build_config() already copies all
three env vars onto extra, and importing this module loads ~/.hermes/.env,
so an env fallback would report "has credential" for every Matrix config
on the host -- including the empty-primary multiplex case (#64674) that
this check exists to evict.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Follow-up to the #100829 salvage. uv pip install writes no __pycache__ by
default (pip does), so --compile-bytecode covers the whole install including
transitive deps, which the per-spec warm never sees. Also skip *.dist-info /
*.egg-info roots in _installed_dist_roots — they own no importable code.
Live: fresh cpython-3.12.13 venv, real uv install of anthropic==0.87.0 via
_venv_pip_install: main -> 0 pyc, first import 0.468s; after -> 1212 pyc
(546 anthropic), first import 0.205s.
Refs #100461
A pip/uv install writes .py sources and no __pycache__ — and reinstalling
the same version still deletes the cache the previous copy had. Nothing in
Hermes compiles them, so the whole compile is paid by whoever imports the
package next. For a lazily installed backend that is the foreground of a
user request, with nothing printed while it runs.
Measured for anthropic==0.87.0 (541 modules) on cpython-3.12.13: the first
import after an install costs 2.2-2.7s against 0.7-1.0s warm, and 10.5s
under concurrent load. N per-profile daemons cold-starting together each
pay it in full, because none of them has written the cache yet.
Compile the freshly installed distributions in _venv_pip_install instead,
on the success path of both the uv and pip tiers. The caller is already
waiting on an installer there and can see why. Package directories are
resolved from each distribution's own file list, so specs whose import
name differs from their package name (python-telegram-bot -> telegram)
are covered. Best-effort: a compile failure never invalidates an install
that succeeded, and sys.dont_write_bytecode is honored.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MhAnkrFktFdmZwf64fUYLE
- test_shutdown_watchdog: the non-POSIX arm now asserts the witness ARMS
over TCP (port published, no AF_UNIX call, no warning, no socket node)
instead of pinning the old witness-absent fail-safe.
- test_update_wedged_gateway: TestLoopTickTcpWitness exercises the
consumer probe against a real loopback listener — stale file + answering
witness stays ALIVE (#90502 shape), stale + silent is WEDGED, fresh +
silent is UNKNOWN, garbage port never counts as armed. Runs on the
Linux lane so the TCP path is not Windows-CI-only.
`asyncio.start_unix_server` does not exist on Windows (no AF_UNIX event-loop
support in asyncio), so arming the loop-tick witness in
`loop_heartbeat_forever` raised AttributeError on every native-Windows
gateway start. The broad except swallowed it and recorded
`loop_tick_socket=False`, so every stale-heartbeat probe classified the
gateway as UNKNOWN — never WEDGED, never ALIVE-with-stalled-write. The
two-witness interlock from a1c83ef9 (issue #90502 follow-up) has been
effectively disabled on Windows since it landed: a wedged native-Windows
gateway could never be detected, and an alive one could never be
distinguished from a stalled heartbeat write.
On non-POSIX platforms the witness now arms over a TCP loopback server on
127.0.0.1 (OS-assigned dynamic port) instead:
- same protocol — connect, read one byte "1"
- same semantics — pure in-memory, zero disk I/O, answered only while the
loop is dispatching, armed by the loop task itself (an awaited
`asyncio.start_server` is structurally loop-owned exactly like the Unix
variant, so a wedged loop cannot keep answering pings)
- the assigned port is published in the heartbeat payload as
`loop_tick_tcp_port`, and `probe_gateway_loop_liveness` prefers the TCP
witness when the producer published a port, falling back to the AF_UNIX
socket for POSIX/legacy producers
POSIX behavior is unchanged: the AF_UNIX arm (including the stale-node
sweep) stays gated behind `os.name == "posix"` so the missing attribute can
never raise on Windows again. Legacy heartbeats without `loop_tick_tcp_port`
keep the existing socket-node contract untouched.
Tested end-to-end on native Windows: witness arms, port is published,
`_probe_loop_tick_tcp` answers from an external thread while the loop
dispatches, and the existing loop-liveness suite passes unchanged (the
AF_UNIX structural test still passes — the Unix arm text is preserved
inside the POSIX branch).
Adds two tests pinning the new behavior: an E2E test that arms the TCP
witness and probes it (skipped on POSIX, where the Unix arm is the real
witness), and a structural test that the TCP arm stays awaited on the loop
task and the AF_UNIX arm stays POSIX-gated.
- ambientRequestFor(gateway): one adapter for the six copy-pasted
`<T>(method, params) => gateway.request(method, params ?? {})` lambdas
the pollers built to call requestForOwnedSession.
- resetBackgroundPollingGuard() with no argument (primary reconnect,
use-gateway-boot.ts) now also clears healsByStoredId. The latch and the
heal budget share a lifetime: a respawned backend re-mints every runtime
id, so a stored session that had exhausted its 3 heals must be healable
again. Previously only the latch was cleared there.
- Leaf module comment states the actual import convention.
The approvals loop and broadcast_session_info fan out UNSCOPED session.info
frames for every live session. The event router attributes an unscoped
frame to the active session, and approvalReplaySessionId then re-pulls
approval.pending for it. When the active runtime is already gone (4001),
that made every fan-out tick a fresh dead-id request — the dominant source
of the 1,369 post-restart approval.pending rejections in #100639.
approvalReplaySessionId now takes the frame's explicitness and a gone
predicate and returns null for an unscoped replay onto a latched runtime.
An explicitly scoped frame is the runtime speaking for itself and is never
skipped.
Refs #100639
The salvaged #95647 commits predate runtime-gone.ts and shipped their own
gone-latch (session-rpc-guard.ts) beside the one main already had. Fold
them: one latch, one classifier, one clear seam.
- session-gone-latch.ts is a dependency-free leaf holding the latch, the
4001 classifier (now also rejecting "mentions session not found" tool
strings and unwrapping IPC bridge prefixes), and the rebind seam. It
exists because session-request-router — imported by every store — must
clear the latch after a successful session.resume/activate without
pulling the session/tile stores into its import graph.
- runtime-gone.ts re-exports the leaf and keeps the heal logic.
- A successful rebind now also refunds the stored session's heal budget.
markRuntimeGone caps consecutive heals at 3 per stored id and only a
successful process.list refunded it, so a backend that reaps a detached
runtime a few times left the view stuck on a phantom id with every
poller latched and the socket-reconnect global clear (removed by the
salvaged commit) re-arming the storm. #100639: 1,230 approval.pending
4001s on one runtime id in 42 minutes, zero recovery.
Refs #100639
Follow-up to the salvaged #100350 commits: replace the per-table
'if table == "delivery_obligations"' branches in session_recovery.py and
session_lost_and_found.py with a single _AUXILIARY_TABLE_SCHEMAS registry
(table -> destination DDL initializer) that both the SQL-level and the
lost_and_found lanes consume, so the next lazily-created state.db table is
one entry, not three code paths. The .recover lane now iterates
_CANONICAL_TABLES + _AUXILIARY_TABLES instead of a duplicated literal list.
Tests: the .recover direct-copy lane creates the missing ledger on the
destination; a source-vs-destination obligation count mismatch fails
verification (complete=False) instead of reporting a clean salvage.
Docs: state.db table inventory lists delivery_obligations.
Addresses #100313
The lazy gateway outbox was missing from the recovery inventory, so a
verified salvage could drop owed replies even when the rows were still
readable. Initialize the destination schema and copy the table.
Drives a real unopenable lock path — a directory where the code expects a
regular file, so open() raises a genuine kernel OSError — rather than
monkeypatching the helpers, standing in for the ENOSPC/EMFILE the field reports
hit without needing to fill a disk.
Both authorities are covered at the primitive and the behavior level:
fts_rebuild_admission refuses admission and rebuild_fts() reports no progress
(asserted against a preceding successful rebuild, so the 0 is the deferral and
not an unrelated no-op); _cross_process_repair_lock refuses the authority and
repair_state_db_schema runs no writable_schema surgery, takes no forensic
backup, and leaves the damaged image byte-identical for the next authorised
pass. A guardrail test pins that a pathless in-memory store is still admitted,
so the fix cannot turn that legitimate no-op into a permanent deferral.
All four deferral assertions fail on the pre-fix code, where the repair test
shows the surgery really did proceed without cross-process authority.
state.db has two cross-process admission authorities gating destructive work
on a file several Hermes processes share: fts_rebuild_admission for full
structural FTS rebuilds, and _cross_process_repair_lock for writable_schema
surgery / VACUUM. Both document themselves as fail-closed, and both honoured
that only for a timed-out acquire. When the lock file could not be open()ed at
all they yielded True and proceeded "with in-process serialisation only" —
which is no cross-process authority whatsoever.
That inversion is reachable exactly when it does the most damage. Creating the
lock file needs a directory entry and an inode, so on a full disk open() raises
ENOSPC — while a sibling that opened ITS handle before the disk filled is still
mid-rebuild or mid-surgery. Every process then ran concurrent destructive work
on the same live DB: precisely the interleaving PR #93200 added these locks to
prevent, and the shape reported in #100368 (disk-full trigger, then a fresh
corruption on every boot with other writers alive, and no re-corruption on a
boot with zero other writers).
Both helpers now yield False on OSError. This routes the error into the
outcome the locks already define and every caller already handles: rebuild_fts
returns 0, _recover_stale_fts leaves canonical writes plus LIKE search
available behind the retryable stale breadcrumb, the startup path detaches FTS
triggers, and repair_state_db_schema re-probes and reports. Nothing reachable
is lost — on a read-only directory the rebuild's and the repair's own writes
could not have committed either. The repair report's error string now names
both ways the authority can be missing, since operators read it directly.
Review point from keeltrace, and it is a real gap: secret redaction and
prompt omission are different contracts, and only the first one is
pattern-shaped.
redact_sensitive_text removes credentials. A provider 4xx that quotes the
request back carries the user's own prose - a paragraph about a person, a
file pulled in by an @ reference - which matches no credential pattern and
so passed through untouched into cause=. The record's stated contract is
that prompt content is not logged, and the previous commit only enforced
the half of it that a regex can see. The existing prompt test could not
catch this: its provider error does not echo the prompt, so it proves the
prompt is not logged directly, not that it cannot arrive by being quoted.
_strip_prompt_echo closes the quoted path directly. Anything the message
shares with the submitted prompt for 24 characters or more becomes
<prompt>. Shingle-set matching rather than a diff, so cost is linear in
both strings on a path that runs for every failed turn and can face an
@-expanded prompt of arbitrary size; the JSON-escaped form of the prompt is
shingled too, because a provider handing back its own request body often
hands it back escaped. The prompt is captured after @-expansion on purpose:
an injected file's contents are exactly the material an echo would carry,
and they are not in the submitted text.
Ordering is load-bearing. The strip runs after the whitespace collapse, so
a re-wrapped quote still matches, and before the length cap, so a quote
cannot survive by being cut mid-run.
What this does not claim: verbatim echo is what it stops. A paraphrase, a
summary, or a re-encoding would survive it. The alternative keeltrace
raised - log only structured provider metadata and drop the message body -
is airtight but costs the diagnosis this PR exists to enable, since the
reporter needed to tell a 402 from a crashed finalizer. Happy to switch if
maintainers prefer the stricter contract.
Tests: the non-secret sentinel keeltrace asked for (a benign phrase present
only in the prompt, echoed by the provider error, asserted absent from the
record), plus guards that a message sharing nothing with the prompt is
untouched, that an overlap below the window is not treated as an echo, that
a prompt shorter than the window cannot blank the message, that a
JSON-escaped echo is stripped, that the strip precedes the length cap, and
that whitespace shape does not hide an echo. The three that cover the new
path fail with the strip removed; the guards pass either way.
Fixes#89117
The whole of #89117 is two log lines:
tui_turn finished: ui_session=0dfcee58 status=error error_retained=True duration=0.9s
A provider 4xx, a budget wall, a billing block and a crashed finalizer all
produce exactly those characters, so an intermittent failure cannot be
triaged from the one record that is guaranteed to exist.
The bookend came from #86865, which added it to trace compression
rotations across #86647 -- identities and a coarse status were the job, and
content was deliberately excluded. What that leaves is a returned-error
path (provider 4xx, budget, billing) which writes no other log line at all.
The exception path at least prints `[gateway-turn] <Type>: <msg>` to
stderr, so the failures that go unlogged are exactly the sub-second ones
this issue is about.
Both failure paths now stash a one-line cause, and the bookend appends it.
The record keeps its shape when nothing failed: a successful turn gains no
new fields.
The cause is redacted with `redact_sensitive_text(force=True)` and capped at
240 characters with a visible ellipsis, because a 4xx body routinely quotes
the request that produced it -- adding the cause without redacting it would
write an Authorization header the user never chose to log. Redaction fails
closed: if the redactor cannot run, the fragment reads `<unredactable>`
rather than the raw message. Whitespace is collapsed so a multi-line
provider body cannot split the record, which is the only property that
makes it greppable for a bug like this one.
12 regression tests. Four mutations proven: disabling the helper fails 9,
dropping redaction fails 2, dropping truncation fails 1, wiring only the
exception path fails 4.
A stdio server that never answers the optional ping (no -32601, no
response at all) produced a bare TimeoutError that _keepalive_probe
classified as a dead transport, tearing down and respawning a healthy
subprocess on every keepalive tick. On a first ping timeout, confirm with
list_tools before declaring death; if it answers, latch _ping_unsupported
and use list_tools from then on. If both fail, propagate as before.
The 10s hygiene_max_turn_hold_seconds budget (#92318) releases the arriving
user turn while the summary model is still streaming. For thinking summary
models (DeepSeek-V4-Flash etc.) whose reasoning prefix alone exceeds 10s,
the abandonment path ALWAYS cancelled the commit fence — 100% of the summary
attempt (including the full thinking prefix) was discarded on every turn,
permanently disabling auto-compression while paying the summary model 10s
of thinking per turn, and the flat 60s retry-after then blocked the
agent-side preflight from a fresh chance.
Structural fix (maintainer-chosen direction in #97963): decouple the turn
from the compression instead of holding the turn longer or making the hold
progress-aware (which would reintroduce the #90845 frozen-turn bug):
- CompressionCommitFence gains mark_commit_watermark_fenced() /
commit_watermark_fenced; compress_context marks the fence right after
capturing get_active_message_watermark() under the durable compression
lock (#75316/#87484) — the property that makes a LATE commit safe: rows
appended after compression start survive both commit paths verbatim as
cloned concurrent tail (archive_and_compact watermark= and
publish_compression_child watermark/watermark_ceiling).
- gateway hygiene turn-hold handler: when the fence is watermark-fenced,
the detached worker (already kept alive via
_defer_agent_cleanup_until_future_done) KEEPS its commit admission; the
user's turn proceeds on the uncompressed transcript at the same 10s
budget, and the summary is adopted at the worker's own watermark-fenced
commit boundary. Unfenced workers are cancelled exactly as before —
never worse than the status quo.
- No retry-after is armed while the kept-admission attempt runs (it would
block preflight adoption via the same-session cooldown); re-attempt
spacing is covered by the durable compression lock
(_session_has_compression_in_flight). If the worker ends WITHOUT
committing, a done-callback restores the flat non-escalating 60s
retry-after; a successful adoption resets the hygiene failure streak.
The streak never advances for a deferral either way.
- Docs: configuration.md hygiene_max_turn_hold_seconds one-liner updated
to describe deferred adoption and the thinking-model case;
config_defaults.py comment updated. Knob stays config.yaml-only.
Invariants preserved:
- 10s user-latency cap stays hard (#90845/#92318):
test_session_hygiene_turn_hold_budget_abandons_streaming_wait passes
UNMODIFIED (its worker is not watermark-fenced, so it pins the cancel
path through the public surface).
- Stale-clobber impossible: adoption only rides commits bounded by the
start watermark; the fence still gates admission and unfenced/late
results are discarded.
New regression tests (tests/gateway/test_session_hygiene_turnhold_adoption.py):
- watermark-fenced worker keeps admission, late summary is committed,
turn still released at the budget, no cooldown while running,
streak reset on adoption;
- kept-admission worker that ends without committing restores the flat
turn-hold retry-after (<=120s, names turn-hold, streak untouched);
- unfenced worker still cancelled and discarded (status quo).
Sabotage-verified: disabling the keep-admission branch fails the two new
adoption tests and leaves the unfenced-cancel test green.
Fixes#97963
PR #99779 gave the streamed chat.completions consumer the host's absolute
compression deadline. The two wires that consume their streams internally
still ran on their own, always-larger budgets after the host gave up:
- Codex Responses: clamp the re-armable watchdog's hard ceiling to the
published host deadline, so a live (re-arming) stream is severed the
instant the host stops waiting instead of at max(600s, 4x timeout).
- Anthropic Messages: the per-event hook now raises at the host deadline
and on an explicit hard cancel; create_anthropic_message lets that
TimeoutError abandon the stream (the with-block closes it) instead of
swallowing it as a callback failure.
Sabotage-verified: without the Codex clamp the new deadline test hangs past
its 25s harness cutoff; without the Anthropic hook the three Anthropic tests
fail.
CompressionCommitFence.set_total_ceiling_seconds documents its deadline as
"shared by the host and worker", but only the host ever read it. The worker's
streamed summary bounds itself with _aux_stream_total_ceiling() instead —
max(600, 4 * aux_timeout) — which is >= the host's total ceiling for every
configured timeout AND starts counting later (after pool admission,
_serialize_for_summary, prompt build and TTFT). A stream that outlives its
abandoned host is therefore not an edge case; it is the guaranteed outcome of
every total-ceiling timeout.
8207862212 closed the first half: a cancelled fence now releases the
compression owner, freeing its pool slot and session lease. Its own comment
leaves the second half open — the isolated provider daemon that holds the
socket keeps streaming "until the auxiliary stream's longer absolute ceiling
expires". With the #99692 reporter's auxiliary.compression.timeout: 600 that
is 2400s of an orphaned ~500K-token summary the fence is already guaranteed to
refuse, and because the session never shrank, every following turn stacks a
fresh orphan on top of the last.
Publish the fence's deadline as an absolute monotonic instant
(CompressionCommitFence.deadline_monotonic) and give the auxiliary layer the
return leg it was missing: aux_stream_deadline() installs it thread-locally,
_ChatStreamAccumulator.feed() stops the stream once it passes, and
_run_protected_sync_provider_call propagates it onto the provider daemon
(thread-locals do not cross that boundary, so an owner-thread-only install
would be inert on exactly the path large-session compression takes).
Absolute, not relative: the deadline is unaffected by however long dispatch and
TTFT took before the accumulator was constructed. Checked as well as — not
instead of — the existing ceiling, so every caller without a host deadline is
byte-for-byte unchanged, and the "timed out" phrasing keeps _is_timeout_error
classification identical to a request timeout.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EKrRS7LVgyHf2WQkEahSwu
The generic tool_call(name, arguments: object) bridge hides a deferred tool's
real parameter schema from provider-native validation. Before this, only
top-level required-key absence was checked, so invalid enums, wrong types,
nested required fields and forbidden extra properties reached the handler
or MCP server. Now the call is coerced (same coerce_tool_args path normal
dispatch uses) and validated with the schema's declared JSON Schema draft;
failures return the path, constraint and parameters schema so the model
repairs the call in one round-trip. Fails open on missing/malformed schemas,
external $ref, or missing jsonschema.
Fixes#73175
Salvaged from #73179 onto current main (post core-tool deferral #97979).
Co-authored-by: teknium1 <teknium@nousresearch.com>
Teknium