F20 (andrexibiza): in --staged/--commit mode the guard jobs ran inside the
snapshot root with the snapshot's git env, but the health job ran in the real
worktree. Its source was pinned to the index tree, yet its engine and policy were
not: Measurer reads scripts/ci/profile_scope_patterns.json from the repo it runs
in, so with an identical `git write-tree`, editing only the unstaged P05
path_regex turned PS-P05 / exit 1 into "ok" / exit 0 and back.
The health job now runs like every other job: cwd = the snapshot root with the
snapshot env, so `python -m scripts.code_health` imports the snapshot's own engine
and reads the snapshot's policy files and ruff pin. The ruff job resolves its pin
from the same root. Unstaged edits to the policy file or the engine no longer move
the verdict, and they stay untouched in the worktree; plain runs are unchanged
(root is the worktree).
unsupportedpastels m4: scripts/check says it runs the lint checks CI runs, but CI
also runs scripts/check-case-collisions.py, scripts/ci/check_lazy_deps_imports.py
and scripts/ci/check_profile_archive_boundary.py in workflows of their own
(case-collision-check.yml, lazy-deps-guard.yml, profile-artifact-check.yml). A
case-colliding pair passed scripts/check and failed CI.
All three are deterministic and local (git ls-files / an os.walk of the checkout;
measured here: 0.2 s, ~7 s, 0.1 s on the full tree), so they run as ordinary guard
jobs - in the snapshot root for --staged/--commit like the other guards - and are
selectable with --only case-collisions / lazy-deps / profile-archives.
unsupportedpastels m3: (a) the switch was found by a regex that accepted only the
exact `ENFORCEMENT = "..."` spelling, so `ENFORCEMENT: str = "off"` silently stayed
blocking; (b) local runs and both hooks read it from the merge-base with
origin/main, so a flip on main reached a branch only after a rebase, contrary to
"CI and local hooks alike".
(a) parse_switch() reads the module-level plain or annotated assignment with ast
(last one wins, as at runtime); missing, unparseable or unknown values are
blocking. (b) Without --base, the base is still the merge-base of the head (a
--head commit, else HEAD) but the switch comes from origin/main when it resolves,
falling back to the base. An explicit --base pins both (CI passes HEAD^1, the
target tip). scripts/check --commit now hands the commit itself to the ratchet so
it derives that commit's merge-base. The head under test never supplies the switch.
(c) A test pins enforcement() on this tree to config.ENFORCEMENT.
F21 (andrexibiza): PS-P05/PS-P06 ran their regexes over raw source lines, so a
comment such as `# Avoid env = os.environ.copy(); use the scoped builder.`, the same
sentence in a docstring, or a comment warning against `os.getenv("DISCORD_TOKEN")`
produced a blocking hit although nothing builds an env or reads a credential.
Before the regexes run, comment tokens (tokenize) and string statements (ast:
docstrings and bare string examples) are blanked in place, preserving every line
number and line break. Ordinary string literals inside code are kept, so the
env-key argument of `os.getenv("DISCORD_TOKEN")` still matches; the real
`env = os.environ.copy()` / `dict(os.environ)` / platform getenv stay flagged. If
tokenizing fails, the raw lines are used (more hits, never fewer).
banditburai (inline report.py:36): `# health: allow BLE001 S110 -- boundary` waived
neither rule and reported "allow comment ignored: it needs a reason", although the
reason is there. _ALLOW accepts spaces in the rule list, but apply_allows split it
on commas only, so "BLE001 S110" was looked up as one rule name.
The rule list is now split on commas and whitespace, so `BLE001 S110`,
`BLE001, S110` and `BLE001,S110` all waive both rules. A reason is still required.
unsupportedpastels m5: when the pinned TypeScript is not cached, ts_measure runs
`npm install`; a failed or timed-out install raised CalledProcessError /
TimeoutExpired, which cli.main did not catch (it caught RuntimeError only). The run
then died with a traceback and exit 1, which reads like a finding.
cli.main now reports those as a broken run: one `code health: measurement tooling
failed: ...` line on stderr and exit 2, like every other environment error.
JoaoMarcos44 (inline cli.py:64): the ENFORCEMENT="off" and "no measured files
changed" branches returned before the --json formatter, so
`python -m scripts.code_health --base HEAD --head HEAD --json` succeeded with prose
on stdout and json.loads() raised.
Both early successes now print the empty findings list `[]` on stdout and move
the human explanation to stderr; findings and zero-finding measured diffs were
already JSON. Exit codes are unchanged on all four paths.
F22 (andrexibiza): `scripts/check --only heath`, `--only ,` and `--only heath,rof`
filtered the job table down to nothing and reported "0 checks, ok" / exit 0, and
`--only health,rof` silently dropped the unknown name. A typo in a developer or
automation command could claim success without running the requested check.
The whole selector set is now validated before any job is built or dispatched:
an empty selection or any unknown name is a usage error (exit 2) that lists the
valid names. A valid selector still runs its check and keeps its status.
Why (cost, same concern as Adolanium's line-matching note): pairing repeated names by body is
all-pairs within a group, and the per-group lookup of base units scanned the whole base file,
so a codemod touching every function of a big file was quadratic (5,000 edited unique names:
5.4 s -> 0.04 s with a per-file name index). A group whose all-pairs check would exceed
250,000 pairs (e.g. hundreds of edited `it(...)` callbacks in one scope) falls back to the
ordinal pairing, which is bounded; measured 2,000-callback group: 0.05 s.
Why (M2 follow-up; invariant 4, no trading): the previous commit let a moved+edited unit's
new-looking hits spend any departed occurrence of its origin unit by line text, so a swallow
dropped at the top of `legacy` paid for one added at the bottom of the moved copy (exit 0;
red on the previous commit), while the same relocation in place blocks. A unit matched
across files (pass 5) now compares its lines with its origin unit's (SequenceMatcher, code
only), exactly like an edit in place: hits on unchanged lines are old, the rest are new.
The departed-occurrence pool is now module-level only.
Why (M1 follow-up; invariant 4, no trading): matching a departed module-level occurrence by
its line alone let `except Exception:` stand for any guard, so deleting
`try: import json / except Exception: pass` from one file paid for a new
`try: import yaml / except Exception: pass` in another (exit 0; red on the previous commit).
A module-level occurrence now moves only with its whole top-level statement unchanged
(found by indentation: a whole try/except block, or a one-line constant). Moving an unchanged
HX005 module constant to another file passes; a different constant still blocks.
Hits inside a unit matched across files keep line matching: only that unit's own departed
occurrences can pay.
Why:
- banditburai (inline, py_structure.py:97) and unsupportedpastels M3: repeated names (Python
`@method(...) def _`, `f#2`; TS `<anon>#n`) were paired by source-order ordinal, so a new
CC-22 `_` inserted above a trimmed one took its cap (exit 0), and inserting a small `_`
above an unchanged CC-22 one made the old one "new" (exit 1). Same-file name matching now
groups units by name without the ordinal and pairs the most similar bodies first
(difflib ratio >= 0.6 over code lines, comments/whitespace dropped); the last unit left
with a name is that name's unit, exactly as a unique name was before. Exact-hash passes
still run first. If a body span is unknown, the group falls back to the ordinal.
- unsupportedpastels M2: cross-file matching was body-hash only, so a function moved to
another file with one constant changed lost its cap (P1 exit 1; same edit in place exit 0).
New last pass: another file, same name, similar body (>= 0.6), only when the name is gone
from its origin file and exactly one unit of that name left and exactly one arrived
unmatched. A copy (origin keeps it), two arrivals, or an unrelated body reusing a deleted
name stay new code (andrexibiza: never arbitrary same-name credit). Its existing hits
follow it through the departed-occurrence pool (one-to-one), a second swallow is new.
Unit gains an optional end_line; until the measurers report it (py_structure: lane A,
measure.py: lane C) the span is estimated from the unit's own FUNC_LINES. Tests pass both ways.
Why (round-5 review, unsupportedpastels M1; Adolanium cost note):
- M1: code outside any unit is not a unit, so its hits could survive only through the line
diff of a file against its own base (or git-detected rename). Moving an unchanged
module-level `try/except Exception: pass` into pkg/a_compat.py reported S110 + BLE001 as new
(exit 1). Now a base occurrence whose line LEFT its file may pay, once, for an identical
occurrence (same rule, same normalized code) arriving in another file in the same diff, at
module level or inside the unit its own unit was matched to. One-to-one: the spent
occurrence no longer counts as credit at its origin. A copy (origin line survives) still
blocks; two arrivals for one departure block; a re-add in the same file stays new; a
module-level hit never pays for one inside a function.
- Adolanium §3: SequenceMatcher ran for every changed file, TS included (1.66 s on desktop
main.ts for a one-line change); line matching is now skipped when neither side has hits.
Why (round-5 review, unsupportedpastels m5, D4): `npm install --prefix <cache>` makes the
cache dir the npm project, so the repo's .npmrc (min-release-age=14) was ignored: `npm config
get min-release-age` from the prefix was undefined. The repo .npmrc is now copied into the
prefix before installing (verified: `npm config get min-release-age --prefix <cache>` -> 14).
A failed or hung install raised CalledProcessError/TimeoutExpired, which cli.main does not
catch, so the run exited 1 with a traceback that read like a finding; both now raise
RuntimeError with npm's stderr, which cli.main maps to exit 2.
Why (round-5 review, unsupportedpastels M3/m1/m6 + §3; Adolanium, helix4u):
- M3: methods of object literals were named `f`, `f#2` and paired by position, so swapping
two objects gave b.f a.f's cap of 31 (V9a). They are now named by the variable/property
that holds the object (`a.f`, `b.f`). Overload signatures (and `declare`/abstract
signatures) have no body and are no longer units, so adding an overload no longer shifts
the implementation to `old#4` and makes it new code (V10a).
- m1: the body hash covered raw text, comments included, so rename + one comment line was new
code in TS (not in Python). It now hashes the parser's token stream (no comments, no
whitespace, no JSDoc), collected once per file.
- §3 (Adolanium, helix4u): `export function f(x: number {` exited 0 because the parser
recovers and sf.parseDiagnostics was never read. A file with parse diagnostics is now an
error -> MEASURE (fails closed), like unparseable Python. No tracked TS file in the tree
has parse diagnostics (4191 files checked).
- m6: the header documents exactly where the CC count deviates from ESLint's complexity
(optional chaining, &&=/||=/??=, default parameters are not counted).
Units also carry their end line (`end`), for body-similarity pairing.
Follow-up to the per-scope handle tracking (Adolanium §4): a repo-wide sweep
of the old vs new engine showed one lost hit,
tui_gateway/host_supervisor.py `_wait_for_exit(self, proc:
subprocess.Popen[str])` -> `proc.wait()`. The old module-wide map flagged it
only because another function happened to bind `proc` to a Popen; scoping
correctly treats a parameter as unknown, which also drops the case where the
type says exactly what it is.
A parameter annotated `subprocess.Popen[...]` (sync) or
`asyncio.subprocess.Process` (async) now starts as a handle of that kind;
unannotated parameters still clear the name.
unsupportedpastels M4 (review 5408300628 §2.3) and m7: the printed fixes would
break code or did not match the engine.
- HX010 led with `is_truthy_value`, which returns False for every real key,
while most hits are "is it set" checks (`bool(os.getenv("..._API_KEY"))`).
It now offers `is not None` / `!= ""` for presence and `is_truthy_value`
only for boolean flags.
- HX001 only said `get_hermes_home()`, which would move a path that lives
under the user's home on purpose (the profiles root); such a path takes an
allow with a reason.
- BLE001: the ratchet runs ruff with --ignore-noqa, so `# noqa: BLE001`
waives nothing, and a message-only `logger.warning(..., e)` still fails.
Verified with pinned ruff 0.15.10 through the real comparison path
(test_ble001_fix_text_matches_the_engine): logger.exception, any log call
with exc_info=True, and re-raise pass; `% e` logging and `# noqa` block;
`# health: allow BLE001 -- why` waives.
Text only: no rule's blocking flag or ENFORCEMENT changed.
unsupportedpastels M4 (review 5408300628 §2.3), each fixture a new file that
exited 1 at 96bb97a:
- HX001 matched any name starting with ".hermes", so
`Path.home() / ".hermes-profile-exports"` (hermes_cli/profiles.py) was a
"hardcoded Hermes home". It now matches only the exact `.hermes` path
component (`.hermes`, `.hermes/x`, `~/.hermes/...`).
- HX003 matched every string containing `ps aux` / `pgrep -f`, including
docstrings that explain why not to use them. Expr-statement string
constants (docstrings, bare string statements) are prose and skipped; real
command strings, assignments and f-string parts stay flagged.
- HX012 flagged `threading.Thread(target=copy_context().run, ...)`, the
context-preserving form the plan lists as a fix (agent/auxiliary_client.py
uses it). A target of `<ctx>.run` is accepted when ctx is
`contextvars.copy_context()` itself or a name this same scope binds only to
`copy_context()` (keyword or positional target). A ctx from another scope,
a rebound name, or any other target stays flagged.
JoaoMarcos44 (inline py_structure.py:49, review 5408439923; reproduced by
andrexibiza): nesting_depth() treated any If that sits alone in an orelse as
an `elif`, but `else:\n if b:` produces the same AST. Seven physical
`else: if` levels measured 1 (adding an inert `pass` made them measure 7), so
the gate depended on an irrelevant statement rather than on indentation.
A real `elif` If node starts at the `elif` keyword, i.e. at its `if`'s
column; an `if` nested under `else:` must be indented deeper (a compound
statement cannot follow `else:` on the same line). Only the former keeps the
depth. Seven physical levels now measure 7 with or without the inert pass
and block; a genuine seven-arm elif chain stays at 1 and passes.
andrexibiza's F15 (changed form, inline on py_rules.py:250): body_hash()
disabled self-reference normalization whenever the textual dump contained
`arg('<name>'` or `Name('<name>', Store())` anywhere in the subtree, so an
unrelated nested `def identity(legacy)` made a pure recursive rename of
`legacy` lose its cap (FUNC_LINES 302 > target 300 as new code). Even with
the check removed, the string replace would also have blanked the nested
function's own `return legacy`, which refers to its parameter.
References are now resolved lexically: only bindings in the function's own
scope (params, assignments, nested def/class names, imports, global/nonlocal,
except/match captures) disable normalization, and a nested def/lambda/class/
comprehension that binds the name keeps its own reads. The matched Name /
Attribute nodes are blanked in place for the dump and restored. Controls: a
rename that leaves the old self-call is different code, an own-scope rebinding
is not a self-reference, a closure's call to its enclosing function is.
andrexibiza's F9 (changed form, inline on py_rules.py:250): definition
traversal kept a def's decorators and defaults but dropped its parameter and
return annotations, which run at def time on 3.11-3.13 (the project floor is
3.11) unless the module has `from __future__ import annotations`; measured on
3.11 here, `def f(x: os.getenv("S"))` freezes the launch value in
__annotations__. Conversely, with the future import, a module/class
`x: os.getenv("PATH")` annotation is never evaluated but the whole AnnAssign
was walked, so it was charged as a capture.
Def annotations now count as import-time exprs unless annotations are
postponed; an AnnAssign contributes its target and value always and its
annotation only when not postponed. A nested def's annotations stay out
(they run when the outer function does).
Adolanium (comment 5983756525 §4) and JoaoMarcos44/andrexibiza: an annotated
`results: list = await asyncio.gather(..., return_exceptions=True)` was not
recognised as a gather result (only ast.Assign was), so a plain `: list`
annotation hid the exact pattern HX009 exists for. Conversely, unpacking a
zip/enumerate tainted every loop variable, so `isinstance(payload, Exception)`
over `zip(payloads, results)` or the `i` of `enumerate(results)` was flagged.
AnnAssign values now propagate results, and loop/comprehension targets are
projected positionally: each element of a zip follows its own argument, an
enumerate taints only its item. Non-tuple targets keep the old whole-target
behaviour.
JoaoMarcos44 (inline py_rules.py:442, review 5408210901; reproduced by
andrexibiza): after `proc.kill()` only an expression or plain-assignment
`proc.wait()` was recognised as reaped, so `return proc.wait()` and
`rc: int = proc.wait()` were flagged while the identical `rc = proc.wait()`
passed, and refactoring an accepted assignment into a return gained a blocker.
Return and AnnAssign values now count as the adjacent wait. The negative
controls stay flagged: no preceding kill, a different killed receiver,
communicate() after kill, and asyncio Process.wait() after kill; an explicit
finite timeout is still accepted. The assignment->return refactor is covered
through the real comparison path.
Adolanium (comment 5983756525 §4), corroborated by andrexibiza (F13 receiver
class) and helix4u: _process_handles() keyed handles by variable spelling for
the whole module, so an async `proc` in one function made a correctly bounded
sync `proc.communicate(timeout=1)` in another function fail HX006 (and a
`def stop(proc)` parameter inherited whatever `proc` meant elsewhere).
A plain-name handle now belongs to the scope that binds it: the innermost
scope binding the name decides (class bodies are invisible to methods,
closures see their enclosing function), and within it the latest binding
before the call, so reassignment (Popen -> asyncio process -> None)
re-classifies the receiver. Parameters and other bindings clear it. Attribute
handles (`self._proc`) stay shared by the methods of their class, keyed by
that class rather than by spelling across the module.
andrexibiza's F19 (review 5407834086, inline): _capture_lines() returned after
the first eager capture it met. The reverse stack reaches an existing dict
entry first, so inserting `"new": os.getenv("HOME"),` above
`"old": os.getenv("PATH"),` produced no new HX005 occurrence and the
comparison could not charge it.
Every independent capture is now emitted (dict entries, list items, defaults);
a capture nested inside another (`expanduser(getenv(...))`) is the same
occurrence and is not entered, and per-line duplicates still collapse in the
measurer. Deferred bodies stay excluded. Tested through the real comparison
path: added dict/list/default captures block, a lambda entry and unchanged
debt pass, a separate assignment blocks.
andrexibiza's F12 (changed form, review 5407834086): canonical_tree() pooled
every parameter, assignment and def in the module into one "rebound" set, so
`from subprocess import run as execute` stopped resolving everywhere as soon
as an unrelated function had a parameter or local named `execute`, and
`launch(cmd): return execute(cmd)` lost its HX006.
Bindings are now recorded per scope (module, class, function/lambda,
comprehension) and each read resolves through the scope chain the way Python
does: the innermost scope that binds the name decides, a scope that binds it
other than by one import makes it a plain local there, class bodies are not
visible to nested functions. Genuine shadowing in the consuming function (or
at module level) still stops resolution.
The replay picked PRs with sort:updated-desc, so the sample changed whenever an old PR got a
comment and two runs could not be compared (unsupportedpastels M6). It now takes every PR merged
inside a closed date window in PR-number order, writes manifest.json (number, base, head), and
--manifest replays exactly those ranges. The summary prints one total: PRs with at least one
blocking finding, instead of mixing blocking and warning-only rows.
- `ENFORCEMENT` in scripts/code_health/config.py (blocking / advisory / off)
turns the ratchet down or off with a one-line commit to main. It is read
from the BASE revision, so it applies to fork PRs (repo variables are not
passed to fork-triggered workflows), CI and local hooks alike, and a PR
cannot relax its own check.
- `scripts/check --install-hook pre-push` judges each pushed commit the way
CI judges the PR (base = that commit's merge-base); `--commit REV` is the
underlying mode, `--uninstall-hook` removes our hooks and leaves foreign
ones. Pre-commit and pre-push share one tree snapshot that never touches
the user's worktree or index.
- A failing report names the local commands; CONTRIBUTING, AGENTS.md and the
PR template point at `python scripts/check`.
SIGKILL bounds a sync Popen.wait() that directly follows kill(), so it no
longer needs a timeout. communicate() after kill() and asyncio's
Process.wait() after kill() stay flagged: both read/close the pipes, and a
grandchild holding them blocks forever (measured: > 20 s with a background
`sleep`; the sync wait() returned in 0.00 s). The HX006 fix text now names
the reap pattern.
Fourth review round (F11-F18, N1-N4, kvnloo, Adolanium):
- Hits are old only where the base occurrence survives: a line the comment- and
whitespace-blind line diff keeps (blame semantics), or anything inside a unit whose
body is unchanged. An identical violation moved past unrelated code is new; removing
an allow comment or re-indenting is not. Hit fingerprints exclude comments (N4).
- Inline allows cover their own line; only a comment-only line covers the next (N3).
- Rules read a copy of the AST with import aliases resolved, so `sp.run`,
`from subprocess import run as execute`, aliased threading/asyncio/load_config are
seen (F12); deadline helpers must resolve to the real asyncio API, and
communicate()/wait() are judged only on receivers holding a spawned process (F13);
infinite deadlines are unbounded (F14).
- HERMES_* writes introduce a name too (N1); bool(os.environ[...]) is HX010 (N2);
HX009 needs the isinstance subject to come from gather(return_exceptions=True).
- Recursive self-references are normalised out of body hashes, so a recursive rename
keeps its cap, py and ts (F15). Decorators belong to their function's scope and
classes are matchable units, so their hits follow proven moves (F16).
- An over-cap file may not grow (F17, Teknium: strict improvement-only).
- A file the checker cannot parse or measure (ast/ruff/TS parse error) fails.
- scripts/check: --staged always lints an index snapshot (ignored files are invisible
to git status, not to guards) (F18); create the --prefix dir for Git for Windows
(F11); --only builds just the selected jobs, so it needs no ruff for non-ruff checks.
- CODEOWNERS covers scripts/check + scripts/code_health; frontend-only PRs skip the
Python-only lint jobs again and run only the ratchet.
Final review on #132646 (R1-R4), each reproduced on d03b089 first:
- Git blobs decode as utf-8-sig like working-tree reads, so a BOM no
longer makes one side unparseable: a new BOM file's debt is measured
(the hook used to let it through) and existing BOM debt is not new (R1).
- One walker, _eager(), yields what evaluating a node runs now: a
lambda/def contributes only its defaults and decorators, a generator
only its first iterable. HX005, HX006 and HX007 all use it, so a
coroutine defined inside `asyncio.timeout()` is not treated as bounded
and a returned sync loader is not a sync call in the coroutine (R2).
- `wait_for(fut=..., timeout=...)` is recognized like the positional
form (R3).
- HX005 also checks lambda defaults, `if`/`while` tests, `with` context
expressions, match guards and walrus bindings; a generator's body stays
deferred (R4).
Second-round review on #132646 (F1-F10), each reproduced on 2531fb2 first:
- Moves are matched one-to-one: unchanged units are reserved first, then
same-file body, then cross-file body only when the origin's name is gone,
then same name. A copy of unchanged debt is new debt; a function moved
over a same-name stub keeps its own cap (F1, F3).
- Every base hit is owned by exactly one head scope, so a file rename no
longer counts each old hit twice (F2).
- Ruff runs when either side has Python, so a delete-only .py diff with a
TS edit no longer crashes on a missing ruff result (F4).
- Deadlines must be finite: timeout=None, **{} and asyncio.timeout(None)
are no deadline; awaited calls inside `async with asyncio.timeout(x)`
are bounded (F5, F8).
- HX005 walks only what runs at import: the import-time branch of a
`__name__` guard (either operator), loops, match, try*, decorators,
class bases; a lambda value is deferred (F9).
- `health: allow` counts only in real comment tokens (Python tokenize, TS
comment trivia), never in string literals (F7).
- TS function lines are owned by the innermost function, so siblings on
one line give it up once (F10).
- The pre-commit hook runs `scripts/check --staged`: health compares the
index tree, and when the working tree differs from the index the other
checkers run on a checkout-index copy with a copied index, so commits are
judged on the staged bytes and unstaged work is never touched (F6).
Independent review of #132646 found five defects, each reproduced and fixed:
- Pattern hits from any vanished scope went into a shared (rule, text) pool, so deleting one
`except Exception:` paid for an unrelated new one elsewhere, and class scopes (never units)
always looked vanished. Hits now transfer only along a unit matched by body hash.
- A unit's prior is now looked up by body hash before qualname, so a TS anonymous callback
whose source-order ordinal shifted keeps its cap.
- The TS body hash included the function name; it is now parameters + body, so a rename keeps
its cap (as in Python).
- HX006 flagged `await asyncio.wait_for(proc.communicate(), timeout=...)`; calls passed to
wait_for/timeout are bounded. The fix text now names the asyncio form.
- scripts/check printed output only on failure, hiding advisories and comment allows; the
health report is now always shown.
Docs now say growth of a file already over 2,000 lines is a warning for now.
scripts/code_health measures the files a change touches on both sides
(base revision and head) and fails when:
- a function or file over target grows past its own value on the base
(CC 20, 300 own lines, nesting 6, file 2,000 lines);
- a new function or file is over target;
- a pattern rule gains a fingerprint (rule + enclosing function + line
text), so fixing one violation never buys another.
Code that moves keeps its cap: functions match by name-independent body
hash, pattern hits by (rule, line text) once their function has left the
old place. Nested functions are their own units, so a closure edit never
counts against the hook or function around it. Python CC is ruff's C901
minus the direct nested defs; TypeScript is parsed with the lockfile's
typescript (no type check).
Rules: swallowed exceptions (S110, BLE001, E722), correctness (F821/F823/
F811/F841, RUF006), hangs (ASYNC230/240, S113, subprocess/urlopen without
timeout, sync config I/O in async def, get_event_loop, gather results
checked against Exception), profile scope (hardcoded ~/.hermes, a new
HERMES_* env var, UnscopedSecretError -> os.getenv, import-time capture,
raw threading.Thread, P05/P06 from profile_scope_patterns.json), argv
process identity, bool() of an env string, and if/elif ladders.
Growth of a file already over 2,000 lines is advisory: replaying 300
merged PRs, a blocking file-size ratchet would have stopped 46 of them
before the 119 over-target files are split. Everything else blocks; the
same replay finds 18 PRs that would have needed a change, every finding
a real hit for its rule.
No baseline file: base is measured on the fly, so an improvement that
lands on main is the new cap at once and parallel PRs never conflict on
ratchet state. `# health: allow <RULE> -- <why>` is the reviewed escape.
The HermesHome/InstallDir guard checked HermesHome first and never looked at
HERMES_RUNTIME_DIR once HermesHome sat inside InstallDir, so the documented
escape hatch could not lift the refusal (Windows lane: store-guard case 4).
The store is HERMES_RUNTIME_DIR when set, else under HermesHome; judge that.
The runtime.ps1 Unicode tests read the helper's answer through a UTF-8 file:
under code page 437/936 the console cannot carry the profile's characters,
so a stdout capture lost them (and the 65001 cell's piped ConvertTo-Json
unrolled the one-element array into a string).
Extends #124526's class fix to the two remaining native captures that can
carry a profile path, and closes the secondary issue from the report:
- scripts/desktop-update/runtime.ps1: the launcher --print-runtime-command
JSON and the legacy user-bin --version 'Install directory:' capture are
both scoped to [Console]::OutputEncoding = UTF-8 and restored in finally,
matching install.ps1's Invoke-Native -Utf8Output and every other consumer
of the boundary (windows_ssh_runtime, the Electron updater, the Rust
bootstrap all decode UTF-8). PS 5.1's OEM decode mojibaked the reported
install root so the legacy identity check never matched on a non-ASCII
profile.
- scripts/install.ps1: -HermesHome equal to or inside -InstallDir puts the
pm tool store inside the checkout; the repository stage's occupied-
directory preflight then refuses every retry after the first run
populated <InstallDir>\tools. Refused up front unless HERMES_RUNTIME_DIR
parks the store elsewhere.
Tests: tests/scripts/desktop_update/test_desktop_update_runtime_unicode.py
(real published launcher, non-ASCII profile, CP936/437/65001, encoding
restore) and scripts/tests/test-install-ps1-store-guard.ps1 (real installer
subprocess through -ShowResolvedPaths: refusal, escape hatch, defaults).
Co-authored-by: 择梦舟 <158067205+dreamfarer-space@users.noreply.github.com>
The libatomic sudo warm-up checked only for a terminal, so
--non-interactive, -NonInteractive and --skip-setup still reached
`sudo -v </dev/tty` inside an SSH session or any PTY-allocating runner.
Gate it on NON_INTERACTIVE like setup and gateway. Reported by
@JoaoMarcos44 on #132664.
The clone and downloads can outlast sudo's timestamp_timeout, after which
PM's sudo -n repair silently falls back to the manual remedy. Refresh the
ticket non-interactively right before PM installs. Compute the bootstrap
target once in stage_prerequisites.
PM's libatomic repair runs `sudo -n` under its install lock, so on a host
whose sudo needs a password the automatic attempt can never succeed and
`curl | bash` still ends in "pm install failed". When the glibc host lacks
libatomic.so.1 and a terminal is attached, cache sudo credentials during
the prerequisites stage so the later non-interactive install goes through.
Root, passwordless sudo, musl, and headless runs are unchanged.
Follow-up to the cherry-picked session handshake, rebased onto the
multiplex bridge ownership that landed in #132518:
- connect(): the foreign-session fatal sits ahead of main's
secondary-aware pidfile/port reaping, so neither path signals a bridge
that serves another profile's session.
- Drop the separate "no session -> restart" branch. A bridge without the
field runs an older bridge.js, so its scriptHash already marks it
stale and it restarts through the existing path.
- Inline sessionIdentity() as path.resolve() in /health and drop its
node test, which only exercised path.resolve and no CI lane runs.
- Collapse the four adapter cells into one parametrized invariant (own
session adopted; another session never adopted or killed, connected or
disconnected). The read-receipt test now reports its own session, so
it still restarts for the read-receipt reason rather than a missing
field.
- Docs: fold the separate-gateway guidance into the existing "Multiple
profiles" section instead of a second multi-profile section.
Co-authored-by: Victor Kyriazakos <victor@rocketfueldev.com>
Two profiles on one host default to the same bridge_port (3000) but have
profile-scoped session paths. _reuse_running_bridge adopted any connected
bridge whose scriptHash and read-receipt config matched, so profile B
adopted profile A's bridge and served A's session; on any mismatch,
connect() fell through to the pidfile/port kill path and killed A's
running bridge.
Contract, keyed on a new /health `session` field (normalized absolute
session dir via bridge_helpers.js sessionIdentity):
1. session present and EQUAL to the adapter's own normalized session
path -> today's logic unchanged (adopt on hash + read-receipt match,
else stale/restart).
2. session present and DIFFERENT -> FOREIGN bridge: not adopted, not
killed. connect() sets a fatal error naming the port, the foreign
session path, and the fix (platforms.whatsapp.extra.bridge_port, one
distinct port per profile).
3. session ABSENT (bridge predates the handshake) -> treated as stale
and restarted, exactly like an absent scriptHash today.
Normalization uses Node path.resolve / Python os.path.abspath - absolute
and `..`-normalized WITHOUT symlink resolution. fs.realpathSync /
Path.resolve() would resolve symlinks asymmetrically (macOS
/tmp -> /private/tmp) and produce false foreign verdicts.
(cherry picked from commit 7e8418678e91fa4bc69d4b4fa6faedd66f2a80e5)
A layout matrix (single/fleet × strip/condensed × local/remote active,
starting from a named profile or the all view) asserts the active
gateway's default is one click away in each. The four fleet+condensed
cases fail on main.
The core E2E lane only starts on PRs touching its guarded paths, so a
profile-rail edit would have met fleet-condensed-default.spec.ts only on
main. Add the rail sources to the lane.
Fleet telemetry labelled every `hermes -z` / `chat -q` / `-Q` run as
entrypoint=interactive because the cli surface mapped straight to
interactive and no caller declared anything else, so 95% of CLI sessions
read as one-turn humans and scripts could not be told apart.
- New closed entrypoint `one_shot` (contract + v3 schema enum). The task
scope start declares it for cli-surface runs carrying the
HERMES_SINGLE_QUERY_SESSION marker the one-shot paths already set (the
signal session source and cache_ttl: auto read). Children Hermes
dispatchers spawn (kanban worker source=kanban, A2A forward source=a2a)
read the existing `background`. Delegated children, other surfaces and
the REPL are unchanged.
- Engagement keeps treating one_shot like interactive (its prior label).
- `hermes -z` exits via os._exit, skipping the atexit hook that closes the
metrics session: its pre-exit cleanup now runs the runtime shutdown, so
task_run.finished and session.count are actually written (before, only
task_run.started survived and the store kept a hot journal).
- Smoke expectations move to one_shot (it drives `chat --query --quiet`).
- relay-shared-metrics.md documents the entrypoint vocabulary.
A login bash reads only the first of ~/.bash_profile, ~/.bash_login and
~/.profile. wire_shell_path always wrote ~/.profile and ~/.bash_profile
when present, so a user whose only login file is ~/.bash_login never saw
~/.local/bin on PATH. Treat ~/.bash_login like ~/.bash_profile: append the
guarded PATH line when the file exists, never create it (creating
~/.bash_profile would shadow it).
The test helper resolves bash by absolute path so the login-PATH tests run
on hosts without /bin/bash (NixOS).
scripts/windows-build-deps.ps1 discovers the ARM64-capable Visual Studio
(vswhere -requires Microsoft.VisualStudio.Component.VC.Tools.ARM64) and
initializes VsDevCmd against it, then builds OpenSSL through vcpkg. But
vcpkg resolves Visual Studio on its own and takes the newest instance. On a
host with a second BuildTools install that lacks the native arm64 toolset,
the arm64-windows triplet fails:
error: in triplet arm64-windows: Unable to find a valid toolchain for
requested target architecture arm64.
The selected Visual Studio instance is at: ...\Microsoft Visual Studio\18\BuildTools
The available toolchain combinations are: x86, amd64, x86_amd64,
amd64_x86, arm64_x86, arm64_amd64
...in the same run that logs "-> ARM64 C++ build tools found: ...\2022\BuildTools".
The install fails, `hermes update` exits 1, the retry fails identically, and
the desktop relaunches on the previous build ("running with the previous
dependencies — run `hermes update` to finish it"), so it never updates.
Export the discovered instance as VCPKG_VISUAL_STUDIO_PATH, the documented
lever vcpkg's own error points at, without clobbering a caller's explicit pin
(same respect-if-set shape as CARGO_HOME/RUSTUP_HOME above).
Extends the initializer fixture: the discovered instance must be pinned when
unset, and a caller pin must survive the call. Needs a Windows runner — the
suite is platforms("windows") and the initializer refuses non-Windows hosts.
`activate` is bash syntax, so fish users had no way to take on the dev
environment. `activate.fish` does what `activate` does: snapshots the keys it
will change, applies the composed env, defines a `hermes` function that
refuses outside the worktree, adds the prompt prefix, and defines
`deactivate`. It shells out to the same bash library, so composition is not
reimplemented.
`pm.environments --format fish` emits the env as `set -gx` statements.
fish's read-only names (`PWD`, `SHLVL`, `_`, ...) are skipped because
`set -gx PWD` aborts the script, and values named `*PATH` stay lists.
`hermes_compose_env` takes the dialect (`sh` or `fish`) as its second argument.
