Skip to content

fix(tools): make apply_patch replacement explicit and compatible#4476

Merged
Hmbown merged 2 commits into
Hmbown:mainfrom
Angel-Hair:fix/apply-patch-replace
Jul 17, 2026
Merged

fix(tools): make apply_patch replacement explicit and compatible#4476
Hmbown merged 2 commits into
Hmbown:mainfrom
Angel-Hair:fix/apply-patch-replace

Conversation

@Angel-Hair

Copy link
Copy Markdown
Contributor

Summary

apply_patch accepts a changes parameter that performs full file replacement, not patching. The name is easily confused with the patch parameter (which applies a unified diff). When both are passed simultaneously, changes silently takes precedence and patch is discarded — no error, no warning, and the file is overwritten with only the changes content.

Root cause

In execute(), the changes branch returns immediately if present, making the patch branch unreachable. The JSON schema declares oneOf mutual exclusivity, but this is advisory — the Rust handler enforces precedence, not rejection.

Fix

  • Rename changes to replace across JSON schema, code, function names, enum variants, error messages, and tests
  • Add explicit conflict detection: if both patch and replace are present, return an error telling the model to choose one

Testing

  • cargo fmt -- --check
  • cargo clippy -p codewhale-tui (zero warnings)
  • cargo check -p codewhale-tui
  • cargo test -p codewhale-tui -- tools::apply_patch (26/26 passed)

Checklist

  • Updated docs or comments as needed
  • Added or updated tests where relevant
  • Verified TUI behavior manually if UI changes
  • Harvested/co-authored credit uses a GitHub numeric noreply address

@Angel-Hair
Angel-Hair requested a review from Hmbown as a code owner July 17, 2026 17:39
@github-actions

Copy link
Copy Markdown
Contributor

Thanks @Angel-Hair for taking the time to contribute.

This repository is observing a maintainer-managed PR intake gate in dry-run mode, so this pull request is staying open. This note helps maintainers prepare the allowlist before any enforcement is considered.

Please read CONTRIBUTING.md for the expected contribution shape. A maintainer can grant recurring PR access by commenting /lgtm on a pull request.

@Hmbown Hmbown added this to the v0.9.1 milestone Jul 17, 2026
@Angel-Hair
Angel-Hair force-pushed the fix/apply-patch-replace branch from 2d71857 to 82a9c35 Compare July 17, 2026 17:53
…t detection

The 'changes' parameter name is easily confused with 'patch'. When both are
passed simultaneously, 'changes' silently takes precedence and 'patch' is
discarded with no error or warning.

- Rename JSON schema key and all internal references from 'changes' to 'replace'
- Add explicit error when both 'patch' and 'replace' are passed simultaneously
- Update function names, enum variants, error messages, and tests accordingly

Fixes: apply_patch changes/replace ambiguity
@Angel-Hair
Angel-Hair force-pushed the fix/apply-patch-replace branch from 82a9c35 to a1980c0 Compare July 17, 2026 18:28
Keep replace as the canonical full-file mode while accepting the legacy changes payload through one normalized parser. Reject every mixed-mode pair before preflight or writes, and route policy, approval, diagnostics, and UI consumers through the same contract.

Also repairs the three schema-note regressions reported by CI run 29601773843.

Follow-up to PR Hmbown#4476 by @Angel-Hair.

Signed-off-by: Hunter B <hmbown@gmail.com>
@Hmbown Hmbown changed the title fix: rename apply_patch 'changes' parameter to 'replace' with conflict detection fix(tools): make apply_patch replacement explicit and compatible Jul 17, 2026
@Hmbown

Hmbown commented Jul 17, 2026

Copy link
Copy Markdown
Owner

Maintainer compatibility amendment is now on the contributor branch at e23cd2b9f.

The original report and contributor commit remain intact. The follow-up keeps replace as the canonical field while accepting deprecated changes, routes execution, preflight, repo-law, approval, diagnostics, and UI previews through one parser, and rejects all three ambiguous pairs before any file write (patch + replace, patch + changes, and replace + changes). It also corrects the Responses schema constraint note that failed the first macOS/Windows run.

Verification on the identical final source tree: 29 apply-patch tests; Responses and schema-sanitizer regressions; repo-law, approval-key, approval-preview, edited-path, and tool-routing coverage; cargo check -p codewhale-tui; formatting and diff checks. Fresh exact-head CI is now the final merge gate.

Thank you @Angel-Hair for finding and fixing the silent precedence bug.

@Hmbown
Hmbown merged commit 760595f into Hmbown:main Jul 17, 2026
14 checks passed
@Angel-Hair
Angel-Hair deleted the fix/apply-patch-replace branch July 17, 2026 18:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants