Skip to content

fix(patch): add idempotency guard to prevent duplicate content loops - #18614

Open
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/issue-18426-patch-duplicate-guard
Open

fix(patch): add idempotency guard to prevent duplicate content loops#18614
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/issue-18426-patch-duplicate-guard

Conversation

@liuhao1024

@liuhao1024 liuhao1024 commented May 2, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Add an idempotency guard to fuzzy_find_and_replace that prevents the patch tool from creating duplicate content when the same patch is applied to an already-patched file.

Related Issue

N/A

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • See commit messages for detailed changes

How to Test

  1. Run pytest tests/ -q — all tests should pass
  2. Verify the specific scenario described above is resolved

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 26.4.1

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture and workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A

When the agent retries a patch without re-reading the file, fuzzy
matching may find the previously-applied replacement and re-apply it,
creating duplicate content blocks. Add a guard that checks whether
the matched region already equals new_string before writing.

Fixes NousResearch#18426
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/tools Tool registry, model_tools, toolsets tool/file File tools (read, write, patch, search) labels May 2, 2026
Cyrene963 pushed a commit to Cyrene963/hermes-agent that referenced this pull request May 3, 2026
Community PRs applied:
- NousResearch#18596: Enable secret redaction by default (SECURITY)
- NousResearch#18650: Sanitize malformed tool messages + auto-recover on API 400
- NousResearch#18607: Emergency compression before max_iterations exhaustion
- NousResearch#18603: Compression fallback to main model on 413 rate limit
- NousResearch#18638: Pass threshold_percent on model switch
- NousResearch#18663: Strip extra_content from tool_calls for strict APIs
- NousResearch#18618: Forward explicit_api_key to OpenRouter
- NousResearch#18632: Show cache tokens in /insights breakdown
- NousResearch#18614: Add idempotency guard for patch duplicate loops
- NousResearch#18600: Raise ValueError when HERMES_HOME unset in profile mode
- NousResearch#18616: Allow ZWJ emoji in context files
- NousResearch#18582: Reload .env on /restart
- NousResearch#18547: Stabilize system prompt prefix for KV cache reuse
- NousResearch#18692: Strip FTS5 operators from session search truncation terms

Fix: Add order_by_last_active=True to list_sessions_rich call
(pre-existing commit 142b4bf code sync)

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tackling the duplicate-content retry path. The bug is still present on current main, but this guard does not cover the reported failure shape.

Problems

  • tools/fuzzy_match.py:102 compares the selected match span with the complete raw new_string. Current context_aware accepts an old-string-sized candidate at 50% line similarity (tools/fuzzy_match.py:696-728), then _apply_replacements replaces only that span (tools/fuzzy_match.py:407-415). After an edit that adds a line, a retry can match a changed prefix rather than the full replacement, bypass this guard, and retain a duplicated trailing line.
  • The added tests do not exercise that multi-line, longer-replacement retry path. The linked #18426 discussion and competing #56570 describe this concrete form; #54575 also targets the permissive fallback strategies.

Suggested changes

  • Add a regression that applies a longer multi-line replacement, retries it, and asserts the content is unchanged with a clean failure.
  • Guard before permissive fallback matching, or reject divergent fallback candidates before replacement while retaining legitimate formatting-tolerant matches.

Automated hermes-sweeper review.

Comment thread tools/fuzzy_match.py
# duplicate content in a retry loop (issue #18426).
matches_to_apply = [
m for m in matches
if content[m[0]:m[1]] != new_string

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only detects a selected span equal to the full new_string. context_aware can select an old-string-sized prefix of an already-applied longer replacement; this comparison is then false, and _apply_replacements inserts new_string while retaining the old trailing line. Please add that longer multi-line retry regression and guard before this permissive fallback is allowed to write.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tools Tool registry, model_tools, toolsets P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades tool/file File tools (read, write, patch, search) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants