Phase 2 review findings on the salvage branch:
C1 (critical): batch and micro summary markers share
COMPRESSED_SUMMARY_METADATA_KEY, and compress() never reset micro state.
After micro absorbed exchanges 1..k, a batch compaction summarizing
1..m (m>k) could fire; the next micro pass's supersede then dropped the
batch marker (whose content the stale rolling summary does NOT contain)
and archive_and_compact immediately made the loss durable. Defrag had
the same hazard: it rewrote "the newest marker" even if that was a
batch marker. Empirically confirmed with a probe (batch marker content
destroyed in one pass).
Fix, three parts:
- Micro-created markers now carry MICRO_COMPACT_MARKER_KEY; supersede
and defrag only ever touch micro-tagged markers. Rehydration in
_resolve_compact_cursor tags the marker it absorbs (containment
proof), which safely covers adopting a batch marker as the new
rolling base after a reset.
- compress() success path resets micro rolling summary/cursor state so
a stale summary can never claim cumulativeness over a batch marker.
- Regression tests for both directions plus the reset.
W4: _splice_micro_compact_result no longer strips _db_persisted stamps
from surviving messages. Micro archives in place under the SAME session
id (unlike batch's child-session rotation, #57491), so surviving stamps
are accurate; stripping them meant an archive_and_compact failure left
every previously-persisted message unstamped and the next append-only
flush re-inserted them all as duplicate active rows.
W5: finalize_turn micro gate now checks agent._persist_disabled —
persistence-isolated fork agents (background review) must not burn an
aux call per review turn, and must never archive_and_compact the
canonical session rows if their compressor ever gains a DB binding.
W1: _serialize_one_exchange now delegates to _serialize_for_summary
(was a ~70-line near-verbatim copy; one serializer, one place to fix).
S4: _find_one_exchange boundary guard rejects only assistant/tool
boundaries (the actual alternation hazard) instead of requiring user —
a stray mid-list system/injected message can no longer wedge the
cursor forever.
5 new regression tests; 38 micro/prune tests, 400 compression-suite
tests, 61 finalize/persist tests pass; ruff clean.
Two integration bugs found during review of #74522, both confirmed with
empirical probes against the production message-repair path:
1. Alternation: the summary marker was role="user" and an exchange was a
single assistant+tools group, so splicing between two user turns produced
user -> marker(user) -> user. The pre-request repair_message_sequence pass
(conversation_loop.py, runs before EVERY API call) then merged the marker
into the neighbouring real user message: metadata gone, cursor
unrecoverable on resume, and the summary text duplicated into the
transcript on every later pass (the transcript GREW every turn).
Fix: an exchange is now a full agent turn (assistant + tools + follow-up
assistant iterations, bounded by user messages), the marker is
assistant-role, and superseding an old marker deliberately merges the two
adjacent real user turns (plain-text \n\n-join, identical to repair
pass 2) so the returned transcript is alternation-valid by construction.
Probe result: repairs 0 (was 2), marker survives, no summary leakage.
2. Defrag destroyed user messages: _defrag_rolling_summary serialized the
whole remaining middle (user turns included) and spliced it away —
8 of 10 user prompts destroyed in one pass, contradicting the feature's
"your messages are never compacted" invariant. Fix: defrag now
re-summarizes only the rolling summary TEXT and rewrites the marker
content in place; transcript shape, cursor, and user turns untouched.
Probe result: 10 of 10 user prompts survive.
Also: marker provenance is now COMPRESSED_SUMMARY_HAS_USER_TURN_KEY=False —
micro markers absorb only assistant/tool content (#64650 invariant), and
real user turns remain in the transcript for provenance detection.
Adds 5 regression tests (repair-pass integration, alternation on
multi-iteration tool turns, defrag user survival, defrag input scope,
marker provenance); updates the two existing tests and the design doc to
the corrected semantics. 28 tests pass.
The on/off switch was the only knob. A pass fired after every completed turn,
absorbed exactly one exchange, and there was no way to ask for less. Since a
pass is also what breaks the prompt-cache prefix, "how often does it run" and
"how often do I pay a cache break" are the same question, and it had no answer.
Add `compression.micro_compact_every_n_turns` (default 1, clamped to >= 1). At 1
the behaviour is what it was; at 5 you get a fifth of the breaks and a fifth of
the reclaim rate. The counter advances per invocation rather than per committed
pass, so a turn that finds nothing to absorb still moves the cadence along and
cannot wedge it, and a bogus 0 or negative degrades to "every turn" instead of
silently disabling compaction.
Also expose `micro_compact_defrag_threshold_tokens`, which has been a hardcoded
attribute on the compressor with no path from config since it was added.
This does not give micro-compaction the prune's reclaim-size gate -- a pass
still commits whatever the single absorbed exchange saved. It makes the break
frequency tunable, which reaches the same end by absorbing less rather than by
waiting for a bigger win. The docs now say that plainly, including that a
reclaim threshold is the obvious follow-up and does not exist yet.
Tests cover the skip-until-due window, the cursor and prefix staying untouched
on skipped turns, the clamp, and that the feature is off unless enabled.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The rolling summary lives only in memory. A resumed session starts with an
empty one while the marker carrying every previously absorbed exchange is
still in the transcript. The first pass after a resume therefore built a
marker from a single exchange and superseded the marker holding the entire
history -- silently discarding everything micro-compaction had accumulated.
This was introduced by the supersede fix. Before it, markers piled up
wastefully, but nothing was ever lost.
Two changes, so a single failure cannot lose data:
Rehydrate. When the cursor is recovered by scanning the transcript -- the
resume path -- also recover the rolling summary from that marker, so the
next pass merges into the existing history instead of replacing it.
Extraction uses rfind for the heading because SUMMARY_PREFIX references the
heading text itself, so the first occurrence is inside the preamble.
Gate superseding. Earlier markers are dropped only when this pass's summary
is demonstrably cumulative, i.e. the rolling summary was non-empty going in.
If rehydration ever fails, the pass keeps both markers: wasteful, but the
history survives.
Tests cover the resume path, the failed-rehydration fallback, and the
round trip of a summary through a marker.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The cursor was set to the pre-splice `exchange_end`. A splice collapses the
absorbed span -- an assistant plus its tool results, often four or more
messages -- into a single marker, and may also drop a superseded marker
further back, so every index after it shifts.
The stale cursor therefore overshot, landing inside a *later* exchange's
tool group. The next pass's `_find_one_exchange` walked forward from there
to the following assistant, so the exchange it had landed inside was never
absorbed at all. On tool-bearing conversations micro-compaction was
silently doing roughly half the work it should.
Traced on a 3-tool-per-exchange transcript: the cursor sat at index 6 when
the marker was at 2, and the message count stalled at 32 instead of
continuing to 28.
Derive the cursor from the marker's actual position in the spliced result
instead, which is self-correcting regardless of how much the splice moved.
Apply it on the defrag path too, which had the same staleness.
Found by a randomized long-horizon harness (480 conversation shapes x 25
passes, varying tool-group sizes and summarizer failure modes) asserting
structural and progress invariants after every pass. Existing tests missed
it because their fixtures have no tool results, so the absorbed span is one
message and nothing shifts. The regression test uses tool groups.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Tokens saved is the wrong headline for this feature. Micro-compaction is
not an efficiency optimisation — the same summarization work happens either
way. What it buys is (a) that work amortized across turns instead of one
stall, and (b) a window kept low enough that a session runs much further
before needing a hard compaction at all.
Neither shows up in "net tokens saved". A session can save nothing on paper
and still be a clear win on both counts.
So the telemetry now carries occupancy: tokens_after as a share of the
compaction threshold, plus the threshold and resolved window it was
computed from. That is the number that says whether a session has headroom
left. The report leads with it, and cross-references the batch
`compression_attempt` lines already in the log so it can show how often the
long pause actually fired — ideally never.
Occupancy is read from the cached threshold only. The public
`threshold_tokens` property resolves lazily and can issue a synchronous
/models probe (#32221); telemetry must never be the thing that blocks a
turn, so an unresolved window reports null. In practice a pass has already
resolved it via the tail calculation, so the field is populated. A test
pins the no-forcing behaviour directly against the emitter.
The report is pure ASCII: `scripts/check_subprocess_stdin.py` currently
dies on a cp1252 console before printing its results, and a diagnostic tool
that crashes on the platform it is diagnosing is worse than no tool.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The existing log line reports message counts, which is the least
informative number available here: absorbing one tool-heavy exchange can
drop hundreds of tokens while moving the count by one. There was no way to
answer "is this actually helping?" from a real session.
Emit one content-free JSON line per pass, in the same shape as the batch
compaction telemetry: before/after tokens, the delta, the size of the
absorbed exchange, the rolling summary size, duration, and running
per-session totals so a whole run can be read off the last line. No
transcript content rides along.
Add scripts/micro_compaction_report.py to aggregate those lines into
passes, outcome mix, net tokens saved, mean exchange size and durations,
with an optional per-session breakdown.
Measuring it immediately surfaced something worth documenting: the first
pass in a session normally *costs* tokens. The summary marker carries a
fixed ~400 tokens of scaffolding, paid on pass one against a single
absorbed exchange. From pass two the marker is replaced rather than added,
so the overhead is already paid and each exchange is close to pure saving.
Break-even is typically the second or third pass. Tests cover the
telemetry contract, the cumulative totals, and that first-pass/later-pass
shape so nobody reads a single turn and concludes it made things worse.
The estimator costs ~5 ms at 600 messages and ~20 ms at 1200, taken twice
per pass, post-turn — and only once an exchange is actually in hand, so
turns that no-op early pay nothing.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`_find_one_exchange`'s docstring described an exchange as "(optional) user
message + assistant message + its tool results", but the walk skips past
user messages and starts at the assistant, so user turns are never absorbed
into the rolling summary.
The code is right and the docstring was wrong. Assistant output is largely
an account of what was done and survives summarising with little loss. The
user's messages are the intent everything else is derived from and cannot be
reconstructed from the work that followed — paraphrasing "use the existing
helper, don't add a new one" into a summary is how an agent ends up doing
the opposite six turns later. They are also cheap: a prompt is normally a
tiny fraction of what one tool result costs.
Correct the docstring, document the property (and its cost — a floor on how
small the middle can get, since user turns accumulate), and add a test so it
stays deliberate.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Batch compaction pauses a session for one large summarization once the
window fills. Micro-compaction spreads that cost out: after each completed
turn, `finalize_turn` folds the single oldest un-absorbed exchange
(assistant message plus its tool results) into a rolling summary, so the
work happens in small increments during post-turn idle time instead of one
long stall.
Mechanics:
- a cursor tracks the first message not yet absorbed, recovered from the
transcript's last summary marker when in-memory state is unavailable;
- protected head and tail windows are never touched, so the system prompt
and recent turns stay verbatim;
- the absorbed span is replaced by a marker carrying the usual
`_compressed_summary` metadata, so resume, handoff and `/compress`
treat it exactly like a batch summary;
- `archive_and_compact` keeps the session DB in step, otherwise the
append-only flush would leave the original rows active and a resume
would double-load both summary and originals;
- when the rolling summary itself passes a token threshold it is
defragged: re-summarized in one shot and the cursor jumps to the tail;
- an exchange the summarizer can't handle is retried a bounded number of
times, then skipped, so one poison exchange can't stall every turn.
Keep only the newest summary marker. The rolling summary is cumulative, so
each marker already contains everything the previous ones held; leaving them
stacked near-duplicate copies of the same text, each with its own heading and
end-marker scaffolding, and the transcript grew on every turn instead of
shrinking. Measured over six turns on a 12-exchange conversation with tool
output: 4104 -> 4797 tokens before, 4104 -> 2572 after.
Off switch: `compression.micro_compact: false` (default on).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Use providers keys as the canonical custom-provider identity while accepting legacy bare keys, display-name slugs, bare custom fallback, and doubled custom prefixes across resolution, pickers, doctor, and runtime reverse lookup.
Co-authored-by: Bakhtier Sizhaev <bakhtiersizhaev@users.noreply.github.com>
Follow-ups on top of #70888's cherry-picked fix:
- Replace the $((0666 & ~0$u)) shell arithmetic with POSIX who-less
'chmod "=rw"'. zsh (reachable via _find_bash's $SHELL fallback on
bash-less hosts) parses leading-zero constants as decimal and silently
chmods a garbage mode (e.g. 0210); the symbolic form is spec-identical
across bash/dash/busybox-ash/zsh and degrades to mktemp's 0600
(pre-fix behavior) rather than corrupting perms if chmod rejects it.
- Move the new-file chmod after the content stream so the temp file
stays owner-writable while cat runs.
- Run the chmod on a '[ ! -e "$t" ]' check after cat instead of the
stat/else branch, keeping the overwrite path untouched.
- Update the stale perms comment #70856 called out (new files did NOT
land with default umask perms pre-fix).
- Tests: select the atomic-write script by content instead of call
order (the previous last-call capture only worked because the bare
MagicMock's falsy-exit early return suppressed later execs), assert
behavior at explicit umasks 0022/0002/0077 via parametrize, add an
overwrite mode-preservation regression guard, and dedupe the
real-subprocess env fake into make_real_subprocess_env() shared with
TestSearchFilesFallbackHiddenPaths.
(webtecnica's email mapping already exists in contributors/emails/ on
current main; the PR's check-attribution red was stale-base only.)
# Conflicts:
# tests/tools/test_file_operations.py
Org-shared skills were unusable past the first propose. Three defects, one
root cause plus two that it masked.
ROOT CAUSE — org reads went to the personal endpoint.
`SyncClient.get_refs()` / `get_object()` only ever called `/v1/sync/refs`
and `/v1/sync/objects/:hash`. Those routes are hard-scoped server-side to
the token's own owner, so asking them for `refs/org/<id>/` returns the
caller's PERSONAL refs rather than an error, and org objects 404. Both org
call sites read org state through them:
- `pull_org_skills` resolved head=None for a populated org and reported
`{"ok": true, "head": null, "updated": []}` — org skills silently never
arrived, which reads as "my org has no skills" rather than as a failure.
- `propose_skill` resolved base_head=None, so the FIRST propose to an org
succeeded by accident (`from: null` happened to be correct) and EVERY
later one CAS'd against a head it had never seen -> 409 -> a raw
`SyncConflict` traceback. Worse, it built its root from an empty skill
map, so a landed CAS would have REPLACED the org set rather than splicing
into it — the 409 was accidentally preventing data loss.
Fix: `org_scope=True` on `get_refs`/`get_object`, threaded through
`get_commit_json`, `get_tree_json`, `_root_tree_of_commit`,
`_skill_trees_of_root`, and `materialize_tree` — walking an org commit needs
the org route on every hop, not just the first. Both org call sites now go
through one `_read_org_head()` helper.
ALSO FIXED
- `propose_skill` retries on conflict. When the org HEAD moves between the
read and the CAS (another member proposing, an admin merging), it
re-splices this one skill onto the NEW head and retries, bounded at 5
attempts. Re-splicing rather than replaying the old root is what stops a
concurrent proposal being dropped.
- An empty `actual` in a 409 means "the ref does not exist", not "here is a
commit". `SyncConflict` normalizes "" to None in its constructor, and the
personal push path redoes the CAS as a create instead of fetching "" as an
object — which surfaced as the baffling `object not found` (doubled
space). This is what a client hits after switching sync planes, since
`.sync_state` is not environment-scoped and carries a foreign head.
THE MOCK WAS THE REASON THIS SHIPPED
The test mock served org refs and org objects off the personal routes, so
21 org tests passed against a client that could not work against the real
plane. The mock now mirrors production: `/v1/sync/org/refs` and
`/v1/sync/org/objects/:hash` exist, org objects live in a separate scope,
and the personal routes refuse org content. Two existing tests had to be
corrected to assert against the org scope — they had been passing on the
mock's over-permissiveness.
Tests: 5 new (org head invisible on the personal route; second propose
splices and preserves the first; pull resolves a real org head; empty
`actual` -> None; push recovers from a stale cross-plane head). Verified
they FAIL without the fix: reverting just `_read_org_head` to the personal
route fails the second-propose test and the pre-existing splice test.
1278 passed / 0 failed across 54 suites via scripts/run_tests.sh.
Verified against PRODUCTION with a real org token, not just the mock:
- `pull_org_skills` -> head `sha256:1adf9333…`, materialized
`software-development/gateway-gateway-connector` into the `_org` mirror
(was head=None, updated=[]).
- A second `hermes sync propose` succeeded where it previously raised, and
the org set afterwards contains BOTH skills with the new commit
descending from the first.
The two exact-list assertions in test_tui_gateway_server froze the toolset
list and broke the moment _RECENTLY_SHIPPED_TOOLSETS back-filled bfl onto a
saved platform list — the exact behavior the sibling change ships on purpose.
Assert the invariant instead: the expected base set is present, and anything
extra must be inside _RECENTLY_SHIPPED_TOOLSETS (vacuously exact again once
that set empties between releases).
send_video got the 60s read_timeout but send_voice/send_audio/send_photo/
send_document/send_media_group/send_animation upload through the same PTB
request path and hit the same server-side processing wait before the
response arrives. Same class, all sites: they all pass
_MEDIA_SEND_READ_TIMEOUT now. Also drops an unused test helper.
test_generated_script_contains_umask_else_branch asserted on shell
script text ('else', 'umask', '(0666 & ~0', 'chmod') rather than
behavior — a change-detector test per AGENTS.md. The behavioral
test (test_new_file_gets_umask_default_permissions) already
covers the actual behavior end-to-end via real subprocess.
The compaction summary's role was selected against the LITERAL
neighbouring messages (compressed[-1] / tail_messages[0]). Mistral-family
chat templates (Devstral, Mistral Small 3.x, Magistral) enforce
user/assistant alternation but exempt the tool flow (tool results and
assistant messages carrying tool_calls) from the check, so a protected
head ending [user, assistant(tool_calls), tool] pinned the summary to
role="user" while the last role the template counts is "user": the
backend rejects the whole request with a Jinja alternation error
(HTTP 500). The summary persists in the stored conversation, every
retry replays the identical poisoned history, and the session is
permanently unrecoverable. Fires on EVERY compaction against a
Mistral-strict backend, captured byte-exact via a tee-proxy in front of
a llama.cpp/llama-swap Devstral deployment.
Fix: compute both neighbour roles through _template_visible_role(),
which skips template-exempt messages. The #52160 (Anthropic user-first)
and #58753 (zero-user-turn) forced-user guards are preserved; their
forced shapes (summary-user followed only by exempt messages) are
alternation-safe. When the visible head ends "assistant" and the
visible tail opens "user", no standalone role can alternate and the
existing merge-into-tail fallback now correctly fires (the literal
logic emitted a standalone user summary there: a second poisoning
shape).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LGN45sMMbwM8cW9T9ga4ou
A model picked mid-turn is applied at the next turn start, but the end-of-turn
session.info (_emit_settled_session_info) reads the still-live agent — the OLD
model — and clobbered the optimistic paint, so the UI showed the new model, then
snapped back to the old one when the turn ended, then switched for real on the
next turn. Report the queued pick's model/provider in _session_info while a
switch is pending (it IS the model the next turn runs), so the display stays on
the user's choice through the settle. Cleared once the switch applies.
* feat(tui): Ctrl+O opens the model picker without clearing your draft
Reaching the model picker meant typing /model, which forces you to wipe
whatever you'd already drafted. Bind Ctrl+O to open the same picker overlay
directly, leaving the composer untouched. Ctrl+O is added to the textInput
pass-through allowlist so the composer doesn't swallow it, mirroring the
existing Ctrl+X session-switcher path.
* feat(tui): apply a mid-turn model switch at the next turn instead of rejecting it
Picking a model while a turn was streaming hit a 4009 'session busy' reject:
switch_model() mutates the agent's model/provider/base_url/client in place and
the worker thread reads those every iteration. Now config.set queues the pick
in session[pending_model_switch] and _apply_pending_model_switch applies it on
the turn thread at the next turn start, before any model call — no race, no
interrupt, no waiting on the client rebuild. The TUI paints the pick optimistically
and notes '(applies next turn)'.
Terminals that rewrite Cmd+Backspace to Ctrl+U already reach
unix-line-discard. Kitty keyboard protocol and xterm modifyOtherKeys
terminals instead report Cmd as the super modifier bit, producing CSI
sequences prompt_toolkit has no entry for — the raw bytes fall through
the VT100 parser and land in the buffer as literal text.
Alias those to the readline kill bindings prompt_toolkit already ships.
Backspace is a CSI-u codepoint (127); ForwardDelete is a CSI tilde key,
so its modifier rides in the CSI 3 ; mod ~ form rather than CSI-u.
Ctrl+ForwardDelete keeps its own binding — that is delete-word on
Linux/Windows, not kill-line.
Salvaged from #57016 by @lEWFkRAD:
- cli.py: handle file:///C:/... drive-letter URIs on nt (strip the
leading slash urlparse leaves); join Termux example paths with literal
forward slashes so hints stay POSIX on Windows.
- gateway/status.py + hermes_cli/gateway.py: normalize backslashes to
forward slashes before the HERMES_HOME substring match so separator
style cannot defeat profile ownership detection.
- hermes_cli/banner.py: cprint degrades to plain print when
prompt_toolkit has no console (NoConsoleScreenBufferError on
redirected/absent Windows stdout).
- hermes_cli/browser_connect.py: posixpath.join for WSL /mnt/c/... bases
(os.path.join would emit backslashes on nt).
- Test hardening: symlink skip-guards, USERPROFILE alongside HOME for
ntpath.expanduser, SIGKILL absence skipif fixed via monkeypatch,
drive-letter URI / separator-normalization / banner-fallback coverage.
Dropped from the original PR: tests/cli/conftest.py fixture and the
AppSession _output monkeypatch — main's merged tests/cli/conftest.py
already handles that prompt_toolkit pollution.
The cross-process update lock (fe8e4d93d) made the in-progress marker
mutually exclusive across every update entrypoint — but the Tauri
updater holds that marker for its WHOLE run and then spawns
hermes update as a child stage. The child read the marker, found its
own parent's live pid, refused with exit 2, and the GUI mapped that to
"Hermes is still running. Close all Hermes windows and try the update
again." Retry spawns a fresh updater that deadlocks against itself the
same way, so every GUI-driven update dead-ends on the failure screen
with no winnable retry (observed: three consecutive self-refusals in
bootstrap-installer.log within 90 seconds).
Hand the claim off explicitly: update_child_env exports
HERMES_UPDATE_HANDOFF_PID naming the updater's own pid, and
UpdateLock.acquire treats a live holder matching that pid as the lock
we are already running under — run without claiming, and release
leaves the parent's marker untouched. The env var alone grants
nothing: the pid must also be the live marker owner, so a stale or
forged value cannot bypass the lock, and a dashboard-spawned
hermes update (no handoff env) is still refused exactly as before.
Slice 1 fell over on eight DB fakes with frozen get_messages_as_conversation
signatures — the new opt-in kwarg is part of the method's contract now, so
the fakes accept **_kwargs like the real SessionDB. Also opts the child-watch
resume projection into row ids: it feeds the same _history_to_messages as the
desktop resume, so reactions on a watched child session address rows the same
way.
CI caught ACP session restore seeing an unexpected _row_id in restored
history — get_messages_as_conversation feeds more than the desktop, and
changing the default shape broke the strictest consumer. Row ids are now
include_row_ids=True, requested only by the gateway's resume/display
projections; ACP restore, export, and inspection get the transcript in its
historical shape.
Reactions live in the existing messages.display_metadata JSON column (no new
table), with iOS Tapback semantics enforced DB-side: one reaction per author
per message, re-tap retracts, different emoji replaces. The desktop catches up
to the reaction contract five platform adapters already ship.
- SessionDB: set/get_message_reaction, latest_message_row_id (role + offset +
require_text so invisible tool-call-only rows are never targeted),
take_unseen_reactions (announce-exactly-once), get_message_role
- message.react RPC: accepts row_id or newest_role for live messages that
haven't learned their durable id yet
- react_to_message tool: desktop-gated (check_fn), defaults to the user's
latest visible message, messages_back for retroactive reactions
- Model context rides run_message only (beside the speech-interrupted note):
the persisted prompt stays clean, so no [The user reacted …] scaffolding in
transcripts, and no cached prefix ever changes
- Resume projection forwards row_id + reactions; _row_id is stripped from
outgoing API copies next to display_metadata
The tests fake the vercel SDK entirely via sys.modules, but
_ensure_vercel_sdk checks installed DISTRIBUTION metadata through
tools.lazy_deps.ensure(): on CI (no vercel dist + lazy installs
disabled) it raised FeatureUnavailable→ImportError before the fake SDK
was ever reached — 16 failures on slice 6, green on dev boxes that
happen to have vercel==0.7.2 installed. Failure mechanism reproduced
locally by forcing _is_satisfied False; fixture patch verified to close
it while the real-dist path stays green (16/16).
The file-local telegram mock in test_dm_topics.py installed unconditionally
(no __file__ guard), registered a separate string-valued telegram.constants
module, and force-popped the adapter — poisoning the session for any later
telegram test in the same process (assert 'MARKDOWN_V2' in "'MarkdownV2'").
Fix at the source:
- conftest: _FakeEnumMember(str) with PTB-faithful str()==value and
repr()==<ChatType.X: 'x'>, satisfying both repr assertions and the
adapter's str(chat.type) normalization; the same object is bound to
mod.ParseMode and mod.constants.ParseMode.
- test_dm_topics.py: delete the divergent local mock installer; import the
shared conftest one.
- release.py: mailmap entry for the author.
Verified: the 5-failure cluster repro (dm_topics + slash_confirm +
approval_buttons + model_picker + network_reconnect + telegram_format in one
process) goes 83/83 green (3x); full tests/gateway single-process run drops
10 -> 5 failed, the remainder being pre-existing discord order-dep failures
out of scope here.
Salvaged from #68873. Credit to @liuhao1024 for the earliest root-cause
diagnosis of this str-enum mock class in PR #33875, two months earlier.
Fixes the telegram-mock order-dependent flake cluster.
Two tests made real HTTP calls to homeassistant.local:8123 — on a LAN
with an actual Home Assistant instance they could turn on real lights,
and otherwise burned ~10s in network timeouts. Replace with AsyncMock at
_async_call_service and assert the exact production call signature
(domain, service, entity_id, data). 35 pass in ~0.2s.
Salvaged from PR #72634 by @jeeves-assistant.
Co-authored-by: Jeeves Assistant <jeevesassistant00@gmail.com>
tests/test_zz_guard_probe_tmp.py was a throwaway verification probe that
was swept into the PR #43299 salvage commit by mistake (and one of its
shell=True cases fails by design of the probe). The durable regression
coverage lives in tests/test_live_system_guard.py.
Autouse fixture also resets approval_module._YOLO_MODE_FROZEN so a
HERMES_YOLO_MODE=1 host env can't poison every case (the one
startup-frozen test still patches it back explicitly). Adds the darwin
'ps -o stat=' zombie branch to _is_alive_like_dispatcher, mirroring
production hermes_cli/kanban_db.py — a no-op on Linux.
Salvaged from PR #34069 by @sunwz1115.
Co-authored-by: sunwz1115 <192549904+sunwz1115@users.noreply.github.com>
Two Windows fixes for the canonical test runner, salvaged from #66496:
- scripts/run_tests.sh: probe the native Windows venv layout
(Scripts/activate → Scripts/python.exe) alongside bin/activate,
adapted to main's pytest-import-guarded loop with SKIPPED_VENVS
reporting. Without it a python -m venv / uv venv on Git Bash/MSYS is
never found and the runner refuses to start.
- scripts/run_tests_parallel.py: _make_stdio_glyph_safe() reconfigures
stdout/stderr to UTF-8 (errors=replace fallback) so the ✓/✗ progress
glyphs cannot crash a cp1252 console when the runner is invoked
directly (run_tests.sh's PYTHONUTF8=1 only covers the wrapped path).
No-op on UTF-8 stdio. Ships 3 OS-independent cp1252 tests plus
encoding=utf-8 in the runner-subprocess assertions.
Dropped from the original PR: the USERPROFILE/HOMEDRIVE/HOMEPATH/
SYSTEMROOT env-forwarding hunk — main's WIN_ENV loop already forwards
a superset (#67385/#70813).
Sibling files (test_billing_rpc etc.) import tui_gateway.server at
collection time, freezing module-level _hermes_home = get_hermes_home()
(server.py:54) to the developer's real home before conftest isolation
runs. _load_cfg() then reads the REAL ~/.hermes/config.yaml — e.g. a
local MoA preset — instead of what _write_moa_config wrote.
The server fixture now monkeypatches _hermes_home to the isolated
HERMES_HOME and resets the mtime-keyed cfg cache (_cfg_cache/_cfg_mtime/
_cfg_path); monkeypatch restores originals on teardown.
Complementary to the #57066 autouse teardown, which only restores the
cfg cache to its pre-test value and never re-points _hermes_home.
Combined tests/tui_gateway + tests/test_tui_gateway_server.py:
792 passed.
Salvaged from PR #63981 by @lEWFkRAD.
Argv-list commands now scan only argv[0] against _PROCESS_KILLERS, ending
false positives like ['cat', '.../skill'] ('skill' is a real util-linux
binary name). Wrapper executables (sh/bash/env/nohup/timeout/sudo/xargs/…)
keep full-token scanning so ['bash', '-c', 'pkill …'] and
['env', …, 'pkill', …] stay blocked; string commands are unchanged.
Adds tests/test_live_system_guard.py pinning both directions.
Salvaged from PR #43299 by @eazye19.
Co-authored-by: Tony (eazye19) <support@captureclient.net>
Contract tests for tools/fal_common.py: queue-URL normalization
(trailing slash / whitespace / empty-raises), _extract_http_status
response-vs-exc precedence and non-int rejection, the
_ManagedFalSyncClient RuntimeError guards against fal_client private
API drift, and submit()'s queue-URL + POST/json/timeout wiring.
Trimmed on landing: test_non_string_coerced_to_string (pins an
implementation accident, not a contract) per the coverage-padding
policy in AGENTS.md.
Salvaged from #52166.
Cross-file leaks made tests/tui_gateway + tests/test_tui_gateway_server.py
fail when run in one process (issue #57068):
- conftest: _hermetic_environment now re-pins hermes_state.DEFAULT_DB_PATH
to the fake home so no test ever touches the developer's real state.db
- conftest: new autouse _reset_tui_gateway_server_state snapshots/restores
_methods, cfg cache, db handle and _real_stdout, and tears down leftover
sessions via the production _close_session_by_id(..., end_reason=
"test_cleanup") boundary
- per-file fixtures scope patch.dict(sys.modules) to the import only
- drop reload()-based teardowns that duplicated atexit hooks
Conflict resolution vs main: kept main's mod._live_transports.clear() in
the test_protocol.py server fixture alongside the snapshot/restore logic.
Combined run now 792 passed / 0 failed.
Salvaged from PR #57066 by @lEWFkRAD. Fixes#57068.
Replace OS-scheduler-dependent elapsed-time ceilings with deterministic
concurrency proofs in three tests:
- test_context_refs_concurrent: asyncio.Barrier rendezvous — all 3 URL
fetches must be in flight simultaneously before any returns.
- test_memory_boundary_commit: positive non-blocking witness — the
provider call list must still be empty when the async commit returns.
- test_mem0_v3 slow-prefetch: threading.Event park/release — prefetch
must return while the backend search is still parked.
The tests/tools/test_mcp_tool.py hunk from the original PR is dropped:
main no longer carries the 'elapsed < 2.5' assertion it targeted
(superseded by a delay-relative bound).
Salvaged from #71913.
Guard the .lazy-refresh-incomplete marker writer (update_cmd), launch-time
recovery (main.py), and _early_recovery repair paths behind a two-condition
check: running under pytest AND the target is this live checkout. Sandboxed
tmp_path tests still exercise the real code paths.
Salvaged from PR #72002 by @fcavalcantirj. Fixes#72000.
Co-authored-by: fcavalcantirj <felipe.cavalcanti.rj@gmail.com>
The guard's sys.modules.get() can observe hermes_cli.kanban_db while a
lazy import is still executing on a fixture boundary — the partially
initialized module has no .connect yet (AttributeError flake in the
full-suite verification run, 1/2460 files). A half-imported module has
no callers to guard; skip this round and let the next fixture patch the
completed module.
hermes_cli/main.py calls setup_logging() at module scope. That resolves
get_hermes_home() and attaches rotating file handlers to the ROOT logger via
a QueueHandler. So merely importing it - which many test modules do, directly
or transitively - points the whole pytest session's logging at
<HERMES_HOME>/logs/agent.log and errors.log.
The _isolate_env fixture already sandboxes HERMES_HOME, but fixtures run
after collection has imported the test modules, and by then the handler holds
an absolute path to the real file. Verified by importing hermes_cli.main in a
clean interpreter and walking the queue listener: both handlers pointed at the
developer's own ~/.hermes/logs/.
Measured on a live install: 126 warnings in a personal agent.log came from
test runs rather than the running gateway - phantom 'FakeTree' Discord
registration failures and 'rejected invalid API key' entries whose paths only
exist in tests/gateway/test_api_server_runs.py. That noise makes genuine
warnings hard to find exactly when someone is debugging.
conftest is imported before any test module, so sandboxing HERMES_HOME there
closes the window. The per-test fixture still applies afterwards.
Also fixes 4 pre-existing failures: tests/gateway/test_channel_directory.py
TestBuildFromSessions was reading the operator's real sessions data for the
same reason.
Full gateway+tools suites: 66 failures on clean origin/main, 62 with this
change, 0 new. The regression guard asserts the value captured AT conftest
import - reading os.environ inside a test passes even with the fix removed,
because the per-test fixture has sandboxed it by then.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
test_session_not_found_goes_to_stdout_in_full_mode passes in isolation but
fails in a full tests/cli run. Two independent leaks from the same neighbor
test conspire:
1. test_cli_init.py's _make_cli() reloads cli.py while prompt_toolkit is
stubbed with MagicMocks and never reloads it back, so sys.modules['cli']
is left with a mock _pt_print/_PT_ANSI and cli._cprint silently no-ops
for every later test. Fixed by reloading cli once more with the real
modules visible (try/finally).
2. prompt_toolkit's print_formatted_text caches its Output on the
process-global default AppSession the first time it renders without an
explicit output=. Under capsys (which swaps sys.stdout per test), the
first CLI test to emit through _cprint locks that cache onto its own
captured stdout, so later capsys tests read an empty buffer. Fixed with
an autouse fixture in a new tests/cli/conftest.py that resets the cached
output around each test.
Neither change touches production code or the flaky test's own assertion.
Related to #59358 (which addresses the same flaky test by mocking _cprint in
the assertion instead; this fixes the two underlying leaks at the source and
does not modify test_resume_quiet_stderr.py).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
