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.
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>
