chore(repo): check out text files as LF on every platform (#1485) #1489

Merged

claude merged 1 commit from chore/1485-gitattributes-lf into main 2026-10-02 11:11:59 +02:00

Conversation 5 Commits 1 Files changed 2 +42

claude commented 2026-10-02 09:57:21 +02:00

Member

• Copy link
• Copy Markdown

Refs #1485

What broke (measured)

A checkout made with core.autocrlf=true (the Git for Windows default) writes all 613 non-empty tracked files into the working tree as CRLF, and three tests fail on it. Measured 2026-10-02 at 02fae6a with Go 1.27.1 and Git 2.55.0.windows.3:

Test	Package	On the CRLF tree
TestDecide_882PendingCutReturnsTypedRecoveryEvidence	internal/decide	changelog: malformed version heading
TestV035HistoricalSectionRemainsClean	internal/gates	check 2 (sections): missing section heade...
TestHelpSnapshot	cmd/rt	rt --help drifted from cmd/rt/testdata/help.txt (Linux container, mounted tree)

What changes

• .gitattributes (new): * text=auto eol=lf, plus explicit text eol=lf for *.go, *.mod, *.sum, *.md, *.json, *.yml, *.yaml, *.txt, *.log, *.sh, *.bats, *.py, VERSION and Dockerfile. eol=lf overrides core.autocrlf for every path it covers.
• changelog.d/1485-gitattributes-eol.internal.md (new): the ### Internal entry, including the one-time rewrite for existing CRLF checkouts.

No stored content changes. Before this PR, no tracked path held a CR byte in the index, no path was binary, and the tree has no .bat/.cmd/.ps1 files, so nothing needs CRLF.

Decision tree

• A repo-level .gitattributes (taken) is right when the tree must look the same to every tool that reads it, whatever each clone's git config says. Go tests, snapshot comparisons and scripts mounted into Linux containers all read the working tree. Cost: an existing CRLF checkout keeps its CRLF files until rewritten; the fragment gives the command.
• Contributor docs only (git config core.autocrlf false) is right when the setting can be relied on. It cannot: it is per clone, and a forgotten setting shows up only as a test failure that reads like a parser bug.
• CRLF-tolerant parsers are right if adopter input can arrive with CRLF. That is a separate question about adopter repositories and is not measured here. It would also not fix TestHelpSnapshot, which compares the text exactly.
• A blanket testdata/** text rule is right only if testdata will never hold a binary fixture. Not taken: * text=auto eol=lf already gives text fixtures LF, and forcing text would normalize, and so corrupt, a future archive or image fixture on commit.

Verification

All on fresh clones made under the system core.autocrlf=true:

• Baseline 02fae6a: 613 i/lf w/crlf; the three tests above fail.
• This branch (36aa902): 615 i/lf w/lf, 7 i/none w/none, 0 w/crlf. git add --renormalize . stages 0 paths. The two native tests pass, and TestHelpSnapshot passes in golang:1.25 on the mounted tree.
• Full suite in golang:1.25: 29/29 packages ok on the LF tree. On the CRLF tree, cmd/rt fails TestHelpSnapshot there too.
• rt fragment-check (content mode) on changelog.d/: kind, summary header, issue reference and density pass, with no warnings.
• Refresh recipe: a CRLF clone that pulled the change kept 613 CRLF files. After git rm -r --cached -q . && git reset --hard, all 615 are LF and git status is clean.

Not run locally: bats, golangci-lint, go vet. Forgejo CI covers them on this PR.

Still failing on native Windows (out of scope)

These have Windows causes, not line-ending ones:

• cmd/rt and internal/release do not compile (syscall.Flock).
• TestRepinFile_PreservesModeAndSkipsNoMatch, TestDiscoverUnreadableIsUngradeable and TestRunPostBumpHooks_theHookDoesNotReceiveTheGitCredential assert POSIX file modes.
• TestScanTree gets backslash paths.
• TestHeaderEmDashLocaleAndByteClassMutation depends on how Git Bash handles locales.

🤖 Generated with Claude Code

Refs #1485 ## What broke (measured) A checkout made with `core.autocrlf=true` (the Git for Windows default) writes all 613 non-empty tracked files into the working tree as CRLF, and three tests fail on it. Measured 2026-10-02 at `02fae6a` with Go 1.27.1 and Git 2.55.0.windows.3: | Test | Package | On the CRLF tree | |---|---|---| | `TestDecide_882PendingCutReturnsTypedRecoveryEvidence` | `internal/decide` | `changelog: malformed version heading` | | `TestV035HistoricalSectionRemainsClean` | `internal/gates` | `check 2 (sections): missing section heade...` | | `TestHelpSnapshot` | `cmd/rt` | `rt --help drifted from cmd/rt/testdata/help.txt` (Linux container, mounted tree) | ## What changes - **`.gitattributes`** (new): `* text=auto eol=lf`, plus explicit `text eol=lf` for `*.go`, `*.mod`, `*.sum`, `*.md`, `*.json`, `*.yml`, `*.yaml`, `*.txt`, `*.log`, `*.sh`, `*.bats`, `*.py`, `VERSION` and `Dockerfile`. `eol=lf` overrides `core.autocrlf` for every path it covers. - **`changelog.d/1485-gitattributes-eol.internal.md`** (new): the `### Internal` entry, including the one-time rewrite for existing CRLF checkouts. No stored content changes. Before this PR, no tracked path held a CR byte in the index, no path was binary, and the tree has no `.bat`/`.cmd`/`.ps1` files, so nothing needs CRLF. ## Decision tree - **A repo-level `.gitattributes`** (taken) is right when the tree must look the same to every tool that reads it, whatever each clone's git config says. Go tests, snapshot comparisons and scripts mounted into Linux containers all read the working tree. Cost: an existing CRLF checkout keeps its CRLF files until rewritten; the fragment gives the command. - **Contributor docs only** (`git config core.autocrlf false`) is right when the setting can be relied on. It cannot: it is per clone, and a forgotten setting shows up only as a test failure that reads like a parser bug. - **CRLF-tolerant parsers** are right if adopter input can arrive with CRLF. That is a separate question about adopter repositories and is not measured here. It would also not fix `TestHelpSnapshot`, which compares the text exactly. - **A blanket `testdata/** text` rule** is right only if testdata will never hold a binary fixture. Not taken: `* text=auto eol=lf` already gives text fixtures LF, and forcing `text` would normalize, and so corrupt, a future archive or image fixture on commit. ## Verification All on fresh clones made under the system `core.autocrlf=true`: - **Baseline `02fae6a`:** 613 `i/lf w/crlf`; the three tests above fail. - **This branch (`36aa902`):** 615 `i/lf w/lf`, 7 `i/none w/none`, 0 `w/crlf`. `git add --renormalize .` stages 0 paths. The two native tests pass, and `TestHelpSnapshot` passes in `golang:1.25` on the mounted tree. - **Full suite in `golang:1.25`:** 29/29 packages ok on the LF tree. On the CRLF tree, `cmd/rt` fails `TestHelpSnapshot` there too. - **`rt fragment-check`** (content mode) on `changelog.d/`: kind, summary header, issue reference and density pass, with no warnings. - **Refresh recipe:** a CRLF clone that pulled the change kept 613 CRLF files. After `git rm -r --cached -q . && git reset --hard`, all 615 are LF and `git status` is clean. Not run locally: bats, golangci-lint, `go vet`. Forgejo CI covers them on this PR. ## Still failing on native Windows (out of scope) These have Windows causes, not line-ending ones: - `cmd/rt` and `internal/release` do not compile (`syscall.Flock`). - `TestRepinFile_PreservesModeAndSkipsNoMatch`, `TestDiscoverUnreadableIsUngradeable` and `TestRunPostBumpHooks_theHookDoesNotReceiveTheGitCredential` assert POSIX file modes. - `TestScanTree` gets backslash paths. - `TestHeaderEmDashLocaleAndByteClassMutation` depends on how Git Bash handles locales. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude added 1 commit 2026-10-02 09:57:21 +02:00

chore(repo): check out text files as LF on every platform (#1485) ... 36aa90201d

A core.autocrlf=true checkout (the Git for Windows default) wrote every
tracked text file into the working tree with CRLF, and the tests that
parse fixtures or compare snapshots failed on it:
TestDecide_882PendingCutReturnsTypedRecoveryEvidence,
TestV035HistoricalSectionRemainsClean and TestHelpSnapshot.

Add a root .gitattributes: `* text=auto eol=lf`, plus explicit
`text eol=lf` for the types the tests parse. The index was already LF
everywhere, so `git add --renormalize .` stages nothing else.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

alex approved these changes 2026-10-02 09:59:55 +02:00

Dismissed

claude reviewed 2026-10-02 11:05:20 +02:00

claude left a comment

• Copy link
• Copy Markdown

Review: reviewed head 36aa90201d0be61fbf948d80497d36cea24fc190

Verdict: the change is correct. One merge blocker, in the PR body: ac-closure-check is red. This is filed as COMMENT, not APPROVED, because this account (claude) also authored the PR, and Forgejo refuses self-approval.

Blocker: ac-closure-check fails on the body (run 18493, job 1)

The log says:

FAIL #1485 — a close keyword targets it, but the PR does not DECLARE that intent. … Either add Intended-targets: #1485 to the PR body …

The same run grades #1485 as clean, no unfinished acceptance criterion, so only the declaration is missing. Fix: add a line Intended-targets: #1485 to the body. Put it at the start of the line, with no bullet or bold. The workflow triggers on pull_request: edited, so a body edit re-runs the gate. No new commit is needed and the reviewed head does not move.

Verified at source (fresh clones under system core.autocrlf=true, Git 2.55.0.windows.3, Go 1.27.1)

• Fresh clone of 36aa902 (git clone -b): 615 i/lf w/lf, 7 i/none w/none, 0 w/crlf. git add --renormalize . stages 0 paths. This matches the body.
• 36aa902 merged with live main 29b541b (the branch is 4 commits behind): 622 i/lf w/lf, 0 w/crlf. No path on main holds a CR byte in the index. The 4 new commits add nothing the attributes mishandle.
• Mutation check (Axis B): on a fresh CRLF clone of 29b541b, TestDecide_882PendingCutReturnsTypedRecoveryEvidence fails (malformed version heading) and TestV035HistoricalSectionRemainsClean fails (check 2 (sections)). On the merged tree both pass natively. The fix discriminates.
• Full suite in golang:1.25 on the merged tree: 29/29 packages ok, including cmd/rt (TestHelpSnapshot).
• Paths outside the explicit text eol=lf list: 10 text=auto, plus .keep/.pub/.likec4/.gitignore/.gitattributes. All resolve to LF, and no binary is mis-tagged.
• Both new files are ASCII-only (section 11). Neither touches a compose-script, so no re-pin is needed (section 2), and check-self-bootstrap is green.
• Every other CI context is green.

Non-blocking

1. Fragment label. The bold label is **internal**, which repeats the kind. Other fragments name the area (**prep**, **release PAT scope**). Something like **line endings** would read better in the release notes. This is optional; fragment-check passes either way.
2. AC ticking. The issue's acceptance boxes were ticked by the author before review. They are accurate (I re-measured each one above), so I am not asking for a revert. The usual flow is for the reviewer to tick them with DONE @ #1489 (sha).
3. Approval order. The existing APPROVED row (alex, 09:59:55) was submitted before CI finished (10:00:19). It stays bound to 36aa902, and the body-only fix will not make it stale.

Once Intended-targets: #1485 is in the body and the gate is green, this is ready to merge from my side.

## Review: reviewed head `36aa90201d0be61fbf948d80497d36cea24fc190` **Verdict: the change is correct. One merge blocker, in the PR body: `ac-closure-check` is red.** This is filed as COMMENT, not APPROVED, because this account (`claude`) also authored the PR, and Forgejo refuses self-approval. ### Blocker: `ac-closure-check` fails on the body (run 18493, job 1) The log says: > `FAIL #1485 — a close keyword targets it, but the PR does not DECLARE that intent. … Either add Intended-targets: #1485 to the PR body …` The same run grades #1485 as `clean, no unfinished acceptance criterion`, so only the declaration is missing. **Fix:** add a line `Intended-targets: #1485` to the body. Put it at the start of the line, with no bullet or bold. The workflow triggers on `pull_request: edited`, so a body edit re-runs the gate. No new commit is needed and the reviewed head does not move. ### Verified at source (fresh clones under system `core.autocrlf=true`, Git 2.55.0.windows.3, Go 1.27.1) - **Fresh clone of `36aa902`** (`git clone -b`): 615 `i/lf w/lf`, 7 `i/none w/none`, 0 `w/crlf`. `git add --renormalize .` stages 0 paths. This matches the body. - **`36aa902` merged with live main `29b541b`** (the branch is 4 commits behind): 622 `i/lf w/lf`, 0 `w/crlf`. No path on main holds a CR byte in the index. The 4 new commits add nothing the attributes mishandle. - **Mutation check (Axis B):** on a fresh CRLF clone of `29b541b`, `TestDecide_882PendingCutReturnsTypedRecoveryEvidence` fails (`malformed version heading`) and `TestV035HistoricalSectionRemainsClean` fails (`check 2 (sections)`). On the merged tree both pass natively. The fix discriminates. - **Full suite in `golang:1.25`** on the merged tree: 29/29 packages ok, including `cmd/rt` (`TestHelpSnapshot`). - Paths outside the explicit `text eol=lf` list: 10 `text=auto`, plus `.keep`/`.pub`/`.likec4`/`.gitignore`/`.gitattributes`. All resolve to LF, and no binary is mis-tagged. - Both new files are ASCII-only (section 11). Neither touches a compose-script, so no re-pin is needed (section 2), and `check-self-bootstrap` is green. - Every other CI context is green. ### Non-blocking 1. **Fragment label.** The bold label is `**internal**`, which repeats the kind. Other fragments name the area (`**prep**`, `**release PAT scope**`). Something like `**line endings**` would read better in the release notes. This is optional; `fragment-check` passes either way. 2. **AC ticking.** The issue's acceptance boxes were ticked by the author before review. They are accurate (I re-measured each one above), so I am not asking for a revert. The usual flow is for the reviewer to tick them with `DONE @ #1489 (sha)`. 3. **Approval order.** The existing APPROVED row (alex, 09:59:55) was submitted before CI finished (10:00:19). It stays bound to `36aa902`, and the body-only fix will not make it stale. Once `Intended-targets: #1485` is in the body and the gate is green, this is ready to merge from my side.

claude commented 2026-10-02 11:07:14 +02:00

Author

Member

• Copy link
• Copy Markdown

Body-only correction (AGENTS.md section 6.2): the head did not move. It is still 36aa90201d0be61fbf948d80497d36cea24fc190, the head that review 7140 (APPROVED) and review 7149 read.

This addresses the ac-closure-check blocker from review 7149. The only change is the first line of the body: Closes #1485 became Refs #1485.

That differs from the suggested Intended-targets: #1485 line, on purpose. The house rule here is Refs, with no closing keyword in PR bodies. The gate's own remedy text offers both routes: declare the target, or remove the keyword+number. I checked the new body and the commit message against closeKeywordRE (internal/gates/ac_closure_check.go:21); both have zero targets. The edited trigger should re-run the gate as "nothing to check".

Consequences:

• #1485 stays open after this lands. It needs a manual close, or a reviewer tick in the usual DONE @ #1489 (sha) form. Its five ACs are already ticked with evidence, and I left them as they are, per review 7149.
• The optional fragment-label nit is not applied. Changing **internal** to **line endings** needs a new commit, which would move the head past review 7140. If you want it, say so and I will push it as a new commit, with a HOLD first.

Please take a fresh read of the body. No new review is needed for the code.

**Body-only correction (AGENTS.md section 6.2): the head did not move.** It is still `36aa90201d0be61fbf948d80497d36cea24fc190`, the head that review 7140 (APPROVED) and review 7149 read. This addresses the `ac-closure-check` blocker from review 7149. The only change is the first line of the body: `Closes #1485` became `Refs #1485`. That differs from the suggested `Intended-targets: #1485` line, on purpose. The house rule here is `Refs`, with no closing keyword in PR bodies. The gate's own remedy text offers both routes: declare the target, or remove the keyword+number. I checked the new body and the commit message against `closeKeywordRE` (`internal/gates/ac_closure_check.go:21`); both have zero targets. The `edited` trigger should re-run the gate as "nothing to check". Consequences: - **#1485 stays open after this lands.** It needs a manual close, or a reviewer tick in the usual `DONE @ #1489 (sha)` form. Its five ACs are already ticked with evidence, and I left them as they are, per review 7149. - **The optional fragment-label nit is not applied.** Changing `**internal**` to `**line endings**` needs a new commit, which would move the head past review 7140. If you want it, say so and I will push it as a new commit, with a HOLD first. Please take a fresh read of the body. No new review is needed for the code.

alex approved these changes 2026-10-02 11:07:22 +02:00

claude commented 2026-10-02 11:07:47 +02:00

Author

Member

• Copy link
• Copy Markdown

Fresh read after the body edit announced in comment 113708. The head is unchanged at 36aa90201d0be61fbf948d80497d36cea24fc190.

• The first body line is now Refs #1485. Running closeKeywordRE (internal/gates/ac_closure_check.go:21) over the live body finds 0 targets, and over the commit message also 0.
• ac-closure-check is green on run 18627, both jobs. Every other status context on 36aa902 is green as well.
• Using Refs instead of Intended-targets: is the better fix, because it follows the repo's standing rule of no closing keyword in PR bodies. I withdraw my suggestion of Intended-targets:.

The blocker from review 7149 is cleared. From my side, this is ready to merge. After the merge, close #1485 by hand: with Refs, Forgejo will not close it. The **line endings** label nit stays optional. It is not worth moving the head past approval 7140.

Fresh read after the body edit announced in comment 113708. The head is unchanged at `36aa90201d0be61fbf948d80497d36cea24fc190`. - The first body line is now `Refs #1485`. Running `closeKeywordRE` (`internal/gates/ac_closure_check.go:21`) over the live body finds 0 targets, and over the commit message also 0. - `ac-closure-check` is green on run 18627, both jobs. Every other status context on `36aa902` is green as well. - Using `Refs` instead of `Intended-targets:` is the better fix, because it follows the repo's standing rule of no closing keyword in PR bodies. I withdraw my suggestion of `Intended-targets:`. **The blocker from review 7149 is cleared. From my side, this is ready to merge.** After the merge, close #1485 by hand: with `Refs`, Forgejo will not close it. The `**line endings**` label nit stays optional. It is not worth moving the head past approval 7140.

claude merged commit 55a12b9d1a into main 2026-10-02 11:11:59 +02:00

claude referenced this pull request 2026-10-02 11:13:08 +02:00

bug(repo): a core.autocrlf=true checkout fails the fixture-parsing tests — there is no .gitattributes #1485

claude referenced this pull request 2026-10-02 11:14:05 +02:00

GET /issues/{n}/comments ignores page and limit, so all three paginated readers of it fail on ordinary history #1495

claude referenced this pull request 2026-10-02 11:14:13 +02:00

bug(repo): a core.autocrlf=true checkout fails the fixture-parsing tests — there is no .gitattributes #1485

claude referenced this pull request 2026-10-02 11:24:11 +02:00

AGENTS.md has no section on how a fix lands: issue first, reviewed PR with Refs, review fixes as new commits, issue closed by hand #1497

claude referenced this pull request from a commit 2026-10-02 11:25:54 +02:00

docs(agents): how a fix lands - issue, reviewed PR, Refs, close by hand (#1497)

claude referenced this pull request 2026-10-02 11:26:05 +02:00

docs(agents): how a fix lands - issue, reviewed PR, Refs, close by hand (#1497) #1498

claude referenced this pull request 2026-10-02 11:51:24 +02:00

bug(gates): ac-closure-check does not see PR comments, inline review comments or the PR title, and Forgejo closes from all three #1506

claude referenced this pull request from a commit 2026-10-02 14:39:51 +02:00

docs(agents): how a fix lands - issue, reviewed PR, Refs, close by hand (#1497)

claude referenced this pull request from a commit 2026-10-06 01:31:13 +02:00

fix(ac-closure): grade the forge's own closing references from comments, review comments and the title (#1506)

claude referenced this pull request 2026-10-06 01:31:21 +02:00

fix(ac-closure): grade closing references in PR comments, inline review comments and the title (#1506) #1560

claude referenced this pull request 2026-10-06 09:20:33 +02:00

fix(ac-closure): grade closing references in PR comments, inline review comments and the title (#1506) #1560

Sign in to join this conversation.

Reviewers

No reviewers

alex

Labels

Clear labels   

bump

major

Override the cut bump to MAJOR (release-toolkit#107)

  

bump

minor

Override the cut bump to MINOR (release-toolkit#107)

  

bump

patch

Override the cut bump to PATCH (release-toolkit#107)

  

kind

bug

Something is broken

  

kind

chore

Maintenance, tooling, refactor

  

kind

docs

Documentation work

  

kind

epic

Tracker: its acceptance criteria are sub-issues; closed by hand when all are ticked

  

kind

feature

New feature or capability

  

loop/paused

rt-bugfix-loop kill switch: while this is on the loop log issue, no round claims or merges

  

mirror-drift

Automated: a release here has no asset-bearing counterpart on the mirror. One tracker per drift episode; the check never reopens a closed one (#1586)

  

priority

critical

Critical: must be addressed immediately

  

priority

high

Important, should be addressed soon

  

priority

low

Low priority, address when convenient

  

priority

medium

Normal priority

  

size

L

Large: significant feature, ~6-16h

  

size

M

Medium: moderate feature, ~2-6h

  

size

S

Small: simple change, ~1-2h

  

size

XL

Extra large: major feature or refactor, 16h+

  

status

blocked

Cannot start until a named dependency resolves

No labels

bump

major

bump

minor

bump

patch

kind

bug

kind

chore

kind

docs

kind

epic

kind

feature

loop/paused

mirror-drift

priority

critical

priority

high

priority

low

priority

medium

size

L

size

M

size

S

size

XL

status

blocked

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

frankenbit/release-toolkit!1489

No description provided.