fix(windows): version_files guard, recover's absolute changelog path and read-only renames (#1526) #1527

Merged

claude merged 5 commits from claude/1526-windows-path-gaps into main 2026-10-03 00:50:14 +02:00

Conversation 6 Commits 5 Files changed 11 +306 -9

claude commented 2026-10-02 23:41:27 +02:00

Member

• Copy link
• Copy Markdown

Refs #1526

#1505 has landed (96d2646), so this branches from main, rebased onto dd871e8. It covers the three Windows path gaps that #1502 left out of scope. Each was measured natively before any change; the measurements are in #1526.

What changes

Gap	Commit	Change	Why Linux is unchanged
1. version_files guard (#181)	2fe9db5, 81086ff	Load runs the existing checks, the basename check included, on filepath.ToSlash(vf), and on Windows also requires filepath.IsLocal(vf)	ToSlash is the identity there; the IsLocal check sits behind runtime.GOOS == "windows"
2. resolveUnderRoot	b46db1d	filepath.IsAbs(p) replaces strings.HasPrefix(p, "/")	on unix IsAbs is HasPrefix(path, "/") (Go 1.27.1 source, internal/filepathlite/path_unix.go)
3. rename over a read-only file	5ca2973	new internal/replacefile.Rename, used by writeFileAtomic, writeRecoveryFileAtomic and the manifest store	it returns os.Rename's result unchanged off Windows

#1502's list did not name the manifest store (internal/manifest/store.go). It has the same rename and fails the same way (measured in #1526).

Decisions

Gap 3: clear the attribute, refuse, or leave it

• Clear the attribute and retry is right when the read-only bit is incidental, not a protection the toolkit must honour. The evidence points that way:

  • git does not record the attribute, so no checkout produces it;
  • on Linux, where the release workflow runs, the same writers already replace a 0444 destination;
  • Go's own Windows os.Remove clears the attribute and retries (src/os/file_windows.go, func Remove). That is why the writers' deferred temp cleanup already works.

  Cost: on Windows, rt overrides an operator's attrib +R, exactly as it overrides chmod 0444 on Linux.

• Refuse with a message naming the attribute is right when the bit is a lock the toolkit should respect. Cost: Windows and Linux then disagree on the same tree. Making them agree means refusing on Linux too, which changes Linux behaviour. The cut would also refuse at the seal write, after earlier steps have run, unless a preflight were added.

• Leave it is right if the case is too rare to carry code. Cost: the error names a temp file and "Access is denied", not the cause.

Chosen: clear and retry, on Windows only.

• The retry runs only when os.Rename failed and the destination is a read-only regular file.
• It clears the attribute and renames once more. If that rename fails too, it puts the attribute back (TestRename_aFailedRetryKeepsTheDestinationReadOnly).
• The destination ends with the mode of the file moved onto it, as on Linux:
  • the cut and the recovery copy the old mode onto their temp file first, so a read-only CHANGELOG stays read-only;
  • the manifest store writes its temp at 0644, so the manifest ends writable.

If you read the bit as a lock, the refusal is a small change inside replacefile.Rename. It needs a decision for Linux as well.

Gap 1: ToSlash and IsLocal, not either alone

• IsLocal alone accepts x\..\y/VERSION. That stays inside the repo, but #181 rejects every .. segment, so Linux rejects the same path written as x/../y/VERSION.
• ToSlash alone misses C:\x/VERSION and C:x/VERSION, because neither starts with /. manifest-check passes vf straight to ReadVersionFile, so a drive path is read as given.
• Both together apply #181's rule on Windows with \ read as the separator it is. IsLocal adds what has no Linux spelling: a drive letter or a reserved device name.

The .. and rooted cases keep their existing messages. Only the drive, colon or device case gets a new one, not a local path on Windows.

The basename (review 7207, N1; 81086ff) is also taken from the slash form. It is then the basename that prep's filepath.Base dispatches on.

• Before: Windows refused sub\VERSION as an unsupported basename but accepted sub\dir/VERSION.
• Now: Windows accepts sub\VERSION, and sub\pom.xml reports unsupported version_files basename: pom.xml.
• On Linux both entries are rejected as before, with the whole sub\VERSION or sub\pom.xml as the basename.

So a config that writes sub\VERSION validates on a Windows workstation and fails in the Linux workflow. Writing / works on both.

Rejecting \ on every OS would make a config portable, but it changes what Linux accepts. #1526 lists it as out of scope.

Gap 2: filepath.IsAbs, keeping root + "/" + p

decide.resolve joins with filepath.Join, which also cleans the path. Switching to it would change the path in recover-pending-cut's error messages on Linux for an input like ./CHANGELOG.md, so only the absolute test changes. On Windows, C:\repo/CHANGELOG.md is a valid path with mixed separators.

Verification

Measured 2026-10-02. Windows: Go 1.27.1 windows/amd64, native, from Git Bash, LF clone. Linux: go1.25.14 in golang:1.25, on a real clone.

Red first. Each new test failed natively on Windows against the unfixed code:

• the six rejection cases of TestLoad_versionFilesBackslashPaths (the local-path control passed);
• TestResolveUnderRootLeavesAnAbsolutePathAlone;
• the three writer tests, each with Access is denied.;
• the two basename cases (sub\VERSION, sub\pom.xml), against 5ca2973.

Suites. On 8086700, go vet ./... is clean and go test -count=1 ./... passes 30 of 30 packages, natively on Windows and in golang:1.25.

Mutation. Run on 8086700, one mutant at a time, each restored from HEAD:

Mutant	Test	Windows	Linux
M1a: grade vf, not ToSlash(vf)	TestLoad_versionFilesBackslashPaths	red	green (equivalent)
M1b: drop the Windows IsLocal check	same	red	green (equivalent)
M1c: take the basename of vf, not of the slash form	same	red	green (equivalent)
M2a: back to HasPrefix(p, "/")	TestResolveUnderRootLeavesAnAbsolutePathAlone	red	green (equivalent)
M3a: Rename is plain os.Rename	TestRename_replacesAReadOnlyDestination	red	green (equivalent)
M3b: no put-back after a failed retry	TestRename_aFailedRetryKeepsTheDestinationReadOnly	red	skipped (Windows only)
M3c: a non-read-only failure returns nil	TestRename_otherFailuresPassThrough	red	green (line not reached)
M3d/e/f: each writer back to os.Rename	that writer's read-only test	red, all three	green (equivalent)

Every Linux survivor is equivalent by construction. The mutated line is the identity there (ToSlash), sits behind runtime.GOOS == "windows", or is the same function (IsAbs). On Linux the new tests pin the existing behaviour and pass before and after.

End to end on Windows. On #1526's fixture (version_files: ['..\outside/VERSION']), native rt at 8086700 refuses in both manifest-check and prep --dry-run (exit 1 each):

config validation failed: config: schema violation: version_files path contains directory traversal (..): ..\outside/VERSION

Both outside VERSION files keep their content (9.9.9 and 7.7.7). Before the change, the first was read and the second overwritten.

Linux byte-identity. rt was built at dd871e8 and at 8086700 and run on the same fixtures. Stdout, stderr and exit code were compared after normalising random temp-file names. All 51 runs are identical:

• --help;
• manifest-check, prep --dry-run and recover-pending-cut --dry-run, each for 15 version_files entries: VERSION, /etc/VERSION, ../VERSION, sub/../VERSION, ..\outside/VERSION, sub\..\..\outside/VERSION, \outside/VERSION, \\host\share/VERSION, C:\outside/VERSION, C:outside/VERSION, C:/outside/VERSION, sub\dir/VERSION, pom.xml, sub\VERSION and sub\pom.xml;
• the same three verbs with an absolute changelog:;
• prep --dry-run and recover-pending-cut --dry-run with a 0444 CHANGELOG.

What those runs do not cover:

• recover-pending-cut stopped at "this is not the state recovery (B) repairs", since no fixture holds a pending cut, so its changelog read and write never ran;
• no run reached the cut's seal or a manifest write.

Those lines are identical on Linux by construction, as above, and the unit tests pin them in golang:1.25.

Self-bootstrap (AGENTS.md section 2.1)

No re-pin: toolkit-self floats @main. rt check-self-bootstrap on 8086700 reports floating-pin (release-toolkit#456): main matches main by construction, result pass. The change is Go only, and adopters get it from the next cut tag.

Observed, not changed

rt prep joins changelog: under the repo root unconditionally (cmd/rt/prep.go:188). An absolute changelog: therefore fails prep with CHANGELOG file not found, on Linux too, while decide and now recover-pending-cut accept it. That predates this PR and holds on every OS. It is filed as #1528, measured per verb, and is not addressed here.

Review

@alex, please verify at source (AGENTS.md section 6):

1. that clear-and-retry is the gap-3 branch you want, rather than a refusal;
2. that replacefile.Rename returns os.Rename's result unchanged off Windows (internal/replacefile/replacefile.go);
3. that the Windows rejection set in TestLoad_versionFilesBackslashPaths matches what you would call "not repo-relative".

🤖 Generated with Claude Code

Refs #1526 #1505 has landed (`96d2646`), so this branches from `main`, rebased onto `dd871e8`. It covers the three Windows path gaps that #1502 left out of scope. Each was measured natively before any change; the measurements are in #1526. ## What changes | Gap | Commit | Change | Why Linux is unchanged | |---|---|---|---| | 1. `version_files` guard (#181) | `2fe9db5`, `81086ff` | `Load` runs the existing checks, the basename check included, on `filepath.ToSlash(vf)`, and on Windows also requires `filepath.IsLocal(vf)` | `ToSlash` is the identity there; the `IsLocal` check sits behind `runtime.GOOS == "windows"` | | 2. `resolveUnderRoot` | `b46db1d` | `filepath.IsAbs(p)` replaces `strings.HasPrefix(p, "/")` | on unix `IsAbs` is `HasPrefix(path, "/")` (Go 1.27.1 source, `internal/filepathlite/path_unix.go`) | | 3. rename over a read-only file | `5ca2973` | new `internal/replacefile.Rename`, used by `writeFileAtomic`, `writeRecoveryFileAtomic` and the manifest store | it returns `os.Rename`'s result unchanged off Windows | #1502's list did not name the manifest store (`internal/manifest/store.go`). It has the same rename and fails the same way (measured in #1526). ## Decisions ### Gap 3: clear the attribute, refuse, or leave it - **Clear the attribute and retry** is right when the read-only bit is incidental, not a protection the toolkit must honour. The evidence points that way: - git does not record the attribute, so no checkout produces it; - on Linux, where the release workflow runs, the same writers already replace a `0444` destination; - Go's own Windows `os.Remove` clears the attribute and retries (`src/os/file_windows.go`, `func Remove`). That is why the writers' deferred temp cleanup already works. Cost: on Windows, rt overrides an operator's `attrib +R`, exactly as it overrides `chmod 0444` on Linux. - **Refuse with a message naming the attribute** is right when the bit is a lock the toolkit should respect. Cost: Windows and Linux then disagree on the same tree. Making them agree means refusing on Linux too, which changes Linux behaviour. The cut would also refuse at the seal write, after earlier steps have run, unless a preflight were added. - **Leave it** is right if the case is too rare to carry code. Cost: the error names a temp file and "Access is denied", not the cause. **Chosen: clear and retry, on Windows only.** - The retry runs only when `os.Rename` failed and the destination is a read-only regular file. - It clears the attribute and renames once more. If that rename fails too, it puts the attribute back (`TestRename_aFailedRetryKeepsTheDestinationReadOnly`). - The destination ends with the mode of the file moved onto it, as on Linux: - the cut and the recovery copy the old mode onto their temp file first, so a read-only CHANGELOG stays read-only; - the manifest store writes its temp at `0644`, so the manifest ends writable. If you read the bit as a lock, the refusal is a small change inside `replacefile.Rename`. It needs a decision for Linux as well. ### Gap 1: `ToSlash` and `IsLocal`, not either alone - **`IsLocal` alone** accepts `x\..\y/VERSION`. That stays inside the repo, but #181 rejects every `..` segment, so Linux rejects the same path written as `x/../y/VERSION`. - **`ToSlash` alone** misses `C:\x/VERSION` and `C:x/VERSION`, because neither starts with `/`. `manifest-check` passes `vf` straight to `ReadVersionFile`, so a drive path is read as given. - **Both together** apply #181's rule on Windows with `\` read as the separator it is. `IsLocal` adds what has no Linux spelling: a drive letter or a reserved device name. The `..` and rooted cases keep their existing messages. Only the drive, colon or device case gets a new one, `not a local path on Windows`. **The basename** (review 7207, N1; `81086ff`) is also taken from the slash form. It is then the basename that prep's `filepath.Base` dispatches on. - Before: Windows refused `sub\VERSION` as an unsupported basename but accepted `sub\dir/VERSION`. - Now: Windows accepts `sub\VERSION`, and `sub\pom.xml` reports `unsupported version_files basename: pom.xml`. - On Linux both entries are rejected as before, with the whole `sub\VERSION` or `sub\pom.xml` as the basename. So a config that writes `sub\VERSION` validates on a Windows workstation and fails in the Linux workflow. Writing `/` works on both. Rejecting `\` on every OS would make a config portable, but it changes what Linux accepts. #1526 lists it as out of scope. ### Gap 2: `filepath.IsAbs`, keeping `root + "/" + p` `decide.resolve` joins with `filepath.Join`, which also cleans the path. Switching to it would change the path in `recover-pending-cut`'s error messages on Linux for an input like `./CHANGELOG.md`, so only the absolute test changes. On Windows, `C:\repo/CHANGELOG.md` is a valid path with mixed separators. ## Verification Measured 2026-10-02. Windows: Go 1.27.1 windows/amd64, native, from Git Bash, LF clone. Linux: go1.25.14 in `golang:1.25`, on a real clone. **Red first.** Each new test failed natively on Windows against the unfixed code: - the six rejection cases of `TestLoad_versionFilesBackslashPaths` (the local-path control passed); - `TestResolveUnderRootLeavesAnAbsolutePathAlone`; - the three writer tests, each with `Access is denied.`; - the two basename cases (`sub\VERSION`, `sub\pom.xml`), against `5ca2973`. **Suites.** On `8086700`, `go vet ./...` is clean and `go test -count=1 ./...` passes 30 of 30 packages, natively on Windows and in `golang:1.25`. **Mutation.** Run on `8086700`, one mutant at a time, each restored from HEAD: | Mutant | Test | Windows | Linux | |---|---|---|---| | M1a: grade `vf`, not `ToSlash(vf)` | `TestLoad_versionFilesBackslashPaths` | red | green (equivalent) | | M1b: drop the Windows `IsLocal` check | same | red | green (equivalent) | | M1c: take the basename of `vf`, not of the slash form | same | red | green (equivalent) | | M2a: back to `HasPrefix(p, "/")` | `TestResolveUnderRootLeavesAnAbsolutePathAlone` | red | green (equivalent) | | M3a: `Rename` is plain `os.Rename` | `TestRename_replacesAReadOnlyDestination` | red | green (equivalent) | | M3b: no put-back after a failed retry | `TestRename_aFailedRetryKeepsTheDestinationReadOnly` | red | skipped (Windows only) | | M3c: a non-read-only failure returns nil | `TestRename_otherFailuresPassThrough` | red | green (line not reached) | | M3d/e/f: each writer back to `os.Rename` | that writer's read-only test | red, all three | green (equivalent) | Every Linux survivor is equivalent by construction. The mutated line is the identity there (`ToSlash`), sits behind `runtime.GOOS == "windows"`, or is the same function (`IsAbs`). On Linux the new tests pin the existing behaviour and pass before and after. **End to end on Windows.** On #1526's fixture (`version_files: ['..\outside/VERSION']`), native `rt` at `8086700` refuses in both `manifest-check` and `prep --dry-run` (exit 1 each): ``` config validation failed: config: schema violation: version_files path contains directory traversal (..): ..\outside/VERSION ``` Both outside `VERSION` files keep their content (`9.9.9` and `7.7.7`). Before the change, the first was read and the second overwritten. **Linux byte-identity.** `rt` was built at `dd871e8` and at `8086700` and run on the same fixtures. Stdout, stderr and exit code were compared after normalising random temp-file names. All 51 runs are identical: - `--help`; - `manifest-check`, `prep --dry-run` and `recover-pending-cut --dry-run`, each for 15 `version_files` entries: `VERSION`, `/etc/VERSION`, `../VERSION`, `sub/../VERSION`, `..\outside/VERSION`, `sub\..\..\outside/VERSION`, `\outside/VERSION`, `\\host\share/VERSION`, `C:\outside/VERSION`, `C:outside/VERSION`, `C:/outside/VERSION`, `sub\dir/VERSION`, `pom.xml`, `sub\VERSION` and `sub\pom.xml`; - the same three verbs with an absolute `changelog:`; - `prep --dry-run` and `recover-pending-cut --dry-run` with a `0444` CHANGELOG. What those runs do not cover: - `recover-pending-cut` stopped at "this is not the state recovery (B) repairs", since no fixture holds a pending cut, so its changelog read and write never ran; - no run reached the cut's seal or a manifest write. Those lines are identical on Linux by construction, as above, and the unit tests pin them in `golang:1.25`. ## Self-bootstrap (AGENTS.md section 2.1) No re-pin: toolkit-self floats `@main`. `rt check-self-bootstrap` on `8086700` reports `floating-pin (release-toolkit#456): main matches main by construction`, result `pass`. The change is Go only, and adopters get it from the next cut tag. ## Observed, not changed `rt prep` joins `changelog:` under the repo root unconditionally (`cmd/rt/prep.go:188`). An absolute `changelog:` therefore fails prep with `CHANGELOG file not found`, on Linux too, while `decide` and now `recover-pending-cut` accept it. That predates this PR and holds on every OS. It is filed as #1528, measured per verb, and is not addressed here. ## Review @alex, please verify at source (AGENTS.md section 6): 1. that clear-and-retry is the gap-3 branch you want, rather than a refusal; 2. that `replacefile.Rename` returns `os.Rename`'s result unchanged off Windows (`internal/replacefile/replacefile.go`); 3. that the Windows rejection set in `TestLoad_versionFilesBackslashPaths` matches what you would call "not repo-relative". 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude added 3 commits 2026-10-02 23:41:27 +02:00

fix(config): read '\' as a separator in the version_files guard on Windows (#1526) ... 2fe9db5b53

The #181 guard tested for a leading '/' and for '..' between '/'
separators. On Windows '\' is a separator too, so '..\outside/VERSION'
passed validation and prep bumped a VERSION file outside the
repository. Grade the slash form, and on Windows also require
filepath.IsLocal, which refuses a drive letter. ToSlash is the identity
off Windows, so Linux accepts and rejects exactly what it did.

Refs #1526

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

fix(recover): take a Windows-absolute changelog path as given (#1526) ... b46db1d377

resolveUnderRoot promised decide.resolve's rule but treated only a
leading '/' as absolute, so on Windows an absolute changelog path was
joined under the repo root into a name no file can have. decide read
the right file and recover-pending-cut could not. Use filepath.IsAbs,
which is that same leading-'/' test on Linux.

Refs #1526

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

fix(release): replace a read-only file on Windows as rename does on Linux (#1526) ... 5ca2973f4b

The cut's CHANGELOG seal, recover-pending-cut and the manifest store
each rename a temp file over their destination. Linux replaces a
read-only destination; Windows refuses with "Access is denied." New
replacefile.Rename clears the attribute and renames once more on
Windows only, the way os.Remove already retries a delete there, and
puts the attribute back if that fails too. Off Windows it returns
os.Rename's result unchanged.

Refs #1526

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

claude requested review from alex 2026-10-02 23:41:27 +02:00

claude reviewed 2026-10-02 23:53:33 +02:00

claude left a comment

• Copy link
• Copy Markdown

Verdict: APPROVE at 5ca2973f4b. Two non-blocking nits are below. This is posted as a COMMENT because the forge identity is claude, the PR author; @alex has to click Approve.

Verified at source

• Diff read in full (11 files, 3 commits on dd871e8). rt review-submit-check returned rc=0 for this head.
• Native Windows (Go 1.27.1, Git Bash, LF clone at 5ca2973): go vet ./... is clean, and go test -count=1 ./... passes 30/30 packages, internal/replacefile included.
• CI: the combined status on the head is success, all 31 contexts.
• Gap 2: resolveUnderRoot now applies the same test as decide.resolve (internal/decide/decide.go:1362, filepath.IsAbs). The join stays root + "/" + p, as the PR says.
• Gap 3: no os.Rename call is left in non-test code outside replacefile, so all three writers are covered.

Your three questions

1. Clear-and-retry vs. refuse. That is alex's call. I agree with clear-and-retry: it gives the Linux outcome and does what Go's own os.Remove already does on Windows. The put-back after a failed retry is the right guard, and TestRename_aFailedRetryKeepsTheDestinationReadOnly pins it.
2. Off Windows, Rename returns os.Rename's result unchanged. Confirmed. if err == nil || runtime.GOOS != "windows" { return err } runs before anything else.
3. The Windows rejection set. It matches. I ran an extra probe natively, not committed:

   Entry	Result on Windows
   a:b/VERSION	rejected, not a local path
   NUL/VERSION	rejected, not a local path
   sub/NUL/VERSION	rejected, not a local path
   sub\..\VERSION	rejected, directory traversal

Nits (non-blocking)

N1. The basename check still splits on / only. internal/config/config.go:211 grades path.Base(vf), but prep dispatches on filepath.Base (internal/prep/versionfiles.go:54, :69, :109, :124, :130). Measured natively on Windows:

Entry	Result
sub\dir/VERSION	accepted
sub\VERSION	unsupported version_files basename: sub\VERSION
.\VERSION	unsupported version_files basename: .\VERSION

This fails closed, so it is not a #181 hole, and Linux rejects the same bytes too. But it doesn't fully match the PR's framing ("#181's rule on Windows with \ read as the separator it is"), and on Windows config and prep now disagree about what the basename is. path.Base(slashed) would fix it in one line, and it is the identity on Linux. Fine as a follow-up.

N2. Typo in the doc comment at internal/replacefile/replacefile.go: "The destination ends with the mode from carries" probably means "the mode from carries".

Neither changes the verdict.

Verdict: APPROVE at 5ca2973f4b8d6ea35fa1c4ed1740bd18f90678fb. Two non-blocking nits are below. This is posted as a COMMENT because the forge identity is `claude`, the PR author; @alex has to click Approve. ## Verified at source - **Diff read in full** (11 files, 3 commits on `dd871e8`). `rt review-submit-check` returned rc=0 for this head. - **Native Windows** (Go 1.27.1, Git Bash, LF clone at `5ca2973`): `go vet ./...` is clean, and `go test -count=1 ./...` passes 30/30 packages, `internal/replacefile` included. - **CI**: the combined status on the head is `success`, all 31 contexts. - **Gap 2**: `resolveUnderRoot` now applies the same test as `decide.resolve` (`internal/decide/decide.go:1362`, `filepath.IsAbs`). The join stays `root + "/" + p`, as the PR says. - **Gap 3**: no `os.Rename` call is left in non-test code outside `replacefile`, so all three writers are covered. ## Your three questions 1. **Clear-and-retry vs. refuse.** That is alex's call. I agree with clear-and-retry: it gives the Linux outcome and does what Go's own `os.Remove` already does on Windows. The put-back after a failed retry is the right guard, and `TestRename_aFailedRetryKeepsTheDestinationReadOnly` pins it. 2. **Off Windows, `Rename` returns `os.Rename`'s result unchanged.** Confirmed. `if err == nil || runtime.GOOS != "windows" { return err }` runs before anything else. 3. **The Windows rejection set.** It matches. I ran an extra probe natively, not committed: | Entry | Result on Windows | |---|---| | `a:b/VERSION` | rejected, `not a local path` | | `NUL/VERSION` | rejected, `not a local path` | | `sub/NUL/VERSION` | rejected, `not a local path` | | `sub\..\VERSION` | rejected, `directory traversal` | ## Nits (non-blocking) **N1. The basename check still splits on `/` only.** `internal/config/config.go:211` grades `path.Base(vf)`, but prep dispatches on `filepath.Base` (`internal/prep/versionfiles.go:54`, `:69`, `:109`, `:124`, `:130`). Measured natively on Windows: | Entry | Result | |---|---| | `sub\dir/VERSION` | accepted | | `sub\VERSION` | `unsupported version_files basename: sub\VERSION` | | `.\VERSION` | `unsupported version_files basename: .\VERSION` | This fails closed, so it is not a #181 hole, and Linux rejects the same bytes too. But it doesn't fully match the PR's framing ("#181's rule on Windows with `\` read as the separator it is"), and on Windows config and prep now disagree about what the basename is. `path.Base(slashed)` would fix it in one line, and it is the identity on Linux. Fine as a follow-up. **N2. Typo in the doc comment** at `internal/replacefile/replacefile.go`: "The destination ends with the mode from carries" probably means "the mode `from` carries". Neither changes the verdict.

claude commented 2026-10-02 23:56:38 +02:00

Author

Member

• Copy link
• Copy Markdown

HOLD: alex asked for both nits from review 7207 (N1, the basename graded after a backslash on Windows; N2, the doc comment). They land as new commits on top of 5ca2973f4b8d6ea35fa1c4ed1740bd18f90678fb, with no amend and no force-push. Please do not approve until I post the new head SHA here.

HOLD: alex asked for both nits from review 7207 (N1, the basename graded after a backslash on Windows; N2, the doc comment). They land as new commits on top of `5ca2973f4b8d6ea35fa1c4ed1740bd18f90678fb`, with no amend and no force-push. Please do not approve until I post the new head SHA here.

claude added 2 commits 2026-10-03 00:01:33 +02:00

fix(config): grade the version_files basename after the last '\' on Windows (#1526) ... 81086fff6d

Review 7207, N1. Load graded path.Base(vf), but prep dispatches on
filepath.Base. On Windows 'sub\VERSION' was therefore refused as an
unsupported basename while 'sub\dir/VERSION' was accepted. Grade the
basename of the slash form, which is the identity off Windows, so Linux
still reads 'sub\VERSION' as one unsupported name.

Refs #1526

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

docs(replacefile): mark the parameter name in Rename's doc comment (#1526) ... 8086700b1a

Review 7207, N2: "the mode from carries" read as a typo. It names the
parameter, so set it in backticks.

Refs #1526

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

claude referenced this pull request 2026-10-03 00:04:08 +02:00

bug(config): an absolute changelog: is read as given by decide and manifest-check, but prep and release join it under the repo root #1528

claude commented 2026-10-03 00:04:35 +02:00

Author

Member

• Copy link
• Copy Markdown

HOLD released. New head: 8086700b1a83ec766f58a29579af79158ccb3729, two new commits on top of the reviewed 5ca2973f4b8d6ea35fa1c4ed1740bd18f90678fb:

• 81086ff, N1: the basename is graded on the slash form. On Windows sub\VERSION is now accepted and sub\pom.xml reports pom.xml; on Linux both are rejected exactly as before. Two new test cases fail on Windows against 5ca2973, and a new mutant M1c is killed there.
• 8086700, N2: from in backticks in the replacefile.Rename doc comment.

Re-measured on 8086700: 30/30 packages natively on Windows and in golang:1.25; all 10 mutants killed on Windows (equivalent on Linux); 51/51 Linux verb runs byte-identical to dd871e8. The body is updated to match, and it links #1528, the absolute-changelog finding.

Review 7207's verdict was given at 5ca2973 and does not carry over. Please re-read and pin with rt review-submit-check --reviewed-head 8086700b1a83ec766f58a29579af79158ccb3729.

HOLD released. New head: `8086700b1a83ec766f58a29579af79158ccb3729`, two new commits on top of the reviewed `5ca2973f4b8d6ea35fa1c4ed1740bd18f90678fb`: - `81086ff`, N1: the basename is graded on the slash form. On Windows `sub\VERSION` is now accepted and `sub\pom.xml` reports `pom.xml`; on Linux both are rejected exactly as before. Two new test cases fail on Windows against `5ca2973`, and a new mutant M1c is killed there. - `8086700`, N2: `from` in backticks in the `replacefile.Rename` doc comment. Re-measured on `8086700`: 30/30 packages natively on Windows and in `golang:1.25`; all 10 mutants killed on Windows (equivalent on Linux); 51/51 Linux verb runs byte-identical to `dd871e8`. The body is updated to match, and it links #1528, the absolute-changelog finding. Review 7207's verdict was given at `5ca2973` and does not carry over. Please re-read and pin with `rt review-submit-check --reviewed-head 8086700b1a83ec766f58a29579af79158ccb3729`.

claude reviewed 2026-10-03 00:07:20 +02:00

claude left a comment

• Copy link
• Copy Markdown

Verdict: APPROVE at 8086700b1a. This replaces review 7207, which was given at 5ca2973. It is posted as a COMMENT because the forge identity is claude, the PR author; @alex has to click Approve.

What I re-read

• Delta 5ca2973..8086700: two new commits on top of the reviewed head, with no rewrite (5ca2973 is an ancestor). The fork point is still dd871e8, which is still main's tip, so no rebase is needed.
  • N1, 81086ff: the basename is now path.Base(slashed). That agrees with prep's filepath.Base dispatch on Windows and is the identity on Linux. Two cases were added: sub\VERSION and sub\pom.xml. The table now pins the non-Windows message for each as well.
  • N2, 8086700: the doc comment only.
• HOLD: the HOLD (114472) and its release (114483) are in the thread. Neither carries a close keyword before an issue number.

Measured

• Native Windows (Go 1.27.1, Git Bash, LF clone at 8086700): go vet ./... is clean, and go test -count=1 ./... passes 30/30 packages.
• Mutant M1c: with the check reverted to path.Base(vf), both new cases fail on Windows:
  • sub\VERSION is rejected when it should be accepted;
  • sub\pom.xml reports sub\pom.xml instead of pom.xml.
    The mutant was killed, and the clone was restored from HEAD.
• CI: the combined status on 8086700 is success.
• Head pin: rt review-submit-check --reviewed-head 8086700b1a83ec766f58a29579af79158ccb3729 returned rc=0.

Both nits from review 7207 are resolved. Nothing new is open.

Verdict: APPROVE at 8086700b1a83ec766f58a29579af79158ccb3729. This replaces review 7207, which was given at `5ca2973`. It is posted as a COMMENT because the forge identity is `claude`, the PR author; @alex has to click Approve. ## What I re-read - **Delta `5ca2973..8086700`**: two new commits on top of the reviewed head, with no rewrite (`5ca2973` is an ancestor). The fork point is still `dd871e8`, which is still main's tip, so no rebase is needed. - **N1, `81086ff`**: the basename is now `path.Base(slashed)`. That agrees with prep's `filepath.Base` dispatch on Windows and is the identity on Linux. Two cases were added: `sub\VERSION` and `sub\pom.xml`. The table now pins the non-Windows message for each as well. - **N2, `8086700`**: the doc comment only. - **HOLD**: the HOLD (114472) and its release (114483) are in the thread. Neither carries a close keyword before an issue number. ## Measured - **Native Windows** (Go 1.27.1, Git Bash, LF clone at `8086700`): `go vet ./...` is clean, and `go test -count=1 ./...` passes 30/30 packages. - **Mutant M1c**: with the check reverted to `path.Base(vf)`, both new cases fail on Windows: - `sub\VERSION` is rejected when it should be accepted; - `sub\pom.xml` reports `sub\pom.xml` instead of `pom.xml`. The mutant was killed, and the clone was restored from HEAD. - **CI**: the combined status on `8086700` is `success`. - **Head pin**: `rt review-submit-check --reviewed-head 8086700b1a83ec766f58a29579af79158ccb3729` returned rc=0. Both nits from review 7207 are resolved. Nothing new is open.

alex approved these changes 2026-10-03 00:40:18 +02:00

claude merged commit 8086700b1a into main 2026-10-03 00:50:14 +02:00

claude deleted branch claude/1526-windows-path-gaps 2026-10-03 00:50:14 +02:00

forgejo-actions commented 2026-10-03 00:51:03 +02:00

• Copy link
• Copy Markdown

Landing identity record

• PR: #1527
• landed commit (server merge_commit_sha): 8086700b1a83ec766f58a29579af79158ccb3729
• effective official approval(s):
  • @alex, review #7209, stamped commit: 8086700b1a83ec766f58a29579af79158ccb3729
• replay comparison: no identity change (stamped SHA equals landed SHA)

This is a post-merge identity record. It does not retroactively review the landed object; it records whether the server landed the object that an official approval named.

<!-- release-toolkit:landing-review-record-v1 landed=8086700b1a83ec766f58a29579af79158ccb3729 --> ## Landing identity record - PR: #1527 - landed commit (server merge_commit_sha): `8086700b1a83ec766f58a29579af79158ccb3729` - effective official approval(s): - @alex, review #7209, stamped commit: `8086700b1a83ec766f58a29579af79158ccb3729` - replay comparison: no identity change (stamped SHA equals landed SHA) This is a post-merge identity record. It does not retroactively review the landed object; it records whether the server landed the object that an official approval named.

claude referenced this pull request 2026-10-03 00:51:09 +02:00

bug(windows): three path gaps - the #181 version_files guard misses ..\ paths, recover-pending-cut joins an absolute changelog, an atomic write fails over a read-only file #1526

claude referenced this pull request 2026-10-03 00:51:25 +02:00

bug(windows): three path gaps - the #181 version_files guard misses ..\ paths, recover-pending-cut joins an absolute changelog, an atomic write fails over a read-only file #1526

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

3 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!1527

No description provided.