Independent review: a nonexistent base ref under --strict reported zero
drops and exited 0 (a mis-fetched CI job would look clean); the script now
verifies both refs and the merge-base and exits 2 otherwise. And a method
moved from a class into a mixin/base defined in the same module that the
class still derives from was reported as removed although the attribute
still resolves; public_methods now collects the methods REACHABLE on each
class through its in-module bases. Replay of #102117 at open: 1,703 names /
341 modules and 126 test defs / 52 files unchanged; methods 1,000 -> 951
(the 49 were in-module mixin extractions, i.e. the false positives).
Test: unresolvable ref -> exit 2 in both modes; a method extracted into an
in-module base is not reported.
The Sep 2026 whole-codebase refactor (PR #102117) opened with 1,703 public
top-level names dropped across 341 modules, 1,000 public/dunder methods in
166, and 126 `def test_` deleted in 52 files. Reviewers found ~30 of the
names by hand; the rest surfaced as post-merge rework: 10 commits restoring
symbols and facade re-exports, 6 restoring tests, and a qwen OAuth break
that passed import smoke because the caller used `module.attr`. Every one
was catchable in seconds; nothing ran the check because it did not exist.
scripts/ci/check_public_surface.py: AST diff of modules present on both
sides of merge-base..HEAD. Public top-level names (defs, classes,
assignments, imported/re-exported names), public and dunder methods of
top-level classes, and `def test_` counts per tests/ file. Deleted modules
and deleted test files are visible decisions and are not flagged; private
names are not flagged. Advisory (exit 0, prints the report) by default;
--strict exits 1 so a refactor brief or a CI lane can gate on it. Wired
into lint.yml as an advisory PR step next to the compat-pointer check.
Replayed on the refactor PR at open (63279301bcb..022785a541) it reports
exactly the figures above in 18 s; on this branch vs main it reports 0.
Test: a throwaway git repo with drops, private drops, a move-with-re-export,
a lost test def and a changed non-source module; asserts the exact report
and the advisory/strict exit codes.
Merge upstream 5e645791ac.
Retain the PM feature-flag owner and add upstream connection options.
Use the deny-only window-open policy while trusted external links keep
the existing IPC path. Keep both session-import and external-link copy.
Preserve captured timeout output when adding terminal yield handoff.
Quickstart tests patch the explicit upstream model-assignment owner.
Migrate incoming legacy OS markers to the branch's platforms gate.
Desktop renderer and Electron typechecks passed. Targeted Electron tests
passed (42 tests), Python conflict checks passed (26 tests, 3 skips),
and the plugin-compat import checker passed. CI owns the broad merge gate.
Prepare dependency generations before selecting them. Keep shipped tool
bytes separate from writable additions, and store facts beside their entries.
Validate proposed plugin sets before config publication. Restore the previous
config if the facts write fails.
Consolidate duplicate updater, backup, setup, and voice helpers. Repair
launcher selection, dependency consumers, download ownership, update feeds,
and native Windows process and file handling.
Verification: 206 changed/prior-failing Python files reported 4630 passed,
one failed, and 330 skipped. Fix the remaining Hindsight fixture boundary.
The final targeted rerun reported 234 passed and two skipped. The store
review regression batch reported 83 passed and one skipped. Desktop
TypeScript checks, 56 selected Electron tests, 24 release tests, and the
removed-import/compatibility guards passed.
This is an integration checkpoint, not full audit acceptance. The complete
Python suite has not run on this fixed tree. Crash-atomic plugin publication,
generation cleanup, receipt correlation, and packaged lifecycle acceptance
remain open in docs/pm-audit-status.md.
33 read sites across 25 files used BOM-intolerant encoding='utf-8'.
Windows tooling BOMs files it touches; json.load on a BOM'd file fails
with 'Expecting value'. Reads now use utf-8-sig (writes unchanged).
check-windows-footguns.py --all: 33 → 0.
`Start-UiServer` printed the -SelfTestUi URL (and opened the browser window)
as soon as the TcpListener was bound, but the runspace that answers /progress
starts asynchronously — BeginInvoke returns before the pipeline is open and
the script block is JIT'd, which is seconds on a loaded runner. The kernel
accepted connections into the backlog during that gap and nobody answered
them. The self-test hit it three times (#90371 and two follow-ups each
widened a timeout instead of removing the race) and it just failed an
unrelated hermes_state.py PR (run 33591547099, two 5s stale-backlog
timeouts = red).
- windows.ps1: readiness handshake after BeginInvoke — one /progress
round-trip must succeed (≤15s) before the server is returned; on failure
tear the listener down and continue without UI. The URL now means
"serving", not "bound". Also fixes the browser opening to a page that never
loads on a slow machine.
- test: 1s per-attempt probe timeout so a single dead backlog socket cannot
consume half the readiness budget.
- CI: new `desktop_updater` classifier lane. tests/test_desktop_update_windows_*.py
spawn the real PowerShell script; the Windows-only job now runs them only
when scripts/desktop-update/**, the Electron updater launcher, conftest,
pyproject, or those tests change (push/dispatch fail open). A PR that
never touched that surface cannot be failed by its process timing.
Route automatic profile exports to a managed store instead of the current checkout, and enforce a CI/Docker boundary that rejects archive files before they can be published.
Introduce the pm store: a unified, hash-verified package store that
replaces lazy_deps and the old installer's ad-hoc tool downloads.
Store tools are provisioned on PATH (ffmpeg, node/npm via pinned uv),
with a resumable 8-way downloader, verify() returning failure reasons,
and adopt() made EPERM-safe. chromium ships in the payload for every
target. The 3600-line install.sh is replaced by a staged bootstrapper
(heavy deps are pm's job after this); setup-hermes.sh, Dockerfile and
nix pin tables are rewired onto the store. Old install-script tests,
lazy_deps/managed_uv/build_info, and the ps1/bash installer test
batteries are removed with the machinery they tested.
Rebuilt from ethie/pm onto upstream/main (ac6c8028e0) after the
utf-8-sig sweep. 16 hot files (main also churned them) hand-merged:
platform adapters, main.py, electron/main.ts, tui_gateway/server.py,
cua_backend, installer-tests workflow, install.sh (full rewrite),
setup-hermes.sh, plugins doc.
xdist-as-standard was the wrong shape for Linux: the per-file subprocess
model ran the full suite there in 3m45s with zero cross-file failures,
while the first xdist run took 7m51s and failed 111 tests. The two
costs are structural opposites:
* xdist multiplies full-tree collection by the worker count — every
worker imports the whole suite and pays pytest's per-item
fixture-closure machinery (profiled: ~98M function calls to
collect 42k items against the conftest's autouse fixtures —
_matchfactories alone 1.37M calls, traverse_fixture_closure 783k).
On the 96-core runner that front-loads 106s before any test runs.
* The per-file model pays a spawn+import wall per file: ~15ms on
Linux (nothing), 0.5-1.5s on Windows (~6-min floor across 3400
files — the dominant cost of that lane).
So scripts/run_tests.sh now dispatches on host: run_tests_parallel.py
(restored verbatim from b8696aedea^, with its two self-tests) on POSIX,
pytest-xdist --dist loadfile on Windows. Both paths share one hermetic
env contract (env -i scrub, TZ=UTC, PYTHONHASHSEED=0, venv probing,
bytecode pre-compile). -j/HERMES_TEST_WORKERS feeds either backend.
The 111 xdist co-scheduling failures on Linux were the per-file
model's isolation guarantee surfacing as test bugs — with the POSIX
lane back on per-file, that class disappears where it never existed;
the Windows lane keeps loadfile (with its remaining co-scheduling
hazards as fix-at-the-test work).
Workflow comments, AGENTS.md, CONTRIBUTING.md, conftest comments and
the classify test follow the dual-path truth.
No shims, one marker system. The full mechanical sweep:
- 165 test files converted: every @pytest.mark.<os>_only decorator and
pytestmark assignment is now platforms("<os>"). Docstring/comment
prose mentioning the trio rewritten to the platforms() vocabulary.
- conftest: _OS_MARKS, the legacy skip loop, the double-mark reject for
the trio, and the selector-tagging shim are deleted. The single
platforms() gate does all host gating; _reject_contradictory_platform_
marks replaces the old reject (one platforms() marker per test — a
module-level pytestmark stacked on a per-test marker is the historic
skipped-everywhere-green-everywhere failure and stays a hard error).
- pyproject: only the platforms marker is registered.
- list_os_marked_tests.py: rewritten for the single vocabulary — takes
a platform name (linux/macos/windows), matches quoted platforms(...)
specs including negated and any-of forms, exits nonzero on empty
selection. Its test file rewritten to match (bare identifiers must
not match; the spec must appear inside the string literal).
- tests.yml macOS lane: marker: macos feeds the lister; the pytest
selection is -m "platforms and not integration" — the marker name is
the selector, the conftest's per-test host skips are the gate.
- test_os_marker_gating.py rewritten for the new reject (the old file
tested deleted machinery).
- AGENTS.md / CONTRIBUTING.md updated to the single vocabulary.
Verified: full-tree compile; marker unit tests (36); lister tests;
macOS-lane simulation (21 files, 180 collected / 429 deselected on a
windows host); broad xdist slices through scripts/run_tests.sh
(367 + 1674 passed, zero refactor-attributable failures — the 6 red
tests in the second slice fail identically with the changes stashed).
The fixed linux_only/macos_only/windows_only trio can only say 'one OS,
no qualifiers' — it cannot express 'anything except macOS', 'Windows
but only arm64', or 'POSIX-family behaviour'. The new platforms marker
takes any number of spec strings (any-of) plus optional arch filters:
@pytest.mark.platforms("linux")
@pytest.mark.platforms("not macos")
@pytest.mark.platforms("windows", arch="arm64")
@pytest.mark.platforms("posix")
Specs: linux / macos / windows / posix / any and 'not <spec>'.
arch matches platform.machine() with alias normalization
(amd64→x86_64, aarch64→arm64); arch_negate inverts it. Unknown specs
and stray keyword arguments are hard UsageErrors — a silent typo would
mean a test skipped on every host, which is the exact green-zero-
coverage failure the marker machinery exists to prevent.
The legacy trio remains accepted as aliases routing through the same
skip path (a mechanical rewrite of the ~500 existing call sites is a
separate sweep); tests/hermes_cli/test_linux_desktop_entry.py converts
as the reference usage. list_os_marked_tests.py now accepts both the
short platform names (matching quoted platforms() specs, including
negated ones) and the legacy _only names, and returns nonzero when a
marker selects nothing. The macOS CI lane passes -m macos.
Also documents the module-mark stacking trap: module-level
platforms/_only plus a per-test host marker trips the conftest's
double-mark hard reject — AGENTS.md now says so. Verified: the earlier
xdist INTERNALERROR crash on test_linux_desktop_entry +
test_browser_real_profile was exactly that stacked double mark, gone
with per-test platforms() marks (76 passed, 44 skipped on Windows).
scripts/run_tests.sh now runs pytest-xdist -n <N> --dist loadfile as
the single canonical path on every OS (Linux and Windows CI lanes both
use it). The per-file subprocess model (run_tests_parallel.py, and the
interim run_xdist.sh experiment) is deleted along with its two
self-tests: persistent xdist workers pay the interpreter+import wall
once per worker instead of once per file (~0.5-1.5s x ~3400 files was
a ~6-minute floor on Windows), and --dist loadfile pins a file's tests
to ONE worker, bounding state pollution to co-scheduled files — which
is exactly the class of flake we are now committing to fix properly.
The serial process-killer quarantine phase is dropped too. It existed
to keep process-tree-sweep tests from killing sibling xdist workers;
the durable fix belongs in those tests (sweeps must target their own
children, not enumerate every python process), and keeping a
divergent two-phase path would hide that work.
Kept from the old wrapper: hermetic env -i scrubbing, Windows
location-var forwarding, venv probing, bytecode pre-compile,
-m 'not integration', and the HERMES_TEST_IMAGE docker-knob
allowlist. HERMES_TEST_FILE_TIMEOUT/FILE_RETRIES/SLICE go away with
the runner they parameterized; -j/HERMES_TEST_WORKERS now map to xdist
-n (Linux CI pins 96, Windows 32, default auto).
Docs updated to match: AGENTS.md (runner contract, flake policy,
isolation section), CONTRIBUTING.md, tests/conftest.py comments,
classify_changes docstring + its lane expectation (a .sh runner no
longer trips the supply-chain scan lane), comfyui README,
hermes-agent contributor guide, debugpy skill and its website doc.
Fold the OS-specific lanes into tests.yml's test job as a three-OS matrix.
Linux and Windows run the full suite (conftest skips foreign-OS markers).
macOS keeps macos_only for now. Delete tests-os.yml.
The browser-tools step ran a bare `npm install` at the repo root, which
resolves the root package.json's `apps/*` workspace glob. That materializes
apps/desktop and with it node-pty, which ships no Linux prebuild and falls
back to `node-gyp rebuild` — so the installer needs make/gcc on a machine
that will never launch Electron or a PTY addon. Since #85297 made a failed
npm install fatal, a host without a C toolchain (a stock CentOS/RHEL box,
for instance) cannot complete a CLI-only install at all; it just reports
"npm install failed or timed out".
Name the workspaces the install actually needs instead. ui-tui and web are
selected when present, with --include-workspace-root so the root's shared
ESLint devDependencies are not pruned by the scoped install — the same
closure `hermes update` already installs. A checkout with neither workspace
falls back to a root-only install, since npm fails hard on a workspace it
cannot find. Desktop dependencies keep coming from install_desktop(), which
is only reachable via --include-desktop.
Against a pristine tree the unscoped install reifies 1362 packages including
node-pty 1.1.0; the scoped one reifies 582 with no native desktop addon.
A fork force-push can 404 the compare API used by detect-changes, which
fail-opens with ci_review=true and blocks the PR on a ci-reviewed label
the install change does not need. Recover the file list from the pull
request files endpoint before that fail-open.
Read text files with the encoding utf-8-sig so a BOM at the start of a
file does not cause a Unicode decode error (Windows editors add BOMs).
Reconstructed from ethie/pm commits 48a32b135b + 013219e814 onto the
current upstream/main base: only the utf-8 -> utf-8-sig transforms were
carried (370 exact line pairs across 205 files); pm-rename hunks that
rode in the original commit were left to the pm-store commit, and
utf8sig hunks entangled with content changes ride their owning commit.
Rebuilt on ethie/pm-clean off ac6c8028e0 (upstream/main).
PR #92092 fixed the same vanished-launcher bug by restoring copies into
the legacy in-checkout hermes-agent\bin from the update tail. That
location is what this branch removes: untracked files there are swept
by the update autostash on every cycle (restore/sweep treadmill, plus a
parked stash entry per update under --keep-stash), and unconditional
exe copies break on relocatable venvs ('uv trampoline failed to
canonicalize script path'). This branch's managed-binary-dir layout
supersedes both mechanisms, so the merge resolves to it:
- drop _sync_windows_cli_launchers and its _ensure_acp_launcher call
(Windows staging/repair lives in ensure_windows_bin_launchers at
process start and migrate_windows_bin_path in the update tail);
_ensure_acp_launcher is a Windows no-op again
- keep #92092's genuinely better installer semantics: staging stays in
a dedicated Install-HermesCommandLaunchers function that throws
BEFORE any PATH mutation when the required launcher cannot be staged
and verified -- previously Set-PathVariable could put an empty dir on
PATH and still print 'hermes command ready'. Reworked for this
branch's layout: caller passes the destination ($HermesHome\bin),
launcher form follows the venv (exe copy vs .cmd delegator), and the
verify step accepts either form
- rework #92092's AST-lifted PowerShell test for the new function
signature, keeping its fail-before-PATH-mutation assertions and
adding relocatable-venv form-selection coverage
- drop tests/hermes_cli/test_windows_cli_launcher_repair.py (pinned the
superseded in-checkout mechanism; equivalent and broader coverage
lives in tests/hermes_cli/test_ensure_windows_bin_launchers.py)
Nothing in CI compiled this crate. `.rs` lives under `apps/`, so the
change classifier matched a Rust edit as `frontend` and ran the
TypeScript matrix, which cannot notice a Rust error — the crate's 58 unit
tests had never executed once, and neither would the pipe-drain tests in
the previous commit.
Adds a `rust` lane and a Linux `cargo test --lib` job. Linux on purpose:
the pipe-drain fixtures need a real process tree whose grandchild
inherits the parent's stdout and are `#[cfg(unix)]`, so a Windows runner
would compile them out and report green over zero coverage. The Windows
half of that contract is `-SelfTestPipeDrain` on the existing Windows
lane.
llms.txt coverage is asserted in Python, but website/ sat on the Python skip
list, so a PR adding a docs page — or regressing the generator — went green
without ever running the test that checks the page is reachable. That is how
the index drifted to 53% coverage unnoticed.
The workflow owns its triggers and ci.yml does not call it. A
reusable-workflow call holds the caller run in progress for the full
build, and GitHub refuses `gh run rerun` on a run that is still in
progress. A separate run reruns and cancels on its own.
The job restores /nix/store from the GitHub Actions cache and saves from
main only. A cache that a PR writes is visible to that PR alone, so a
save there spends the quota of the repository and helps no later run.
The docker.yml gate held its own copy of the build formula, in shell.
classify_changes.py now owns a derived docker lane, and the nix lane in
the next commit derives from the same file. Two formulas in two
languages drift apart, and one Python function with tests does not.
The publisher read the PR number from the CI run's pull_requests
payload. GitHub keeps that payload empty for fork runs, so the job
printed 'No pull request is associated' and stopped on every fork PR.
Resolve the PR from the run's head owner, branch, and SHA instead.
The SHA match skips runs that a newer push superseded.
A fork PR also has no CI review comment, because the live poller
skips forks. The publisher now logs this and exits clean instead of
raising; the evidence stays in the workflow artifact.
The poller job set GITHUB_RUN_ID in env: to point at the CI run.
The Actions runner sets the GITHUB_* defaults itself and ignores
the override. Thus the poller read its own run id and watched
itself. Its own run stays in_progress while the poller runs, so
runs_all_completed() was never true. The comment froze at
'waiting for jobs to start' and the job burned its full 3000s
timeout on every PR.
Rename the variable to CI_RUN_ID. Also drop the GITHUB_REPOSITORY
override — it was a no-op for the same reason, and the runner
default already holds the correct value.
The live comment poller inferred completion from the job list. An empty
job list looks the same as a finished run: GitHub has not spawned the
jobs yet, so nothing is pending, and the poller posted a final
"all good!" comment and exited.
The run status is now the authoritative signal. collect_run_jobs()
returns whether the CI run and every watched sibling run report
status=completed, and the loop exits only when no job is pending AND
all runs are complete. While a run is still queued or in progress with
no visible jobs, the comment shows "waiting for jobs to start" instead
of a final banner.
the markers from the previous commit skip off-host. without a host to
run them on, every marked test is a silent skip. this commit adds the
hosts.
- tests-os.yml runs -m macos_only on macos-latest and -m windows_only
on windows-latest. ci.yml requires both lanes in all-checks-pass.
- a lane fails on pytest exit code 5 (zero tests selected). a renamed
marker cannot produce a green job that ran nothing.
- each lane repeats 'not integration' because a command-line -m
replaces the addopts filter.
- scripts/ci/list_os_marked_tests.py selects which files each lane
imports. -m filters after collection, and collection imports every
module. without this helper, one unrelated ImportError on the
foreign host fails a job whose own tests passed. the helper exits
non-zero when a marker matches no file, and writes bytes with
explicit lf so windows crlf translation cannot corrupt the bash
file list. it has its own tests in tests/ci/.
- the local runner now reports the skipped count and prints a note:
macos_only/windows_only tests were skipped on this host, and this
ci lane runs them. a green local run on linux no longer reads as
coverage of the other hosts.
- the runner default job count is now #cpu, not #cpu*2.
The CI run stayed in progress until its last job ended. Two advisory jobs
set that time: the review-comment poller (40 minutes) and the Docker image
build (45 minutes). Neither job was required to merge.
GitHub refuses `gh run rerun` on a run that is in progress. Thus a reviewer
who added the `ci-reviewed` label had to wait for the two slow jobs, and
label-rerun.yml carried a 2100-second wait loop for this reason. The fast
required jobs were ready long before.
Each slow job now runs in its own workflow:
- docker.yml owns its `pull_request` trigger and does its own change
detection. The new `detect` job runs the same composite action with the
same condition that ci.yml applied, so a tests-only PR still skips the
build. The `workflow_call` trigger is gone.
- ci-review-comment.yml starts on `workflow_run` when CI starts. It reads
the workflow and the scripts from the default branch, which is the trust
boundary that the old job got from its `ref: default_branch` checkout.
The poller reads job results through the API, so it can report on a run
that it does not belong to. `WATCH_WORKFLOWS` names sibling workflows for
the same commit, and `select_watched_runs` keeps the newest run for each
name. Thus the comment still shows the Docker results. The list is
newline-separated, because a workflow name can contain a comma.
The poller always exits 0 now. It reports on the CI run from a different
run, so a failed CI job is not a failure of the poller. The CI run has its
own gate for that.
Also correct a parse error in label-rerun.yml. STATUS came from the already
truncated RUN_ID, so its value was the run id and never "completed". Thus
the wait branch always ran.
ci.yml no longer needs `packages: write`, because the image build has left.
The artifact download URL returns a 302 redirect to a signed blob URL.
urllib sent the Authorization header to the blob, and the blob rejected it
with a 401 error. The download now has two hops. The first hop authenticates
to the API. The second hop follows the redirect without the auth header.
The query runs?event=workflow_call returns nothing for this repository.
GitHub flattens reusable-workflow jobs and their artifacts into the caller
run. The fetch now lists the artifacts on the orchestrator run only. The
dead sub-run enumeration is gone. Two API calls per cycle are gone with it.
The 'artifact statuses updated' reason never appeared. The code updated the
count before the comparison. Now the code compares first and updates after.
The code rejects zip members that contain '..' or start with '/'.
tests/ci/test_live_comment.py is deleted. This repository does not keep
tests for CI infrastructure.
The live comment poller got its review statuses from two sources. The first
was the REVIEW_STATUSES environment variable, fixed at the start of the
comment-live job. The second was one ci-timings artifact, downloaded at the
end of the run. Status details (error messages, action_required items)
appeared only after all jobs finished. The job pass/fail results were visible
as each job completed.
Now every status-producing workflow_call uploads a small review-status
artifact when it completes. The poller lists all review-status-* artifacts
from the orchestrator run and its workflow_call runs every cycle. It
downloads each artifact and merges the statuses into the comment. A status
appears as soon as its job finishes.
Changes:
- live_comment.py: _fetch_artifact_statuses became fetch_all_review_statuses.
The new function lists the artifacts via the API, downloads each one, and
parses it. Removed the review_statuses_json parameter, the
--review-statuses-file argument, and the subprocess import.
- ci.yml: removed the REVIEW_STATUSES environment variable, the inline Python
merger, and the --review-statuses-file argument. Renamed the
ci-timings-review-status artifact to review-status-ci-timings.
- Eight workflow_call files: added a step that writes review-status.json and
uploads it as an artifact after each review_status output.
- test_live_comment.py: added tests for _parse_status_file and
_merge_statuses.
The poller logs transitions between polls. It reports newly completed jobs
(with their results), newly appeared jobs, and jobs that left the pending
list. Each comment update shows the reason for the change. For example:
'1 new completion(s); artifact statuses updated'. When nothing changed, the
poller lists the jobs that are still pending. The status line shows the raw
job count from the API and the number of infra jobs that the filter removed.
scripts/tests/ has held three PowerShell suites that no workflow ever
invoked -- there is no Windows runner in CI, so they have been inert since
they landed. A regression test nothing executes is worse than none: it
reads as coverage.
Adds a windows-latest job, gated on a new `installer` lane so it only fires
for PRs touching install.ps1 or its tests. The 8.3 suite runs under both
pwsh 7 and Windows PowerShell 5.1, since install.ps1 arrives via `irm | iex`
into whichever shell the user already has and 5.1 is what ships with Windows.
Only the 8.3 suite is wired up. The other two fail on main today for
unrelated reasons; they can join once they are fixed.
Hermes installs runtimes for itself — `uv` at `$HERMES_HOME/bin/uv`, Node
at `$HERMES_HOME/node` — and neither directory is on an arbitrary
process's PATH. Every `shutil.which("node"/"npm"/"npx"/"uv")` in Hermes's
own code therefore has two failure modes: the managed runtime is invisible,
so the caller reports "not installed" or degrades to a slower tier on a
machine that has exactly what it needed; and when a system copy also
exists, the one Hermes does not own wins.
Routed the Hermes-owned call sites through managed-aware resolvers:
- `agent/lsp/install.py`, `hermes_cli/dep_ensure.py`, `hermes_cli/main.py`
(`_make_tui_argv`), `hermes_cli/tools_config.py` (`_run_post_setup`) now
use `find_node_executable()`.
- `hermes_cli/tools_config.py::_pip_install` and `hermes_cli/setup.py`'s
vercel install use `ensure_uv()` (installing uv is in scope during setup,
and the Windows installer's `uv venv` does not seed pip, so the fallback
tier is "No module named pip"). `tools/lazy_deps.py` uses `resolve_uv()`
— a lookup, not a bootstrap, because it runs mid-turn for an optional
dependency and downloading a runtime as a side effect exceeds what the
caller asked for.
- `hermes_cli/gateway.py`: extracted `_append_node_dir_for_service()`,
shared by the systemd unit and launchd plist generators, which appends
the managed dirs before the PATH-resolved one. A service definition is
written once and survives reboots, so resolving a system Node that
happens to lead the installing shell's PATH bakes the wrong interpreter
in permanently. Managed dirs are profile-scoped, so each profile's unit
still names its own Node; the existing symlink-parent rule (don't
`.resolve()`) is preserved verbatim.
- `tools/environments/local.py`: the terminal tool's subshell PATH gains
the managed dirs, appended alongside the sane entries rather than
prepended — a tool the user deliberately put on their own PATH still
wins, and the managed one only fills a gap. This is also what makes the
bare `which("uv")` in `tools/env_probe.py` correct: that probe reports
the environment the *model* sees, and the model can only run what is on
that subshell's PATH.
`scripts/install.ps1`: the persisted User PATH update becomes
`Set-ManagedNodeFirstOnUserPath`, a move-to-front rather than an
add-if-missing. Installs made by an older install.ps1 already have the
managed dir in User PATH — at the tail, behind a system Node — and an
add-if-missing check sees it present and leaves that ordering in place
forever, so the users the bug hurt would never be repaired. Unrelated
entries keep their relative order (empty segments included; a trailing
`;` is legal and the installer's other PATH code preserves them),
duplicates collapse, and it writes only when the string actually changes.
Tests:
- `tests/test_managed_runtime_resolution.py` — AST guard that fails any
new bare `which()` for a managed runtime, with a short justified
allow-list and a companion test that fails when an allow-list entry goes
stale. Reading source is banned by AGENTS.md and this is the documented
exception: the property is "no call site anywhere spells it this way",
which no runtime seam can observe.
- `scripts/ci/test_install_ps1_path_migration.ps1` — behavioral, not a
source regex: it lifts the real `Set-ManagedNodeFirstOnUserPath` out of
install.ps1's AST and rewrites only the two registry calls into an
in-memory store, so the shipped split/dedupe/prepend/change-detection
logic executes for real. Not in the default lane (Linux runners have no
PowerShell host); runs under `pwsh`. 13/13 assertions pass.
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.
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).
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).
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.
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)
#66373 swapped GITHUB_TOKEN -> AUTOFIX_BOT_PAT across the workflows and
#66577 restored the `|| github.token` fork fallback for detect-changes and
the label gates -- but it missed the ci-timings "Collect timings and
generate report" step, which still passes a bare AUTOFIX_BOT_PAT. On fork
PRs that PAT is empty, so timings_report.py hard-fails at
expect_env("GITHUB_TOKEN") before it can reach its own "degraded run must
never redden the PR" soft-fail path. Every fork PR gets a red run from this
advisory job (e.g. #66573).
- ci.yml: apply the same `secrets.AUTOFIX_BOT_PAT || github.token` fallback
to the timings step. github.token has `actions: read`, enough to read the
run's job/step durations on forks.
- timings_report.py: treat a missing/empty GITHUB_TOKEN as a degraded run
(TimingsUnavailable) instead of a hard ValueError, so this whole class of
failure can never redden a PR again even if a future workflow drops the
token. Still writes no JSON, so no empty baseline is ever cached.
git diff on a lockfile is unreadable: npm reorders entries, rewrites
integrity hashes, and moves packages between nesting levels, so a
one-line package.json bump produces a thousand-line textual diff.
scripts/ci/lockfile_diff.py instead parses the `packages` map out of
both versions of every tracked package-lock.json (via `git show`),
reduces each to {install path: version}, and set-diffs the maps —
reorder/hash churn vanishes, leaving only actual version movement
(added / removed / updated, with nested dedup copies tracked
separately).
The lockfile-diff workflow posts the result as a Markdown table in a
PR comment gated behind a hidden marker: subsequent pushes PATCH the
existing comment instead of stacking new ones, and a push that reverts
all lockfile changes updates the comment to say so. Advisory only —
never fails on findings; fork PRs (read-only token) degrade to a
warning.
Wired through the ci.yml orchestrator with a new npm_lock lane in
classify_changes.py (fails open on .github/ changes per the existing
contract).
