Patched files have mixed line endings on Windows

[ctkm-aelf]

Auto-mirrored from upstream for the consensus-loop backlog. Do not edit the section below by hand.

Upstream: openai#4003
Area / crate: codex-apply-patch (codex-rs/apply-patch)
Why selected: high impact (51 👍), deterministic, single-crate, unit-testable without Windows.

Acceptance criteria

• Patch application must preserve each file's existing newline style: LF-only files stay LF, CRLF files stay CRLF.
• Add unit tests in codex-apply-patch that apply a patch to CRLF content and assert CRLF is preserved, plus an LF control case.
• No changes outside codex-rs/apply-patch unless strictly required.

---

Original upstream report (openai#4003)

When codex makes changes to files, it does not adhere to the line endings in the file but appears to always use Unix-style LF. On Windows with files using CRLF, this leads to inconsistent line endings in the file. When opening these files in Visual Studio, for example, VS warns about this fact and asks whether line endings should be normalized.

Expected behavior: codex should use LF line endings if the file already uses LF, and CRLF endings if the file already uses CRLF.

Repro platform: Windows NT 10.0.26120 x64, Codex 0.39.0.

[ctkm-aelf]

Minimal solver framing for issue 1:

The smallest viable fix is in codex-rs/apply-patch/src/lib.rs, at the existing-file update path. derive_new_contents_from_chunks currently reads the original file, computes line replacements, then always writes new_lines.join("\n"); that is the narrow point that turns CRLF files into mixed/LF output. Detect the original file's newline style there and join the computed output with "\r\n" for CRLF files, otherwise keep "\n".

Concrete plan:

• Touch codex-rs/apply-patch/src/lib.rs only for implementation, with roughly +8 / -1 LOC.
• Add two byte-exact scenario fixtures under codex-rs/apply-patch/tests/fixtures/scenarios: one CRLF update that expects CRLF bytes, and one LF control update that expects LF bytes.
• No parser, patch grammar, dependency, or ruleset changes.
• Validate with cd codex-rs && just fmt, cd codex-rs && just test -p codex-apply-patch, and cd codex-rs && just fix -p codex-apply-patch before finalizing the actual code change.

Main tradeoff: mixed-line-ending files need a deterministic local policy. I would keep the minimal rule to preserve CRLF only when CRLF is clearly the file style, otherwise use LF, and leave broader normalization policy out of scope.

⟦AI:AUTO-LOOP⟧

[calvintkm]

🤖 Delete solver abstains on CRLF apply-patch issue

TL;DR

• What this is: Delete-framing solver output for issue Patched files have mixed line endings on Windows #1.
• Current state or conclusion: I found no safe deletion or collapse target because the CRLF preservation requirement is a live apply-patch update invariant.
• What the maintainer should do OR what the controller does next: Controller should let the minimal or structural solver own the scoped fix in codex-rs/apply-patch.

---

Delete-solver conclusion

derive_new_contents_from_chunks is the local function that turns parsed patch chunks into new file contents for existing-file updates and unified-diff previews. Deleting it would remove ordinary patch update behavior, not dead code.

I checked the call path in codex-rs/apply-patch/src/lib.rs, the scenario test harness, .gitattributes, and repo history around the earlier CRLF fix and revert. The earlier revert removed a broad helper/test footprint, including codex-rs/core coverage, but it does not make the user-facing preservation requirement unnecessary.

Option	Trade-off
Delete/collapse	Unsafe: no duplicate implementation or disabled feature was found, and deletion would break apply-patch updates.
Abstain	Best fit: preserve deletion-first discipline while allowing a narrow implementation solver to fix new_lines.join("\n").

📎 Full raw codex output (archival)

---
solver: delete
issue: 1
cluster: issue-1
verdict: abstain
---

## Classification
e) Genuinely needed and right-sized; no current deletion/collapse is justified.

## Recommended action
Abstain from deletion. The requested behavior is a real preservation invariant for `codex-apply-patch`: updating an existing CRLF file should not silently convert the rewritten file body to LF. The load-bearing code path is `codex-rs/apply-patch/src/lib.rs::derive_new_contents_from_chunks`, which is called by both patch application and unified-diff preview, and the current implementation reads the original text, computes line replacements, then serializes with `new_lines.join("\n")`. There is no dead feature, disabled endpoint, or duplicate newline-preservation implementation in this crate to remove or redirect to. A minimal implementation solver should keep the fix scoped to `codex-rs/apply-patch`; deletion should not compete here.

## Concrete plan (if propose)
- Files to delete: none
- Caller migrations: none
- Tests to delete: none
- LOC delta: 0
- Philosophy/CLAUDE.md/SPEC/Tier changes (if any): none

## Reverse-evidence (why this is safe to delete)
- No deletion is recommended, so no public API break is introduced by this solver.
- `derive_new_contents_from_chunks` has two local load-bearing callers in `codex-rs/apply-patch/src/lib.rs`: the existing-file update branch and `unified_diff_from_chunks_with_context`.
- Removing the update derivation path would remove ordinary apply-patch update behavior, not an unused feature.
- `rg` across `codex-rs` did not find an existing reusable CRLF preservation abstraction that `codex-apply-patch` could redirect to instead of owning a small local policy.
- Git history shows a previous broad CRLF fix was added in `6c9c563fa` and reverted in `9e91e49ed`; the revert removed a large helper/test footprint across `codex-rs/core`, but it does not prove the user-facing preservation requirement is unnecessary.
- Current scenario tests compare byte snapshots, which is the right shape for CRLF coverage, but `codex-rs/apply-patch/tests/fixtures/scenarios/.gitattributes` currently forces `** text eol=lf`; an implementation plan should account for that if it uses committed CRLF fixtures.

## Risks
- If `ExecutorFileSystem::read_file_text` normalizes line endings before `derive_new_contents_from_chunks`, detecting style from `original_contents` would be insufficient; the implementation solver should verify byte preservation through the CLI/scenario path.
- Mixed-line-ending files need a deterministic policy. Deletion cannot resolve that policy question; the narrow policy should be "preserve CRLF only when the existing file is clearly CRLF, otherwise keep LF."
- A fixture-only test plan can be invalidated by `.gitattributes` normalization unless the test writes CRLF bytes at runtime or marks the CRLF fixture path as binary/non-normalized.

## Escalation triggers (if any)
- none

## Reasoning trace (internal — for meta-judge)
- Why I claim this can be deleted: I do not claim deletion is safe. The value chain is live: user applies patch to an existing file, `derive_new_contents_from_chunks` computes the updated content, and the writer persists that content. Deleting this path removes the core update capability.
- What I checked to verify safety: inspected `codex-rs/apply-patch/src/lib.rs`, confirmed the two callers of `derive_new_contents_from_chunks`, inspected scenario test structure and byte snapshot comparison, checked `.gitattributes`, searched for reusable newline helpers across `codex-rs`, and reviewed git history around the prior CRLF fix/revert.
- What I cannot decide alone: the final exact mixed-newline policy and test fixture representation belong to the implementation solver/judge, but they do not create a deletion or collapse opportunity.

⟦AI:AUTO-LOOP⟧
SOLVER_DONE:delete:abstain:CRLF preservation is a live apply-patch update invariant, and the current code path is load-bearing with no duplicate abstraction to delete or redirect.

⟦AI:AUTO-LOOP⟧

[calvintkm]

🤖 Structural solver proposes a scoped apply-patch newline fix

TL;DR

• What this is: Structural design proposal for issue Patched files have mixed line endings on Windows #1, focused on codex-apply-patch preserving existing file newline style.
• Current state or conclusion: The clean fix point is the update-content derivation path, where current code serializes all updated files with LF.
• What the maintainer should do OR what the controller does next: The controller can compare this with other solver outputs and select a byte-exact scenario-test implementation plan.

---

Structural recommendation

The proposal keeps the change inside codex-rs/apply-patch. derive_new_contents_from_chunks is the private function that reads the current file, computes replacements, and produces the updated file contents; it currently joins computed lines with "\n", which explains why CRLF files can be rewritten with LF.

Recommended shape: represent the source file's newline style as a private LineEnding value, normalize CRLF input to logical lines before matching patch chunks, then serialize the computed result with the original separator. This keeps the invariant local to patch derivation rather than adding Windows-specific behavior to the parser, filesystem layer, or CLI surface.

Option	Trade-off
Private LineEnding in codex-rs/apply-patch/src/lib.rs	Small, reviewable, and keeps newline preservation attached to update derivation.
Parser-level newline handling	Wider blast radius without evidence that patch grammar needs a new concept.
Filesystem-level newline handling	Too broad; add/delete/write callers do not all want text newline policy.

File references:

• codex-rs/apply-patch/src/lib.rs:681 matters because new_lines.join("\n") is the narrow serialization point that converts updates to LF.
• codex-rs/apply-patch/tests/suite/scenarios.rs:65 matters because scenario snapshots compare Vec<u8>, giving byte-exact coverage for CRLF and LF fixtures.
• codex-rs/apply-patch/tests/fixtures/scenarios/.gitattributes:1 matters because the current ** text eol=lf rule would normalize CRLF fixture bytes unless scoped overrides are added.

Expected implementation size: about +54 changed lines total, all under codex-rs/apply-patch, with two new scenario fixtures: one CRLF update and one LF control update.

📎 Full raw codex output (archival)

---

solver: structural
issue: 1
cluster: issue-1
verdict: propose

PROJECT_RULES clause violated (quoted verbatim)

FI-006 Changes must be verifiable and evidence-based; failures, gaps, and out-of-scope promises must be exposed explicitly, and tests must not be disabled to force a pass.

When writing tests, prefer comparing the equality of entire objects over fields one by one.

Avoid bool or ambiguous Option parameters that force callers to write hard-to-read code such as foo(false) or bar(None). Prefer enums, named methods, newtypes, or other idiomatic Rust API shapes when they keep the callsite self-documenting.

Do not create small helper methods that are referenced only once.

Recommended framing

把换行风格作为 codex-apply-patch 更新路径内部的文件属性处理，而不是在写入点做一次字符串例外替换：在 derive_new_contents_from_chunks 读取原文件后，解析出一个私有 LineEnding 值，后续仍用现有的行替换算法计算 new_lines，最后由 LineEnding 统一决定 join 分隔符。这样 CRLF/LF 保留成为该 crate 的补丁派生不变量，测试用现有 scenario harness 做字节级端到端验证，避免把 Windows 规则、全局配置或跨 crate API 引入核心路径。

Concrete plan

• New abstractions (if any): private enum LineEnding { Lf, Crlf } in codex-rs/apply-patch/src/lib.rs; interface fn detect(contents: &str) -> Self and fn separator(self) -> &'static str only if both are used by the update path and tests, otherwise inline detection to respect the one-use helper rule. Detection policy: return Crlf only when the original text contains \r\n and no earlier stronger LF-only evidence is needed for this issue; return Lf for LF-only, empty, or ambiguous/mixed content.
• Files: codex-rs/apply-patch/src/lib.rs update derive_new_contents_from_chunks at lines 660-681 so it records original newline style before splitting, strips any trailing \r from split lines for CRLF input, applies existing replacements, and joins with "\r\n" for CRLF or "\n" for LF. codex-rs/apply-patch/tests/fixtures/scenarios/.gitattributes add scoped overrides for the CRLF fixture files so Git preserves expected bytes despite the current ** text eol=lf rule. codex-rs/apply-patch/tests/fixtures/scenarios/023_update_preserves_crlf/{input,expected}/crlf.txt plus patch.txt add byte-exact CRLF coverage. codex-rs/apply-patch/tests/fixtures/scenarios/024_update_preserves_lf/{input,expected}/lf.txt plus patch.txt add LF control coverage.
• LOC delta: ~+24 / -1 in implementation, ~+5 in .gitattributes, ~+24 fixture lines across 6 fixture files; total changed lines ~54, all inside codex-rs/apply-patch.
• Tests to add: scenario 023_update_preserves_crlf asserts an update to CRLF input writes expected CRLF bytes; scenario 024_update_preserves_lf asserts the same update on LF input stays LF. The existing codex-rs/apply-patch/tests/suite/scenarios.rs harness snapshots files as Vec<u8> at lines 65-101, so these are byte-level behavior tests, not line-count tests.
• Schema/protocol changes (if any): none; no CLI argument, app-server API, config, or serialized protocol changes.
• Governance/ruleset changes (if any): none. The plan follows AGENTS.md by staying under the 800-line guidance, avoiding codex-core, and using whole-file byte equality through the existing scenario snapshot primitive.
• Runtime cost: one str::contains("\r\n") scan over the original file, O(n) byte comparisons, usually one extra pass over data already in memory; zero filesystem calls; zero async hops; one enum or &str local allocation-free value. For a 100 KB file this is roughly 100 KB additional memory reads and no heap allocation beyond the existing line vectors.
• If the structural answer is issue decomposition, use the controller-private IssueDecompositionPlan boundary and child body artifacts; forbid solver/judge/worker GitHub lifecycle, public issue factories, and any wakeup_plan.py decompose action/projection. Not applicable; this is a single-crate fix.

Risks

• Preserving CRLF by detecting \r\n means deliberately choosing a deterministic local policy for mixed-ending input. The bounded policy should be: CRLF only for clear CRLF files, LF otherwise; do not normalize mixed files or attempt per-line preservation in this issue.
• Adding a named enum for two separators could be over-engineering if implemented as a public API. Keep it private to lib.rs, or inline the separator local if the enum would only be used once after tests are added through fixtures.
• The existing split path uses split('\n'); CRLF input leaves trailing \r in line strings unless removed before matching. The implementation must normalize internal line vectors to logical lines before compute_replacements, then reapply the detected separator at serialization.
• Fixture storage can be silently normalized by Git. The .gitattributes override for the two CRLF fixture files is required so the behavior remains verifiable across platforms.

Escalation triggers (if any)

• none

Reasoning trace (internal — for meta-judge)

• Why this structure beats a documented exception: the failure is in codex-rs/apply-patch/src/lib.rs:681, where all updates serialize through new_lines.join("\n"); encoding the source line ending as an internal value makes preservation part of the file-update derivation rather than an external Windows note.
• Why I picked this abstraction over alternatives: a private LineEnding value is enough for two concrete call paths through the same derivation result: direct patch application and unified_diff_from_chunks both call derive_new_contents_from_chunks. A parser-level or filesystem-level abstraction would touch more layers without new evidence.
• What I cannot decide alone: the long-term policy for genuinely mixed line endings. The issue acceptance criteria only require LF-only and CRLF files to preserve their existing style, so broader mixed-file normalization should remain out of scope unless maintainers request it.

⟦AI:AUTO-LOOP⟧
SOLVER_DONE:structural:propose:private apply-patch newline-style preservation with byte-exact CRLF and LF scenarios

⟦AI:AUTO-LOOP⟧

[calvintkm]

🤖 Meta-judge reached consensus for issue #1

TL;DR

• What this is: Meta-judge decision for design-consensus round 1 on issue Patched files have mixed line endings on Windows #1.
• Current state or conclusion: Consensus is reached on a scoped codex-apply-patch fix that preserves existing LF/CRLF file newline style during updates.
• What the maintainer should do OR what the controller does next: The controller should auto-dispatch implementation with the scope and verification plan below.

---

Consensus decision

The two implementation-bearing solvers agree on the same boundary: update codex-rs/apply-patch so existing-file patch updates detect the target file's clear newline style and serialize the derived content with the matching separator. The delete solver abstained with reverse-evidence that the update path is load-bearing and has no duplicate abstraction to remove.

Option	Decision	Trade-off
Minimal implementation	Accepted as part of consensus	Smallest viable change at the update serialization point.
Structural framing	Accepted as part of consensus	Same scoped fix, with the newline policy treated as a private apply-patch invariant.
Delete/collapse	Neutral abstain	No deletion target; removing the path would break normal patch updates.

Implementation scope:

• codex-rs/apply-patch/src/lib.rs: detect original LF/CRLF style in the existing-file update derivation path and stop using unconditional LF serialization.
• codex-rs/apply-patch/tests/fixtures/scenarios/.gitattributes: preserve CRLF fixture bytes if committed scenario fixtures are used.
• codex-rs/apply-patch/tests/fixtures/scenarios/023_update_preserves_crlf/: byte-exact CRLF update scenario.
• codex-rs/apply-patch/tests/fixtures/scenarios/024_update_preserves_lf/: byte-exact LF control scenario.

Verification commands:

• cd codex-rs && just fmt
• cd codex-rs && just test -p codex-apply-patch
• cd codex-rs && just fix -p codex-apply-patch

No CLAUDE.md, AGENTS.md, SPEC, Tier, schema, protocol, or GitHub lifecycle changes are authorized by this decision.

📎 Full raw codex output (archival)

---

issue: 1
cluster: issue-1
convergence_round: 1
solver_verdicts:
minimal: propose
structural: propose
delete: abstain
decision: consensus

Decision

达成共识：minimal 与 structural 都把实现边界限定在 codex-rs/apply-patch 的 existing-file update 派生路径，核心变化都是检测原文件 LF/CRLF 风格并在 derive_new_contents_from_chunks 输出时使用对应分隔符，同时添加 CRLF 与 LF 的字节级 scenario 覆盖；delete solver 明确 abstain，并给出 reverse-evidence 说明该更新路径是 load-bearing、没有可删除或可重定向的重复抽象。该组合满足 Path A manual-issue compatible-neutral exception：2 个 implementation-bearing proposals 同意同一具体方案，delete abstain 不矛盾且证明没有当前 deletion target。

If consensus

• Chosen framing: hybrid-A+B
• Implement plan (structured fields read by wakeup-plan from this judge artifact only, not from solver artifacts or prompt-body free text):
  • scope_paths:
    • codex-rs/apply-patch/src/lib.rs
    • codex-rs/apply-patch/tests/fixtures/scenarios/.gitattributes
    • codex-rs/apply-patch/tests/fixtures/scenarios/023_update_preserves_crlf/
    • codex-rs/apply-patch/tests/fixtures/scenarios/024_update_preserves_lf/
  • old_pattern: existing-file patch updates compute logical replacements and then serialize with unconditional new_lines.join("\n"), which converts CRLF files to LF/mixed output.
  • new_principle: existing-file patch updates preserve the target file's clear newline style by detecting LF vs CRLF from original contents, normalizing lines for matching, and serializing with the original separator; LF remains the fallback for LF-only, empty, or ambiguous/mixed content.
  • verification_hints: cd codex-rs && just fmt; cd codex-rs && just test -p codex-apply-patch; cd codex-rs && just fix -p codex-apply-patch. If fixture files are committed with CRLF bytes, add scoped .gitattributes entries so Git does not normalize the CRLF input/expected fixtures.
• Philosophy/CLAUDE.md/SPEC/Tier changes included: none
• Implementation owner: dispatch implement codex with cluster_id=issue-1, design_decision_path=.refactor-loop/runs/phase9-issue1-r1-judge.md
• Add crnd:triage:resume-requested label to issue 1
• For large-issue decomposition consensus, direction consensus may authorize an IssueDecompositionPlan artifact and child body artifacts only. To authorize the later Pls let plus users use, i dont have money to use this token by token, i only have fxxking 20USD/mo, thanks openai/codex#396 named apply projection, this plan-level judge artifact must carry structured fields controller_action="apply_issue_decomposition_plan", plan_level_design_consensus_judge_artifact, issue_decomposition_plan_path, issue_decomposition_plan_digest, and issue_decomposition_proof; the first META_JUDGE_DONE:consensus:decompose, solver artifacts, prompt body, validator output, worker output, and .refactor-loop/host.env are not apply authorization. Do not authorize solver/judge/implement worker GitHub lifecycle calls, public issue factory commands, parent close/reopen/body-title edits, or generic wakeup_plan.py decompose projections.

If converge

• Convergence question (specific): none
• What each solver should address explicitly: none
• Round number this fires: 2; marker payload uses source round 1

Round audit trail (links to local artifacts)

• solver-minimal: .refactor-loop/logs/phase9-issue1-r1-minimal.log
• solver-structural: .refactor-loop/logs/phase9-issue1-r1-structural.log
• solver-delete: .refactor-loop/logs/phase9-issue1-r1-delete.log
• work-unit-producer: manual-issue (prompt-only provenance)
• work-unit-source-ref: gh-issue-1

⟦AI:AUTO-LOOP⟧
META_JUDGE_DONE:consensus:hybrid-A+B:preserve apply-patch existing-file newline style in codex-rs/apply-patch with byte-exact CRLF and LF scenarios

⟦AI:AUTO-LOOP⟧