The referenced WAL-index-lock docstring on _live_writer_holds_db was removed
by the extraction; describe hermes_state_holders.live_writer_holds_db instead.
Follow-up on the #97330 salvage (#97329): main grew `_canonical_sqlite_path`
after the PR's base; the holder module re-implemented it as a closure. Export
`canonical_sqlite_path` from hermes_state_holders and alias the legacy name in
hermes_state. Drop the unused `_read_proc_argv` / `_looks_like_hermes`
re-exports (no in-tree callers).
Move live-holder inspection into a bounded helper and make repair fail closed when a process still owns the state database, including deleted WAL/SHM descriptors and ambiguous procfs reads.
Preserve the contributor lineage from the original four-commit review train while presenting one coherent release object on current main.
Co-authored-by: fangliquanflq <fangliquan@qq.com>
Co-authored-by: ciabata <296402666+ciabata-git@users.noreply.github.com>
Each TUI instance spawned three stdio MCP server copies: one in the
CLI wrapper, one in tui_gateway.entry, one in the slash worker. The
wrapper's copy is dead weight — _launch_tui blocks in subprocess.call
until the TUI exits, so its registered MCP tools are never invoked,
yet the server process (35-85 MB) lives for the whole session.
Root cause: _is_tui_chat_launch() only detected --tui / HERMES_TUI=1,
so bare `hermes` with display.interface: tui fell through to
background MCP discovery in the wrapper while the TUI gateway
(spawned moments later) ran a second discovery.
Fix: _is_tui_chat_launch() now consults _resolve_use_tui() — the exact
TUI-vs-classic decision cmd_chat makes — for chat commands only
(command in {None, "chat"}), leaving mcp serve / gateway / acp / cron
discovery behavior untouched.
Verified: unit tests (RED->GREEN); E2E with a canary stdio MCP server
in a scratch HERMES_HOME counted 2 spawned copies pre-fix vs 1
post-fix (gateway's only), and the wrapper's RSS dropped ~43 MB.
Related: #71928 (same per-process duplication class), #11115 (lazy
non-core discovery).
Follow-up on the #101437 salvage (#101415). `_own_live_lease_ids()` snapshots
the server's session records under `_sessions_lock`, but the registry file
lock is taken afterwards, and a sibling session attaches its lease to its
record only after `try_acquire_active_session` has already written the entry.
A finalize racing that acquire would read the brand-new lease as an orphan
and drop it. Entries this process wrote inside a 30 s grace window are kept
regardless of the snapshot; real orphans are minutes old.
Follow-ups on the #101406 salvage (#101391):
- The issue's measured incident was a healthy connect followed by silent
polling loss; nothing republished for 11 h. `_schedule_polling_recovery`
(the single entry to the recovery ladder) now publishes "retrying" while
the adapter is running; `_record_polling_progress` already flips it back.
- `BasePlatformAdapter.send_path_degraded` property (default False) replaces
the runner's `getattr(adapter, "_send_path_degraded")` reach into a
plugin-private attribute; Telegram overrides it. `_mark_connected()` reads
the property instead of taking a `degraded=` kwarg, and `_mark_degraded()`
+ `DEGRADED_STATUS_MESSAGE` replace three copies of the same write/string.
- The startup connect stamp (sibling of the reconnect stamp) honours the
same flag.
- Tests: mid-session death publishes retrying; pre-connect death does not;
progress never flips while not running / fatal; runner reconnect stamp
honours the adapter flag.
connect() intentionally returns True when Telegram polling starts in
degraded mode (or a reconnect's require_progress=False skips the strict
readiness gate), so background recovery can retry without failing
gateway startup. But _mark_connected() published platform_state
"connected" unconditionally, and the reconnect watcher in gateway/run.py
stamped "connected" again right after -- so gateway_state.json was
indistinguishable from a healthy adapter for as long as recovery took
(observed ~11h on one seat).
_send_path_degraded already tracks exactly this (set at polling
generation start, cleared on the first confirmed getUpdates round-trip),
so:
- BasePlatformAdapter._mark_connected() takes a `degraded` flag and
publishes "retrying" instead of "connected" when set.
- TelegramAdapter passes its current _send_path_degraded into
_mark_connected().
- The gateway/run.py reconnect watcher checks the same flag before
overwriting the adapter's own status write.
- _record_polling_progress() republishes "connected" the moment
polling actually proves progress, so the state does not stay wedged
at "retrying" until the next disconnect.
Fixes#101391
_sanitize_ws_text now delegates to agent.message_sanitization._sanitize_surrogates
(the same U+FFFD replacement gateway/run.py, oneshot.py, and the chat helpers
use) instead of a second encode/decode round-trip implementation.
Starlette encodes send_text as UTF-8. A lone UTF-16 surrogate in a
status payload used to UnicodeEncodeError and latch the whole Desktop
connection closed. Sanitize the frame and skip only that send.
Final-diff review measured the worst text the 4 MiB walk budget still
admitted at ~20 s of shlex (cost is linear in admitted bytes, ~7 lex passes
per line, so the line cap was not the binding knob). 1 MiB brings it to
~4 s while a 200-script wrapper graph (~5 KB) and a ~1 MB build script are
still allowed.
lifecycle_scan_root_within_budget: state that it uses a fresh budget,
independent of the full guard's walk (review point on #83821).
- one _capped_read_limit() for local and remote reads (the file's history
is local/remote divergence bugs: #76762, #77703)
- line cap 256 KiB -> 64 KiB: shlex is quadratic on a single token, so the
worst text the budget still admits drops from ~60 s to ~17 s of lexing;
real shell lines are far below either
- fix a comment that described an encode the code does not do; trim the
header comment to the non-obvious WHY
The referenced-script walk in cron/lifecycle_guard.py capped each file
(1 MiB) and the recursion depth (8) but not the walk: a command referencing
hundreds of scripts, or one enormous shlex token, held the GIL for minutes
on every gateway terminal call (#78398).
Add a per-walk _LifecycleScanBudget (bytes, lines, longest line, unique
paths, remote reads) charged BEFORE any text reaches shlex, and cap each
referenced read at the remaining byte budget so an oversized file is never
read whole. Exhaustion fails closed (the existing contract for one oversized
file) and is logged at WARNING so operators can tell it from a genuine
lifecycle block. Limits are sized so real wrapper graphs never hit them:
a 200-script benign graph is allowed and a restart hidden behind it is
still caught.
tools/terminal_tool.py gates its optional launchctl pre-scan (which also
tokenizes) on the same budget; the full guard still runs afterwards.
Redesigned from #83821 by @Riccardo-Vecchi, which introduced the budget
idea but blocked benign wide graphs (64-path cap) and bundled a suffix
classification change that is left out here.
Refs #78398
Follow-up on the #42278 salvage (#98222): `a && b & &>/dev/null c` is valid
bash but rewrote to `a && { b & } &>/dev/null c`, which binds the redirect to
the brace group and orphans `c`. Treat a suffix starting with `&>` as command
text and insert the `;` separator. Adds bash -n coverage for that shape, for
the `;;` case-arm terminator (must NOT gain a separator), a multi-line mixed
form, and separator idempotence.
`_rewrite_compound_background` rewrites `A && B &` into `A && { B & }` to
avoid the subshell-wait process leak. When another statement follows the
backgrounded compound on the same line (`A && B & C`), the trailing `&`
was the only separator between the compound and `C`. The rewrite consumed
that `&` into the brace group and produced `A && { B & } C`. A brace group
must be terminated by `;`, `&`, `|`, a newline, or `)`/`}` before the next
command, so the result is a bash syntax error and the entire command fails
to run — neither `A`, `B`, nor `C` execute. The rewrite is applied by
default to every foreground command, so a valid command is silently
mangled into one that errors out.
This restores a `;` separator after the closing `}` when the suffix
resumes with command text. Only spaces and tabs are stripped before the
check; a newline already terminates the group, and an existing separator
(`;`, `&`, `|`, `)`, `}`) is left untouched, so previously-correct rewrites
are unchanged.
## What does this PR do?
Fixes a rewrite in `_rewrite_compound_background` that turned a valid
foreground command of the form `A && B & C` into the bash syntax error
`A && { B & } C`, causing the whole command to fail. A `;` is now inserted
after the brace group whenever a further statement follows on the same
line, while leaving commands that already end in a separator/newline
untouched.
## Related Issue
N/A
## Type of Change
- [x] 🐛 Bug fix (non-breaking change that fixes an issue)
- [ ] ✨ New feature (non-breaking change that adds functionality)
- [ ] 🔒 Security fix
- [ ] 📝 Documentation update
- [ ] ✅ Tests (adding or improving test coverage)
- [ ] ♻️ Refactor (no behavior change)
- [ ] 🎯 New skill (bundled or hub)
## Changes Made
- `tools/terminal_tool.py`: in `_rewrite_compound_background`, insert a `;`
after the rewritten `{ ... & }` brace group when the trailing suffix
begins with command text rather than a separator/terminator.
- `tests/tools/test_terminal_compound_background.py`: add
`TestTrailingStatementSeparator` for the string-level rewrites and
`TestRewriteIsValidBash`, which runs the rewriter output through
`bash -n` for parse validity plus one end-to-end execution check.
## How to Test
1. Run `pytest tests/tools/test_terminal_compound_background.py -q`.
2. Before the fix, `_rewrite_compound_background("echo hi && sleep 5 & echo done")`
returns `echo hi && { sleep 5 & } echo done`; `bash -n -c` on that string
exits 2 with `syntax error near unexpected token 'echo'`.
3. After the fix it returns `echo hi && { sleep 5 & } ; echo done`, which
parses and runs, and the trailing statement executes.
## Checklist
### Code
- [x] I've read the Contributing Guide
- [x] My commit messages follow Conventional Commits
- [x] I searched for existing PRs to make sure this isn't a duplicate
- [x] My PR contains only changes related to this fix
- [x] I've run `pytest tests/tools/test_terminal_compound_background.py -q` (50 passed)
- [x] I've added tests for my changes
- [x] I've tested on my platform: macOS 15.5
### Documentation & Housekeeping
- [x] I've updated relevant documentation — N/A
- [x] I've updated `cli-config.yaml.example` if I added/changed config keys — N/A
- [x] I've updated `CONTRIBUTING.md` or `AGENTS.md` — N/A
- [x] I've considered cross-platform impact: the rewrite is
platform-independent; the `bash -n` test is skipped when bash is absent
- [x] I've updated tool descriptions/schemas — N/A
The plausibility gate excluded stub rows by a hard-coded title literal that
session_lost_and_found builds inline at two sites; rewording either would have
turned every stub into a mis-mapped row. One STUB_TITLE_PREFIX constant.
Follow-ups on the #101423 salvage (#101409):
- `stub_missing_parent_sessions` legitimately writes `started_at = 0.0` when
no timestamped message survived; a salvage where only stubs remain is
depleted, not mis-mapped. Stub rows leave the sessions denominator.
- Reuse `_EPOCH_LOW` from session_lost_and_found instead of a second
2001-epoch constant; collapse the two per-table blocks into one loop.
- Tests: a real upgraded-layout source (started_at physically appended) with
page 1 zeroed goes through the real `.recover` lane and the report comes
back `verified: False`; a stub-only output is not flagged; a mapped row
with a NULL title (the mis-mapped shape) still counts as mapped.
The lost_and_found salvage lane maps cells positionally onto the
destination template's declared column order, but a source upgraded
via ALTER TABLE has its columns in the order they were added, which
differs from SCHEMA_SQL whenever a column was inserted mid-definition
(#101409). Every row still inserts, so integrity/FK/FTS/count checks
stay green and the report ends up verified: true — while all 1,875
sessions in the reporter's DB got started_at = 0.0 with counters and
URLs shifted into the wrong columns.
Add a semantic plausibility gate to the salvage lane: when every
salvaged sessions.started_at or messages.timestamp is NULL or before
2001-09, the cells were mapped onto the wrong columns and the
recovery is reported with errors and healthy: false, so verified no
longer claims a mis-mapped output. A partially damaged column (torn
cells on some rows) does not trip the gate — only a systematic
violation does.
Fixing the positional mapping itself needs a historical-layouts
table derived from schema-version history; that design decision is
left to maintainers (suggested fix 2 in the issue).
Follow-ups on the #101586 salvage (#101356):
- `_is_same_auth_store` uses `Path.samefile` instead of a hand-rolled
st_dev/st_ino compare.
- The same-store check now runs after the mtime fingerprint short-circuit and
records the clean mark, so a symlinked-profile process pays the resolve +
stat pair once per file change instead of on every `load_pool()` call.
- A profile `.anthropic_oauth.json` aliased to root's singleton is one shared
grant too; the singleton block no longer self-compares and unlinks it.
The forked-grant heal treated a profile auth.json that is an *alias* of the
root store (symlink, hardlink, bind-mount) as a forked copy: it loaded the
same file as both the profile store and the root store, matched every OAuth
row against itself, stripped the profile rows / providers.<id> block, and
_save_auth_store() wrote that strip back through the alias — deleting the
shared credential (openai-codex reported).
#100339 / PR #100929 fixed *copied* stores; a shared store has no other side
to consolidate. Guard the heal with _is_same_auth_store(), which reuses the
resolving _same_path() (covers symlinks and equivalent paths) and falls back
to device+inode identity (covers hardlinks), and return None without writing.
Regression tests cover an openai-codex pool row plus providers block behind a
symlinked and a hardlinked profile auth.json; the copied-fork heals from
#100339 are unchanged.
Fixes#101356
A search_files hit inside a serialized dump (multi-MB single-line JSON,
minified bundle) made rg/grep emit the entire matched line into stdout:
head -n counts lines, so a 40MB match line crossed the exec transport
untruncated and was buffered whole into Python before the per-match
[:500] clamp ran. Measured on main: 42MB transport payload and ~180MB
peak Python allocation for a single match.
Fix at the engine layer, all three pipelines:
- rg: --max-columns 2000 --max-columns-preview (preview keeps the match
visible instead of omitting it)
- grep fallback + darwin pruned-grep fallback: | cut -c1-2000
- files_only/count modes skipped (lines are paths/counts, never giant)
2000 cols exceeds the existing 500-char content clamp, so no
previously-visible content changes.
Adapted from cline/cline#13525 (search_codebase RangeError OOM crash on
giant single-line files) — hermes buffers in Python rather than a JS
string, so the failure mode is memory/transport blowup rather than an
uncaughtException, but the class is identical.
content and structuredContent are now alternatives — never both forwarded
to the model. Spec-following servers render their data into content (the
verbatim dual-emit SHOULD or a faithful reorganisation), so forwarding
both sent the same information twice per tool call. structuredContent
fills in only when content blocks render effectively empty (whitespace-only
text counts as empty), preserving structuredContent-only servers. _meta
passthrough is unchanged.
Also surfaces unsupported content-block drops to the MODEL as
'[MCP content dropped: unsupported block (...)]' notices carrying
type/mime/uri/size handles (kimi-code#3227) instead of a log-only warning;
drop notices do not count as usable content for the arbitration.
Live E2E: real stdio MCP server (mcp 2.x structured_output tool) through
register_mcp_servers + _make_tool_handler — origin/main forwarded the
payload twice, branch forwards content only; text-only path unchanged.
/simplify-code pass on the salvage stack. The hand-rolled module-global
probe cache ignored its binary argument (stale verdict survives a binary
swap) and logged per launch; switch to the repo's established
functools.lru_cache capability-probe pattern (cua_backend, browser_tool),
probe stdout+stderr, share the --http-cache-dir literal via a module
constant, tighten the probe timeout to 3s, and cover the probe's
except branch in tests. Mutation-checked: gate tests fail with the
gate removed and with the probe hardcoded True.
Salvage follow-up for PR #100269. The flag landed upstream in 0.3.x;
binaries before it (e.g. 0.2.8, verified locally) fatally reject the
flag with 'unknown argument', breaking every Browser Use launch.
Probe 'lightpanda help' once per process and omit the flag when the
binary predates it. Also soften the unverified concurrency claim in
the _http_cache_dir docstring and tell docs readers to stop sessions
before deleting the live sqlite cache.
Lightpanda's HTTP cache is opt-in (`--http-cache-dir`, off by default), and
the launcher never passed it, so every Browser Use navigation re-fetched
every asset.
Point all Hermes-spawned instances at one shared cache under
$HERMES_HOME/cache/browser-use/lightpanda/http-cache. Sharing it across
sessions keeps assets warm through session churn; Lightpanda stores it in
sqlite (WAL), so a write that loses a race degrades to a cache miss rather
than a failed load, and --http-cache-entry-limit (default 1000) bounds the
directory without Hermes managing eviction.
Measured over 25 navigations across 5 sites, median warm navigation drops
from 0.40s to 0.18s on news.ycombinator.com and 0.14s to 0.10s on
wikipedia; total navigation time 7.0s -> 5.9s.
The flag has existed since Lightpanda 0.3.x (April 2026), so this needs no
minimum-version bump.
With servers configured but none selected by -t/--toolsets, discovery still
paid the ~260ms mcp SDK import before discovering it had nothing to spawn.
Filter first; the test now asserts the SDK probe is not reached.
The cherry-picked commit added the allowed_mcp_names filter to
discover_mcp_tools(). Since then CLI startup grew a second discovery path —
start_background_mcp_discovery / the deferred desktop start in
hermes_cli/mcp_startup.py — so wiring the filter only into the inline call
would leave `hermes chat -t terminal` (the default backgrounded path) still
spawning every server.
Store the filter once in mcp_startup (set_mcp_server_filter, called from
_prepare_agent_startup from args.toolsets; `all`/`*`/empty clears it) and
have both the inline and the background discovery honor it. The unfiltered
call shape is unchanged so zero-arg test stubs keep working.
Dropped from the original PR: the atexit/SIGINT/SIGTERM oneshot MCP reap —
main already does this in _cleanup_oneshot_runtime() -> shutdown_mcp_servers().
E2E (3 configured stdio servers, real subprocesses, 5 runs median):
no filter 3 spawned / 2.0 s; `-t terminal` 0 spawned / 1 ms.
Guards the off-loop assertion against becoming vacuous if run.py ever hoists
the loops imports out of the watcher body (the class/module patches would then
be bypassed and on_loop_calls would be trivially empty).
The writer-lock test exits at the no-adapter branch, so it only covered the
first of the three off-loop calls. Add a full-path test (adapter present,
slash-command loop) asserting each SessionDB call runs off the loop thread;
with main's run.py it fails naming all three.
Follow-up to the two cherry-picked commits: use _run_in_executor_with_context
(the file's established off-loop helper — keeps the multiplex HERMES_HOME
override alive in the worker thread, same as the warm-up above) instead of a
bare run_in_executor(None, ...), drop the now-stale rationale that blamed
list_meta_prefix's writer lock (main reads via _read_ctx), and add a
regression test that holds the SessionDB writer lock from another thread
during one watcher scan and asserts the event loop keeps ticking. On main
the same test measures a 0.53 s loop stall for a 0.6 s hold.
- Route list_meta_prefix() through _read_ctx() instead of self._lock,
fixing contention at the source for every caller (CLI, TUI, gateway),
not just this one call site
- Wrap fire_tick() and complete_tick() in run_in_executor() to avoid
blocking the event loop on writes (BEGIN IMMEDIATE) that acquire
self._lock
- Use asyncio.get_running_loop() instead of get_event_loop() to match
established convention in the file
These changes fix the actual freeze case (when a loop IS due and writes
happen), not just the common case (when no loop is due).
list_active_loops() calls list_meta_prefix() which acquires self._lock,
and that lock is also held by writers doing BEGIN IMMEDIATE and periodic
FTS5-merge/WAL-checkpoint work. A slow write holding the lock while the
watcher blocks the entire event loop on the same lock froze it for 90+
seconds until the liveness watchdog force-exited.
Moving list_active_loops() to run_in_executor() prevents the event loop
from blocking on the SQLite lock acquisition.
Fixes dynamic-hermes-agent#130
Expose the read deadline as a top-level config.yaml key beside
context_file_max_chars (same load_config_readonly resolution shape), default
5s, documented in context-files.md. Narrow the reader thread's catch from
BaseException to Exception: control-flow exceptions can't originate inside
read_text on a worker thread, and re-raising one would bypass the sites'
except Exception / except (OSError, UnicodeDecodeError) handlers.
Context files (SOUL.md, .hermes.md/HERMES.md, AGENTS.md, CLAUDE.md,
.cursorrules, .cursor/rules/*.mdc) and subdirectory hint files are read on
the startup / per-turn path. On network-backed filesystems (iCloud Drive,
OneDrive, NFS) a cold read of an evicted file can block indefinitely, which
stalls system-prompt assembly before the first turn.
Read them on a daemon thread with a 5s deadline via _read_text_with_timeout;
a timed-out file is logged at WARNING and skipped so the next context source
still loads. Read errors propagate exactly as before, so each site's
existing try/except handling is unchanged.
Re-cut against current main from PR #10110 (the original diff predates the
context_length plumbing and the AGENTS.md directory-chain loader); helper,
sites and regression tests follow the original.
Follow-ups from review of the two salvaged #87490 commits:
- _pressure_with_real_floor now applies only on the rough fallback branch.
A valid usage anchor is provider-exact and wins as-is: on MoA turns the
anchor deliberately uses the pre-fold aggregator usage while
last_real_prompt_tokens holds the folded figure, so flooring the anchored
value would re-add fan-out tokens the anchor exists to exclude. Docstring
rewritten to describe the real path split (anchor since d3a1c46510).
- estimate_tokens_rough: encode with errors="replace". main's estimator
never raised; text.encode() on a lone surrogate (routine in tool output,
see message_sanitization) raised UnicodeEncodeError and would abort a
turn where main produced a slightly-off number.
- Record the cl100k/o200k/Qwen2.5 calibration for the bytes/4 rule.
- tests: accented Latin within +10% of the ASCII rule; mixed Cyrillic/ASCII
counts ASCII at one byte; lone surrogates don't raise; anchored pressure
is never floored (wiring shape).
The ~4 chars/token rule is calibrated for ASCII; Cyrillic, Greek, Arabic and
similar 2-byte scripts tokenize at ~2-3 chars/token, so chars/4 under-counts
them ~2x and the pre-flight pressure figure trails real usage by tens of
percent on non-English sessions. Counting UTF-8 BYTES at ~4/token uses the
encoding width itself as the corrective: ASCII is unchanged (1 byte/char),
2-byte scripts count at chars/2, and the CJK dense path keeps its explicit
~1 token/char rule with the sparse remainder byte-counted. The ASCII
isascii() O(1) fast path is preserved; the non-ASCII paths add a single
C-level encode over text that was already being regex-scanned.
Complements the last-real-prompt floor: the floor catches sessions that are
already at the ceiling, this keeps the estimate from lagging in the first
place.
The chars/4 rough estimate under-counts Cyrillic and other non-ASCII scripts
by up to ~2x, so a session can ride the provider's real context ceiling while
the rough pressure stays under the compaction threshold. On providers that
silently clip over-window prompts (ollama /v1) the reactive overflow handler
never fires either, and the length-continuation retry path re-enters the API
call without passing the post-response gate — reproducing the truncation
death spiral this branch already addresses (observed live after the first
commit: real prompts 64,842 -> 64,995 against a 55,705 threshold, output
room shrinking 694 -> 541 tokens).
Floor the pre-API pressure figure at the provider's last reported
prompt_tokens — authoritative, script-independent — except for the one turn
after a compaction when that value is known-stale (#36718's
awaiting_real_usage_after_compression window).
Idle compaction (`compression.idle_compact_after_seconds`) skipped work only
when the context was at or below `threshold_tokens × summary_target_ratio`.
That is a *theoretical* post-compaction target: a real pass lands well above
it because the system prompt, the tool schemas and the protected head/tail
are an incompressible floor.
So a session that compacted to well above the target stayed above it forever,
and every later idle resume re-ran a full summarisation over a transcript that
had not grown. In the reported session the 17:01 pass reduced 64,105 -> 44,579
tokens; the 17:36 resume re-fired because 44,579 was still above the 25,502
theoretical floor, blocking the prompt for another 256 s on a ~55 tok/s local
route and reclaiming nothing. Neither pass was followed by a single API call.
`_should_idle_compact` now also honours `last_compression_rough_tokens` — what
the previous pass on this session actually produced, recorded by
`compress_context` with the same `estimate_request_tokens_rough` shape the idle
estimate uses. When it is known, the transcript must accumulate at least one
`floor_tokens` worth of new content on top of it before another pass is worth
its wall clock. A raised floor is a deferral, not an off switch.
`0` — nothing compacted yet, or the counter cleared by a rebind/recalibration
— keeps the original semantics exactly, so the first idle compaction of any
session is unaffected. The call site type-pins the read so compressor doubles
that expose a Mock there fall back to the original floor.
Fixes a root cause behind #97239. Deliberately does not touch the TUI status
surface (#97253), the digest chain / total ceiling (#93241), or turn-settle
scheduling (#96891).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HV3k1v5nx9ag5d5wXSFZ7o
Two /simplify-code findings on the salvaged #95160:
- The native read walked every line of the whole file with per-line
Python bookkeeping even after the requested window had passed, because
total_lines and the trailing byte are still needed. Once lineno is past
end_line, switch to chunk.count(b'\\n') for the remainder -- same result,
C speed. Page 1 of a 3M-line (123 MB) file: 448 ms -> 49 ms. Parity
harness native vs shell: 20 shapes (1 MiB boundary, no trailing newline,
past-EOF offsets, long lines) identical.
- _native_read_enabled re-implemented the LocalEnvironment check that
_lsp_local_only() already does in this class (same env-None / import
policy) and memoised it on self although self.env is bound once in
__init__ and never rebound. Call the existing helper, drop the cache.
Follow-up to the salvaged #95160. The test wrote the NFC spelling of
café.txt and read the NFD spelling. APFS resolves NFD lookups to the NFC
file directly, so on macOS the compound probe simply found the file, the
retry path never ran and r.hint was None -> TypeError (Linux CI passed
because ext4 does not alias normalisation forms).
Use a curly-vs-ASCII apostrophe instead: still visually identical, still
mapped by _unicode_variant_match, and never aliased by any filesystem, so
the retry path is exercised everywhere.
fangliquanflq