Follow-ups from sweeper review of #43763:
- tests/docker/test_tini_compat_shim.py asserts the dispatcher
ENTRYPOINT (with /init delegation check) instead of a bare /init
- tests/docker/test_smoke.py gains a docker run --init regression
for the non-PID-1 fallback (#38349)
- website/docs/user-guide/docker.md and the s6 supervision skill
document the dispatcher and its wrapped-runtime fallback
Replace the bare /init ENTRYPOINT with entrypoint-dispatch.sh: exec
/init + main-wrapper when the image owns PID 1, fall back to a direct
stage2 bootstrap (with the s6 helper PATH restored) on wrapped runtimes
where s6-overlay-suexec would abort with 'can only run as pid 1'
(Fly Machines, docker run --init, podman/FreeBSD setups).
Cherry-picked from PR #43763 by @konsisumer, conflicts with current
main resolved (tests/test_dockerfile_tini_compat_shim.py was moved to
tests/docker/, container_boot argv tests were reshaped upstream).
Fixes#38349
A local pin/unpin fires reconcile synchronously via the
$pinnedSessionIds listener — before any PATCH exists — so
pullRemotePins() read the still-stale server row and immediately
undid the user's action: !row.pinned && heldLocally reverted a fresh
pin, and row.pinned && !heldLocally re-pinned a fresh unpin, after
which the push pass saw nothing to write and no PATCH ever fired.
Fence local intent ahead of the pull:
- Run the push pass first, so pending/unconfirmed record the local
intent before the page is read, then pull.
- Skip the pull for ids still in `pending` (row not yet resolved) —
local intent awaiting its PATCH is newer than any loaded page.
- Update mirrored bookkeeping before mutating the pin store inside
the pull, so the re-entrant reconcile doesn't echo adopted state
back as redundant PATCHes.
Regression tests cover both directions (fresh pin over a stale
pinned=false row, fresh unpin over a stale pinned=true row) plus the
deferred-pin case where the row loads stale after the toggle. All
three fail against the previous reconcile order.
Fixes#74570
Extends the shared _GATEWAY_LIFECYCLE_PATTERN (used by BOTH the cron
creation-time guard in cron/lifecycle_guard.py and the terminal
execution-time hard-block in tools/terminal_tool.py) so Branch B covers
launchctl submit and bootstrap alongside kickstart/unload/load/stop/
restart, and normalizes POSIX shell line continuations before matching
so the exact multi-line reported shape in #62891 cannot slip past.
Also extends the execution-aware, label-independent detector
(contains_launchctl_submit_command, cherry-picked from #63272) to cover
launchctl bootstrap, since a neutral label like ai.hermes.svc-reload-tmp
defeats any label-anchored regex — the second production reproduction.
Regression tests cover both sites, including
`launchctl submit -l com.foo -- /path/gateway` and the bootstrap
variant, plus outside-gateway pass-through.
Branch B regex extension and continuation normalization drawn from
PR #62896; bootstrap coverage and test shapes drawn from PR #51003.
Co-authored-by: JackJin <1037461232@qq.com>
Co-authored-by: joelbrilliant <joelbrilliant1@gmail.com>
- Resolve guard cwd against get_session_cwd(session_key); fall back to env.cwd
when no session record exists yet, matching current main's per-session cwd
architecture.
- Make referenced-script reads backend-aware: local read first; if missing,
fall back to env.execute('cat ...') for SSH/Modal/Daytona backends.
- Reuse the recursive scanner in check_gateway_lifecycle so nested cron
wrapper scripts are caught, and resolve relative refs inside a script
against that script's directory.
- Add regression tests for remote-backend reads, two-session cwd, and nested
cron wrappers.
Verification: 80 passed tests/hermes_cli/test_gateway_restart_loop.py;
694 passed tests/cron; ruff + git diff --check clean.
A successful finalize edit can carry only the last streamed preview
snapshot: deltas generated between the last preview edit and stream
completion never reach any Bot API call, yet final_response_sent /
final_content_delivered were set from the call's success and suppressed
the gateway's normal final send — losing the tail permanently.
The stream consumer now records the exact cleaned payload of every
turn-final delivery (delivered_final_matches tri-state), and gateway/run.py
reconciles that record against the completed final_response before
trusting either suppression flag. On a demonstrable mismatch it edits the
streamed message up to the complete response, falling back to the normal
final send if the edit fails. Multi-message split deliveries and legacy
paths without a record keep the existing flag-trusting behavior, so
overflow splits and the failed-finalize handling (#51828/#33793) are
untouched.
Fixes#71643
CI's plugin-test slice runs without the discord optional extra; the raw
import failed with ModuleNotFoundError while every other test in the
file uses injected mock modules.
Under gateway.multiplex_profiles, Discord and Telegram authorization gates
(allowed/ignored channels, allowed users/roles, allow-all flags) were read
from process-global os.environ, populated first-writer-wins by the YAML->env
bridge in each adapter's _apply_yaml_config. The first profile to initialize
pinned its allow/deny lists — and its ALLOW_ALL flags — for every other
profile in the process (issue #72348, incl. the Telegram mirror reported in
the thread).
Fix (per-adapter-instance gate reads, whole class):
- gateway/authz_mixin.py: new _platform_gate_env — scope-authoritative gate
read: under an installed profile secret scope with multiplex active, a
missing key returns the default instead of falling through to os.environ
(which may hold another profile's value). Single-profile behavior is
byte-identical to os.getenv.
- Discord adapter:
- connect() snapshots all gate env vars (_GATE_ENV_KEYS) inside the owning
profile's runtime scope into a per-adapter dict; new accessors
(_get_allowed_channels/_get_ignored_channels/_get_allowed_users/
_get_allowed_roles/_get_no_thread_channels/_discord_allow_all_users/
_gateway_allow_all_users/_get_allow_bots) resolve snapshot -> config.extra
-> scope-aware env, replacing every raw os.getenv gate read: on_message
channel gates, _is_allowed_user allow-all flags, slash authorization,
fail-closed diagnostics, missed-message backfill, bot-message gating,
and _component_check_auth (component buttons).
- _apply_yaml_config always seeds gate values into PlatformConfig.extra
(incl. new allowed_roles / allow_all_users keys) and SKIPS the
process-global env writes when loading a profile-scoped config under
multiplex; the legacy first-writer env bridge is preserved verbatim for
single-profile deployments.
- _resolve_allowed_usernames no longer unconditionally rewrites
os.environ[DISCORD_ALLOWED_USERS] — under multiplex the resolved IDs stay
adapter-local (snapshot refresh); single-profile keeps the env rewrite.
- Telegram adapter (mirror of the same class): intake prefilter and
callback-auth fallbacks, _telegram_auth_env_configured, and the
allowed/ignored chats-topics-threads getters now read via the scoped gate
reader; _apply_yaml_config skips authorization env writes for
profile-scoped loads and seeds free_response_chats/ignored_threads extras.
Regression tests (tests/plugins/platforms/test_discord_gate_isolation.py):
two adapter instances with different allow-lists enforce their OWN lists
order-independently across message, slash, and component gates; negative
allow-all case proves profile A's open-access flag cannot authorize profile
B; username-resolution env-clobber; YAML-bridge seeding/skip matrix; and the
Telegram scoped-reader matrix. Sabotage-verified: reverting either the
Discord snapshot accessors or the Telegram scoped reader fails 12/2 tests
respectively.
Credit: builds on the per-adapter accessor direction of PR #72427
(@JonthanaHanh) and the scope-aware-reader approach validated live on v0.19.0
by @yournetworkplug-ctrl for the Telegram mirror; scope corrections from
jackjin1997's and cal88's analysis in the issue thread (allow-all flags,
unguarded username-resolution env write, per-site channel reads).
Fixes#72348
Builds on #72787's current_root guard (cherry-picked with authorship
preserved). Two further hardenings for #72776:
- require the settled cwd and the workspace to share the same common .git
dir (the shape 'git worktree add' produces), so a git workspace visiting
an UNRELATED repo is a browsing visit, not a re-home (repro'd by
Johnny-xuan in the issue thread);
- never reconcile over an explicitly chosen workspace (explicit_cwd),
while a settle-adopted cwd stays followable via a cwd_from_settle
marker cleared by _set_session_cwd / project switch.
Fixes#72776
`_reconcile_session_cwd_from_terminal()` treats a settled terminal cwd in a
different git working tree than the session's workspace as a relocation. But
when the session's workspace is not itself in a git repo, `_git_repo_root_for_cwd(current)`
is None, so the `landed == _git_repo_root_for_cwd(current)` guard never matches
and the FIRST git directory a tool call steps into hijacks the session: its
cwd/git_repo_root flip to that repo, later tools run from the wrong place, and
the repo's AGENTS.md gets injected into an unrelated conversation.
Only reconcile when both the current and settled cwds resolve to valid, differing
git roots. A non-git workspace visiting a git repo to read a file or run a command
is a browsing visit, not a re-home — matching the docstring's own intent that
`cd`-ing away must not re-home the chat. The intended "follow into a worktree"
behavior is unaffected: that path starts from a git checkout, so current_root is
valid there.
Adds a regression test covering a non-git workspace that touches a git repo.
Follow-up to the a11d0bdb01 dedupe of test_doctor.py. Two gaps in the
surviving copies:
- Nine tests stubbed get_nous_auth_status, but run_doctor calls
get_nous_auth_status_local (hermes_cli/doctor.py:1373-1380) — the
stubs weren't stubbing the called function. Point them at the local
variant, keeping the gemini OAuth stub where the newer copies had it.
- The catalog-alias parametrize lost its nvidia and moa cases in the
dedupe; restore them alongside ai-gateway.
54/54 pass (test_doctor.py + the shadow guard).
handOffWindowsBootstrapRecovery() writes the update marker
unconditionally, same as applyUpdates() before the previous commit.
It's reachable during boot whenever resolveHermesBackend reports
bootstrap-needed, which a relaunch mid-update can plausibly trigger
on Windows -- clobbering a live updater's marker through this second
path. Apply the same updateHandoffConflict() guard here: refuse to
spawn a second updater when one is already alive, and quit instead
so the live updater can finish and restart us.
writeUpdateMarker unconditionally overwrites HERMES_HOME/.hermes-update-in-progress
before every hand-off. If the user retries "Update" while a prior updater is
still alive and parked (e.g. waiting for the desktop to exit), the retry's
pre-write clobbers the still-running updater's claim, so the older updater is
no longer recorded as the owner even though it's actively mutating the
checkout. A second updater can then run concurrently over the same tree.
Add updateHandoffConflict() to check for a live foreign marker owner before
spawning a new updater, and refuse the hand-off (surfacing an "update already
running" message) instead of overwriting the marker.
Ref: #75778
Two amplifiers behind the 779K api.osv.dev DNS queries/16h report:
1. tools/osv_check.py: check_package_for_malware() hit OSV on EVERY
call. MCP reconnect ladders, stdio recycles, and parked-server
self-probes re-run the preflight for the same package on every spawn
attempt, so a flapping server became a sustained OSV query/DNS
stream. Verdicts (clean or blocked) are now cached for 1h
(OSV_CHECK_CACHE_TTL to tune); network failures stay uncached so
fail-open never masks a real advisory once connectivity returns.
2. hermes_cli/security_audit.py: cmd_security_audit() ran full
component discovery twice per audit (_count_components + run_audit).
Discovery now runs once via _discover_components() and run_audit()
accepts the pre-discovered list.
Both regression tests fail against the previous code (verified via
sabotage run).
The parallel tool-batch planner treated search_files as unconditionally
parallel-safe (_PARALLEL_SAFE_TOOLS) with no path reservation, so a
batch of patch(path=X) + search_files(path=dir(X)) landed in one
concurrent segment and the search could observe pre-mutation file
content — a same-block write->read stale-read race.
Fix the class, not the site: path-scoped reservations now carry a
reader/writer role.
- search_files joins _PATH_SCOPED_TOOLS as a READER, reserving its
search root (default '.', matching the tool's default) instead of
bypassing path checks entirely.
- Overlap only conflicts when a WRITER is on either side: a write into
a searched/read subtree splits segments (ordered behind the write),
while reader<->reader overlap — previously split needlessly — now
stays parallel (concurrent reads commute).
- write_file/patch keep their existing writer barrier semantics.
Prior art surveyed for this design: Codex CLI's RwLock read/write
barrier (readers share, writers exclusive), Claude Code's
isConcurrencySafe partitioning, and gemini-cli's contiguous
parallelizable batching — all converge on reader-shared/writer-
exclusive with contiguous-order preservation, which this planner
already had for read_file/write_file/patch; this closes the
search_files gap and adds the missing reader/reader concession.
Verified by sabotage run (tests fail against the old planner) and an
E2E script exercising the real planner + real file I/O.
Live staging (2026-08-01): relay semantic thread rename still declined
silently despite both #74482 and #75581 deployed — thread kept its
initial-words name, session title generated fine. Root cause is the
no-clobber guard string mismatch (see paired gateway-gateway PR): the
gateway can't reproduce the thread's initial name byte-for-byte, so the
connector's only_if_current_name check always failed.
- relay rename lane now passes prefer_connector_created=True instead of
the fragile initial-name string; the connector resolves the guard from
its own created-name memory. Native-marker lane keeps the legacy
only_if_current_name string (source carries the real initial name).
- rename_thread: prefer_connector_created param -> only_if_connector_created
on the wire, precedence over the legacy string.
- INFO logs at rename dispatch (thread/lane/new_title) and result
(applied=bool): the whole failure hunt needed telemetry the gateway
never emitted — this makes the outcome visible in fly logs.
Tests: connector-guard wire shape + precedence over legacy string; the
title-turn race test updated to assert the connector-owned guard. Relay
suite 149 passed; ruff + footguns clean.
Converged Phase 2 finding (two reviewers independently): _discard_staged
only ran when phase-1 staging failed. A phase-2 (commit) failure rolled the
live tree back correctly but orphaned staging copies for every not-yet-
swapped entry — up to most of a full tree. The retry's up-front free-space
check runs BEFORE the lazy per-entry leftover cleanup, so the litter makes
the retry fail 'not enough free disk space' on exactly the space-constrained
machines the 1.2x threshold was chosen for: the same 'retry fails harder'
failure mode _discard_staged's docstring says it exists to prevent.
Two tests: a behavioral one pinning rollback+discard leaves the old tree
intact with zero litter, and an AST wiring contract on _update_via_zip so a
refactor can't silently drop the cleanup. Mutation-verified: removing the
try/except around _commit_staged_replacements fails the wiring test.
Phase 2 review findings: (1) _commit_staged_replacements' docstring cited
os.replace while the code uses os.rename — the atomicity claim holds (same-
filesystem rename is atomic on POSIX and NTFS) but named the wrong function.
(2) venv_bin_dir's remaining hand-rolled site list missed agent/lsp/servers.py:270.
Phase 2 review HIGH (empirically reproduced): a hard kill between
os.rename(dst, backup) and os.rename(staging, dst) leaves dst missing and
the backup as the ONLY copy of that entry. On retry, _stage_replacement
deleted that backup as a 'leftover' BEFORE staging the fresh copy — so a
staging failure (disk exhaustion is likeliest exactly after writing a full
staging copy) left a hole in the install with nothing to roll back to.
Restore the backup to dst first when dst is missing; it's a same-filesystem
rename. Mutation-verified: removing the restore makes the new test fail.
CI slice 8/8 red:
test_verify_core_dependencies.py::test_uses_virtual_env_from_environment
AssertionError: assert None == PosixPath('.../newvenv/Scripts/python.exe')
The Phase 2 reviewer flagged this exact risk (W4) and I under-weighted it as
"latent, not broken". It was neither — it was already failing.
The suite exercises Windows-only paths on Linux CI by patching predicates
(`hermes_cli.main._is_windows`, `is_windows`, `platform.system`). Routing
those call sites through a helper that reads `sys.platform` unconditionally
meant the patches no longer reached the path derivation: the test built
`Scripts/python.exe` while the code looked for `bin/python`.
venv_bin_dir/venv_python_path now take an optional `windows=` verdict,
defaulting to the host. Every converted site passes its own predicate, so
the patched-predicate coverage is restored — the dedup keeps the layout in
one place without hijacking the platform decision.
Verified by causation: dropping `windows=` reproduces the CI failure exactly;
restoring it goes green. Added two regression tests, including one asserting
a patched `_is_windows` still reaches the derivation.
Phase 2 review findings on the first commit.
C1 (critical) — the two-phase replace covered directories only, so the 20
first-party modules at the repo root (run_agent.py, cli.py,
hermes_constants.py, model_tools.py, toolsets.py, ...) were still copied
one-at-a-time with shutil.copy2 straight onto live paths. A failure in that
loop left all directories new and the root modules stale: precisely the
ImportError shape this PR exists to prevent. Worse, copy2 truncates in place,
so a crash mid-copy could leave a half-written cli.py — strictly worse than
stale on the flaky-AV path this code runs on.
Stage files the same way as directories and swap them in the same commit
phase. The docstring's "wholly new or wholly old" is now actually true.
C2 (critical) — a phase-1 failure (disk exhaustion being the likely one)
orphaned one staging copy per entry already processed, up to a second copy
of the tree. The user then follows our "re-run hermes update" advice with
LESS free space and the retry fails harder. Added _discard_staged() on the
staging path. Verified: staging failure now leaves zero litter.
W1 — _stage_replacement duplicated _atomic_replace_dir's first half verbatim.
_atomic_replace_dir is now a 1-line shim over the two-phase helpers; its
#49145 regression test still passes.
W2 — the failure message still said "some directories were replaced and
others were not", which the fix makes false. Now says the install was left
in place.
W3 — the free-space gate demanded 2x the tree when only the staging copy is
new (the live tree already occupies its space; swaps are renames). Relaxed
to need * 1.2, so we stop blocking updates that would have succeeded on the
space-constrained machines most likely to hit this.
W5/W6 — the lint-style guard used `"if" in line`, which matches "modify" and
"verify" and still missed os.path.join(venv, "Scripts"). Rewritten as an AST
check; it immediately found the real offender the substring version missed
(stdio.py, now explicitly exempted — it lists literal Windows-only PATH
candidates, not a cross-platform derivation). Softened venv_bin_dir's
"single source of truth" claim, since sites outside hermes_cli/ remain.
S1 — the rollback loop now logs instead of silently swallowing OSError.
Both C1 and C2 fixes are mutation-verified: reverting either makes the new
tests fail.
Closes#76104, closes#76105.
#76104 — `_atomic_replace_dir` (#49145) made each individual directory swap
safe, but `_update_via_zip` replaced ~70 top-level entries in a loop with no
atomicity across iterations. `agent/` lands at os.listdir index 13 and
`tools/` at 66, so an interruption between them left the new
`agent/context_compressor.py` (module-level `from tools.todo_tool import
TODO_INJECTION_HEADER`) beside a stale `tools/todo_tool.py` — every file
valid Python, the tree unbootable. That is the mechanism behind the
ImportError fixed in #76091, and the "partial update" field report in #63717.
Split into stage-all-then-swap-all:
- `_stage_replacement` copies each dir to a sibling staging path, touching
nothing live, so a failure during the long copy phase is a no-op.
- `_commit_staged_replacements` performs the renames and, if any fails,
restores every entry already swapped — the tree lands wholly new or
wholly old, never mixed.
This shrinks the failure window from a full tree copy to N renames and makes
what remains recoverable. Added an up-front free-space check, since staging
needs a second copy of the tree; a clear error beats running out mid-swap.
#76105 — venv interpreter resolution was open-coded in 7 places across 4
files using 3 different Windows predicates. #76091 added the seventh because
the correct behaviour lived 2400 lines away. Hoisted `venv_bin_dir()` /
`venv_python_path()` into hermes_constants (import-safe, no new imports) and
routed every site through them; `managed_uv._venv_python` now delegates so
its 6 callers are untouched.
`_atomic_replace_dir` is retained — it is re-exported from main.py and has
its own #49145 regression test; removing it is out of scope here.
Tests: 10 new (rollback-on-mid-swap-failure is mutation-verified — it fails
when the rollback loop is removed), plus a guard that fails if a new call
site hand-rolls Scripts/bin again. E2E-verified against the real staging +
commit helpers with a live tree.
/simplify-code reuse reviewer (HIGH): the probe and the user-facing hint
each carried their own hand-written list of first-party package roots,
and they had already diverged on day one —
module probe hint
cli False True <- rollback with no explanation
hermesx True False <- third-party blamed on our updater
Hoist a single FIRST_PARTY_MODULE_ROOTS + is_first_party_module() into
hermes_constants (import-safe, no new imports) and have both consume it;
the probe gets the set injected into its source rather than re-typing it.
Also completes the roster — cron, utils, run_agent, model_tools,
toolsets, tui_gateway, acp_adapter were missing from both copies.
Verified by executing the real probe source against 19 module roots:
0 disagreements. Added a test that fails if either side grows a private
copy again.
Phase 2 review (C2) and /simplify-code findings.
C2 — the git path ran the import guard before `_clear_bytecode_cache`,
wired into the syntax guard's `git reset --hard` rollback. But
`cannot import name 'X'` is ALSO the documented signature of the
stale-bytecode class (#6207, #60242, see
_sweep_stale_bytecode_if_checkout_changed), which the very next steps —
and the launch-time sweep — already self-heal. A false positive there
would destroy a good update over a state that fixes itself.
Remove the guard from the rollback path entirely and re-add it at the
end of the git path, after bytecode sweep + dependency reinstall + lazy
refresh, as a WARNING only. By then every benign source of a transient
ImportError has run, and we never reset the user's checkout.
W6 — the headline regression test was vacuous: it patched
`hermes_main._UPDATE_CRITICAL_FILES`, but the syntax guard reads
`update_cmd`'s global, so the stub files were never examined and the
(True, None, None) came from "no files found" rather than "parses
clean". Patch the right module; mutation-checked (the test now fails
when the guard is disabled).
S5 — `startswith(("tools","agent","hermes","gateway"))` also matched
third-party `agents`/`agentops`/`toolsets`. Compare the first dotted
segment against an exact set instead.
S6 — hoist the per-line ChatConsole() instantiation.
Phase 2 review caught a false-rollback I introduced: on the git path the
import guard runs at the post-pull syntax check, which is BEFORE the
dependency sync. A release that adds a new third-party requirement would
fail the probe and trigger `git reset --hard` on a perfectly good update.
Rather than reorder the git path (the guard belongs with the rollback it
feeds), make the probe ignore a missing module that isn't ours. A missing
third-party package means deps aren't installed yet; a missing first-party
module means the update dropped a file, which IS the skew we're hunting.
This also makes the ZIP path's ordering non-load-bearing.
Verified: third-party absent -> (True, None, None); first-party absent ->
flagged; and the original TODO_INJECTION_HEADER skew is still caught.
Self-review against the sibling probe `_venv_core_imports_healthy`
surfaced this: that helper deliberately resolves the project venv's
python rather than using `sys.executable`, because `hermes update` may
be driven by a different interpreter than the install's own.
The new import guard had the same requirement and missed it. Probing
`sys.executable` would validate a tree the user never actually runs —
and that divergence is most likely on Windows, the exact platform this
guard was added for.
Falls back to the running interpreter when there is no venv (normal in
a dev checkout). Regression test asserts the venv python is chosen; it
fails when the fix is reverted.
A Windows user reported every startup dying with `ImportError: cannot
import name 'TODO_INJECTION_HEADER' from 'tools.todo_tool'`. The symbol
exists on main; their tree had the new `agent/context_compressor.py`
(which imports it at module level) alongside a pre-update
`tools/todo_tool.py`.
The post-update guard missed it. `_validate_critical_files_syntax` only
py_compiles files, and every file in a skewed tree parses fine — it is
the combination that is broken. The guard reported success and the
update completed over an install that could not start.
The ZIP-update path (Windows-only, used when git file I/O is broken)
is where the skew comes from: its copy loop replaces top-level entries
one at a time in `os.listdir` order, so `agent/` lands at index 13 and
`tools/` at index 66. Any failure between them leaves exactly this
mismatch — and that path had no post-copy validation or rollback at all.
- Add `_validate_critical_modules_import`: imports the four startup
modules in a subprocess (~0.4s) so cross-module breakage is caught.
Non-import errors (config/env) are ignored; a probe that cannot spawn
is non-fatal so we never block an update on our own tooling.
- Run it after the syntax guard on the git path, reusing the existing
auto-rollback.
- Run it on the ZIP path after dependency install (so a genuinely-new
requirement is not misreported as a partial copy), and make the ZIP
failure message state the install may be half-updated.
- Add `partial_update_hint()` and print it under "Failed to initialize
agent", so users see "re-run hermes update" instead of a bare
ImportError. Stays silent for ModuleNotFoundError and third-party
imports, which need different remediation.
Verified by simulating the exact skew: the syntax guard returns ok=True
while the import guard returns the user's error verbatim.
- Reuse telemetry['middle_window_tokens'] for the skip's middle estimate
(is-None fallback to a fresh estimate) so log and telemetry agree
- Declare prellm_skip_count in the base telemetry schema (fixed shape)
- Defer _derive_auto_focus_topic into the non-skip branch (user-turn scan
was wasted work on every skip)
- Document the skip in compress()'s Algorithm list and force: arg doc
- Drop dead call_llm patches from 7 tests (unreachable with
_generate_summary mocked)
When the middle section is < 10% of threshold tokens AND at least one prior
real-usage ineffectiveness strike has been recorded, skip the expensive LLM
summarization call and fall through to the deterministic message-dropping
path. Without this guard, a tool-heavy session where the protected tail
already holds most of the tokens can burn 500+ seconds on a summary call
that replaces a few lightweight messages with negligible token savings.
Key design decisions per GottZ review on PR #60451:
1. Separate _prellm_skip_count counter — never increments
_ineffective_compression_count (the strike counter that latches at >=2
to disable compression entirely). One real strike + one skip must NOT
permanently lock out compression until /new.
2. feasibility_skip sentinel flag — exempts skips from the abort branch
(abort_on_summary_failure / _last_summary_auth_failure /
_last_summary_network_failure). A stale failure flag from a prior
cycle must not turn a deliberate skip into a full abort.
3. reason=None for feasibility-skip fallbacks — a stale _last_summary_error
from an earlier real failure must not be embedded into the skip's
deterministic fallback marker.
4. info-level logging for feasibility-skip fallbacks (not warning) — this
is an intentional optimization, not a failure.
Skipped when force=True (manual /compress) so auth/error handling paths
are always exercised on explicit user request.
Adds 6 regression tests (TestPreLlmFeasibilityCheck) covering:
- Strike counter isolation
- Stale auth/network failure flag immunity
- force=True bypass
- No-skip when no prior strikes
- Counter reset on session reset
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: TRON <tron-agent@agentmail.to>
- Relax timing bound from 1.0s to 2.0s (CI machine robustness).
- Add test_long_dotted_run_with_keyword_completes_fast: includes a secret
keyword so the pre-gate does NOT skip _CFG_DOTTED_RE — directly exercises
the possessive-quantifier regex, not just the pre-gate.
- Add test_yaml_assign_redos_resistance: _YAML_ASSIGN_RE was modified but
had no ReDoS test — add 100-line stress input.
- Add test_yaml_assign_secret_still_redacted: verify YAML matching behavior
preserved with possessive quantifiers.
_CFG_DOTTED_RE's nested quantifier (?:[A-Za-z0-9_\-]+\.)+ backtracks
exponentially on long non-matching dotted runs (doubles every ~4
segments). Flatten it and use possessive quantifiers (py3.11+) in
_CFG_DOTTED_RE and _YAML_ASSIGN_RE wherever the successor is disjoint.
Zero behavior change: equivalence fuzz-verified over 120k structured
and random inputs comparing full sub() output including groups. Adds a
ReDoS regression test.
_call_fallback_candidate_sync replans messages/tools for each resolved
destination, but its async mirror still shipped the caller's decorated
sections verbatim — the primary destination's markers (including a
direct-native tool marker) leaked to fallback candidates with different
cache contracts, and the relay saw the display label instead of the
resolved provider/api_mode. Mirror the sync path: resolve the
destination, replan both sections, thread provider/api_mode through
_relay_async_completion, and replan again for the auth-refresh retry
client. Mutation-checked: the new parity test fails on the verbatim
pass-through shape.
Follow-up to #76032 (#20880).
The count walked every message part and tool schema on every request but
is consumed only by tests. Compute it on demand via a property instead.
Follow-up to #76032 (#20880).
Three copies of the same logic landed with #76032:
- MoA's _call_prepared_aggregator and auxiliary_client's
_replan_synchronous_cache_sections both implemented stub → policy →
strip → plan for a resolved destination. Extract
plan_cache_sections_for_destination() into agent_runtime_helpers (which
already owns the policy functions) and route both through it. Also
removes a redundant full-transcript deepcopy+strip per request (the
caller pre-stripped what build_prompt_cache_plan strips again).
- The fallback_chain[N] label regex + chain-entry lookup lived in
_fallback_entry_timeout AND _fallback_destination. Extract
_fallback_chain_entry() and reuse.
MoA's cache-plan failure log is promoted debug → warning: the call-block
site skips MoA, so this block is the aggregator's only decoration path —
a silent failure ships an undecorated request (the 0%-cache MoA bug class).
Behavior-preserving; 195 targeted tests green.
Follow-up to #76032 (#20880).
_apply_static_prefix_marker duplicated _apply_system_cache_markers' split
logic minus its empty-suffix guard: when the stored system prompt equals
the static prefix exactly, the tool-cache plan emitted a two-part split
with a trailing empty text block — HTTP 400 on native Anthropic. Fold the
tool-cache layout into the existing helper via mark_suffix /
fallback_to_whole flags; the empty-suffix case now marks the prompt as
one whole block. Behavior-parity verified against the merged planner for
every non-empty-suffix shape.
Follow-up to #76032 (#20880).
Review pass 2 (reuse reviewer HIGH): the step-3b probe-down fallback for
custom/local endpoints returns the same silent 256K default but only
logged at INFO - invisible by default, and it is the MORE common path
for small local models (the exact users the warning exists for).
Extract _warn_context_length_fallback() (deduped per model+base_url)
and call it from both fallback sites, per the fix-the-whole-bug-class
rule. Regression test drives the custom-endpoint path and fails without
the widening (mutation-checked).
ethernet