The apt/docker CLI tests pinned exit 1; refusals are now exit 2
(refused-by-contract, distinct from errors). The web_server guards
patched the module-local detect_install_method alias, which the shared
admission gate no longer consults — patch hermes_cli.config directly.
Every surface that can start an in-place mutation — hermes update
(apply), update --check, and the dashboard's update endpoint — now
routes through evaluate_update_admission(): the baked image-provenance
marker first (authoritative; a bind-mounted checkout inside a container
looks like git to the heuristics while the filesystem is an immutable
image), then the pre-existing docker/nix/apt heuristics verbatim.
A refusal prints the real update command for the deployment kind,
records a 'refused' receipt (fleet tooling sees 'not updatable in
place, use <cmd>' instead of a silent non-update), and exits 2 on CLI
surfaces — distinct from exit-1 errors. The dashboard response keeps
the per-kind error codes its UI already keys on. collect_runtime
inventory()'s updatable_in_place also honors the marker, so --plan and
receipts report image-managed truthfully even with a bind-mounted
checkout.
Live E2E (real hermes update subprocesses, real marker file): apply and
--check both refuse exit-2 with docker-pull guidance, receipts land as
refused/image-marker, an in-place corrupted marker still refuses
(fail-closed), removing the marker admits the git checkout.
* fix(tui_gateway): ask before queueing a guarded model picked mid-turn
config.set model on a running session cannot swap the agent in place, so it
stashes the pick in session["pending_model_switch"] and applies it at the
next turn start. That branch answered confirm_required=False without ever
running the selection guards.
A client that implements the confirm round-trip was therefore told no
consent was needed and never prompted. One turn later
_apply_pending_model_switch ran the guards with the stashed (unconfirmed)
flag, saw the warning, and dropped the switch by design. The model reverted
with no confirm ever offered, because the only moment a round-trip was
possible had already passed.
Evaluate the guards before stashing, where the client still has a live
response to turn into a prompt. Nothing is queued for an unconfirmed
guarded pick, so the session is left exactly as it was and the re-send
carrying confirm_expensive_model queues it for real. The apply-time check
stays as the backstop for guards that can only decide after resolution.
The data-policy guard keys on the model id alone, which is all this branch
can see before resolution. The cost guard returns None when pricing is
unknown and its models.dev lookup is allow_network=False, so calling it
early can only under-fire and never blocks the RPC thread.
* test(tui_gateway): pin provider forwarding, name the canonical confirm field
Two review follow-ups, no behavior change.
_pending_switch_selection_warning forwards `provider=provider or None`, but
nothing asserted it: a guarded model id fires the data-policy guard on the
model alone, so the existing tests passed with `provider` dropped entirely.
Record the kwargs instead. Dropping the argument fails the first test;
removing the `or None` normalization fails the second.
The confirm responses carry `warning` and `confirm_message` with identical
text, which reads like an accident. Name which one clients should read
(`confirm_message`; `warning` is the pre-confirm-era alias that
_apply_pending_model_switch already treats as a fallback) so the two do not
drift apart later.
Both raised by @Enough1122 in review.
slack_bolt builds a fresh AsyncWebClient for every inbound request and
copies proxy=app.client.proxy into its constructor, where slack_sdk reads
a None/blank proxy *argument* as "unspecified" and reloads HTTP(S)_PROXY
from the environment. aiohttp then treats that env value as an explicit
proxy and skips its own NO_PROXY check, so the adapter's resolved decision
to go direct - a NO_PROXY bypass, or a proxy scheme aiohttp cannot use -
holds on every client except the one authorization spends on auth.test.
The failure looks like a healthy bot: Socket Mode connects, outbound sends
keep working, and every inbound event is rejected with "Failed to authorize
with the given token" - forever, since a failed auth_test_result is not
cached and never retried differently.
Re-apply the resolved proxy through AsyncApp(before_authorize=...), which
bolt inserts before the authorization middleware: the request-scoped client
already exists there and has not been used yet. Assigning the attribute
post-construction is the only way to express "no proxy" to slack_sdk.
Co-authored-by: Junie <junie@jetbrains.com>
`completed` already puts the worker's summary inside the synthetic wake
turn, so the woken creator sees what was done. `review_requested` did
not: the summary rode the passive ping only, and the wake turn said just
"handed off for review", forcing the woken reviewer to re-read the board
(and losing the PR link the worker had already written).
Reuse the same first-line handoff the `completed` branch builds, so the
existing `gateway.kanban.wake.handoff` string renders it — no new locale
keys, no change to the passive message.
`review_requested` and `block_loop_detected` are terminal event kinds that
hand a decision back to the origin subscriber, but neither was listed in the
gateway notifier's `_WAKE_KINDS`. A `notify+wake` subscription therefore got
the passive ping only and the origin agent never took a turn — so an agent
that delegated implementation work slept through the "ready for review"
handoff and through a task being routed to triage, while the equivalent
`blocked` event woke it.
Add both kinds to the wake set, add their status strings to the synthetic
wake message in every locale, and document which events wake.
Slack sends an authored message twice: flat in `event.text` and structurally
in `event.blocks`. The blocks are rendered so quoted and forwarded content is
not lost, and whatever the render carries beyond the flat text is appended to
the message. That comparison had several ways to fail on the *same* sentence,
each of which showed the author their own words a second time:
1. HTML entities — the flat copy escapes `&`/`<`/`>` while `blocks[].link.url`
stays raw, so any link with query parameters (every "Copy link" on a
thread) mismatched.
2. Permalink unfurls — the live inbound path skips `is_msg_unfurl`
attachments, thread/parent hydration did not, so the linked message's body
was appended again.
3. The Block Kit dump — it serialized the authored `rich_text` alongside the
UI blocks it exists for, and its allowlist drops `url`, so the sentence
reappeared with every link removed.
4. Unknown inline elements — the renderer knew eight types and silently
dropped the rest. A pasted message permalink arrives as `message_mention`,
so the link vanished from the render and the sides stopped comparing equal.
5. `message_mention` without a url — `url` is optional on that element while
`channel_id` and `message_ts` are not, so the element rendered as nothing
and the sentence came back with a blank in the link's place.
6. `date` elements — `fallback` and `url` are both optional, and the flat
`<!date^…>` form was never read down to what the rich text renders.
7. Labelled mentions — Slack may attach a label (`<@U…|name>`,
`<#C…|general>`, `<!subteam^S…|@marketing>`, `<!here|@here>`) in the flat
text while the blocks carry the bare id. The bot's own mention is one of
these, and stripping only its bare form left it in the flat copy.
8. Autolink schemes — only `https` and `mailto` were matched, so a `tel:` link
kept its angle brackets and mismatched too.
Unknown inline types are now read by their `url`/`text`/`fallback` so a type
Slack adds later still renders, and `team`, `color` and a fallback-less `date`
render into the flat form Slack sends. Every field is read as a string or
not at all: Block Kit carries text as an object in many places, and a
non-string one reaches the renderer's `str.join` and raises there, which
costs the whole message. `channel_id` and `message_ts` are the
permalink's own components, so a url-less `message_mention` renders the
permalink's tail; the workspace host and the thread query cannot be rebuilt
from the element, so a permalink on either side is reduced to that same tail.
Canonicalization is used for matching only -- the authored text still reaches
the agent verbatim, so a mistake here can cost an unrendered element, never an
altered or missing message.
An element carrying neither a url nor a label still renders as nothing, and a
message containing one is still appended twice. Suppressing such a render was
tried and is worse: an app message whose body lives only in the blocks
disappears, and a forwarded quote is dropped. Genuinely additional content --
quotes, lists, code blocks, attachments, interactive bot blocks -- is
unaffected throughout.
Tests cover both merge sites (live inbound and thread hydration) and the
negative cases.
An ACP client talks to a CLI over subprocess stdio: it returns a plain
completion object rather than an iterable stream, and it does not implement
the Responses API surface. Both exclusions spelled out `acp://copilot`, so the
next ACP client silently inherited the wrong defaults — a Responses upgrade
its shim cannot serve, and a streaming call that tries to iterate a
`SimpleNamespace`.
Match on the `acp://` scheme instead. `acp+tcp://` was already handled this
way; copilot-acp's behaviour is unchanged, and the new tests pin that a
non-ACP URL still upgrades, so this is not a blanket opt-out.
The review fork's entire job is to emit `memory` / `skill_manage` tool calls,
and by default it inherits the parent's live runtime. A provider that IS an
autonomous agent reaches Hermes through a client shim; if that shim cannot
carry Hermes tool calls back, the fork is a guaranteed no-op that still pays
for a full agent spawn — a whole CLI process, sometimes a JVM — on every
review cadence.
A client declares `SUPPORTS_HERMES_TOOL_CALLS = False` and the fork is
skipped with a warning naming the `auxiliary.background_review.{provider,model}`
override that routes the review to a normal model instead. Anything that says
nothing is assumed capable, so ordinary providers are untouched.
The check runs before the thread-scoped silence so the warning is not
swallowed, and only resolves the review runtime once the cheap capability
test has already failed, so the normal path does not resolve it twice.
Most providers are models: they ask Hermes to run a tool and Hermes runs it,
so the transcript and the loop's counters see every tool iteration. Some
providers are agents — an ACP CLI behind a client shim, or the codex
app-server, which already takes an analogous path in `agent/codex_runtime.py`.
They execute their own read/edit/execute tools inside their own session, and
by the time Hermes sees the response that work is done.
Those calls must never come back as pending `tool_calls` — Hermes would
re-run finished work. But summarising them into `reasoning` blinds two
subsystems:
- the self-improvement loop, which distils memories and skills by replaying
`messages`; a one-line activity feed teaches it nothing;
- the skill-review nudge, whose `_iters_since_skill` counter only moves on
Hermes tool iterations, of which there are none.
So a client may hand both back on the completion object —
`hermes_projected_messages` (completed assistant(tool_calls) + tool(result)
rows) and `hermes_provider_tool_iterations` — and
`splice_provider_projection` applies them. Rows go through `append_message`
like every other live-transcript append, so they carry a timestamp and
persist the same way the codex projection path's rows do.
The splice is append-only, sits before this turn's assistant message so the
order reads call -> result -> answer, and is a no-op for every client that
sets neither attribute, i.e. every ordinary OpenAI-compatible provider.
Garbage attribute values are tolerated rather than allowed to break the turn.
ACP has no OpenAI `tools`/`tool_calls` channel: a prompt is text and a
response is text plus the agent's own tool notifications. Hermes' agentic
surface — memory, todo, skill_manage — is dispatched from OpenAI-shaped
tool_calls, so on an ACP provider it only works if the schemas travel into
the prompt as text and the calls are parsed back out of the response text.
copilot-acp already carried that bridge as private module-level helpers.
Lift it verbatim into `agent/acp_openai_bridge.py` so every ACP client
shares one implementation of the wire contract instead of re-deriving it —
`agent/claude_code_acp_client.py` (#81375) is currently a third copy of the
same four functions, and each copy is a place the `<tool_call>` contract can
drift.
copilot-acp is migrated onto it as the in-tree consumer and loses 176 lines
of duplication; its prompt shape is unchanged, which the new tests pin.
Two things the shared version adds over the copy:
- `render_tool_bridge_sections(..., allowlist=)`. A CLI with no tools of its
own forwards Hermes' whole toolset (copilot, unchanged: no allowlist). A
CLI that *is* an autonomous agent must forward only Hermes' agent-level
tools — re-offering the overlapping read/edit/execute ones makes Hermes
re-run work the agent already finished.
- `StreamChunks`, a list subclass that keeps response-level attributes.
Hermes reads provider extras off the object returned by
`chat.completions.create`; the old plain-list return silently dropped them
whenever a caller asked for `stream=True`.
Windows updates forced a choice between 'gateway survives' and 'update
proceeds': the pause machinery's only tools were the planned-stop marker
poll and the force-kill ladder, so a mid-turn gateway was tree-killed and
its active turn lost. Step 2 of the socket migration adds the
pause-for-update verb: the updater ASKS the gateway to drain in-flight
turns and exit cleanly — releasing every venv file handle on the way out
— through the same request_restart(via_service=True) drain path SIGUSR1
and service restarts already use.
- gateway/run.py: pause-for-update verb handler registered on the
existing control server; marshals onto the loop thread, ACKs with
{pausing, already_stopping, pid, drain_timeout}.
- gateway/control_socket.py: pause_gateway_for_update() client — None on
no-answer (older gateway / no socket), so every caller keeps the
legacy path when the verb is missing.
- update_cmd.py (_pause_windows_gateways_for_update): socket-first ask
per mapped profile gateway before the drain wait; positive ACKs extend
the wait to the gateway's own declared drain budget (+ teardown grace)
so a mid-turn gateway isn't force-killed at the end of a too-short
local default. Marker write + force-kill ladder retained verbatim as
the fallback.
Live E2E: real gateway process (isolated HERMES_HOME), real socket:
identify -> pause ACK {pausing: true} -> gateway drained and exited on
its own (rc=75, zero signals) -> dead-gateway re-ask returns None.
A step-1 gateway without the verb answers ok:false -> client None ->
legacy path (pinned by test).
The three existing assertions are absence checks, so the test also
passed when the loop got no iteration inside the sleep window (with
interval=5.0 it passes without the gate ever executing). Checking
_scale_to_zero_no_suspend_logged proves the branch was taken.
Drop the try/except AttributeError guard in the new regression test
now that the slot is always defined, and note in the run() comment
that _ever_connected is set once and never cleared.
These pre-existing tests fake a successful first connect by calling
only _ready.set(), which is what the real code did before this PR.
Now that run() gates the initial-vs-reconnect branch on the new sticky
_ever_connected flag instead, their later simulated reconnect failures
were misclassified as never-connected and hit the 3-attempt ladder,
failing test_reconnect_counter_resets_after_successful_session,
test_parked_server_self_probes_and_revives, and
test_retry_attempts_log_debug_transitions_warn in CI. Set the flag
alongside _ready.set() to mirror the real success sites, same as the
new test added in tools/mcp_tool.py's own PR.
MCPServerTask.run() used `_ready.is_set()` to tell a genuine first
connection attempt from a later reconnect. `_ready` is cleared on every
reconnect cycle, so once a server has already registered its tools and
then drops (keepalive failure, transient TaskGroup exit, etc.), the next
failed reconnect attempt is misclassified as "never connected" and burns
the 3-attempt initial-connect ladder instead of the 5-attempt reconnect
budget, parking the server much sooner and logging "failed initial
connection after 3 attempts" even though tools were already registered.
Add a sticky `_ever_connected` flag, set once alongside `_ready.set()`
right after a successful `_discover_tools()` call and never cleared, and
gate the initial-vs-reconnect branch on it instead.
Fixes#94654
The stale-dashboard sweep at the end of hermes update snapshots each killed
backend's HERMES_HOME (_hermes_home_for_pid) but only used it as the per-profile
dedupe key. _respawn_dashboard_processes replays the argv with no env=, so a
backend belonging to a second install (e.g. a launchd KeepAlive sidecar) came
back running on the updating install's default home and stole the sidecar's
fixed port: the supervisor crash-looped on EADDRINUSE and clients on that port
silently talked to the wrong backend.
Drop such candidates in _filter_dashboard_respawn_candidates: a backend whose
captured HERMES_HOME differs from the updater's own get_hermes_home() is not
replayed at all — its own supervisor/user owns its lifecycle. Homes are
normalized the same way _profile_key_for_respawn normalizes home: keys, so
symlinked roots compare equal. An unreadable home (None) stays eligible,
keeping the pre-fix fail-open behaviour.
The old assertions pinned the phrasing that HID serve backends — the
exact asymmetry #81564 reports. Re-pinned to the new message and
strengthened: a serve-mode row must now appear, tagged [serve].
A manually-launched `hermes serve --host <ip>` powering a remote Desktop
was invisible to the entire update pipeline: not in the runtime
inventory, a permanent exit-2 dead-end at the Windows venv-holder guard,
and — when anything killed it — never relaunched, stranding the remote
client on a dead endpoint (#63206). Serve backends were also visible to
`hermes dashboard --stop` but hidden from `--status` (#81564's
asymmetry), so operators could kill what they couldn't see.
Built on the spawn ledger (positive identity, never argv guessing):
- process_identity.py: LedgerEntry gains structured host/port/profile
(backward-compatible — readers .get()); register_self accepts detail=;
argv capture widened 6→10 tokens so profiled launches survive.
- web_server.py: serve/dashboard registration moved AFTER the bind and
now records the ACTUAL bound host/port/profile.
- update_inventory.py: serve/dashboard collector reading the ledger —
manual backends inventory as supervisor=manual-serve with
restart_via=respawn-argv; Desktop-owned ones (live recorded spawner)
as desktop. Plan/receipts/fleet matrix see them for free.
- update_cmd.py: new venv-guard rung — manual serve/dashboard holders
are stopped for the update and relaunched via an idempotent atexit
token built from structured identity (same contract as the gateway
pause/resume); receipts record serve_pause/serve_relaunch.
Desktop-owned backends keep the refusal (the app respawns what we
kill).
- dashboard_procs.py: the process scan is augmented with live ledger
rows, so profiled launches (`hermes --profile p serve ...`) that match
no substring pattern are finally visible to kill/respawn.
- main.py: `--status` now lists serve-mode backends too, tagged [serve]
— closing the #81564 status/stop asymmetry.
Salvage note: detection deliberately does NOT reuse #70742's psutil
cmdline-pattern scan (the argv-guessing class this campaign retires);
its resume-token lifecycle (atexit + idempotent flag) and don't-replay
guard shaped the relaunch contract here — credit @Tranquil-Flow.
Co-authored-by: Tranquil-Flow <66773372+Tranquil-Flow@users.noreply.github.com>
Enough1122 review points on #94417:
1. Precedence hazard fixed: the busy-guard assertion now locates the
rebind helper body precisely and asserts the guard INSIDE it, instead
of a 2000-char window with an (m and X) or Y precedence trap.
2. stored_session_id guarantee: documented + pinned — the gateway always
stamps it ('stored_session_id': session_key or "" in server.py), and
the rebind's typeof check refuses non-string/empty values, so an
unnamed rebuilt runtime is never adopted as lineage proof.
3. New third assertion pins that refusal contract.
Structural smoke tests remain structural by design; the behavior
contract for the rebind is exercised end-to-end by the model-switch
manual repro path — a vitest harness driving handleSessionInfoEvent is
the follow-up candidate noted in the reply.
A mid-conversation model/provider switch rebuilds the agent runtime. The
rebuilt runtime emits session.info (and all later events) under a NEW
explicit session_id while the pane still holds the dead one as its
active id — isActiveEvent is false for the same conversation from that
moment on, so view-scoped updates stop and the chat freezes until a
full resume (#93942 scenario B; backend even logs 'client should resume
the stored session', but the client never does).
Fix: when a session.info event lineage-matches the selected conversation
(sessionMatchesStoredId over stored_session_id) but carries a different
runtime id, adopt the new runtime id as the active session id — keeping
the durable selection untouched — so every subsequent isActiveEvent gate
keeps matching without a resume. Guarded: the old runtime must show no
live turn (not busy/awaiting/streaming) or the adoption is refused, so
an overlapping manual switch can never split one conversation across
two panes.
The existing compression-rotation path does not cover this case: it
fires when the SAME runtime's stored id rotates, while a rebuild
produces a NEW runtime with a NEW stored id.
Together with #94255 (tile reconcile on sessions.changed), closes
#93942.
Regression tests verified failing pre-fix on 41447a6d70.
Bot canonical chats open as workspace tiles (workspaceMode: 'bots') and
are deliberately hidden from $sessions/$messagingSessions, so the
sessions.changed transcript refresh skipped them twice over: it covers
only the main pane's selection, and its resolveSession() bails on hidden
sessions. A background delivery (bot-to-bot DM via bot_relay.deliver, a
cron run's output, another machine) therefore never reached an open bot
chat — the roster updated but the pane stayed stale until remount
(#93942 scenario A).
Fix: the sessions.changed tick now also reconciles every visible
workspace tile through a dedicated signature-gated path. Each tile
carries its own stored↔runtime id pair so no resolution step is needed;
per-tile signatures make no-change ticks free; busy tiles are skipped
(their own stream owns the view); closed/superseded tiles discard their
in-flight read.
Slice 1 of 2 for #93942 (scenario A only). Scenario B (stream re-key
after mid-conversation model switch) follows separately.
Fixes part of #93942
Continuity tests pin tail_mode=legacy (they assert the raw LLM text
terminates the stored summary; lean's verbatim-user appendix follows it by
design and the contract under test is mode-independent). The #57491
rotation fixture grows 200→2000 chars/message: at ~2.5K total tokens the
old fixture fit entirely inside lean's 10K tail floor, so the no-growth
guard correctly refused the rotation the test exercises.
The legacy tail budget scales as threshold×target_ratio, which was designed
around 128K windows at a 50% trigger (~13K tail). On modern big-window
models with raised thresholds it silently hoards: a 1M-window session at
threshold 0.85 keeps a 170K-token verbatim tail (255K soft ceiling) out of
EVERY compaction, so a 540K manual /compress lands at ~290K and every
subsequent turn re-ships the hoard. Nobody chooses this; it is an artifact
of the formula outside its design envelope.
Lean mode (#87326, compaction-v2) was built for exactly this and its recall
was validated in the before/after eval (evals/compaction/results/): clamped
2.5%-of-window tail (10K floor / 25K cap), continuity carried by the
upgraded summary (digests, anchor index, verbatim user messages,
session_search recovery pointers). This flips the DEFAULT to lean; explicit
'tail_mode: legacy' in config keeps the old behavior exactly.
Also fixes a latent bug the flip exposed: update_model() re-assigned the
LEGACY formula directly when recomputing budgets, silently reverting a lean
compressor to the hoard on every mid-session model switch. The recompute
now routes through the mode-aware tail_token_budget property (regression
test included).
Surfaces: context_compressor.py defaults + getattr fallbacks, agent_init
parse default, DEFAULT_CONFIG, gateway _CACHE_BUSTING_CONFIG_KEYS gains
compression.tail_mode (mode changes now evict cached gateway agents like
target_ratio changes do), user + developer docs. Tests: 3 new default
contracts, legacy tests pinned explicitly, feasibility-skip scenario pinned
to legacy (under lean its payloads correctly become compressible).
E2E counterfactual (real imports, 1M window @ 0.85):
main default: legacy, tail 170,000 (ceiling 255,000)
head default: lean, tail 25,000 (ceiling 37,500)
head legacy: 170,000 (opt-out intact)
update_model to 400K: 10,000 (lean preserved across switch)
De-flakes tests/hermes_cli/test_ssh_ownership_endpoint.py, which failed CI
twice on PR #95563 with teardown-time daemon-thread excepthook crashes — a
different test in the file each attempt, always green in isolation. Root
cause: three PROCESS-GLOBAL monkeypatches leaked into every other thread
sharing the per-file worker:
- monkeypatch.setattr(web_server.os, 'stat', ...) — web_server.os IS the os
module; any daemon thread from an earlier test that stat()ed during the
patch window got the fake 2-field stat and died in its excepthook, which
fired at interpreter teardown.
- monkeypatch.setattr('builtins.open', ...) — same class, worse blast radius.
- monkeypatch.setattr(web_server.sysconfig, 'get_paths', ...) — sysconfig is
process-global too.
Fixes, none of which weaken coverage:
- replaced-runtime test: a REAL tmp_path purelib whose recorded inode
deliberately mismatches (st_ino + 1) — real os.stat, same code path.
- readonly-purelib test: chmod 0o555 on the real directory instead of an
open() interceptor — exercises the genuine OSError branch (root-skipped,
where mode bits aren't enforced).
- sysconfig patches swapped for a SimpleNamespace on the web_server module
attribute — module-scoped, invisible to other threads.
Verified: 14 consecutive full-file runs green; sabotaging
_ssh_runtime_intact to always-True still fails 2 tests (coverage intact).
Reverts the interpreter-anchor halves of #95131 and #95478 (the anchor
module, its doctor check, and the update-time refresh). On real Macs the
anchored real-file copy of the uv interpreter dies in dyld: its LC_RPATH
(@executable_path/../lib) resolves into venv/lib/, which holds no
libpython — bricking EVERY hermes command including update and doctor
(#95425), and the re-pointed python3 aliases lost the stdlib
(ModuleNotFoundError: encodings, #95541). Linux CI could not catch this:
the fixture interpreters were one-byte fakes with no dynamic linking.
Kept: managed_uv._macos_sign_managed_python (#82529, @notkisk) — the
identifier-DR signing of repair generations is independent of the anchor
and unaffected by the dyld issue (it signs binaries IN PLACE in their
store, where their rpath is valid).
Added: doctor's check_macos_tcc_anchor_removed() heals venvs the anchor
already converted — restores bin/python to a symlink at the recorded
source (the anchor's own marker file) and re-points aliases; prints the
manual one-liner if the heal itself fails. Users whose CLI is fully
bricked can run the workaround from #95425 directly.
Re-land criteria: a dylib-complete anchor design (bundle libpython or
rewrite LC_RPATH), verified on macOS hardware BEFORE merge. Credit to
@kim-miram (#95358), @kokhlo (#95476), @zengzheqing (#95551) for the
forward-fix diagnoses that mapped the failure, and to the #95425/#95541
reporters.
Addresses review feedback on the regression test. The test previously parsed
the update_cmd.py AST to assert that each auto-restore call site cleared the
destination's sidecars before copying. That bound the fix to source text rather
than behaviour, and would break on unrelated refactors.
Extract _restore_state_db_from_snapshot(state_path, snap_state), which performs
the clear -> copy -> verify sequence as one unit and returns whether the
restored file passes its integrity check. Both auto-restore paths now call it,
so the ordering is guaranteed by construction instead of by inspection, and the
two byte-identical blocks collapse to a single call each.
The regression test now exercises that helper directly against a database that
still owns a hot WAL: removing the clear from inside the helper fails it with
201 rows where 400 were expected, so the guard remains bound to behaviour.
Also covers the two failure modes the callers already handle: a snapshot that
does not survive the copy returns False, and a missing snapshot raises OSError.
The post-update integrity guard (#68474) restores state.db from a pre-update
quick snapshot with a plain shutil.copy2, at both auto-restore sites: the
ZIP-update path in _update_via_zip and the git-pull path in _cmd_update_impl.
The snapshot image is produced by backup._safe_copy_db through sqlite3.backup(),
so it is already checkpointed and owns no WAL. That is precisely why
backup._EXCLUDED_SUFFIXES refuses to ship -wal/-shm/-journal inside a snapshot:
"shipping the live WAL / shared-memory / rollback-journal alongside would pair a
fresh snapshot with stale sidecar state and produce a torn restore on the next
open." The backup side excludes sidecars for that reason; the restore side never
cleared the destination's.
copy2 replaces only the main database file. A state.db-wal belonging to the old,
corrupt database survives the copy and is replayed over the fresh image on the
next open. The restored file then passes PRAGMA integrity_check while serving
the discarded database's contents, so _restored_ok reports valid and the CLI
prints "Auto-restored from snapshot" over data the user has lost. The first
subsequent checkpoint folds the stale WAL in permanently.
A hot -wal is reachable at exactly this moment: a second Hermes holder the
updater's drain did not stop, or the crash that corrupted state.db in the first
place, which is the very trigger for this code path.
Clearing the destination's sidecars is safe here specifically -- they belong to a
database the caller has already declared corrupt and is about to discard.
Contrast preflight_db_writability, which correctly refuses to delete a live WAL.
Reproduced against real SQLite: restoring a 400-row snapshot over a database
with a hot WAL yields 0 of the 400 rows, all 201 visible rows coming from the
old WAL, with integrity_check reporting ok.
The last piece of the macOS permissions campaign (#52010 follow-up): macOS
prompts per-folder (Desktop, then Downloads, then Documents, ...) as the
agent touches each one — a drip-feed of dialogs on first use. ONE Full Disk
Access grant covers all of them permanently, and with the stable signing
identities merged this week it survives every update. Nothing in Hermes
taught users that.
- hermes doctor: check_macos_full_disk_access() — prompt-free probe (the
FDA-gated TCC db dir returns EPERM without a dialog; TCC only prompts on
protected-CATEGORY paths), reports granted state or prints the one-switch
setup with the Privacy_AllFiles deep link.
- hermes setup: same probe at the end of onboarding — the moment users are
primed to do system setup — silent when already granted, indeterminate,
or non-macOS.
- docs: desktop.md TCC section now leads with the one-switch guidance.
- 7 tests (granted / denied / indeterminate / non-macOS, both surfaces).
Two fixups the #75785 review required before landing:
- grep fallback no longer uses --exclude-dir for protected dirs: grep
matches exclude-dir globs against BASENAMES anywhere in the tree, so
--exclude-dir=Downloads silently skipped every nested directory named
Downloads (a repo's own Downloads/ included). Protected-dir searches now
route through find's path-scoped -prune (same traversal-prevention the
find backend uses) feeding grep via -exec. Regression test proves a
nested work/repo/Downloads/notes.txt is still found while ~/Downloads is
not (live filesystem, real find+grep).
- exclusions gated on env.is_local (new BaseEnvironment flag, True on
LocalEnvironment): sys.platform/Path.home() describe the controller, not
the execution host — a macOS controller driving a Linux SSH/container
backend must not prune the remote's unprotected Downloads. Environments
without the flag default to local semantics (warning-carrying skip,
never data loss).
Both sabotage-verified: restoring basename --exclude-dir fails 2 tests.
Follow-up on the cherry-picked #82644: the (st_dev, st_ino) snapshot of
site-packages does not survive contact with ext4 — a recreated directory
routinely REUSES the freed inode, so the exact reported repro
(rm -rf venv && uv venv) passed the intact check undetected. Proven live
during salvage: the E2E's replaced venv came back with the identical
inode and runtimeIntact stayed true.
Primary identity is now a marker file written into site-packages when
the SSH owner nonce activates: it deterministically dies with the old
tree on ANY replacement (same or different Python version) and survives
in-place pip/uv installs (no false stales). The stat snapshot remains as
the fallback for read-only site-packages, where it still catches
cross-device moves and version-bump path changes. Client classifier
semantics unchanged: only an explicit runtimeIntact:false rejects, so
older remotes stay compatible.
Three new tests: recreated-venv-with-reused-inode (the live-proven
case), in-place-install stays intact, read-only fallback arms the stat
tier.
Folds review findings: surface failed_deletes in CLI and gateway
/rollback output (new gateway.rollback.failed_deletes locale key, 17
locales), emit skipped_oversize on the nothing-to-restore early return
too, document all three report keys in the restore() docstring, and pin
the failed_deletes contract from both sides in tests.
Follow-up to #95491. The restore result dict had two inconsistent
reporting surfaces: skipped_oversize was only present when non-empty
(unlike skipped_user_edits), and failed_deletes was filtered from
restored_files but never surfaced to the user at all (debug-level log
only). Both are the same silent-omission class #95491 fixed for
oversize files; this completes the cleanup.
Record correction: the previous commit's message says the async flavor
offloads mark_suspect via asyncio.to_thread — it does NOT (and must not).
The mark is deliberately inline on the event loop: running it
synchronously guarantees mark-happens-before-BoundedResult-return and
mark-before-on_abandon-cleanup (cleanup is ensure_future'd and cannot
start until the next loop tick). An offloaded mark would race both.
The trade-off is that a slow adopter mark_suspect would block the loop
(measured: a 2s mark stalls every coroutine for 2.003s), so the adopter
contract is now explicit in the Protocol docstring and at the async call
site: mark_suspect must be cheap, non-blocking, lock-free; expensive
recycle work belongs in ensure_healthy.
New pins so the negotiated semantics can't silently regress:
- test_sync_mark_happens_before_on_timeout (the review-round ordering)
- test_async_mark_happens_before_on_abandon_cleanup (the scheduling
invariant an offloaded mark would break)
- test_sync_completion_never_marks_backend (sync counterpart of the
async completion test)
- async timeout marks once with a label-carrying rounded-timeout reason
- completion never marks
- sync flavor marks on timeout
- non-adopting backends keep the real timeout result
- a raising mark_suspect cannot eat the timeout or the label
On macOS with TCC (Transparency, Consent, and Control), os.getcwd()
raises PermissionError: [Errno 1] Operation not permitted — not
FileNotFoundError — when the process CWD is under a protected location
(~/Documents, ~/Desktop, ~/Downloads) and the calling process lacks
Full Disk Access.
_safe_getcwd() only caught FileNotFoundError (deleted CWD), so the
terminal-tool cleanup thread, which calls _get_env_config() →
_safe_getcwd() every 60 s, logged a full stack trace on every tick.
This accumulated hundreds of MB of noise in mcp-stderr.log (observed
184 MB on a single-day session) without breaking functionality — the
cleanup thread's outer try/except swallowed the exception, but
exc_info=True kept emitting the traceback.
Fix: add PermissionError to the existing except clause so the fallback
chain (TERMINAL_CWD → $HOME) runs, matching the existing pattern for
deleted-CWD recovery (#17558). Complements #66306, which handles
PermissionError from subprocess.Popen(cwd=...) for an inaccessible
configured cwd on Linux; this handles the distinct case where the
live process CWD itself is TCC-blocked.
Tests cover: PermissionError fallback to $HOME, TERMINAL_CWD priority,
FileNotFoundError regression, happy path unchanged, and unrelated
OSError (NotADirectoryError) still propagating instead of being
swallowed.
The verify_fresh_state verdict now carries an optional human hint; assert
the decision + additive-field absence instead of the exact dict shape
(same contract loosening as test_computer_use_delivery_ladder.py).
Teknium