scripts/ci/check_agents_md_size.py (in scripts/check and lint.yml): root <= 12k, every
AGENTS.md plus its ancestors <= 30k (the joined root->cwd block on a 128k window, minus
the per-file labels), every area file <= _MAX_HINT_CHARS read from the source so the two
cannot drift. On origin/main it reports the root and all 11 root-to-area chains over.
test_ci_only_guards_run_in_scripts_check wrote pkg/Notes.md and pkg/NOTES.md to disk,
which a case-insensitive filesystem (Windows, macOS) holds as one file, so the fixture
could not build its own premise. Each path is now staged from a blob with
`git hash-object -w` + `git update-index --cacheinfo`, which is what --staged judges.
test_staged_health_ignores_unstaged_policy_and_engine compared two reports byte for
byte, elapsed "(0.1s)" included, so it flaked whenever the two runs straddled a tenth
of a second (seen on native Windows). The comparison now drops the elapsed time.
bodyWithoutSelf treated a declaration of the function's name at any depth as a rebind,
so a renamed recursive function containing an unchanged `.map((legacy) => legacy)` kept
its old self-call text, no longer matched its base body, and lost its cap ("CC 22 >
target 20 (new code)"). Only the function's own scope (parameters, a named function
expression's name, declarations outside nested functions and classes) now stops the
normalization, and nested functions that bind the name keep their own reads, as the
Python measurer already does.
_Binder.visit_Global marked the name ambiguous in the module, so `global execute` in a
function hid `from subprocess import run as execute` and HX006 (missing timeout) never
fired on `execute(cmd)`; nonlocal did the same for an enclosing function's import. A
declaration selects the outer binding, it does not rebind it: declared names now resolve
in the scope they point at, and only an assignment (or import) through the declaration
rebinds that outer name.
The hooks' judge() built its private index and tree under `mktemp -d`, a POSIX /c/...
path in Git Bash, and passed it to native git.exe and python.exe through GIT_INDEX_FILE,
--prefix and the checker path. Hermes' Windows terminal exports MSYS_NO_PATHCONV=1 and
MSYS2_ARG_CONV_EXCL=*, so those paths arrived unconverted and the hook died
("Unable to create '/c/.../index.lock'") before any check ran. judge() now hands native
tools the `cygpath -m` spelling when cygpath exists; elsewhere nothing changes.
Measurer.measure wrote each blob's decoded text into the temp tree with write_text's
default newline translation. On Windows that turns a CRLF blob into CR CR LF, so ruff
counted lines differently from the AST and comment tables: hits moved to <module>,
their `# health: allow` waivers were missed, and valid code blocked. Writing with
newline="" keeps the blob's own line endings. The test forces the Windows write
translation so it also runs on Linux.
A manifest row whose range could not be built (base "", head "error: ...") or replayed
(an OID the clone does not have) was counted in the "N of M PRs had a blocking finding"
denominator with no findings, and the run exited 0, so a replay that measured nothing
read as evidence that a rule never fires. Such rows now carry an "error" in replay.jsonl,
stay out of the totals and the denominator, are listed by number, and fail the run.
HX001, HX003, HX010, HX012 and PS-P05/P06 now print as advisory findings instead of
blocking. Reviewers' samples put them between ~12% and ~60% precision, and the plan
ships weak rules as warnings first. Promote one by flipping it to blocking with replay
evidence in the PR. The ENFORCEMENT comment now says where the switch is read from.
Tests that used PS-P05 as a stand-in blocking violation now use a blind except.
No behaviour change: line wrapping in the hook text, parse_switch docstring,
commit_or_none and the new tests; the test helpers lose an unused env parameter.
JoaoMarcos44 (inline scripts/check:40): the hooks began with
`[ -f "$repo/scripts/check" ] || exit 0`, before reading the pushed SHAs, so the
checked-out branch decided whether another revision was checked. Pushing an
invalid checker-containing branch while an older branch was checked out succeeded
without judging it.
Eligibility is now decided from the candidate tree: pre-push runs
`git cat-file -e <local_sha>:scripts/check` per pushed ref (a candidate that
predates the checker is skipped, deletions are still skipped), pre-commit does the
same for the index tree. An eligible candidate is checked out alone into a temp
dir (private index + checkout-index; the worktree, index and stash are untouched)
and judged by its own scripts/check with GIT_DIR pointing at the repo - with the
previous commit's snapshot-bound health job, implementation, job table, config and
policy all come from the judged artifact, whatever is checked out. pre-commit
exports an absolute GIT_INDEX_FILE because git may hand hooks a relative one.
unsupportedpastels M5: both hooks ran whatever python3 came first on PATH, but
scripts/check imports ruff_runner, which imports tomllib (3.11+, the project
minimum). With Ubuntu 22.04's python3 3.10 every hooked commit or push died with
"ModuleNotFoundError: No module named 'tomllib'".
The hooks now use the repo's virtualenv interpreter when there is one (.venv/ or
venv/, bin/python or Scripts/python.exe; this worktree first, then the main
checkout) and fall back to python3, then python. scripts/check checks
sys.version_info before importing the engine and exits 2 with a one-line message
naming the virtualenv instead of a traceback. Verified with a real CPython 3.10.21:
exit 2 and the message, where 96bb97a crashed in ruff_runner.
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 (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.
- `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>
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).
`activate` doubled as the thing scripts sourced non-interactively, so it
carried `--` argument guards and every caller had to tolerate its prompt and
`hermes` function. Composition (setup sync, bootstrap Python, pm env) now
lives in scripts/_activation.sh, and there are two consumers:
- `activate`: interactive, reversible (`deactivate`).
- new `scripts/run-in-hermes-env CMD...`: applies the env to one process and
execs into it. Replaces `scripts/_hermes-python`; every Python script's
shebang and `scripts/run_tests.sh` (which re-executes itself under it,
however it was launched) use it.
`pm.environments --format sh` emits the env as shell exports, so the inline
JSON-to-export Python is gone from `activate`. An install that did not build
the test environment now reads as stale for every consumer.
Tests are split along the same seams: activation support fixtures, bash/ps1
activation, env-script quoting, and the runner's staleness.
`release.py abandon` deleted the draft and wrote the marker but left the
attempt's Stable Release run going. A live run keeps building, and with
autopublish on it can publish the draft that abandon just cleared.
abandon now lists the claim ref's runs of stable-release.yml and runs
`gh run cancel <id> --force` on every one that has not completed. This
happens before the draft is deleted and before the marker is written: the
marker removes the attempt from the outstanding set, so a cancel failure
after it could never be retried. A published release is still refused
before anything is cancelled. The `delete` callable is renamed `execute`
since it now runs cancels too.
The MSIX package carried a single Square44x44Logo.png, so every taskbar and
Start slot on a HiDPI Windows 11 machine was Windows upscaling a 44px bitmap,
and with no altform variants Windows plated the icon itself. The generator now
emits the qualified catalog Windows resolves by itself: <Logo>.scale-{125,150,
200,400} for all four manifest logos at Microsoft's pixel sizes, and
Square44x44Logo.targetsize-{16..256} in the three required theme forms —
unqualified and altform-lightunplated carry the light tile, altform-unplated
(dark theme) the dark tile, the same split macOS dark mode gets. before-build
stages the whole assets/appx directory; app-builder-lib runs makepri and
`makeappx /l` as soon as it sees a qualified asset, so nothing else changes.
The hand-listed *-dark.png logos, generated but never staged, are gone.
Exposed along the way: the commit badge sat outside the portrait's silhouette
clip, and at 16–30px its anti-aliased edge stacked coverage on the plate's
corner pixels and changed the alpha channel between build flavors. The badge
now shares the clip, is plain black, and is left off direct renders below
32px where no SHA is legible anyway.
