A volume that already owns /workspace skipped the configured working
directory, so tools treated that host path as unmounted. Bind it at a
second mount, or point tools at the volume that already has it, for any
drive path.
A shell with `set -x` (user rc, BASH_ENV) traces `+ echo <sentinel>` into
the merged output. That line is an extra separator for _split_segments, so
the segment count mismatched and read_file_raw (the V4A/replace write-back
source) failed with "Failed to read file".
_fenced_read now turns xtrace off before the fence; `set +x`'s own trace
goes to the group's discarded stderr.
Co-authored-by: JoaoMarcos44 <joaomarcosdias444@gmail.com>
The previous fold guarded each Delete/Move entry by running
get_write_denied_error on dirname(path). That coordinate is wrong both ways:
- runtime self-protection treats ANCESTORS of the running venv/interpreter
as protected, so a plain file directly in ~, ~/.hermes, the checkout root
or the uv python dir could no longer be deleted or moved ("'/Users/x' is a
protected system/credential file");
- credential-dir prefixes end in os.sep and match via startswith, so the
bare dir ~/.ssh never matched and a link directly inside ~/.ssh, ~/.aws,
~/.gnupg, ... was unlinked/renamed.
The existing classifier gains an entry=True mode (get_write_denied_error /
_classify_write_denial, and is_protected_path(follow=False)) that vets
realpath(parent)/basename — the entry, leaf not dereferenced — in addition
to the resolved target. A file inside a protected dir or prefix is denied;
a file merely beside the venv is allowed. delete_file and move_file make
one entry-mode call per entry instead of the duplicated path+dirname loop.
The new parametrized test covers both directions (plain Delete/Move next to
a monkeypatched runtime venv succeeds; a link in <home>/.ssh is refused with
link and target intact); all four cases fail on the previous fold head.
Delete and Move remove or rename the directory entry itself (a symlink,
not its target), but get_write_denied_error realpaths its argument, so it
only ever vetted the link's target. A link outside HERMES_WRITE_SAFE_ROOT
(or inside ~/.hermes/sessions) pointing at a file inside the root passed
the guard and the link was deleted/renamed outside the allowed area.
delete_file and move_file now also run the same guard on each entry's
parent directory, which realpaths to where the entry really lives.
The symlink test gains a safe-root case (red without this change), and
its Move cases always assert success, the rename, the link target and
files_modified instead of tolerating a refusing move primitive; expected
values are parameters rather than header introspection, and json is a
module-level import.
[ -f ] and [ -e ] follow symlinks, so a dangling link probed as missing and
read_file_raw reported not_found: V4A Add followed the link and created its
target, Move replaced the link, both reporting success. The shell size
probe, the compound read probe and the native stat now classify any
symlink entry as not a regular file.
The fence drops backend output around the read, not output printed while
the transport runs: a BASH_ENV DEBUG hook firing for base64 alone puts
text inside the payload that still decodes (TERM is b"LDL"), and the edit
paths wrote it back. Emit wc -c in its own fenced segment and hand bytes to
a writer only when their length matches; the od fallback is checked the
same way.
Consolidates #120559 by @JoaoMarcos44 into this PR. Both PRs landed on the same
shape for #120514 — read_file_raw must be a byte-preserving mutation boundary,
framed so the backend's merged stdout is never decoded as payload — but the
sibling one step in FRONT of it was still unfenced here.
_sample_file_bytes whitespace-joined the whole reply before base64-decoding it,
so a backend that announces something on connect had that noise decoded into the
sample: "TERM" is four base64 characters and prefixes the sample with b"LDL".
That sample is the binary-admission gate read_file_raw consults, so noise there
decides whether a file is editable at all and what a refusal reports about its
bytes. Same class as the read this PR already fixed, one call earlier.
_fenced_read() is that framing extracted once — sentinel, payload segment, and
the body's own exit status in its own trailing segment — and the three byte
transports (sample, base64, od fallback) now share it instead of repeating the
split/parse/decode triplet, with _failed_read() for the diagnostic they all
surfaced by hand.
The mock double for the sample transport moves to the fenced reply shape the
other doubles in that file already build, so it stays honest about the fence.
Co-authored-by: JoaoMarcos44 <joaomarcosdias444@gmail.com>
Three defects in the byte-exact read this PR introduced.
base64 is not on every backend (busybox, distroless). The sample path
already degrades when it is missing; this one returned the read as a
failure, and read_file_raw is also _apply_add's existence check, which
treated any error as "the path is free". A backend with cat but no base64
turned `*** Add File` over an existing file into a silent overwrite that
reported success:
main: refused, "file already exists — use Update File"
PR head: b'KEEP ME -- months of work\n' -> b'clobbered by Add'
So: fall back to od (POSIX, in busybox) when base64 exits 127, and when
neither exists report a transport error. ReadResult grows not_found, set
only where the path is genuinely absent, and _apply_add refuses unless it
sees that flag — a read that FAILED can no longer pass as an absent path.
getattr keeps a producer without the field failing closed rather than
raising. The doubles in test_patch_parser that meant "absent" now say so.
The native fast path stat'd the path and then opened it, two lookups on a
name. A swap to a FIFO in between blocks the thread, and nothing times out
that. One O_RDONLY|O_NONBLOCK open, fstat on THAT descriptor, then read, so
a non-regular file is handed to the shell path and its timeout instead.
Tests: the od fallback round-trips byte-exactly and patches; an Add over a
file the backend cannot read is refused with the file intact; the native
read hands a FIFO to the shell rather than opening it. The first two go red
if their fix is removed. The third pins the property, not the race — with
one descriptor there is no window left to swap into, so reverting to
stat-then-open does not flip it.
Not addressed here: inserted text is still encoded UTF-8 regardless of the
file's declared encoding, so an edit adding non-ASCII to a latin-1 file
writes mixed bytes. That is the write half, it predates this PR, and it
needs source-encoding detection.
The byte-exact read whitespace-joined the whole reply before decoding, so
anything a backend prepends to its merged stdout became part of the base64.
A remote shell announcing TERM is four base64 characters, which decodes to
b"LDL" and lands at the head of the file the edit paths then write back:
patching HEADER\nVERSION=1\n left LDLHEADER\nVERSION=2\n on disk.
Fence the payload between a per-call sentinel, the shape the compound read
probe already uses and for the same reason. _new_sentinel's underscores are
outside the base64 alphabet, so noise that lands inside the fence fails
validation instead of decoding into bytes, and noise outside it is dropped.
The read's exit status rides in its own trailing segment, so a failed read
is still told apart from an empty file without && chaining or exit.
stderr stays merged rather than silenced: on a failed read that segment
carries the backend's own diagnostic ("No such file or directory"), which
the callers surface, and a reply with no fence at all hands the backend's
text straight back so a wrapper cd failure still reports itself.
The two doubles this PR added keyed off the old command string; they now
build a fenced reply from the sentinel in the command they were handed, so
they stay honest about the fence the transport depends on.
The new test drives the noisy backend through the real transport: the
existing tests use LocalEnvironment, which takes the native read path on
POSIX and never exercises base64 at all.
Review follow-up (ehz0ah, teknium1). Keying the cleanup on the
__HERMES_FENCE_ marker still rewrote a real line that contains that
text, and nothing has emitted the wrapper since the spawn-per-call layer
(d684d7ee7e), so the cleanup can only ever eat the file's own bytes.
The same class also hit mode="replace", the default: patch_replace (and
V4A past the 1000-byte binary sample) read the file through the text
transport, which decodes with errors="replace", so any byte UTF-8 cannot
decode came back as U+FFFD and was persisted on lines the edit never
touched, while the diff and the post-write check (same lossy read) showed
nothing.
_read_exact_bytes reads natively on the local POSIX host (regular files
only) and over base64 elsewhere; read_file_raw, patch_replace and
_verify_patch_persisted decode it with surrogateescape, which write_file's
encode already inverts (#79178), so every untouched byte is written back
exactly and no readable file becomes a read error (V4A's Add/Move/Delete
existence checks are unchanged). A garbled transport reply refuses
instead of writing stray output into the file. The Python linter parses
bytes so a declared legacy coding still lints clean. Local edits spawn
two fewer shells (replace 5 -> 3, V4A 9 -> 7).
read_file_raw feeds the V4A write-back, and it ran the whole file through the
terminal fence-leak cleanup, which deletes every OSC sequence and BEL byte. A
prompt script's title escape or a beep line was silently rewritten on apply, and
the diff and post-write hash both started from the stripped text, so nothing
showed it. A leaked wrapper always carries the fence marker: clean only those
lines when the content is going to be written back.
The exec wrapper's own `builtin cd -- <cwd> || exit 126` failure was only
named on paths that embed stdout (write_file, cat). Stat-probe reads
(read_file/read_file_raw/read_file_bytes) reported "Terminal environment
unavailable", the patch pre-image read "Failed to read file", and search
"requires rg or find" — and _has_command cached {'find': False} for the
instance lifetime from a probe that never ran.
Carry the signal as ExecuteResult.cwd_error: _probe_regular_file /
_env_unavailable_error, patch_replace and the search existence probe surface
the named working-directory error verbatim; _has_command returns False
without caching when the probe died at the wrapper.
Wording: "check terminal.cwd or the session cwd"; the in-container path hint
is appended only when the env's env_type is a container backend, so a
local/ssh cwd failure no longer gets a docker hint.
Follow-up to the salvaged #113946 pick (source-side guard on the live-env
cwd write, @kokhlo):
- ShellFileOperations._exec: when the backend's own command wrapper fails at
`builtin cd -- <cwd> || exit 126`, the surfaced error now names the real
problem (working directory does not exist on the active backend;
terminal.cwd is not valid for it) ahead of the shell's raw `cd:` line, so a
host path configured for a container backend no longer reads like a
sandbox/mount fault at the requested file path.
- Trim the contributor test file to one parametrized invariant (mounted host
workspace remaps to /workspace; unmounted host path leaves the live env
untouched; in-sandbox and local overrides apply verbatim; the session record
keeps the raw path) and add one invariant for the surfaced error.
- Docs: note under docker_mount_cwd_to_workspace.
The consumer-side re-validation from #113931 is not taken: with the single
unsanitized writer fixed, a per-exec guard plus constructor re-validation and
an env_type kwarg threaded through file_tools would be a second mechanism for
the same state. Earlier filers of the same class on #98723 (#98730 @tylerbrevard,
#98731 @kokhlo, #98734 @io614) are superseded by this pick.
Co-authored-by: chelsealong <chelsealong@126.com>
Co-authored-by: tylerbrevard <lyonrt@icloud.com>
file_operations.py checked `os.path.splitext(path)[1].lower() in
BINARY_EXTENSIONS` at five sites, so a `.db-wal` / `.sqlite3-shm` sidecar
(final suffix never in the set) was read and edited as text in the sandbox
paths. Route all five through has_binary_extension(), which strips the
sidecar marker. Behaviour is otherwise identical: same lower-cased final
suffix membership (rfind vs splitext differ only on leading-dot basenames
like `.bashrc`, which are in neither set). binary_extensions imports nothing
from tools, so no cycle.
_add_line_numbers split on '\n', so a file ending in a newline (the normal,
well-formed case) produced a trailing empty element that got its own line
number. read_file therefore showed a phantom '<N+1>|' line that is not in the
file, on every terminal backend and every OS, matching neither cat -n nor the
reported total_lines. Drop the single terminating newline before splitting.
Fixes#49451
wc -l counts newlines, not lines, so a file without a trailing newline
reported one fewer total_lines than the content it returned. The read
paths already probe the last byte (file_ends_with_newline) to strip cut's
phantom newline; use that same signal at the shared assembler choke point
so total_lines, truncation, and the past-EOF guard agree on every path
(compound, sequential, native).
Fixes#3907. Supersedes #3908: single adjustment instead of a per-path
helper, covering the native and sequential paths as well.
The shared quoting helper doubled regex backslashes for remote shells
because the controller ran Windows. Apply compensation only when the
backend executes locally; serialized POSIX command text stays literal.
Path translation and file-write payloads remain unchanged.
Verified with the real LocalEnvironment argv path and JSON command text
delivered to a real Bash stdin. Tests compare received bytes for quotes,
metacharacters, newlines, empty values and backslash runs. The serialized
case fails before the fix while the local case already passes.
Integrated: 114 tests passed, 6 host skips. Ruff and whitespace checks pass.
No Docker/SSH service or POSIX host run is claimed. The rejected grep
multiline rewrite is not included.
Merge d86627a7f3 into pm-clean. Keep the shared bounded ripgrep transport and preserve translate_path=False for probe patterns. Targeted canonical search tests: 19 passed, 3 POSIX-only skips on Windows. Full integrated CI has not been run for this merge.
_native_rg_enabled was a pass-through to _native_read_enabled (a wrapper with
no behaviour); call the gate directly and note in its docstring that it
covers search too. The rg --files invocation was assembled twice (argv list
for the native lane, string for the shell lane) with room to drift; build the
command string once and hand it to either transport.
Merge upstream 5e645791ac.
Retain the PM feature-flag owner and add upstream connection options.
Use the deny-only window-open policy while trusted external links keep
the existing IPC path. Keep both session-import and external-link copy.
Preserve captured timeout output when adding terminal yield handoff.
Quickstart tests patch the explicit upstream model-assignment owner.
Migrate incoming legacy OS markers to the branch's platforms gate.
Desktop renderer and Electron typechecks passed. Targeted Electron tests
passed (42 tests), Python conflict checks passed (26 tests, 3 skips),
and the plugin-compat import checker passed. CI owns the broad merge gate.
Prepare dependency generations before selecting them. Keep shipped tool
bytes separate from writable additions, and store facts beside their entries.
Validate proposed plugin sets before config publication. Restore the previous
config if the facts write fails.
Consolidate duplicate updater, backup, setup, and voice helpers. Repair
launcher selection, dependency consumers, download ownership, update feeds,
and native Windows process and file handling.
Verification: 206 changed/prior-failing Python files reported 4630 passed,
one failed, and 330 skipped. Fix the remaining Hindsight fixture boundary.
The final targeted rerun reported 234 passed and two skipped. The store
review regression batch reported 83 passed and one skipped. Desktop
TypeScript checks, 56 selected Electron tests, 24 release tests, and the
removed-import/compatibility guards passed.
This is an integration checkpoint, not full audit acceptance. The complete
Python suite has not run on this fixed tree. Crash-atomic plugin publication,
generation cleanup, receipt correlation, and packaged lifecycle acceptance
remain open in docs/pm-audit-status.md.
_probe_regular_file returned 'missing' for ANY non-zero exit, so a docker container that
was still starting, paused, or otherwise unable to exec made read_file / read_file_raw /
read_file_bytes report a false 'File not found' — which the model then trusted for the
rest of the session. The probe now echoes a sentinel for a genuinely missing path and a
non-zero exit without either sentinel is reported as an environment failure with a retry
hint; search_files does the same when its existence probe returns neither marker.
Salvaged from #44753 by @dredozubov.
Every PLUGIN-COMPAT __getattr__ now calls hermes_cli.plugin_compat.warn_once(facade, name, target) before
resolving, emitting a HermesPluginCompatWarning (FutureWarning) once per process per name: old path, new
path, removal target. Importing a facade for its live API stays silent; only resolving a moved name warns.
COMPAT_MANIFEST.md documents the warning and how to silence it during migration.
Verified the runtime never routes through a pointer: every entry point (run_agent, cli, hermes_cli.main,
gateway.run, tui_gateway.server, web_server, model_tools + tool discovery, hermes_state, cron.scheduler,
browser_tool, mcp_tool, kanban, auth) imports clean and `hermes doctor` runs end to end with the warning
promoted to an error.
Also restores the check_compat_pointers CI step to .github/workflows/lint.yml, which a0be177aac dropped
when the compat layer was regenerated (the lint script itself was present; the workflow step was not).
hermes_cli/plugin_compat.py, tests/test_plugin_compat_warning.py and the two-line insert per facade are
part of the compat layer and go away with it.
The Sep 2026 decomposition (PR #102117) makes internal import paths a non-API: names now live in
the focused modules that define them. This commit is the ONLY thing keeping the old paths alive,
so external plugins have time to update. It is deliberately a single, unsquashed commit:
git revert <this sha>
removes every shim, stub and manifest at once on the announced date. Nothing in-tree may depend on
these pointers: scripts/check_compat_pointers.py (wired into lint.yml) fails CI if it does.
What it adds (see COMPAT_MANIFEST.md, compat_manifest.json):
- 332 facade modules get one delimited `PLUGIN-COMPAT` block appended at the end of the file
- 1,172 moved names resolved lazily via a module `__getattr__` (PEP 562) — never a top-level import,
so no import cycles; facades that already had `__getattr__` get a chained one
- 592 third-party/stdlib names the old modules used to expose, with their original import statements
- 266 public definitions that had been deleted as unused, restored byte-for-byte from the pre-decomposition
tree (+40 private helpers and 16 imports pulled in only because a restored definition needs them)
- 3 deleted modules recreated as re-export stubs (gateway/startup_watchdog, hermes_cli/observability/
relay_runtime, tools/environments/modal_utils)
- private names (`_x`) get no pointer: they were never API (3,792 skipped)
Verified: all 335 touched modules import under a fresh HERMES_HOME and every manifest name resolves;
the lint reports zero in-tree uses; ruff clean; targeted suites unchanged.
For each issue anchor present in BASE 63279301bc non-test .py and absent on HEAD, the BASE comment/docstring block was re-attached at the HEAD location of the code it explained (matched by the distinctive code line / enclosing def). Sentences already covered by an existing HEAD comment were deduped; the issue number always survives. Insert-only: no code lines changed.
