Two paths let a pre-existing system Node win over the Hermes-managed one.
The desktop backend spawn built its managed-Node PATH entry as
`<home>/node/bin` only. That is the POSIX layout install.sh produces;
install.ps1 unpacks portable Node straight into `%LOCALAPPDATA%\hermes\node`
with node.exe at the root and no `bin\`. On Windows the entry therefore
pointed at a directory that does not exist, and the backend fell through to
whatever Node was already on PATH.
main.ts already had the correct platform-ordered list, behind a "keep this
in sync with iter_hermes_node_dirs()" comment on a second copy of the rule.
The two copies had drifted. Export the ordering from backend-env.ts and have
main.ts consume it so there is one source of truth on the Node side (the
Electron main process cannot import hermes_constants.py, so a mirror is
unavoidable — but one mirror, not two).
install.ps1 appended the node dir to the persisted User PATH instead of
prepending it. The session PATH was already prepended correctly, so this only
bit later processes: any shell opened after install, and a standalone
hermes-setup.exe run that inherits User PATH rather than a curated env, both
resolved a system Node ahead of the bundled one.
Not a bug, for the record: update.rs's prepend list omits the same Windows
root, but it inherits PATH from the desktop, which supplies the correct
entries — so it is redundant rather than broken, and no installer rebuild is
needed for this fix.
Tests: managed dirs lead with the platform-native layout while always
offering both shapes, empty without a home, and every managed dir outranks
the inherited PATH on darwin and win32. The three existing tests that pinned
`entries[1]` by index asserted the old single-dir shape and now assert the
relationship instead.
install.ps1 has no behavioral test here: CI has no PowerShell host, and
AGENTS.md bans source-reading tests (the neighbouring
test_install_ps1_node_path_for_npm.py predates that rule).
The npm 12 requirement (f88ed6c717) strands every system-Node install:
no shipping Node bundles npm >=12, engine-strict makes EBADENGINE fatal,
and the recovery in npm_engine.py refuses to touch a system npm — so
'hermes update' leaves the install in a mixed state (updated code, stale
Node deps, no TUI/web/desktop rebuild) with only a manual-fix hint.
Instead of modifying the user's toolchain (still never done), the
EBADENGINE recovery now provisions Hermes' own managed Node tree under
$HERMES_HOME/node — the same pinned-nodejs.org path install.sh and
install.ps1 use — upgrades THAT npm into the required range, and hands
the caller the managed npm for its single retry.
- hermes_constants.bootstrap_hermes_managed_node(): cross-platform
provisioning (POSIX via node-bootstrap.sh _nb_install_bundled_node,
Windows via the existing portable-zip download); reuses a healthy tree.
- node-bootstrap.sh: HERMES_NODE_SKIP_LINKS=1 skips the ~/.local/bin
node/npm/npx symlinks so the private tree never shadows the user's
own toolchain on PATH.
- maybe_repair_npm_engine() now returns the npm path to retry with
(managed-in-place upgrade or freshly provisioned runtime); both call
sites retry with the returned path and put the managed tree first on
PATH so npm lifecycle scripts resolve the managed node.
- Node-only mismatches on a foreign npm are now also recoverable (the
managed tree ships a supported Node); on a managed npm they still
correctly decline.
E2E (real download, temp HERMES_HOME): provisioned node v22.23.2,
upgraded bundled npm 10.9.4 -> 12.0.2, system npm byte-identical after,
no ~/.local/bin links re-pointed, healthy-tree reuse in 0.05s.
After the test-suite prune, the Python slices (~2.3m each) are no longer
CI's critical path — Desktop E2E (5.2m, the longest job) and the Docker
build are, and both run on every python-lane PR even when the diff never
leaves tests/. Neither consumes the test suite: Playwright drives the
built app + hermes serve backend, and the image copies installed code.
New python_prod lane = python minus tests-only diffs. e2e-desktop and
docker gate on it; every pytest/lint lane keeps gating on python.
Fail-open contract preserved: .github/ changes and empty diffs set
python_prod=true, and runner infrastructure (scripts/run_tests.sh,
run_tests_parallel.py) is deliberately NOT tests-only since a bad
runner edit can mask real failures.
Replay over the last 231 main commits: 39 (17%) would skip both jobs,
cutting their critical path from ~8m to ~3m. E2E-verified through the
real script entrypoint (tests-only/prod/mixed/fail-open) + 83 tests/ci
green.
Setting prompt_caching.cache_ttl to a falsy value (false, null, off,
disabled, no, none) now fully disables prompt caching instead of
being silently ignored.
The disable propagates through anthropic_prompt_cache_policy() (early
return when _cache_disabled flag is set) and restore_primary_runtime()
(override after snapshot restore), so it survives /model switches and
fallback re-derivation — the gap that caused #56105 to be reverted in
#56126.
Salvage of #33555 by @BB-light, with model-switch/fallback survival
gap fixed on top.
Co-authored-by: BB-light <BB-light@users.noreply.github.com>
setup_path() only wrote a 'hermes' launcher into the command-link dir,
even though pyproject.toml declares three [project.scripts]:
hermes, hermes-agent (run_agent:main), hermes-acp (acp_adapter.entry:main).
Fresh venv installs (the common case on macOS/Linux) leave
~/.local/bin/{hermes-agent,hermes-acp} empty, so external tools
expecting 'hermes-acp' as a standalone command (documented as
first-tier supported in website/docs/user-guide/features/acp.md)
fail to find it after a fully successful install.
Loop over the three console-script names, writing a shim per entry
that exec's the venv interpreter with the right checked-in
entrypoint. --no-venv keeps the old single-shim behaviour since
it does not manage the venv and only 'hermes' is guaranteed on PATH.
Fixes#74819
Two fixes for the Skills Hub "View source" links on ClawHub skills:
1. Source URL generation was missing the required {owner} segment —
https://clawhub.ai/skills/{slug} → 404. Correct format is
https://clawhub.ai/{owner}/skills/{slug}. When the owner handle is
unavailable, source_url is now "" (card omits the button) instead of
emitting a broken link.
2. _fetch_owner_handle() previously delegated to _get_json() which
returned None on any non-200 response with no retry. Under HTTP 429
rate-limiting the "50 consecutive failures" safety rail in
enrich_owners() fired immediately — the documented claim "Respects
HTTP 429 rate-limit responses with exponential backoff" was not
actually implemented. Now has its own retry loop: 3 attempts, honours
Retry-After on 429, exponential backoff on 5xx/transport errors, no
retry on 4xx.
Changes:
- tools/skills_hub.py: _coerce_skill_payload carries owner from top-level
response; inspect() captures owner from detail API; _fetch_owner_handle()
added with bounded retry/backoff; enrich_owners() batch method with
safety rails (30 workers, early termination at 50 consecutive failures).
- website/scripts/extract-skills.py: _source_url() reads extra["owner"]
for ClawHub.
- scripts/build_skills_index.py: batch enrichment step after crawling.
- tests: 35 URL/enrichment tests + 7 retry tests (42 total).
Signed-off-by: dongjiang <dongjiang1989@126.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>
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 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).
Combines the Windows-hermeticity cluster (#67512 by @webtecnica, earliest;
#71112 by @Sanjays2402; #67196 by @anatolijlaptev1991-ctrl) into one fix:
- scripts/run_tests.sh: env -i forwarded only HOME, but native Windows
CPython resolves Path.home() from USERPROFILE (or HOMEDRIVE+HOMEPATH),
stdlib paths from LOCALAPPDATA/APPDATA, ssl/sockets need SYSTEMROOT,
tempfile needs TEMP/TMP — the strip broke collection tree-wide on
native Windows (issues #67385, #70813). Location vars (never
credentials) are now forwarded, each only when actually set, so
POSIX runs are byte-for-byte unchanged (probe-verified both ways).
PYTHONUTF8=1 added for legacy-codepage consoles printing the
runner's glyphs.
- tests/plugins/memory/test_hindsight_provider.py: _clean_env patched
HOME only; on Windows Path.home() ignores HOME. Now patches
Path.home directly into tmp_path (from #67196).
Not ported: #71112's guard test — it regex-reads run_tests.sh source,
which the test policy bans (never read source code in tests).
Fixes#67385. Fixes#70813.
hermes-setup.exe bakes its build-time commit into the binary
(BUILD_PIN_COMMIT) and passes it as -Commit on every install-mode run,
including the retry the desktop's "Update didn't finish" screen kicks
off. The repository stage checked that SHA out unconditionally, so an
installer built months earlier rewound a current managed checkout to its
build commit -- 9,160 commits in the reported case -- leaving ancient
source against a current venv. npm then failed on workspaces that did not
exist yet at that commit, and every later update ran against the wrong
tree.
Skip the pin when its target is already an ancestor of HEAD. Fresh clones
have no such ancestry so reproducible/CI pinning is unchanged, and
--force-commit / -ForceCommit still rolls back on purpose.
install.sh probed pypi.org and duckduckgo.com serially with
--max-time 8 each, so a fully blocked network cost 16s before the user
saw any useful guidance. The two probes are independent; running them
as background jobs and gathering verdicts caps the worst case at one
--max-time (8s) while the good path stays instant.
Verified live: reachable URLs 0.24s (both probes concurrent);
blackholed 10.255.255.x URLs 8.02s total (was 2x8s), warning text
unchanged. bash -n clean.
The Skills Hub 'Marketplace' tab showed a single useless entry: Anthropic
changed .claude-plugin/marketplace.json to bundle-shaped plugins whose
source is './', so all plugins collapsed to one identifier pointing at the
repo root, and the second marketplace repo (aiskillstore/marketplace) is
gone (404). Everything in anthropics/skills is already surfaced by the
GitHub tap as the Anthropic tab, making this source fully redundant.
Removes ClaudeMarketplaceSource and all wiring: source router, index
builder (crawl + floors + sort order + rate-limit messaging), extract
labels/install/URL mapping, hub UI tab, web server labels, CLI limits,
docs (en + zh), the legacy index-cache snapshot, and test fixtures.
Stale skills-index entries with source 'claude-marketplace' still install
fine: HermesIndexSource fetches via resolved GitHub paths generically.
install.sh duplicated the raw deep ad-hoc re-sign, so install/repair and
self-update could disagree about the app's signing identity — an update
signed with the stable identity would be clobbered back to a cdhash-only
DR by the next installer repair. Call the shared Python fixup (passing
the shell's publisher-signing decision explicitly), and branch into the
historical xattr + deep ad-hoc repair when the venv helper is missing or
fails so a broken venv never leaves the bundle unlaunchable.
Co-authored-by: cipry0200 <cipry0200@users.noreply.github.com>
Co-authored-by: natebransc <natebransc@users.noreply.github.com>
Co-authored-by: caseyanthony <caseyanthony@users.noreply.github.com>
Running the suite could take over the machine it ran on. Two real
effects, both now closed:
- **The suite spoke through the speakers.** Once any test drove the
`voice.toggle` RPC with `action="tts"`, the handler set
`HERMES_VOICE_TTS=1` in the *live process environment*, and the flag
outlived that test. Every later test that drove a turn to completion
then fed its final response text to `hermes_cli.voice.speak_text` on a
background thread - real synthesis, real playback, no API key needed
(the default `edge` provider is keyless). A developer heard the fixture
string "partial answer complete" out loud. Because the flag is set from
inside the process, `scripts/run_tests.sh`'s `env -i` never protected
against this.
`tests/conftest.py` now blanks `HERMES_VOICE`/`HERMES_VOICE_TTS` per
test, and a new autouse `_audio_playback_guard` stubs `speak_text` and
its playback binding outright, so the speakers stay shut even inside the
test that sets the flag itself. `@pytest.mark.real_audio_playback` opts
out.
- **The suite launched Chrome.** `tests/tools/test_browser_supervisor.py`
spawned a real browser on any machine with Chrome on `PATH`. Its
docstring promised a `HERMES_E2E_BROWSER=1` gate that existed nowhere in
the code. That gate is now real, and the file is marked `integration`
so the default marker filter excludes it. `scripts/run_tests.sh`
forwards `HERMES_E2E_BROWSER` so the documented manual run still works.
Miscellanea
- `tests/test_audio_playback_guard.py`: regression cover for both defences,
driving the real `voice.toggle` handler rather than a stand-in.
setup_path() wrote a `hermes` launcher to ~/.local/bin but nothing for
`hermes-acp`. That console script exists only inside the venv, which is
not on the login-shell PATH.
ACP hosts resolve the agent by command name against that PATH, so an
otherwise healthy install looks absent to them. Buzz Desktop ships a
Hermes preset that spawns `hermes-acp` and reports the runtime as
unavailable; Zed and JetBrains configs that name the bare command have
the same problem.
Write a hermes-acp launcher next to the hermes one, dispatching to the
acp subcommand. Same PYTHONPATH/PYTHONHOME clearing, and the same rm -f
before cat > so an older symlink into the venv cannot be followed and
stomp the console script (#21454). Uninstall removes both launchers.
tests/test_install_sh_acp_launcher.py drives the block out of install.sh
rather than asserting on a copy, covering the venv and non-venv branches
plus the symlink-stomp case. Reverting the install.sh change turns all
three red.
Signed-off-by: SHL0MS <SHL0MS@users.noreply.github.com>
test_install_ps1_is_pure_ascii guards against PowerShell 5.1 ANSI
codepage misdecoding (issues #66994/#67000); the Install-DesktopVoiceDeps
comment had an em-dash.
Two fixes from live testing (Teknium):
1. Desktop installs now ship the wake/voice stacks up front.
install.sh + install.ps1 desktop stages run 'uv pip install
-e .[wake,voice]' (best-effort, lazy-install remains the fallback)
before building the app, so the first ear-click arms instantly
instead of sitting through a multi-minute onnxruntime download.
CLI-only installs keep the lazy path — [all] curation unchanged.
2. The vanished ear: the STT/TTS gate made wake.status call
check_tts_requirements(), whose edge path runs _import_edge_tts →
lazy_deps.ensure — a synchronous PIP INSTALL inside a status poll.
On a venv without edge-tts that blew the desktop's 30s RPC timeout,
armWakeWord caught the error, the atom never learned enabled=true,
and the ear unmounted. _tts_ready is now a pure probe: deps missing
+ lazy installs allowed counts as ready (installs at first speak)
WITHOUT touching pip; check_tts_requirements only runs once deps
are present. Regression test asserts the probe never calls it while
deps are missing.
install.ps1 wrote the marker on Windows and the Rust installer now writes it,
but install.sh -- the path every Mac and Linux CLI install takes -- never did.
A machine set up with install.sh therefore looked uninstalled to the desktop
app, which re-ran first-run bootstrap on every launch.
Stamp the same schema-v1 payload install.ps1 writes, from both the staged
`complete` stage and monolithic main(). An unresolvable HEAD skips the marker
rather than writing one the desktop validator rejects: absent reads as a clean
"bootstrap needed", malformed reads as a confusing half-state.
The Windows-footgun check is red on current main, not just on this branch:
a2c42be93c added `encoding="utf-8"` to one `write_text` in this file and
missed the other two.
Line 190 is what the checker reports. Line 211 has the same defect but the
call is split across lines, so the single-line regex never flagged it —
fixing only the reported site would have left the same bug in the file and
re-armed it for the next reader.
Both now pass an explicit encoding, matching line 68.
Bridge vs listing only (Opus 4.8, 830 real UE schemas, 3 reps/cell).
Excluding one both-modes mock artifact: listing 24/24 vs bridge 20/24,
searches/task 0.2 vs 4.0. Bridge failures: core-tool substitution at
frontier tier (ran the host test suite via terminal instead of
discovering RunTests, 2/3 reps), up to 8 searches to prove a negative,
and search-vocabulary misses on paraphrase. Listing asserts absence in
zero searches and answers a 5-way capability survey in 1 API call.
Scenarios target real confusion clusters in Epic's UE 5.8 catalog
(StaticMesh vs SkeletalMesh set_material, three tag systems, CurveTable
vs DataTable rows, Niagara Component vs System variables, four capture
variants, zero-keyword phrasing). Mocks return realistic editor errors
on wrong-type calls; scoring is strict (clean solve = correct tool with
zero distractor calls; first-call accuracy tracked separately).
Key result: first-call selection is unreliable in EVERY mode — eager
with all 199K of schemas in context managed 2/10 — but clean solves stay
75-95% because agents probe (get_components, get_material_slots) before
committing. The probe loop works through the 3-tool bridge at 1/4 the
cost of eager ($1.60-1.69 vs $6.49/task, Opus 4.8). On Haiku the
listing beats bare bridge 18/20 vs 15/20 (core-tool substitution again).
Zero distractor invocations across all 50 Opus runs.
Replays the actual tool schemas captured from Epic's UE 5.8
ModelContextProtocol + AllToolsets plugins (830 tools / 52 toolsets) as
live registry tools with mocked editor responses, then benchmarks
eager vs bare-bridge vs bridge+listing at two scales (62-tool editor
subset, full 830) on Claude Opus 4.8 (1M ctx; eager at 830 does not fit
any 200K model — first call requests ~266K tokens).
Headline (full 830, mean per task, rescored): eager 8/8 at 810,578
input tokens ($4.05); bare bridge 16/16 at 160,844 ($0.80); listing
16/16 at 257,264 ($1.29). Frontier model erases the accuracy gap in
every mode; cost is the differentiator. At 62 tools eager wins on cost
— consistent with the auto-threshold design.
Also parameterizes livetest harness model + listing_max_tokens via
env/args (TS_UE_MODEL, TS_UE_SCALE, TS_UE_MODES, TS_UE_LISTING_MAX).
Deferred MCP/plugin tools become invisible once the tool_search bridge
activates — live benchmarking (48 runs, Claude Haiku 4.5) showed models
substituting visible core tools (terminal/web_search/browser) for deferred
capabilities or declaring them nonexistent instead of searching: 16/24
task success vs 24/24 with eager loading.
Skills never had this failure mode because every skill keeps a ~21-token
name+description listing line in the system prompt. This ports that exact
pattern to the tool bridge: when tool_search activates, a grouped manifest
of every deferred tool (name + first sentence of description, clipped to
60 chars, grouped per MCP server / toolset) is embedded in the tool_search
bridge description.
- tools/tool_search.py: build_catalog_listing() with deterministic
ordering (byte-stable across assemblies -> prompt prefix stays
cacheable); token-budget fallbacks full -> names-only -> legacy bare
count; bridge_tool_schemas(listing=...) embeds it and instructs the
model to skip tool_search when the exact name is visible (one fewer
round-trip per use)
- config: tools.tool_search.listing auto|on|off (default auto),
listing_max_tokens (default 4000, clamped 200..20000); legacy bool
shapes keep working
- tests: 8 new tests (config parsing/clamps, short-desc clipping,
deterministic rendering, budget fallbacks, bridge embedding, assembly
on/off paths); full file green (47 passed)
- docs: tool-search.md config table + rationale
- scripts/tool_search_livetest2.py: benchmark harness v2 with real
per-call token accounting (normalize_usage spy) and a third 'listing'
mode for A/B/C comparison
Three foot-guns in the canonical test runner, each of which cost real
debugging time by making an unverified run look verified.
1. Zero collection across the whole run reported success-shaped output.
Per-file rc=5 is rewritten to rc=0 so a platform-gated file (every test
skipped on this OS) doesn't fail the suite — correct, but it also meant a
run where NOTHING was collected anywhere printed
"0 tests passed, 0 failed (100% complete)" and, with no failures
recorded, could exit 0. Now the run-level guard counts every collected
outcome (passed/failed/skipped/errors/xfailed/xpassed): an all-skipped
file still passes, but zero-collected-anywhere prints an explicit
"✗ NO TESTS RAN — this is NOT a pass" block naming the likely causes and
returns 1.
2. A venv without pytest was selected merely for existing. The probe
accepted any directory with bin/activate, so in a checkout/worktree
without a local .venv it picked the RELEASE venv
(~/.hermes/hermes-agent/venv, no pytest). Every file then died with
"No module named pytest" and the run reported 0 tests. Candidates are now
import-checked for pytest — the same guard the HERMES_PYTHON fallback
already applied — and a skipped candidate is named on stderr.
3. Pytest node ids were silently discarded. This runner is file-granular,
so `tests/foo.py::TestBar::test_baz` isn't an existing path: discovery
dropped it and the run ended "No test files to run" while the selector
looked accepted. Node ids are now translated to the FILE plus an inferred
`-k` on the leaf name (parametrized ids reduced to the function name),
with a note explaining the translation. An explicit caller `-k` wins over
the inferred one.
Tests: 4 behavior contracts in tests/test_run_tests_parallel.py. Verified
by sabotage — reverting the runner fails 3 of the 4 (the fourth pins the
pre-existing all-skipped tolerance so fix 1 can't regress it).
GitHub's current signup rules forbid consecutive hyphens, but legacy
accounts with them exist and are valid (Roger--Han, hit live during the
July 24 sweep — the mapping had to be written by hand). Accept any
alphanumeric/hyphen login that doesn't start or end with a hyphen.
AST-driven pass over every Path.read_text()/write_text() without an
explicit encoding= across non-test code: 71 sites in 34 files
(skills_hub, hermes_cli/main+profiles+service_manager+container_boot,
mem0/hindsight/honcho plugins, achievements dashboard, release/CI
scripts, productivity+comfyui skill helpers, agent/*). Verified zero
positional-encoding collisions before insertion; per-file compile()
check after.
Adds a check-windows-footguns rule flagging bare single-line
read_text/write_text (multi-line forms stay covered by the AST guard
test from #38985). Together with the salvaged contributor commits this
retires the ~169-site bare file-I/O class (#37423's long tail).
Addresses hermes-sweeper review on PR #54866: the installer runs
tools/skills_sync.py as a child python.exe whose PYTHONIOENCODING /
PYTHONUTF8 the scoped install.ps1 block sets, but there was no
regression test for this child-Python UTF-8 path. The existing
test_child_process_inherits_utf8_mode covers a different (bootstrap
entry-point) flow.
Add TestSkillsSyncUtf8Guard: three subprocess tests that import
skills_sync (triggering its import-time stdout/stderr reconfigure)
and assert the checkmark/up-arrow glyphs the script prints at
tools/skills_sync.py:596,675 emit valid UTF-8 and exit 0 even when
the child env is left unset or explicitly hostile (gbk). A third
test proves the guard is load-bearing by reproducing the crash
without it.
Also keep the new install.ps1 comment ASCII-only (the checkmark
spelled out as U+2713) per the file's PS 5.1 parser-compatibility
contract at scripts/install.ps1:79-80; the literal glyph in the
comment violated that contract.
On Windows with a non-UTF-8 system locale (e.g. CP936/GBK on zh-CN),
Python defaults stdout/stderr to the active codepage. tools/skills_sync.py
prints glyphs such as checkmark (U+2713) and up-arrow (U+2191) that GBK
cannot encode, raising UnicodeEncodeError mid-run.
The installer (scripts/install.ps1) captures this script's stdout and the
Rust bootstrap parses it as UTF-8 expecting a JSON result frame. A GBK
byte stream (or the traceback it triggers) surfaces as:
WARN stdout read error: stream did not contain valid UTF-8
stage=config-templates state=Failed
error=install.ps1 -Stage config-templates produced no JSON result frame
(exit=Some(0))
i.e. the stage fails even though the script exits 0. install.ps1 already
sets [Console]::OutputEncoding = UTF8, but that does not propagate to the
python.exe child (Python reads PYTHONIOENCODING / locale, not the console
encoding).
Fix in two places for defense in depth:
- tools/skills_sync.py: reconfigure sys.stdout/stderr to UTF-8 at import so
output is valid UTF-8 regardless of caller or active codepage.
- scripts/install.ps1: set PYTHONIOENCODING=utf-8 and PYTHONUTF8=1 (scoped
to the call, restored afterwards) around the skills_sync.py invocation.
- Cut the leftover telemetry.* DEFAULT_CONFIG block (nothing reads it) and
the legacy telemetry-key fallback in policy.py.
- install_id: persist the minted UUID back to config.yaml on first use so
service.instance.id survives gateway restarts (fail-open when the write
is not possible); regression test covers the restart path.
- Remove the no-op gateway_health_export.redaction config keys. Redaction
is always-on by design and deliberately not configurable; status output
now says so.
- Collapse redaction to one unconditional secrets+PII scrub: drop the
none/pii content modes (they served the dropped trajectories plane) and
fold gateway_health.py's duplicate bearer/token/email/phone regex layer
into agent/monitoring/redaction.py.
Salvages the event-spine foundation from feat/telemetry-observability
(emitter, typed events, OTLP streaming, redaction — authorship preserved in
the preceding commits) and scopes it to the plane enterprise operators need
today: gateway Service Health Monitoring plus redacted Operational
Diagnostics, exported over OTLP.
Dropped from the salvaged branch, deliberately:
- run/model/tool trajectory capture (plugins/telemetry hooks, tel_spans)
- the local JSONL + state.db tel_* store (monitoring is egress, not storage)
- usage rollups/metrics, /insights integration, bulk export
- hermes telemetry CLI (replaced by hermes monitoring status)
Those planes — shared client usage metrics and enterprise trace telemetry —
are being designed on the NeMo Relay integration with distinct consent,
policy, and export boundaries; this keeps the monitoring plane content-free
and independently enableable.
Renames agent/telemetry -> agent/monitoring, config telemetry.* ->
monitoring.*, and pins the otlp extra at OpenTelemetry 1.39.1 (matching
uv.lock; 1.30.0 conflicts with mistralai>=2.4 on opentelemetry-api).
AST-driven pass over every subprocess.run/Popen/check_output/check_call/call
with text=True (or universal_newlines=True) and no explicit encoding=:
append encoding='utf-8', errors='replace' at the kwarg site. 136 call
sites across 28 files (cli.py, hermes_cli/main.py, tools_config.py,
environments, computer_use, gateway, scripts, skills helpers, agent/*).
Together with the salvaged #55339/#60741 commits this closes out issue
#53428's bug class; the salvaged #60751 linter rule in
check-windows-footguns.py now enforces it repo-wide (verified: 807 files
scanned, zero findings).
Adds a new rule to scripts/check-windows-footguns.py that flags
subprocess.run/Popen/call/check_output/check_call(..., text=True, ...) calls
missing an explicit encoding= kwarg.
On Chinese Windows (cp936/GBK) and other non-UTF-8 default codepages,
text=True without encoding= decodes child output with
locale.getpreferredencoding(False), crashing _readerthread with
UnicodeDecodeError on non-default-codepage bytes (issues #47939, #53428,
rule prevents future regressions.
Rule design:
- Pattern matches 'text=True' / 'text = True'
- post_filter skips lines that:
- already pass encoding= on the same line
- are method definitions (def text)
- contain text=True inside string literals
- are not subprocess-shaped calls (heuristic via _is_likely_subprocess_call)
- Two helper functions: _is_likely_subprocess_call, _looks_like_string_literal
- Multi-line calls where subprocess.X( and text=True are on different lines
are not flagged (acceptable false negative for a line-based scanner)
Also fixes the linter's own footgun: get_staged_files() and get_diff_files()
used subprocess.check_output(text=True) without encoding= — now fixed.
Suppresses 4 false positives on non-Windows platform-exclusive calls:
- tools/voice_mode.py (Termux/Android)
- tools/environments/singularity.py (Linux HPC)
- plugins/google_meet/cli.py (macOS system_profiler)
Test plan:
- 21 unit tests in tests/scripts/test_footgun_subprocess_encoding.py
- TestDetection: 6 cases verifying the rule flags real subprocess calls
- TestSuppression: 7 cases verifying false-positive avoidance
- TestHelpers: 7 cases for the two helper functions
- TestFullRepoScan: scans the whole tree and asserts the new rule finds
only the 7 call sites that PR #60741 fixes (or zero, once #60741 merges)
Verified: full-repo scan reports 7 matches on main (the #60741 sites),
4 platform-exclusive calls correctly suppressed, zero false positives.
On Windows with Chinese locale (GBK), subprocess.run(text=True) without
explicit encoding causes UnicodeDecodeError crashes. This fix adds
encoding='utf-8', errors='replace' to all subprocess.run() and
subprocess.Popen() calls that use text=True across 76 non-test Python files.
Fixes#53428 (master tracker for Windows GBK locale crash).
Note: credential_pool.py and electron changes excluded per reviewer request —
those will be submitted as separate focused PRs.
Print gh-image stdout and stderr on attachment failures, then replace the
pending inline-evidence marker in the PR review comment with an escaped
failure notice before preserving the failing workflow result.
Keep the live PR review comment polling for a short grace period after
visible jobs complete. Preserve composite-action jobs in timing and live
status collection, and split the timing HTML from its linked review-status
artifact.
* fix(ci): publish inline E2E evidence
Upload bounded screenshot evidence from E2E, then publish validated images
from a trusted workflow_run job to commit-pinned branches in the evidence repo.
Wait briefly for the live CI review comment marker before publishing, so
GitHub's read-after-write delay cannot leave an orphaned evidence branch.
* fix(ci): isolate privileged credentials from PR jobs
Keep App private keys and Docker Hub credentials out of PR-controlled
workflows. Use protected environments for trusted publishing and a public
repository variable for the App client ID.
* fix(ci): attach E2E evidence with restricted bot session
Replace the App-backed evidence repository publisher with gh-image uploads
from a dedicated bot session in the gh-image environment.
* fix(ci): publish validated E2E evidence from forks
Let the trusted default-branch publisher handle bounded, validated evidence
artifacts from fork PR CI without checking out or executing fork code.
* ci: surface E2E screenshots in review comment
* ci: mark completed review commits in past tense
* ci: surface approved sensitive-file reviews
* ci: link sensitive files to reviewed changes
* ci: stage desktop E2E visual evidence
Track screenshots newly introduced against main and package visual diffs for a trusted publisher.
* fix(ci): pass E2E evidence output paths
Supply the manifest and staging-directory arguments required by the screenshot status helper.
* fix(ci): download the OSV SARIF artifact
Match the artifact name and result filename emitted by the pinned upstream reusable workflow.
Removes Homebrew and PyPI wheel/sdist as Hermes distribution paths while
preserving the supported source, Docker, and Nix workflows.
Changes:
- Removes the Homebrew formula, PyPI publish workflow, sdist manifest
(MANIFEST.in), and wheel/sdist release-attachment logic from scripts/release.py.
- Keeps setuptools metadata and entry points required by editable installs
and Docker/Nix builds, but adds a setup.py guard that rejects wheel/sdist
builds outside a sealed Nix derivation (HERMES_NIX_BUILD=1).
- Removes pip/Homebrew install detection, PyPI update checks, the pip
self-update path, the deprecation-banner state, the postinstall subcommand,
wheel data-directory fallbacks in agent/i18n.py and hermes_constants.py,
and the ACP Registry manifest/version-lockstep release logic.
- Adds /nix/store/ path detection so `nix run` / `nix profile install`
installs (which don't set HERMES_MANAGED) are correctly identified as
"nix" rather than falling through to "git"/"unknown".
- Retired install-method values ("pip", "homebrew") in existing
.install_method stamps (both code-scoped and home-scoped) are ignored by
the allowlist reader and fall through to "unknown" instead of resurrecting
a retired enum value.
- Updates Nix packaging to ship bare runtime data (locales, optional-mcps)
through store symlinks and wrapper env vars instead of wheel data-files.
- Removes the ACP Registry manifest/icon and their version-lockstep tests.
- Deletes or rewrites packaging, pip-update, Homebrew, and ACP Registry
tests; adds parametrized coverage for the packaging build guard covering
BOTH sdist and wheel paths (the guards live in separate cmdclass entries
— a passing sdist test proves nothing about the wheel path).
- Updates installation/platform documentation and related user-facing copy.
- Adjusts the supply-chain scan so deleted install-hook files do not trigger
a finding, while additions or modifications still require the existing
ci-reviewed label gate.
Supported installation paths (unchanged):
- git installer (install.sh)
- Docker
- Nix/NixOS
- editable development installs (uv sync, uv pip install -e ., pip install -e .)
Let the scanner report critical findings without failing. The review-label
gate owns the action-required status and blocking result, allowing the
ci-reviewed label rerun to clear both CI and the PR comment.
Replace the static comment-pending + comment-results two-job pattern
with a live-updating comment system that polls the GitHub Actions API
every 15s, re-assembles the review comment from whatever results are
available, and upserts it via the <!-- hermes-ci-review-bot --> marker.
The comment updates in real time as each job finishes — no waiting for
the full pipeline.
Every CI job that wants to appear in the review comment emits a
review_status output — a JSON array of objects, each with a source
and a results array:
[
{
"source": "review-label-gate",
"results": [
{"kind": "action_required", "title": "...", "summary": "...",
"how_to_fix": "..."},
{"kind": "info", "title": "...", "summary": "..."}
]
},
{
"source": "ci timing",
"results": [
{"kind": "warning", "title": "CI timings", "summary": "...",
"detail": "...", "link": "..."}
]
}
]
One job can emit multiple results of different kinds. The source field
is used to exclude the corresponding job from the synthesized error
list (case-insensitive, hyphen-normalized matching against GitHub
Actions job display names).
| job | source | kind (on failure) | section |
|----------------------------|--------------------------|---------------------------|----------------------|
| review-labels | review label gate | action_required / info | Action required |
| lockfile-diff | lockfile-diff | action_required | Action required |
| ci-timings | ci timing | warning / info | Warnings |
| supply-chain scan | supply chain | error / (none) | Job failures |
| supply-chain dep-bounds | supply chain | action_required / (none) | Action required |
| osv-scanner | osv scan | warning / (none) | Warnings |
| uv-lockfile-check | uv.lock check | action_required / (none) | Action required |
| history-check | unrelated histories | action_required | Action required |
| contributor-check | contributor attribution | action_required | Action required |
Jobs that find nothing emit [] (empty array) — no noise info items.
A single comment-live job polls the GitHub Actions API every 15s,
classifies jobs into (completed, pending), assembles the comment, and
upserts it. Merges review_status outputs from all needs jobs via
toJSON(needs.*.outputs.review_status), and downloads the ci-timings
artifact when it becomes available. Shows commit SHA + message below
the header.
The assembler has ZERO job-specific knowledge. It just:
1. collect_from_statuses() — flattens all nested status objects into ReviewItems
2. collect_failed_jobs() — synthesizes errors for failed jobs with no declared status
3. _attach_job_urls() — fills in per-job log links for ALL items
4. render_comment() — groups by severity, renders with group headers
Each item shows links inline next to the title: View report (job-emitted
URL) and View job (auto-attached logs link). Each info item is its own
collapsible <details> block.
# ૮ >ﻌ< ა ci review
running on abc1234 — commit message first line
## ❌ Job failures
### {title} · [View job](url)
{summary}
## ⚠️ Action required
### {title} · [View job](url)
{summary}
**How to fix:**
{how_to_fix}
## ⚠️ Warnings
### {title} · [View report](url) · [View job](url)
{summary}
{detail}
<details><summary>{title}</summary>
{content}
</details>
Still running 3 jobs: ci-timings, docker
- test_assemble_review_comment.py (48 tests): collect_from_statuses,
collect_failed_jobs with exclude_sources, _attach_job_urls,
render_comment (group headers, inline links, commit info, per-item
details, pending footer), assemble integration
- test_live_comment.py (16 tests): classify_jobs pure function
- test_timings_report.py (10 tests): generate_review_status nested format
- test_lockfile_diff.py (6 tests)
- test_classify_changes.py (32 tests, pre-existing)
Follow-up fixes for salvaged PR #67686 (issue #67639):
1. Windows regression: bare 'import fcntl' at module level in entry.py and
slash_worker.py would crash on Windows (fcntl is POSIX-only). entry.py
explicitly aims to 'import cleanly on Windows'. Extracted all fcntl/socket
logic to tui_gateway/_stdin_recovery.py with try/except import guards.
2. fd leak: socket.fromfd() dups fd 0; s.detach() returned the fd number
without closing it, leaking one fd per recovery call. Changed to s.close()
(safe — fromfd duped the fd, closing won't close stdin).
3. Code duplication: ~80 lines of recovery loop + diagnostic were copy-pasted
between entry.py and slash_worker.py. Extracted to shared
tui_gateway/_stdin_recovery.py (handle_spurious_eof + diagnose_stdin_state).
4. Profile-unsafe path: scanner used Path.home() / '.hermes' / 'plugins' but
canonical plugin discovery uses get_hermes_home() / 'plugins'. With
HERMES_HOME pointing elsewhere, scanner missed the actual plugin dir.
Also gated project plugins behind HERMES_ENABLE_PROJECT_PLUGINS to match
hermes_cli/plugins.py.
5. SO_RCVTIMEO not cleared: recovery only called os.set_blocking(0, True)
but a child that set SO_RCVTIMEO would cause the next readline to time
out and loop. Now clears SO_RCVTIMEO alongside O_NONBLOCK.
* fix(desktop): allow write-build-stamp from non-git checkouts
Stop hard-failing npm pack when neither GITHUB_SHA nor git HEAD is
available (ZIP installs / broken .git). Emit an explicit fallback stamp
instead so local Windows desktop builds can finish (#50823).
* fix(desktop): treat fallback stamps as unpinned; harden Windows install
Keep all-zero fallback commits out of -Commit/--commit pins and fetch
install.ps1 by branch instead. After bootstrap, pin the marker to the
checkout HEAD so isBootstrapComplete accepts it. On Windows, force ZIP
checkout, seed GITHUB_SHA (ASCII-only install.ps1), and avoid the pack
stamp failure.
* fix(install): pin core.autocrlf=false before ZIP-path checkout (#50823 review)
The ZIP-fallback path added in #67643 runs `git checkout -f FETCH_HEAD`
before core.autocrlf gets pinned (which only happened later, on the
shared clone-path config). On Git for Windows -- where core.autocrlf
defaults to true -- that renormalizes the repo's LF text files to CRLF in
the working tree during checkout, leaving the freshly-created managed
checkout dirty versus HEAD and aborting the next `hermes update`. That is
the exact "dirty tree the user never touched" failure the surrounding
code already guards against (install.ps1:1461-1469, 1750-1753).
Move the `config core.autocrlf false` pin to run immediately after
`git init`, before the fetch/checkout. The later idempotent pin on the
shared clone path is retained so git-clone installs are unaffected.
Addresses teknium1's review on #67643 and supersedes it, preserving the
original author's two commits.
Co-authored-by: HexLab98 <8422520+HexLab98@users.noreply.github.com>
* chore(contributors): map austinpickett commit email for attribution
The check-attribution CI gate flagged austinpickett@users.noreply.github.com
as an unmapped commit-author email (introduced by the autocrlf fix commit
on this PR). Add the per-email mapping file as the gate instructs (the
legacy AUTHOR_MAP in scripts/release.py is frozen).
---------
Co-authored-by: HexLab98 <liruixinch@outlook.com>
Co-authored-by: austinpickett <austinpickett@users.noreply.github.com>
Co-authored-by: HexLab98 <8422520+HexLab98@users.noreply.github.com>
Follow-up hardening on top of the salvaged #66637 commits:
- _insert_real_user_anchor could place the restored human turn directly
next to user-role scaffolding (index-0 insert before a leading synthetic
user turn, or a scaffolding-only transcript), breaking the strict
alternation contract (#55677). Restoration now merges into trailing
scaffolding (anchor text leads, synthetic flags cleared) and appends
after a user-role compaction summary instead of inserting adjacent.
- _is_real_user_message now also rejects user-role compaction summaries
(the compressor pins the summary to role=user when the tail opens with
an assistant turn), so a summary can no longer satisfy the human-anchor
check and skip restoration.
- _latest_user_task_snapshot reuses the same real-user predicate, so the
deterministic task snapshot can no longer anchor on todo snapshots,
truncation notices, or background-process reports.
- The Historical Task Snapshot rewrite keeps the section terminated with
a blank line; the previous replacement consumed the boundary newlines,
gluing the next '## ' heading mid-line and deleting all later sections
on the next iterative compaction.
- Drop _length_continuation_synthetic (no producer anywhere).
- AUTHOR_MAP entry for enzo-adami.
The first LLM call of every gateway turn gets ~0% provider prompt-cache
hit rate (in-turn calls: 97-99%) because the bytes sent for a turn's
user message are not the bytes replayed next turn: memory-prefetch and
pre_llm_call context are injected into the API copy only, the #48677
persist override writes cleaned content to the DB row, and
get_messages_as_conversation sanitize/strips user and assistant content
on load. Any of these diverges the request prefix at that message and
re-prefills everything after it — measured 27.9s for the first call vs
2.4-5.8s cached at a median ~156k-token context.
Persist what you send: a nullable messages.api_content column stores the
exact content string sent to the API when it differs from the clean
stored content, and replay substitutes it verbatim (no sanitize, no
strip). The injection composition lives in one helper
(turn_context.compose_user_api_content); the turn prologue stamps its
output onto the live user message, the api_messages build sends the
stamped bytes, and every outgoing copy pops the field so it never
reaches a provider. The crash-resilience user-turn persist moves after
prefetch/pre_llm_call so the user row is written once with its final
sidecar; _ensure_db_session stays before preflight compression (session
rotation needs the parent row under PRAGMA foreign_keys=ON). The
current-turn index trackers are re-anchored after compaction rebuilds
the message list, in-place preflight compaction backfills the stamp onto
the already-inserted row, and gateway replay forwards the sidecar only
when the replay pipeline did not rewrite the content. Rewrite paths that
would leave stale bytes (historical image strip, merge-summary-into-
tail, consecutive-user repair merge, stale-confirmation redaction) drop
the sidecar; the chat-completions transport and the max-iterations
summary path strip it defensively. codex_app_server and MoA turns are
excluded from stamping because their wire bytes differ from the
composition. A missing or dropped sidecar degrades to today's behavior
(one cache-boundary miss), never to wrong content.
A commit added a bullet and an em-dash inside two Write-Host/Write-Info string
literals in scripts/install.ps1. The file has no UTF-8 BOM, so Windows
PowerShell 5.1 (which the bootstrap runs the cached script under) reads it in
the system ANSI code page (CP1252), not UTF-8. The em-dash's UTF-8 tail byte
decodes to a smart close-quote (U+201D) that the tokenizer treats as a string
delimiter, prematurely closing the string and desyncing the parser -- surfacing
as the reported cascade of syntax errors at lines 1619/1770 and aborting the
Windows GUI installer before it does anything.
Non-ASCII bytes in '#' comments are harmless (skipped to end-of-line), so the
file carried em-dashes in comments for months; only the two chars in code
broke it. Convert all non-ASCII to ASCII equivalents (em-dash -> '--', already
this file's own comment convention; bullet -> '-') and add a source-level test
locking the pure-ASCII invariant, since Linux CI cannot run the PS installer.
Fixes#66994Fixes#67000
Follow-up to PR #66479 salvage. The three new env vars
(HERMES_LOCAL_STREAM_STALE_TIMEOUT, HERMES_GATEWAY_MAX_STARTS,
HERMES_GATEWAY_START_WINDOW_S) were introduced as bare env-var reads with no
config.yaml surface or documentation — violating the .env-is-for-secrets-only
policy (behavioral settings must live in config.yaml, bridged to env internally).
- config.yaml: add gateway.respawn_storm {max_starts, window_seconds} to
DEFAULT_CONFIG, mirroring the existing restart_loop_guard pattern.
- gateway.py: read config.yaml first, env vars override as escape-hatch.
- environment-variables.md: document all three new env vars, noting the
config.yaml alternative for the gateway ones.
- chat_completion_helpers.py: cross-reference the env-var docs from the
local stale timeout comment.