Keep the shouldQuitOnLastChatClosed decision matrix and the activateWindow
restore -> show -> focus test. The two scenario tests rebuilt main.ts state
in local variables and then asserted it (tautological), and the remaining
activateWindow cases only pin no-op paths.
The salvaged fix added decideSecondInstanceAction, which re-implemented the
create / defer / focus branches ensureMainWindow already owns, plus a
shouldQuitOnAllClosed helper that only restated the existing
window-all-closed condition. Route second-instance through
ensureMainWindow(focusWindow: activateRestoredWindow) like the tray path,
keep one rememberLog line so a relaunch stays visible in desktop.log, and
inline the window-all-closed check back.
activateWindow wrapped restore/isVisible/show/focus in try/catch and
typeof-probed each method; the window is already checked with
isDestroyed() and Electron's calls do not throw here, so type the window
with required methods and call them directly. Also fixes a comment typo.
#132141 landed the shared outcome helper; render the write_file refusal through it
so the stub reads like the other marked stubs ("[write_file] a.md FAILED: ...") and
fold the test into the TestSummarizeToolResultOutcome tables (one failed row, one
success row) instead of a separate parametrized function.
_sum_write_file always rendered "[write_file] wrote to PATH (N lines)" from
the call's args, so a write the stale-write guard refused
({"error": "Refusing to overwrite ...", "stale_write_blocked": true}) - or
any other write_file error - compressed into the same stub as a successful
write, and the post-compaction agent reported a change that never landed.
Read the result's error first and say FAILED with a bounded preview.
Salvaged from #131247 (write_file half only; the patch-tool half is covered
by the templated-stub outcome fix in #132141).
Gate finding (2ab Low, toothless): removing the pidns stamp from the
flock holder record or from the compression lock holder string left every
kept test green, since the tests only fed hand-built stamped strings to
the readers; a dropped record stamp silently brings back the
foreign-namespace flock break. Assert both writers stamp our namespace in
the existing flock-record test.
Gate finding (2ab Low, toothless): dropping the "don't cache a failed
lookup" guard in _local_pid_namespace left the kept tests green because
the failed-lookup rows never exercised recovery. After those rows, let the
resolver succeed and assert a stamped dead flock record breaks again.
Gate finding (2ab Low, toothless): flipping holder_pid_checkable to probe
unstamped holders again (unstamped_checkable=True) left every kept test
green, so the stack's deliberate behaviour change for pre-upgrade TTL rows
was unpinned. Assert in the existing sibling-namespace test that an
unstamped dead-pid turn lease is not reclaimed by a pidns-stamped
contender before its TTL.
Gate quality findings (Low): hermes_state_pidns is stdlib-only (os, re,
sys, typing), so the function-local imports guarded no cycle — verified
with fresh-interpreter imports of hermes_state, hermes_state_common,
agent.conversation_compression and agent.turn_facade_lease. Import it at
module level. Compare the namespace stamp as the str it already is,
make recorded_namespace private (no external user), and replace the
STRICT/LEGACY explanations repeated at each call site with one-line
pointers to the hermes_state_pidns module docstring that owns them.
conversation_compression.py stays at its base line count.
Gate 2c + quality finding (Low, convergent): _parse_pid_namespace_link was
split out "so the parsing can be tested" but no test uses it, leaving a
one-call helper with a stale docstring. Inline the regex at its only
call site.
Gate 2c finding (Low, regression): _qualify returned False for any record
once our own /proc/self/ns/pid read failed, before consulting the
unstamped policy. An unstamped (pre-upgrade) flock record never expires,
so on such hosts main's #100108 orphaned-lock break was silently lost.
Check the unstamped policy first: the namespace lookup only matters when
there is a stamp to compare against. STRICT TTL rows are unaffected
(unstamped is still False there).
Same content (stamp rule, strict vs legacy unstamped policy, unsupported
platforms, uncached failed lookup) in a third of the lines. Note for
rollout: on upgrade an unstamped turn lease/compression lock left by a
pre-upgrade process is no longer reclaimed early; it waits out its TTL
(<= 300s), once.
Keep one behavioural test per probe site: a sibling-namespace session
turn lease is not stolen (hermes_state lease/lock gate), and a
foreign-namespace flock holder record is not broken while same-ns and
unstamped dead records still are (hermes_state_common). The local
namespace is pinned via _LOCAL_PID_NS so both run on every host rather
than only on Linux. The matrix/parse/helper tests re-asserted the same
rule through private seams.
A pid=<n> holder (compression lock, session turn lease, flock holder
record) is namespace-relative: process siblings sharing one state.db from
different PID namespaces (gateway + webui containers on one volume,
systemd PrivatePIDs=) see disjoint PID sets, so a sibling's live pid=
reads as absent to the namespace-blind probe and its unexpired row is
reclaimed under it. For a turn lease that ends the sibling's in-flight
turn ('another Hermes process took over this session'); for a compression
lock it splits the lineage; for a flock record it breaks a live lock.
Stamp every structured holder with the writer's PID-namespace identity
(/proc/self/ns/pid inode, via hermes_state_pidns) and qualify each probe:
* TTL-bounded rows (compression locks, turn leases): strict — a foreign
or unstamped holder is never probed, it defers to its own expiry (at
most the remaining TTL). A false defer self-heals at TTL; a false
reclaim loses a reply. Strictness also stops the steal during a
mixed-version rollout as soon as the reader restarts (the reader is
the long-lived side of a container pair that restarts at different
times), before every writer has restarted to stamp.
* No-expiry flock holder records: legal rollout — a foreign-namespace
record is never probed, while an unstamped one keeps main's behavior
(refusing to probe it would permanently disable orphaned-lock cleanup,
the #100108 path). Protection begins as writers restamp.
Probe doubt already defers for the same reason (a wrongly-kept lease
self-heals; a wrongly-stolen one does not), so this extends an existing
contract rather than adding a new one.
(cherry picked from commit 4b1a29f63b8e6dec0a2724b5c2c4fac2bcc31c0e)
Gate finding BN03.final.2ab (Low): the raw-text branch
(`not error and content.startswith("BLOCKED")`) had no test -- every
refusal row was a JSON envelope carrying `error`, so disabling the
branch stayed green. Make the write_guard row the raw "BLOCKED: ..."
string (the shape tools/file_tools_write_guards.py builds before any
envelope); the envelope path stays covered by the terminal rows. The row
goes red when the fallback is removed.
Gate finding BN03.final.2ab (Low): the `status == "blocked"` disjunct in
_summarize_refused_tool_result had no test -- dropping it left every row
green. It is the only reason the terminal workdir guard's refusal
(_error_json(_validate_workdir(...), status="blocked"), error text
"Blocked: workdir contains ...", not upper-case "BLOCKED") is summarized
as "BLOCKED, not run" instead of "ran `ls` -> exit -1". Add that real
producer as a row in the existing parametrize; it goes red when the
disjunct is removed.
Gate finding BN03.r1.2c (Medium): status == "blocked" alone was treated
as a refusal, but successful kanban results use it as a task state --
kanban_block (and kanban_create with initial_status="blocked") return
{"ok": true, "status": "blocked", ...}. Those summarized as
"[kanban_block] BLOCKED, not run", so after pruning or fallback
compaction the model would read its own successful block/create as
refused and might repeat it (e.g. a duplicate card).
Every real refusal envelope carries a non-empty error string
(terminal _error_json / _blocked_json, tool_error); the kanban success
payloads have none. Require it. Adds the kanban_block success payload
as a negative row in the existing parametrize.
Gate finding BN03 quality (Low): after the test trim no kept case reached
the status == "pending_approval" branch of _summarize_refused_tool_result,
so deleting it left the suite green. Add a pending row (real
terminal_tool._error_json producer) to the existing parametrize, with an
expected-substrings column so the row asserts "awaiting the user's
approval, not run" while the denial rows keep asserting the no-consent
note. Still two test functions.
Gate finding BN03 quality (Low): the parametrize ids list sat apart from
the _refusals() rows, so adding or reordering a case meant editing both
by hand. Return pytest.param(..., id=...) rows instead and drop ids=.
No behaviour change; the prune test reads the first row's .values.
Keep the parametrized denial test (real approval-gate and write-guard
producers) and the prune-across-passes test; both go red without the fix.
The pending-approval and ordinary-result cases were exact-string checks
that add little beyond those two.
Phase-1 pruning and the fallback summary describe each old tool result with
a per-tool one-liner. For a call an approval or write guard refused, those
read as done: a denied `rm -rf build` became "[terminal] ran `rm -rf build`
-> exit -1, 1 lines output", a denied protected write became
"[write_file] wrote to AGENTS.md (2 lines)", and the "The user has NOT
consented ... Do NOT retry" instruction was dropped. Denials are 300+ chars,
over the 200-char prune floor, so this hits every long session.
Summarize a refused result (BLOCKED error, status blocked/pending_approval)
as "BLOCKED, not run" and keep the no-consent note, for every tool. Like the
clarify summary (#128346), it stays below _PRUNE_MIN_CHARS so later passes
keep it. Other results are summarized exactly as before.
(cherry picked from commit fd506bbccd3df8783dce0c0a1895f1aaf1585cb4)
Gate finding 2ab: the #131412 escape requires the anchored region to hold
tool_calls, but no test pinned that sub-gate — dropping it left every test
green. Add a row to test_assistant_anchor_still_binds: an over-ceiling region
of plain text rows (no tool_calls) with allow_split_turn=True must still
anchor to the older turn's closer. Mutation-proved: removing the tool_calls
condition turns this row red.
The _reasoning_heavy_small_turn docstring referenced an internal review
tag ("BN04 2c") that means nothing upstream; point at the issue instead
(gate fold2 quality finding). Comment-only change.
The #131412 escape gate summed _estimate_msg_budget_tokens(m) with its
default charge_stale_thinking=True, so every older assistant row was
charged for reasoning the wire never carries, and the native-Anthropic
budget was ignored. On reasoning routes that strip stale thinking the
gate fired on regions under the soft ceiling on the wire, dropped the
last visible reply from the tail and reopened#29824 (probe: no
reasoning cut=2 reply kept; 1200-char reasoning_content on two older
tool-call rows cut=5 reply dropped; main keeps cut=2).
Factor the walk's per-row pricer out of _walk_tail_budget into
_tail_row_pricer and use it for both the walk and the gate, so the
escape agrees with the #84371 accounting. The existing anchor test now
parametrizes the reasoning-heavy shape; it is red without this change
(cut 5 != 2) and green with it. (BN04 2c High; quality/reuse Medium.)
Both new tests hedged: `cut <= older_closer_idx + 1` also passed when
the closer itself was summarized (the very anchor loss it guards), and
`cut > active_user_idx + 2` plus a tail-smaller-than-turn check passed
for a cut that kept nearly the whole over-ceiling turn. Pin the exact
cuts the code produces: the closer aligned back before its tool group,
and the walk's group-aligned cut two groups before the anchored nudge
(BN04 quality Lows).
The comment above the #131412 escape gate said it fires whenever the
anchored region is over the soft ceiling, but the gate also requires
tool_calls in that region. Name both conditions so the comment matches
the code (BN04 gate finding, Low).
The over-ceiling escape added for #131412 measured the anchored region with
_walk_tail_budget from the anchored cut, which reports "over the ceiling"
whenever any single region after a visible reply exceeds the soft ceiling
(tiny budgets, one big tool output). That dropped the last visible assistant
reply from the tail and reopened#29824
(test_reporter_repro_long_tool_run_after_visible_reply,
test_oversized_tool_output_does_not_strand_reply).
Escape only when the anchored region actually holds tool-call bodies to
reclaim (mirroring the #80449 user-split's tool_calls gate) AND its raw
token sum exceeds the soft ceiling. The textless oversized active turn is
still bounded; benign #29824 shapes keep the reply anchor.
Co-authored-by: liuhao1024 <sunsky.lau@gmail.com>
The #80449 oversized-turn split only relaxes the USER anchor, and only
when that anchor actually pulls the cut back (user_anchored_cut <
cut_idx). In a long agentic turn whose assistant rows carry only
tool_calls, the latest user message sits inside the token-budget tail,
so the user anchor does not move, the split never fires, and
_ensure_last_assistant_message_in_tail then pulls the cut back to the
newest TEXT-BEARING assistant — which is the PREVIOUS turn's closer.
The whole oversized active turn rides the protected tail, the middle
collapses (middle_window_tokens=0), and every manual/auto compress
no-ops in ~30ms with failure_class=no_progress: the session is
permanently uncompressible (#131412, same wedge class as #80449/#105663).
Give the assistant anchor the same bound the user anchor got: when the
anchored region exceeds the tail soft ceiling (measured with the walk's
own accounting, #84371) and mid-turn splits are allowed, keep the walk's
tool-group-aligned cut instead of anchoring. Rolling micro-compaction
(allow_split_turn=False) keeps the old anchoring behavior, and the
N-user promise (#70250) is untouched.
(cherry picked from commit 9259ea925c1eb95b3da7bc2fb5ad4499c6afd46b)
activeWorkByWebContents dropped an entry only on 'destroyed'. A renderer
that dies keeps its webContents, and some render-process-gone exits never
reload, so its last in-flight count stayed in the merged picture until
quit: stream throttling stayed off and, now, the 'while-working' blocker
stayed held. Forget the entry on render-process-gone too; a reloaded
renderer re-reports its live turns on mount.
The mode union and its validator were written three times: power-save.ts,
the renderer store and global.d.ts. The store and the preload typing now
import KeepAwakeMode and parseKeepAwakeMode from electron/power-save (no
electron import, as pool-limits-setting already does), so a new mode
cannot drift between main and the picker.
The three-way keep-awake setting shipped labels in en, ja, ru, zh and
zh-hant only, so German, French and Spanish showed English "Off / While
working / Always" beside a description that still promised the old
always-on behaviour. Translate the labels and the new description, add
"while working" / "turn" to the settings-search keywords, and register the
three new keys in locales/_keys.desktop.json for language-pack validation.
运行时 / 執行時 parse as "runtime" at first glance; 运行期间 / 執行期間 are
the unambiguous "while a run is in progress". Review nit from #102209.
(cherry picked from commit 176c77ea693af9c2d398c63fac2c7f1857afed5d)
"Keep computer awake" was a boolean: off, or a machine-global power-save
blocker held for as long as Hermes Desktop is open. That is the wrong shape
for its own stated purpose. An unattended overnight run dies when the
machine sleeps unless the user predicted, before starting, that they would
need the toggle — and a user who did flip it leaves a laptop pinned awake
all week, including the idle hours after the run finished (#101991).
Main already knows, live, whether any turn is in flight:
`mergeActiveWork(activeWorkByWebContents.values()).count > 0` is the exact
expression that drives stream throttling. This change wires the blocker to
that same signal behind a three-way mode:
off never hold the blocker (unchanged default)
while-working hold it only while a turn is in flight; release at idle
always hold it whenever the app is open (what `on` used to mean)
Both persisted copies migrate in place. The renderer's localStorage boolean
(`hermes.desktop.keepAwake.v1`) and main's `keep-awake.json` `{ on }` are
read once and mapped `true -> always`, `false -> off`, so an existing user
sees exactly the behaviour they had. The IPC channel keeps its name and
accepts either the mode or the old boolean. An unrecognised saved value is
refused rather than trusted and falls back to `off`.
No new listener, timer, or IPC channel: `updateStreamThrottleFromActiveWork`
computes the merged picture once and hands it to both consumers, and the
cold-launch apply reads the mode instead of the flag. No trailing delay
between back-to-back turns — a sub-second release cannot let a machine
sleep, and `powerSaveBlocker` start/stop are cheap.
The policy is three pure functions in power-save.ts (parse, read, wanted)
so it is testable without the main process; main only wires them.
(cherry picked from commit c6dab931a2891ea7026eea2c07c271bcf48446b9)
Gate finding BN21 r1 quality (Lows):
- The race/eager-import snippets put the repo on sys.path three ways; PYTHONPATH and
cwd already do it, so drop the in-snippet sys.path.insert and the REPO_ROOT templating.
- The deadlock explanation was spelled out in the module docstring, again in the
__getattr__ docstring, and a third time in the test module. Cut it to a 2-line
pointer to the test, and give __getattr__ a one-line docstring that describes what
it does (cache the name in globals()).
- Unquote the dict[str, str] / list[str] annotations: requires-python is >=3.11.
No behaviour change. Both kept tests still pass on HEAD and fail with BASE
tools/connectors/__init__.py.
Gate finding BN21 r1 2c (Medium, flaky): the race snippet's own purge() pops
tools.connectors* from sys.modules while sibling threads are mid-import, so
importlib also raises AttributeError ('NoneType' object has no attribute
'__dict__') for tools.connectors.tool. That is a harness artifact, not the
deadlock, yet the old `!= "KeyError"` filter kept it and the fixed tree went
red intermittently (8/160 runs under 8x parallel load).
Filter on the bug class itself. Measured: HEAD 30/30 green serial and 160/160
under the same load; BASE tools/connectors/__init__.py 30/30 red
(_DeadlockError in every run).
Drop the __all__/dir() surface check: it is a change-detector over the export
list and the public names are already exercised by the existing connector
tests (from tools.connectors import ..., monkeypatch of
tools.connectors.connectors_available). The no-eager-submodule test and the
package-vs-submodule race test cover the deadlock invariant.
Co-authored-by: Rostislav Přidal <5829155+rpridal@users.noreply.github.com>
`tools/connectors/__init__.py` imported all five of its submodules eagerly, so a
thread importing the package held the package's module lock while reaching for a
submodule that another thread already held. Python 3.14 aborts one of the two
with:
_frozen_importlib._DeadlockError: deadlock detected by
_ModuleLock('tools.connectors.tool')
which is the failure in the Group Chat hosted-room worker startup path
(`gateway/run_startup.py::_ensure_hosted_room_worker` -> `import tui_gateway.server`
-> `methods_connectors._install_update_hook` -> `from tools.connectors import
operation`) while the tool registry's discovery scan imports
`tools.connectors.tool` from another thread.
Exporting the cross-package surface through a module-level `__getattr__` (PEP 562)
removes the parent->child edge entirely: the package `__init__` now imports no
submodule, so it cannot hold its lock while waiting for one. Attribute access,
`from tools.connectors import x` and `__all__` are unchanged.
Measured on Python 3.14.7 by racing `import tools.connectors` (3 threads) against
`import tools.connectors.tool` (3 threads), 40 iterations each:
| tree | outcomes |
| --- | --- |
| before | 120 `_DeadlockError`, 0 successes |
| after | 0 `_DeadlockError`, 0 threads left alive, `import tools.connectors` loads no submodule |
Regression tests run in subprocesses so they assert on a pristine `sys.modules`
(`tests/tools/test_connectors_import_deadlock.py`).
Co-Authored-By: Paperclip <noreply@paperclip.ing>
(cherry picked from commit 1803f53b1ed34487ff6a60cdbde915726be21338)
Gate finding (quality, Low): the ValueError branch added for malformed
Origin authorities copy-pasted the origin_mismatch reason f-string, so the
format lived in two places. Map the parse failure to parsed=None and let
the existing mismatch return handle it. Behaviour unchanged (still fails
closed with 4403 origin_mismatch).
The "[::1].evil.test" rows only raise in urlparse on Python 3.14; on the
3.11-3.13 interpreters the repo supports, the media proxy answers 403 and
the custom endpoint saves (200), so those rows fail outside 3.14. Drop
them; the "[::1" and "[not-an-ip]" rows still pin the fix.
Replace the new 111-line test_web_server_ws_origin.py (a full
login/ticket/heartbeat matrix that re-tests existing auth) with two
malformed-origin rows on the existing gated cross-site origin test.
urlparse raises ValueError on bracketed authorities like "http://[::1" or
"https://[not-an-ip]:80". Three user-input boundaries let that escape:
the WebSocket Origin guard, /api/media/proxy and the custom-endpoint
base_url save, turning a bad header/param into an unhandled 500 instead of
a clean rejection. Fail closed there: 4403 origin_mismatch for the WS
guard, HTTP 400 for the two routes.
Salvaged from #131323 (net diff; the interim loopback-origin exemption the
PR added and then removed is squashed out).
BS04 gate (quality, Low): the fence test's `stale` param mixed False/True
with a "validation_error" sentinel that took a separate early-return
branch calling compress_now directly — two tests behind one function.
Share the agent/SessionDB fixture via a helper, keep `stale` a bool, and
move the raising-callback lease-release check into
test_manual_compress_releases_lease_when_snapshot_check_raises. The file
stays at two tests.
BS04 gate (quality, Low): the stale-reject lease.release() sat inside the
try/except BaseException, so a raising release() ran release() a second
time and the guard read as covering the abort path. Guard just the host
callback (release + re-raise), then branch on its result outside.
BS04 gate (2c, Low): the post-admission stale-snapshot abort released the
lease and returned without emitting the aborted-attempt telemetry, so a
stale manual /compress never reached the compression_attempt log line or
the success/failed counter (the thread's pending attempt was left open).
Emit `snapshot_stale` like the other abort paths, and assert the log line
in the stale cases of the existing fence test.
Keep the headline invariant (stale full and 'here 2' snapshots are
rejected after lease admission, benign compress still commits, a raising
validator releases the lease) and drop the empty-summary case, which
covers a pre-existing path, plus the duplicate 'here 2' validation error.
