Skip to content

fix(coding-agent): preserve line endings when apply_patch updates a file - #2639

Merged
code-yeongyu merged 4 commits into
mainfrom
fix/apply-patch-preserve-line-endings
Oct 3, 2026
Merged

code-yeongyu merged 4 commits into
mainfrom
fix/apply-patch-preserve-line-endings

Conversation

@code-yeongyu

@code-yeongyu code-yeongyu commented Oct 3, 2026 •

Copy link
Copy Markdown
Owner

Fixes #2638. The standalone package's same fix is code-yeongyu/pi-apply-patch#48; senpi's builtin is a hand-maintained port (MANUAL_PACKAGES), so it gets its own fix.

What

  • The bug: updates split files on LF and joined them back with LF, so a one-line patch rewrote every line of a CRLF or mixed-ending file. That showed in git, and in the tool's preview as a whole-file diff.
  • line-endings.ts (new): SourceText keeps each line's own ending (\r\n, \n, lone \r). replacementsAroundContext splits a matched chunk around its context lines, so those stay exactly as in the file. Inserted lines take the file's first ending, and every line still ends with one.
  • Where the codebase changes: replaceChunks uses it, and the parsers record each chunk's contextLineIndices.
  • Codex comparison: this is Codex's PreserveLineEndings model (text_file.rs, compute_replacements). In Codex it is behind an under-development flag, off by default, and its conformance scenarios run with it on. oh-my-pi restores the detected ending after edits.
  • Two user-visible changes: endings are preserved, and a context line matched only after trimming is left as it was instead of being rewritten to the patch's copy. LF files produce identical bytes.

Tests (test/suite/gpt-apply-patch-line-endings.test.ts)

TEST AUTHORING GATE:

  1. What it protects: the update file-content contract (only changed lines are rewritten).
  2. The regression it catches: a return to LF-normalizing whole-file rewrites.
  3. Why existing tests miss it: no owner test uses CRLF or mixed-ending sources.
  4. New production seam: none; the tests use applyPatchDetailed and the tool's execute.
Case main This PR
CRLF change plus insertion fails passes
Mixed endings fails passes
Trimmed-match context line untouched fails passes
One-line CRLF change previews as +1 -1 fails passes
LF file without a final newline passes passes (regression guard)
Failed hunk leaves CRLF bytes untouched passes passes (regression guard)

Verification


Summary by cubic

Fixes apply_patch rewriting every line of CRLF and mixed-ending files as LF on update, so a one-line patch no longer becomes a whole-file diff in git and in the preview (#2638).

Files are now parsed into lines that keep their own ending; only touched lines are replaced, context lines stay exactly as they were, and inserted lines take the file's first ending. A context line matched only after trimming is also left untouched. LF files produce identical bytes.

Pins pi-apply-patch to 0.1.4, which carries this fix and the indented-header fix from #2637 upstream; the grammar declaration it added is already native here.

Tests

Written for commit 8ac5ce9. Summary will update on new commits.

Review in cubic

@code-yeongyu

Copy link
Copy Markdown
Owner Author

Lead in-session review at 1814736: PASS. Same model as pi-apply-patch #48. SourceText.parse splits on exactly the endings normalizePatchText used to fold (CRLF, lone CR, LF), so dropping that call from replaceChunks changes only the output endings, never matching. Both the batch parser and the streaming parser record contextLineIndices on the same two branches (' ' and the bare empty line), so context lines keep their source text and ending on both paths; inserted lines take the file's first ending; LF-only output is unchanged. Lands after the 2026.10.4 dispatch with #2637.

@code-yeongyu

Copy link
Copy Markdown
Owner Author

Real-CLI QA (senpi-qa Channel 3 pattern, zero tokens). The real senpi --print CLI runs in an isolated sandbox against the local fake model server, with a scripted gpt-5.5 model whose only tool call is apply_patch. The real credential file's hash was unchanged.

Scenario main (control) this branch (1814736)
CRLF lines.txt (one\r\ntwo\r\nthree\r\n): change one line, insert one FAIL: rewritten to ONE\ntwo\nbetween\nthree\n PASS: ONE\r\ntwo\r\nbetween\r\nthree\r\n

@code-yeongyu
code-yeongyu force-pushed the fix/apply-patch-preserve-line-endings branch from 1814736 to f89ea49 Compare October 3, 2026 14:13
The built-in apply_patch split files on LF and joined them back with LF, so
a one-line patch rewrote every line of a CRLF or mixed-ending file. Files are
now parsed into lines that keep their own ending; a chunk is replaced around
its context lines, which stay untouched, and inserted lines take the file's
first ending (Codex's PreserveLineEndings model).

Fixes #2638
Record the upstream version whose apply_patch fixes this builtin now carries
(indented headers in #2637, line endings in this PR); the grammar declaration
upstream added is already native here.

Refs #2638
@code-yeongyu
code-yeongyu force-pushed the fix/apply-patch-preserve-line-endings branch from 713c9dd to b6762bf Compare October 3, 2026 15:13
…rve-line-endings

# Conflicts:
#	packages/coding-agent/CHANGELOG.md
#	packages/coding-agent/src/core/extensions/builtin/gpt-apply-patch/changes.md
#	packages/coding-agent/src/core/extensions/builtin/gpt-apply-patch/parser.ts
@code-yeongyu
code-yeongyu merged commit 7e42127 into main Oct 3, 2026
33 checks passed
@code-yeongyu
code-yeongyu deleted the fix/apply-patch-preserve-line-endings branch October 3, 2026 16:20
code-yeongyu added a commit that referenced this pull request Oct 3, 2026
…n (senpi#2656)

Proves the main-merge kept all three behaviors: an indented Update File header
applies (#2637), CRLF is preserved (#2639), and a + line duplicating a context
line counts (+1 -0). Fails on main (count) and on the pre-merge head
(header/CRLF), passes on the merged head.

Refs #2656
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.

apply_patch rewrites CRLF and mixed line endings to LF on every update

1 participant