The 3-sentence identical-edit message was snapshot-asserted verbatim in
two tests. House style avoids exact-string change-detector assertions;
both tests now import the constant from tools/fuzzy_match so rewording
the message can't silently break them.
* fix: Windows path handling in search_files rg calls and patch escape drift
Two related Windows failures from a live session (Windows 10, git-bash
terminal backend, winget-installed native ripgrep):
1. search_files was unusable on drive-letter paths. _escape_shell_arg
rewrites C:\... to the MSYS form /c/... so bash builtins resolve it,
but rg is a native Windows binary and Hermes disables MSYS argument
conversion for its bash subprocesses (MSYS_NO_PATHCONV=1 /
MSYS2_ARG_CONV_EXCL=*, see _apply_windows_msys_bash_env_defaults) —
so nothing ever translated /c/... back and every search failed with
'The system cannot find the path specified. (os error 3)'.
Fix: new _escape_native_tool_arg emits the forward-slash NATIVE form
(C:/Users/...), which native binaries accept, bash passes through
untouched, and MSYS builds also handle. Applied to the six rg call
sites (content search, --files search x2, zero-match probe x3); the
grep fallback keeps the MSYS form since MSYS grep wants it.
2. The patch tool silently doubled backslash runs when tool-call args
arrived JSON-escaped one extra time (file had \ where old_string
had \\). Similarity strategies (context_aware) matched the region
anyway and wrote new_string verbatim, corrupting every backslash run
(reproduced: 6 backslashes on the line became 12). _detect_escape_drift
now also blocks when every backslash run in old_string is exactly twice
its counterpart in the matched region and new_string repeats the
doubling — with guardrails so exact matches, intentional backslash
edits, model-corrected new_strings, and single weak-signal runs all
still apply. Blocking returns the standard escape-drift guidance so
the model re-reads and retries with correct counts.
Tests: TestEscapeNativeToolArg (5 cases, including an end-to-end
_search_with_rg command capture) and TestBackslashDoublingDrift (6
cases). The 8 pre-existing failures in tests/tools/test_file_operations.py
on a Windows host (umask/symlink POSIX assumptions) are identical on
unmodified main and unrelated.
* fix: shell linters get native Windows paths too (node C:\c\... double-prefix)
Same class as the rg fix: LINTERS commands (python -m py_compile,
node --check, npx tsc, go vet, rustfmt) invoke native Windows binaries,
but _check_lint interpolated the MSYS /c/... form. node resolves that
as C:\c\Users\... (double-prefixed), so on Windows hosts every .js
write reported a phantom ENOENT lint failure that could mask real
syntax errors (issue #84303). Route the {file} arg through
_escape_native_tool_arg like the rg call sites.
Regression test asserts node --check receives 'C:/...' and never
'/c/...'.
Three lower-severity core-tool robustness fixes from a targeted audit, each
reproduced live:
1. terminal_tool did not validate non-positive timeouts. 'timeout or default'
silently coerced 0 to the config default (0 can't mean 'no timeout'), and a
negative value is truthy so it flowed into 'deadline = now + timeout' and
fired an immediate '-Ns' timeout. Reject timeout <= 0 with a clear message.
2. fuzzy_find_and_replace accepted a whitespace-only old_string, which matches
trivially (blank line / run of spaces) and mass-replaces under replace_all
or raises an opaque ambiguity error. Reject it alongside the empty check.
3. The '/private/var/' sensitive-path prefix over-blocked ALL macOS temp-file
writes: , /tmp, and /var/folders realpath into /private/var/folders
on macOS (and paths are resolved through symlinks), and /private/var/tmp is
a normal temp dir. Narrowed to the genuinely-sensitive subtrees
(/private/var/db, /private/var/root); /etc and /private/etc stay blocked.
All verified with sabotage-checked regression tests. 85 terminal/fuzzy/file
tests pass; normal timeouts, legit replacements, and /var + /boot + /etc
blocking are unaffected.
Strategy 9 (context_aware, the last-resort fuzzy strategy used by
patch_replace, V4A UPDATE hunks, and skill_manage) had two serious flaws,
both reproduced live against current main:
1. CORRECTNESS: it accepted a block when >=50% of its lines were >=0.80
similar. A 2-line pattern with one real line and one garbage line matched,
silently deleting the non-matching line and persisting a wrong edit as
success. Now requires the first AND last lines to anchor-match and EVERY
non-blank pattern line to be >=0.80 similar — one garbage line disqualifies
the block.
2. PERFORMANCE: it scored every content window with per-line SequenceMatcher,
so every failed match paid O(file_lines x pattern_lines) — measured ~5.5s
for a single 40-line no-match on a 10k-line file, per hunk. The first/last
line anchor pre-filter skips non-candidate windows: same case now ~160ms
(34x faster).
Also gate replace_all: a similarity-based strategy (block_anchor,
context_aware) with multiple matches under replace_all would overwrite every
approximate block, not just exact ones. Now refused with a clear error
directing the caller to precise text.
All verified with sabotage-checked regression tests (fail against the old
50% logic). 158 file/patch/fuzzy tests pass; legit fuzzy edits (indent drift,
unique near-match) unaffected.
Second, deeper pass over tools/gateway/hermes_cli plus first pass over
the trees wave 1 missed (acp, acp_adapter, skills, computer_use, docker,
dashboard, conformance, monitoring, secret_sources, hermes_state,
providers). Same rubric as wave 1 (AGENTS.md test policy); security,
alternation/caching invariants, issue-number regressions, and E2E kept.
Real test-quality fixes found and rooted out along the way:
- tests/tools/test_command_guards.py made real auxiliary-LLM HTTPS calls
(DEFAULT_CONFIG smart-approval leaked in) — pinned approval
mode=manual via autouse fixture: 17.4s → 0.4s.
- test_model_switch_custom_providers.py / test_user_providers_model_switch.py
silently probed live provider catalogs (~2s/test) — stubbed
cached_provider_model_ids/provider_model_ids/fetch_api_models.
- test_telegram_noise_filter.py: 15-platform copy-paste matrix over
shared gateway.run logic → 3 representative platforms (55s → 3.9s).
- test_gateway_shutdown.py: stop()'s 5s interrupt-deadline loop spun on
MagicMock agents — interrupt.side_effect now clears _running_agents
(22s → 1.0s).
- test_gateway_inactivity_timeout.py poll-harness timings shrunk 3-5x
(24s → 1.1s); test_mcp_stability.py backoff/SIGTERM-grace sleeps
patched (15.4s → 2.5s); test_async_delegation.py negative-drain wait
5s → 0.5s.
- test_telegram_init_deadline.py: loop-block margin restored to 1.0s
with rationale comment — the watchdog-dump assertion needs the loop
blocked well past deadline+grace under parallel load (flaked once in
the 40-worker verification run at a 0.2s margin).
Verification: full hermetic suite via scripts/run_tests.sh —
2,438 files, 21,718 tests passed, 0 failed, 293.9s wall.
Suite totals vs original baseline: 46,820 → 19,757 test functions
(−57.8%), wall 583.5s → 293.9s (−50%), subprocess CPU 13,564s → 11,623s.
Port from anomalyco/opencode#38133/#38134 (patch Unicode matching corpus):
extend UNICODE_MAP with the Zs space-separator family (en/em quad, en/em/
three-per-em/four-per-em/six-per-em/figure/punctuation/thin/hair spaces,
narrow NBSP, medium mathematical space, CJK ideographic space) and the
Unicode minus sign U+2212.
Before: a file containing typographic spacing (French narrow NBSP, CJK
ideographic spaces, math minus) never matched a model's ASCII old_string
via the precise strategies — the edit only succeeded through the
similarity-based context_aware fallback, which (a) can pick the wrong
region (#54572 family) and (b) silently flattens the file's Unicode to
ASCII on replacement. After: these match at unicode_normalized (strategy
7), whose _preserve_unicode_in_replacement keeps the file's typographic
characters in unchanged spans.
All additions are 1:1 mappings, so the existing position-mapping and
preservation logic apply unchanged. Proven live before/after with a
multi-line probe; 59 fuzzy-match + 188 file-tools/patch/skill-manager
tests pass.
The patch tool's strategy 7 (unicode_normalized) matches ASCII old_string
against a file containing real Unicode (em-dashes, smart quotes, ellipsis,
non-breaking spaces). Writing new_string verbatim silently replaced the
file's Unicode with the LLM's ASCII equivalents.
_preserve_unicode_in_replacement() diffs old_string->new_string and applies
only the actual edits to the file's original Unicode text, preserving
unchanged characters.
Salvaged from #50540 by @aj-nt. Only the Unicode-preservation half is
carried over; the write_file line-number-strip half was dropped (the
existing _looks_like_read_file_line_numbered_content reject guard already
covers its target case, and the strip's looser threshold risks silently
mutating legitimate pipe-delimited content).
_strategy_exact advanced its scan cursor by pos+1 instead of
pos+len(pattern), so self-overlapping patterns (e.g. "aa" in "aaaa")
matched at overlapping offsets. _apply_replacements works in reverse
order, so the second replacement operated on already-modified content
using stale offsets — corrupting the file and reporting the wrong count
under replace_all=True. Advancing by len(pattern) matches str.replace()
semantics.
The trailing-whitespace expansion in _map_normalized_positions
unconditionally consumed whitespace after the matched region — including
the word-boundary space that separates the match from the next token.
This caused silent file corruption when the fuzzy matcher fell back to
the whitespace_normalized strategy.
Guard the expansion on the normalized match actually ending with
whitespace (i.e. the original had a run of spaces that were collapsed).
When the match ends with a non-space character, the first whitespace in
the original is a boundary and must not be consumed.
Fixes#52491
Extends @liuhao1024's escape-normalized fix so the patch tool also
recovers when old_string carries a real tab byte and matches via the
`exact` strategy — which is the headline reproduction in the issue and
the most common case in practice (LLMs frequently get old_string right
because they re-read the file, but still serialize new_string's tabs as
two-character `\t`).
Instead of gating on the match strategy, decide per-sequence by looking
at the *matched region of the file*: only convert `\t` -> tab and
`\r` -> CR when the file region we're replacing actually contains the
corresponding control byte. That mirrors the region-based heuristic in
`_detect_escape_drift` and keeps legitimate writes of the literal
two-character string `"\t"` (e.g. patching `sep = "\t"` in Python
source) untouched — those files have a backslash+t in the matched
region, not a real tab, so new_string passes through verbatim. `\n` is
still excluded because newlines serialize correctly through JSON and
unescaping would corrupt source escape sequences far more often than
help.
E2E verified against the live `patch` tool: tab-indented file + literal
`\t` in new_string under both `exact` (Variant 1) and `escape_normalized`
(Variant 2) strategies now produces real tab bytes; a Python source line
containing `sep = "\t"` (legitimate literal backslash-t) survives a
patch unchanged.
Tests updated to cover both strategies and the legitimate-literal case,
and to assert that `\n` is intentionally preserved.
Refs #33733
When the patch tool matches via the escape_normalized strategy, old_string
contains literal \t, \n, \r sequences that get unescaped to match real
control characters in the file. However, new_string was written as-is,
leaving literal backslash sequences in the output.
Add _unescape_common_sequences() helper and apply it to new_string when
the matching strategy is escape_normalized. This ensures LLM-generated
tab/newline sequences become real bytes in the patched file.
Fixes#33733
Three granular patch-tool refinements from the Roo Code deep-dive (#507).
## Indentation preservation (fuzzy_match.py)
When fuzzy_find_and_replace matches via a non-exact strategy, the file's
indentation may differ from what the LLM sent in old_string/new_string
(common case: model sends zero-indent old/new for a method body that
lives inside an 8-space-indented class). Before this commit the
replacement was spliced in verbatim, producing a file with a broken
indent level that may still parse but is logically wrong.
The fix computes the indent delta between old_string's first meaningful
line and the matched region's first meaningful line, then re-indents
every line of new_string by that delta. Exact-strategy matches are
untouched (passthrough). Same approach as Roo Code's
multi-search-replace.ts:466-500.
## CRLF preservation (file_operations.py)
Models nearly always send tool args with bare LF endings (JSON-encoded),
but the file on disk may have CRLF (Windows-line-ending configs, .bat,
.cmd, .ini files). Before this commit:
- write_file silently normalized CRLF to LF on every overwrite
- patch produced mixed-ending files: the substituted region had LF,
the surrounding context kept CRLF
The fix detects the file's existing line endings (via pre_content if
already read for lint/LSP, otherwise a tiny head -c 4096 probe), and
normalizes the entire write to that ending. New files are written
verbatim (no detection possible).
## Per-file failure escalation (file_tools.py)
When the agent fails to patch the same file 3+ times in a row, the
existing 'old_string not found' hint isn't strong enough — the model
keeps retrying with variations against a stale view of the file.
The fix tracks consecutive failures per (task_id, resolved_path) and
injects an escalating hint after 3 failures: 'This is failure #N
patching X. Stop retrying. Either re-read fresh, use longer context,
or fall back to write_file.' Counter resets on a successful patch to
the same path.
## Validation
- 22 new tests across tests/tools/test_fuzzy_match.py (5),
test_line_ending_preservation.py (12), test_patch_failure_tracking.py (5)
- All existing tests pass (165/165 in the touched files)
- E2E verified with real _handle_patch / _handle_write_file calls
against real CRLF files and real failure loops
Closes part of #507. The remaining open items in #507 (2b start_line
hint, behavioral rules) were declined after audit:
- 2b adds schema bloat for a problem the existing 'multiple matches'
contract already handles
- Behavioral rules conflict with the personality system
Items 1, 2d, 2e, 3, 4 of #507 were already landed in earlier work.
Follow-ups on top of @teyrebaz33's cherry-picked commit:
1. New shared helper format_no_match_hint() in fuzzy_match.py with a
startswith('Could not find') gate so the snippet only appends to
genuine no-match errors — not to 'Found N matches' (ambiguous),
'Escape-drift detected', or 'identical strings' errors, which would
all mislead the model.
2. file_tools.patch_tool suppresses the legacy generic '[Hint: old_string
not found...]' string when the rich 'Did you mean?' snippet is
already attached — no more double-hint.
3. Wire the same helper into patch_parser.py (V4A patch mode, both
_validate_operations and _apply_update) and skill_manager_tool.py so
all three fuzzy callers surface the hint consistently.
Tests: 7 new gating tests in TestFormatNoMatchHint cover every error
class (ambiguous, drift, identical, non-zero match count, None error,
no similar content, happy path). 34/34 test_fuzzy_match, 96/96
test_file_tools + test_patch_parser + test_skill_manager_tool pass.
E2E verified across all four scenarios: no-match-with-similar,
no-match-no-similar, ambiguous, success. V4A mode confirmed
end-to-end with a non-matching hunk.
When patch_replace() cannot find old_string in a file, the error message
now includes the closest matching lines from the file with line numbers
and context. This helps the LLM self-correct without a separate read_file
call.
Implements Phase 1 of #536: enhanced patch error feedback with no
architectural changes.
- tools/fuzzy_match.py: new find_closest_lines() using SequenceMatcher
- tools/file_operations.py: attach closest-lines hint to patch errors
- tests/tools/test_fuzzy_match.py: 5 new tests for find_closest_lines
Two hardening layers in the patch tool, triggered by a real silent failure
in the previous session:
(1) Post-write verification in patch_replace — after write_file succeeds,
re-read the file and confirm the bytes on disk match the intended write.
If not, return an error instead of the current success-with-diff. Catches
silent persistence failures from any cause (backend FS oddities, stdin
pipe truncation, concurrent task races, mount drift).
(2) Escape-drift guard in fuzzy_find_and_replace — when a non-exact
strategy matches and both old_string and new_string contain literal
\' or \" sequences but the matched file region does not, reject the
patch with a clear error pointing at the likely cause (tool-call
serialization adding a spurious backslash around apostrophes/quotes).
Exact matches bypass the guard, and legitimate edits that add or
preserve escape sequences in files that already have them still work.
Why: in a prior tool call, old_string was sent with \' where the file
has ' (tool-call transport drift). The fuzzy matcher's block_anchor
strategy matched anyway and produced a diff the tool reported as
successful — but the file was never modified on disk. The agent moved
on believing the edit landed when it hadn't.
Tests: added TestPatchReplacePostWriteVerification (3 cases) and
TestEscapeDriftGuard (6 cases). All pass, existing fuzzy match and
file_operations tests unaffected.
- Bug 1: replace read_file(limit=10000) with read_file_raw in _apply_update,
preventing silent truncation of files >2000 lines and corruption of lines
>2000 chars; add read_file_raw to FileOperations abstract interface and
ShellFileOperations
- Bug 2: split apply_v4a_operations into validate-then-apply phases; if any
hunk fails validation, zero writes occur (was: continue after failure,
leaving filesystem partially modified)
- Bug 3: parse_v4a_patch now returns an error for begin-marker-with-no-ops,
empty file paths, and moves missing a destination (was: always returned
error=None)
- Bug 4: raise strategy 7 (block anchor) single-candidate similarity threshold
from 0.10 to 0.50, eliminating false-positive matches in repetitive code
- Bug 5: add _strategy_unicode_normalized (new strategy 7) with position
mapping via _build_orig_to_norm_map; smart quotes and em-dashes in
LLM-generated patches now match via strategies 1-6 before falling through
to fuzzy strategies
- Bug 6: extend fuzzy_find_and_replace to return 4-tuple (content, count,
error, strategy); update all 5 call sites across patch_parser.py,
file_operations.py, and skill_manager_tool.py
- Bug 7: guard in _apply_update returns error when addition-only context hint
is ambiguous (>1 occurrences); validation phase errors on both 0 and >1
- Bug 8: _apply_delete returns error (not silent success) on missing file
- Bug 9: _validate_operations checks source existence and destination absence
for MOVE operations before any write occurs
kshitij
Elisa Martinez Abad
Teknium
AJ
liuhao1024
Teknium
teyrebaz33
KUSH42
0xbyt4