Fleet review (glm)

Lens: requirements + adversarial. The diff satisfies issue #370's expected direction: the Stat_t-based identity read is behind a build-tagged directoryIdentity function, the unix variant preserves the previous behavior exactly (same Dev/Ino read, same Ino==0 refusal), and the Windows variant opens the directory with FILE_FLAG_BACKUP_SEMANTICS|FILE_FLAG_OPEN_REPARSE_POINT and refuses non-directories, reparse points, zero file index, and any handle whose stat does not pass os.SameFile against the caller's Lstat result, so divergence between path and Lstat fails closed. Verified at head 62ad16b: the syscall import in directories.go is still used (EINVAL/ENOTSUP at line 414), so the import-hunk absence in the diff is not a build break. The expected-files count in tests/test_community_policy.py is updated 406->409, exactly the three new files. No blocking findings found: every Windows-only edge I attacked (replaced path, stale Lstat, reparse point, long-path open failure) fails closed to an 'identity unavailable'/'directory changed' error rather than wrong behavior. Three non-blocking items: the ReFS 128-bit file-ID limitation the author already disclosed (the persisted AcceptedDirectory shape is uint64, so a low-64-bit collision on ReFS could make a replacement directory look accepted), the scope narrowing versus the test-pass expectation recorded in #370's plan, and long paths (>260 chars) failing the Windows identity open without an extended-length prefix. Windows filesystem behavior is explicitly not verified yet, so none of these gate this build fix.

Suggestion (2)

• ReFS 128-bit file IDs can collide in the 64-bit index persisted as identity (internal/filetxn/identity_windows.go:48): GetFileInformationByHandle exposes only the low 64 bits of the file ID. On ReFS, file IDs are 128-bit and Microsoft does not guarantee the 64-bit index is unique per volume. Evidence: the PR body itself discloses 'A Windows ReFS volume can use 128-bit file IDs, and GetFileInformationByHandle returns the 64-bit index only. NTFS is fully covered.', and the persisted AcceptedDirectory.Device/Inode shape is uint64 so it cannot hold 128 bits. This identity is the guard that rollback/restart uses to accept a directory, so a collision would make a replacement directory indistinguishable from the accepted one.
• Paths longer than 260 chars fail the Windows identity open (internal/filetxn/identity_windows.go:17): syscall.CreateFile is called with UTF16PtrFromString(path) and no extended-length \?\ prefix, so on a default Windows configuration CreateFile fails with ERROR_PATH_NOT_FOUND for paths longer than MAX_PATH, while os.Lstat of the same path may still succeed via Go's long-path handling. Evidence: CreateFile call at identity_windows.go:20-31 with the plain path string; the codebase's unix behavior has no equivalent length limit. The failure direction is safe (identity unavailable, transaction refused), but it means cortex transactions would be refused on deeply nested Windows directories even though the files are reachable.

Question (1)

• Scope narrowing versus the test-pass expectation recorded in Windows build fails: internal/filetxn uses syscall.Stat_t without a Windows fallback #370 (docs/verification.md:16): Issue Windows build fails: internal/filetxn uses syscall.Stat_t without a Windows fallback #370's expected direction only requires the build to succeed on windows/amd64, and the PR satisfies it. However, the PR body states the plan in Windows build fails: internal/filetxn uses syscall.Stat_t without a Windows fallback #370 said 'go test ./internal/filetxn/...' would pass on Windows, which this PR does not achieve (fsync, POSIX mode bits, and symlink-privilege failures remain). The author already asked whether to open a separate issue for filetxn Windows semantics; confirm that split is recorded before merge so the unmet expectation in Windows build fails: internal/filetxn uses syscall.Stat_t without a Windows fallback #370 does not silently disappear. Evidence: 'Chain Dependencies / Out of Scope' section of the PR body listing the pre-existing Windows failures.

Verdict: ready to merge

Fleet review (deepseek)

Lens: evidence + test design. The diff does what issue #370 asks for: the directory-identity read moves behind build-tagged variants (identity_unix.go keeps the previous Stat_t device/inode read including the Ino == 0 refusal, identity_windows.go uses GetFileInformationByHandle), the four call sites stop referencing syscall.Stat_t, the persisted AcceptedDirectory shape and the unix behavior are unchanged, and docs/verification.md plus the publication inventory are updated consistently. The build claim on windows/amd64 is backed by a stated before/after (undefined: syscall.Stat_t at four sites, then go build ./... passing on win/linux/darwin, go vet ./internal/filetxn/, and the full Linux suite on a fresh clone). What the evidence does not cover: none of the new Windows refusal branches (non-directory, reparse point, zero index, os.SameFile mismatch) is exercised by any test, so those guards can be removed without a red test, and the stale-lstat subtest passes on ok == false, which cannot distinguish a correct drift refusal from an unrelated identity-read failure. No blocking finding: no acceptance criterion of #370 is contradicted and I found no concrete failure scenario on a supported platform. The PR's own scope question (build fix only vs. the remaining Windows fsync/mode work) is answered below as a question, not a gate.

Suggestion (3)

• Windows refusal branches have no test coverage (internal/filetxn/identity_windows.go:40): The new Windows implementation rejects four conditions before it returns an identity: not a real directory (isRealDirectory), FILE_ATTRIBUTE_DIRECTORY absent, FILE_ATTRIBUTE_REPARSE_POINT present, a zero file index, and a handle that os.SameFile says is not the caller's Lstat result. Every current test goes through the happy path, so deleting the reparse-point check or the os.SameFile check keeps the suite green while a junction would start being accepted as a stable directory identity. Missing test conditions: (1) a directory replaced by a junction/reparse point, with an Lstat FileInfo captured before the replacement - expect ok == false; (2) a regular file path - expect ok == false; (3) a directory path that no longer names the FileInfo the caller holds (fresh Lstat captured, directory swapped, old FileInfo passed) - expect ok == false, and a fresh Lstat at the same path - expect ok == true. This is the security-relevant part of the change and it is the only part without evidence.
• Stale-lstat subtest is satisfied by any failure to read an identity (internal/filetxn/identity_test.go:64): The assertion is if _, inode, ok := directoryIdentity(...); ok && inode == currentInode, so the subtest passes whenever ok is false, for any reason at all (missing path, privilege, malformed path, a future regression to a broken Windows open). It therefore cannot tell a correct drift refusal from an unrelated identity-read failure, and on unix it is vacuous because the path argument is ignored and the returned inode comes from the stale FileInfo by construction. Missing test condition: in the same subtest, assert that a fresh Lstat of the same path still returns ok == true immediately before the stale call, so ok == false can only be attributed to the mismatch rather than to an environment failure. Without that pin the Windows evidence for check 4 of the PR body does not distinguish the two cases.
• Identity variants cover only unix and windows (internal/filetxn/identity_unix.go:1): The two build tags leave plan9, js/wasm and wasip1 without a directoryIdentity definition, so those targets fail to compile with an undefined symbol rather than with a deterministic refusal. This is not a regression (the previous unconditional syscall.Stat_t read failed there too), and the published targets are macOS arm64 and Linux amd64, so it is low priority; the reason to mention it is that Windows build fails: internal/filetxn uses syscall.Stat_t without a Windows fallback #370 asks for per-OS variants and a third tag returning ok == false would complete that set and make the platform surface explicit. Please state the intended target list either way.

Question (2)

• Long paths are opened without the long-path prefix os.Lstat applies (internal/filetxn/identity_windows.go:17): syscall.UTF16PtrFromString(path) + syscall.CreateFile open the literal path, while os.Lstat on Windows applies the \?\ prefix for paths beyond MAX_PATH. A directory whose full path exceeds 260 characters would therefore Lstat successfully and then fail to open here, returning ok == false: through acceptCreatedDirectories that surfaces as "accepted directory identity is unavailable", and through restartDirectoryMatchesAccepted as "restart directory changed"/"restart directory drifted". Is that acceptable while Windows stays explicitly unsupported and unverified, or does it belong to the follow-up that makes the Windows filetxn path usable? I am not filing this as blocking because Windows filesystem transaction behavior is documented as unverified in this PR.
• Which Windows check gates the merge, given the disclosed pre-existing failures? (internal/qapi/profile_test.go): The verified Windows claim is go build ./... and go vet ./internal/filetxn/; the PR body discloses that go vet ./... still fails on Windows (syscall.Mkfifo in this file) and that go test ./internal/filetxn/ does not pass there for fsync and POSIX-mode reasons. If a Windows job in CI runs go vet ./... or go test ./..., this head will not turn it green and the build fix will look regressed later. Which command is the Windows gate for this PR? Also, on the scope question in the PR body: I would keep this PR to the build fix and open the follow-up for the filetxn Windows semantics, since anything touching directory fsync or permission bits changes the manifest evidence contract rather than the build.

Verdict: ready to merge

Thanks for the review. Taking the three items in order:

ReFS file IDs. Agreed. Fixing it here would need either a manifest identity format change or refusing ReFS (which would include Dev Drives), so I put it in the follow-up issue as a design point: #389.

Long paths. I couldn't reproduce this one. On Windows 11 with go1.26.5 and LongPathsEnabled=0 in the registry, directoryIdentity succeeds for a 326-character directory path. The Go runtime opts the process into long-path support at startup (initLongPathSupport in runtime/os_windows.go), and that also covers direct syscall.CreateFile calls. Happy to add a regression test for it if you want one.

Scope split. Recorded in #389 as evidence for the Phase B mode/backup semantics design (scope item 2 in docs/design/issue-332-powershell-windows.md): directory fsync, mode semantics, symlink tests, ReFS IDs, and the Mkfifo vet failure, each with the decision it needs. It doesn't ask for implementation before the readiness gate.

Thanks, these were worth fixing. I pushed 4e3e08c.

Windows refusal branches. identity_windows_test.go now covers a regular file, a junction (mklink /J, which needs no privilege), a directory replaced by a junction after Lstat, a directory replaced by another directory after Lstat (where a fresh Lstat must still resolve), and a path longer than MAX_PATH. To check that the tests actually pin the guards, I disabled each guard on Windows and ran them:

The single-guard rows stay green because those checks overlap: removing one of them changes no behavior, since another check rejects the same input. Every removal that changes behavior turns a test red.

Stale-Lstat subtest. It now pins a fresh Lstat first and states the expected result per OS: a refusal on Windows, where the identity is read from the path, and the original identity on unix, where it comes from the Lstat result itself (so it is no longer vacuous there).

Other platforms. identity_other.go (!unix && !windows) returns ok == false, so js/wasm and wasip1 now compile and fail closed. The intended targets stay as published (macOS arm64, Linux amd64); Windows builds but stays unsupported. plan9 still fails on the pre-existing syscall.ENOTSUP in directories.go, which I left alone.

Long paths. Covered by the new path_longer_than_MAX_PATH subtest. A 300+ character directory resolves with LongPathsEnabled=0, because the Go runtime opts the process into long-path support (initLongPathSupport), and that also covers syscall.CreateFile.

Windows gate. For this PR: go build ./... on windows/amd64 and go test -run TestDirectoryIdentity ./internal/filetxn/. go vet ./... and the full filetxn suite on Windows are tracked in #389 and don't gate this PR. I agree on keeping it to the build fix.

Verification: Windows 11 go1.26.5 (identity tests and the mutation runs above), and Linux (golang:1.26 container, non-root, LF): gofmt, go vet ./..., and go test ./... all clean; the policy contracts pass with the inventory at 411.

Fleet review (glm)

Lens: requirements + adversarial. The diff satisfies issue #370's expected direction: the syscall.Stat_t reads in acceptCreatedDirectories and restartDirectoryMatchesAccepted (plus the two restart_test.go call sites) now go through a build-tagged directoryIdentity function, with identity_unix.go preserving the exact previous behavior (same Stat_t read, same Ino == 0 refusal), identity_windows.go reading volume serial + file index through GetFileInformationByHandle, and identity_other.go failing closed on remaining platforms. Adversarially, the Windows variant is fail-closed where it matters: it refuses non-directories, reparse points (so junctions are rejected even though the handle is opened with FILE_FLAG_OPEN_REPARSE_POINT), a zero index, and any case where os.SameFile(info, opened) disagrees; in the restart paths a TOCTOU replacement between Lstat and the handle open still ends in a mismatch against the persisted accepted identity, so a replacement is refused rather than accepted. The stale-Lstat tests pin the lazily loaded Windows identity the same way production callers do via os.SameFile, and the 406-to-411 inventory update matches the five new files. The PR also honestly discloses that #370's plan promised go test ./internal/filetxn/... on Windows while this PR only delivers the build fix and the identity tests; the author asked the maintainer how to slice it. That scope gap is the one thing to pin down before merge so it does not silently disappear. No blocking findings.

Suggestion (1)

• Document the ReFS identity limitation next to the Windows caveat (docs/verification.md:16): The PR body discloses that ReFS can use 128-bit file IDs while GetFileInformationByHandle returns only the 64-bit index, but docs/verification.md (the file this PR edits) does not mention it. One line noting that directory identity on ReFS volumes is not guaranteed would keep the documented Windows caveat aligned with the actual limitation.

Question (1)

• Scope gap vs Windows build fails: internal/filetxn uses syscall.Stat_t without a Windows fallback #370's plan needs a tracked follow-up before merge (internal/filetxn/identity_windows.go): Issue Windows build fails: internal/filetxn uses syscall.Stat_t without a Windows fallback #370's plan said go test ./internal/filetxn/... would pass on Windows; this PR only makes the build and the identity tests pass. The PR body asks the maintainer whether to split, but the decision is still open. If the follow-up issue for the remaining Windows semantics (directory fsync, permission bits, symlink privilege) is not created and referenced, the gap has no owner after merge.

Verdict: ready to merge

Fleet review (deepseek)

Lens: evidence + test design. The extraction is sound: identity_unix.go is a literal move of the previous Stat_t read (same Dev/Ino and the Ino == 0 refusal), identity_other.go now completes the platform set by failing closed, both call sites in directories.go and restart.go pass the same path they Lstat'ed, and the persisted AcceptedDirectory shape is unchanged. Two earlier gaps are closed at this head: identity_windows_test.go now exercises the regular-file, junction, replaced-after-lstat and long-path refusals, and the stale-lstat subtest pins a fresh Lstat before the stale call, so ok == false is attributable to the mismatch. Two problems remain on the Windows side. First, the new identity read converts the caller's path directly with syscall.UTF16PtrFromString and syscall.CreateFile, so it has none of the long-path handling the os package applies to every other operation in the transaction; the path longer than MAX_PATH subtest therefore asserts a result that depends on the host's LongPathsEnabled registry value, which the PR body does not state. Second, identity_test.go has no build tag while the refuse-by-design variant it compiles against on !unix && !windows always returns ok=false, so that file cannot pass where identity_other.go is the active implementation.

Blocking (1)

• Windows identity read has no long-path handling, and the >MAX_PATH subtest asserts a host-dependent result (internal/filetxn/identity_windows.go:22): directoryIdentity hands the path verbatim to syscall.UTF16PtrFromString (line 18) and syscall.CreateFile (line 22). Neither applies the long-path handling that the os package adds to its own calls (os.Lstat, os.MkdirAll, os.Open), so the same path succeeds for the rest of the transaction and fails only in the identity read. The Windows refusals test then asserts the opposite for a long path: identity_windows_test.go:74 builds a path past 300 characters with os.MkdirAll and requires ok == true, which holds only when the host has the LongPathsEnabled registry value set. The PR body records go test -run TestDirectoryIdentity ./internal/filetxn/ as passing on Windows 11 without stating that host setting, so the evidence is not reproducible from the PR on a default Windows install.
  • Failure scenario: On a default Windows 11 host (LongPathsEnabled unset), go test -run TestDirectoryIdentityWindowsRefusals ./internal/filetxn/ fails the path longer than MAX_PATH subtest with long path (N chars) has no identity: os.MkdirAll creates the 300+ character directory through the os package, then syscall.CreateFile on the unprefixed name returns ERROR_PATH_NOT_FOUND and directoryIdentity reports ok=false. In the product, an install whose directory target path exceeds MAX_PATH creates its directories and then aborts the transaction in acceptCreatedDirectories with accepted directory identity is unavailable: <path>, instead of recording the identity the way it does for shorter paths.

Suggestion (1)

• Portable identity test cannot pass where the refuse-by-design variant is compiled (internal/filetxn/identity_test.go:21): identity_test.go has no build constraint, while identity_other.go covers !unix && !windows and always returns ok=false. TestDirectoryIdentity and its lstatDirectoryIdentity helper fatal whenever the platform refuses, so on the platforms identity_other.go exists for (js/wasm, wasip1, plan9) the package builds and the test fails. The coverage rationale gap is that the fail-closed variant is exercised only in configurations where the test is guaranteed to fail, and never with an assertion of its intended refusal behavior.

Question (1)

• Which Windows command gates this merge, given the disclosed pre-existing failures? (internal/qapi/profile_test.go): The verified Windows claim is go build ./... and go vet ./internal/filetxn/; the PR body also discloses that go vet ./... still fails on Windows because this file uses syscall.Mkfifo, and that go test ./internal/filetxn/ does not pass there for directory-fsync and POSIX-mode reasons. If a Windows job in CI runs go vet ./... or go test ./..., this head will not turn it green and the build fix will look regressed later. Naming the Windows gate command in the PR makes the evidence auditable; the follow-up split for the remaining filetxn semantics is a scope decision for the author, not a defect in this diff.

Verdict: not ready

Fleet review (qwen38)

Lens: evidence. Issue #370's acceptance criterion is met with real evidence: the identity read is extracted behind build-tagged directoryIdentity variants, the unix variant preserves the previous Stat_t read exactly (including the Ino == 0 refusal), both production call sites (directories.go acceptCreatedDirectories, restart.go restartDirectoryMatchesAccepted) and both restart_test.go helpers route through it, identity_other.go fails closed elsewhere, and the PR reports go build/vet plus GOOS=linux|darwin|windows cross-builds passing and TestDirectoryIdentity passing on native Windows 11. The publication inventory bump 406->411 matches the five new files exactly, and the persisted AcceptedDirectory shape is unchanged. No blocking defect found: the windows variant refuses non-directories, reparse points, and a zero file index, and guards the handle against the caller's Lstat with os.SameFile. Remaining findings are evidence gaps: the Windows-only refusal suite TestDirectoryIdentityWindowsRefusals has no reported run result, its long-path subtest depends on an unstated host setting, and the disclosed ReFS 128-bit file-ID truncation is recorded only in the PR body.

Suggestion (1)

• ReFS 64-bit file-index truncation disclosed only in the PR body (internal/filetxn/identity_windows.go:46): On ReFS volumes a directory's file ID can be 128-bit while GetFileInformationByHandle returns only the 64-bit high/low index, so two distinct ReFS directories could in principle share a truncated identity, weakening the accepted-directory restart check on such volumes. The PR body states NTFS is fully covered and notes this, but the code at the index derivation and docs/verification.md carry no trace. Suggest a comment at internal/filetxn/identity_windows.go where FileIndexHigh/FileIndexLow are combined (and a line in docs/verification.md) so the constraint survives in the repository, not only in the PR discussion.

Question (2)

• No reported run result for TestDirectoryIdentityWindowsRefusals (internal/filetxn/identity_windows_test.go:28): The PR's Focused Tests section records four subtests of TestDirectoryIdentity passing on Windows 11, but TestDirectoryIdentityWindowsRefusals (five subtests: regular file, junction, directory replaced by a junction after lstat, directory replaced by another directory after lstat, path longer than MAX_PATH) has no stated outcome anywhere in the PR. These are the subtests that pin the Windows refusal behavior (reparse rejection, stale-Lstat rejection via os.SameFile). Please confirm the command and observed result for 'go test -run TestDirectoryIdentityWindowsRefusals ./internal/filetxn/' on the native Windows 11 host, or state if it was not executed.
• Long-path support depends on host LongPathsEnabled, asserted silently (internal/filetxn/identity_windows.go:18): directoryIdentity converts the path with syscall.UTF16PtrFromString and passes it to syscall.CreateFileW directly; unlike the os package, the syscall package does not add the extended-length prefix, so CreateFileW accepts paths over MAX_PATH only when the volume/host has long paths enabled (Windows 11 default; not guaranteed). The long-path subtest of TestDirectoryIdentityWindowsRefusals asserts success for a >300-character path, so its reported result doubles as evidence for this environment assumption — but combined with the previous finding, that run was never reported. On a LongPathsEnabled=0 host the runtime behavior is fail-closed (identity unavailable, accept/restart refuses), which is safe, so this is evidence and documentation rather than a defect.

Verdict: ready to merge

Fleet review (glmflash)

Lens: adversarial. The per-OS split is structurally sound: unix behavior is byte-for-byte the old Stat_t read, identity_other.go fails closed, and the Windows variant refuses non-directories, reparse points, zero indexes, and identity mismatches, so every adversarial path I attacked (junction swap after Lstat, directory replacement between Lstat and CreateFile, unpinned stale info in classifyRestartDirectories/rollbackRestartDirectories) ends in a refusal or in an identity that then fails the accepted-Device/Inode comparison. The accepted-identity property itself survives the TOCTOU windows because the identity always comes from the handle and is compared against the persisted accepted values. One blocking problem: the new Windows test suite asserts long-path success that a direct syscall.CreateFile cannot deliver on stock Windows, and the test evidence in the body is consistent with those Windows-only subtests never having passed.

Blocking (1)

• Long-path subtest asserts behavior syscall.CreateFile cannot provide on stock Windows (internal/filetxn/identity_windows_test.go:77): The 'path longer than MAX_PATH' subtest builds a 300+ character path and requires directoryIdentity to succeed. But directoryIdentity calls syscall.CreateFile directly; long-path support for a raw CreateFileW call requires both LongPathsEnabled in the registry AND a longPathAware manifest embedded in the binary. Verified against the local go1.27.1 source tree: the string longPathAware appears nowhere in Go's source, and fixLongPath (which prefixes \?) exists only in the os package, which syscall.CreateFile bypasses. So on a stock Windows install CreateFileW fails with ERROR_PATH_NOT_FOUND, directoryIdentity returns ok=false, and the subtest's t.Fatalf fires. The product behavior in that case is fail-closed and acceptable; the test expectation is the defect.
  • Failure scenario: Running 'go test -run TestDirectoryIdentity ./internal/filetxn/' on stock Windows 11 fails at TestDirectoryIdentityWindowsRefusals/path_longer_than_MAX_PATH with 'long path (3xx chars) has no identity', because syscall.CreateFile receives the unprefixed 300+ char path and Windows rejects it before the volume-serial/index read.

Question (2)

• Windows test evidence: -run TestDirectoryIdentity also matches the refusals test (internal/filetxn/identity_windows_test.go:39): The body claims 'go test -run TestDirectoryIdentity ./internal/filetxn/ passes all four subtests', but -run patterns are unanchored regexes, so that command on Windows also runs the five subtests of TestDirectoryIdentityWindowsRefusals (nine subtests total, not four). Either those five subtests were not actually run or they did not pass, which is consistent with the long-path blocking finding above. Can you confirm the exact commands and subtest counts from the Windows run?
• Windows acceptance binds identity to the path object, not the created object (internal/filetxn/identity_windows.go:44): In acceptCreatedDirectories, directory.createdInfo is typically unpinned until the os.SameFile check, and on Windows SameFile lazily loads a file ID from the stored path. If the directory is replaced between creation and acceptance, createdInfo resolves to the replacement, SameFile passes, and the snapshot records the replacement's identity — the unix guarantee that the accepted directory is the object actually created degrades to path identity. This is not a regression (Windows did not build before) and the platform is documented as unsupported, but it is a semantic weakening worth tracking for the Feature: support PowerShell as a runtime target and a Windows platform for the CLI #332 Phase B work. Is this limitation recorded anywhere besides this PR body?

Verdict: not ready

Fleet review (mimo)

Lens: requirements. Issue #370's expected direction - extract the directory-identity read behind a build-tagged interface (unix Stat_t variant, Windows GetFileInformationByHandle variant) so ./cmd/cortex builds on windows/amd64 - is satisfied at head 4e3e08c: both production call sites (directories.go acceptCreatedDirectories, restart.go restartDirectoryMatchesAccepted) and both restart_test.go helpers route through directoryIdentity, a read of the head files shows only identity_unix.go still references syscall.Stat_t, exactly one variant compiles per GOOS, directories.go keeps its syscall import for the EINVAL/ENOTSUP sync check at line 414 (so the missing import hunk is not a build break), the unix read is byte-identical to the previous one (same Dev/Ino read and Ino==0 refusal, so unix behavior is unchanged as the PR claims), the persisted AcceptedDirectory shape is unchanged, and the publication inventory moves 406->411 for exactly the five new files. I do not share the first-line blocking verdict on the >MAX_PATH subtest: the blocking scenarios assume the os package adds long-path handling that syscall.CreateFile lacks, but on any Windows build >=15063 running a Go binary (including the named verification host, Windows 11 go1.26.5) the Go runtime sets the PEB IsLongPathAwareProcess bit at startup and os.fixLongPath then returns the path unmodified, so os.Lstat/os.MkdirAll and the new identity read pass raw long paths identically - a failure would hit the subtest's own os.MkdirAll first, not directoryIdentity, and the author reports the subtest passing with LongPathsEnabled=0. Four non-blocking findings follow: the still-undocumented host/OS precondition of that subtest, the Windows accept-baseline laziness question, a build-tag gap in identity_test.go, and the stale test evidence in the PR body for this head.

Suggestion (1)

• identity_test.go has no build constraint, so it fails by design wherever identity_other.go is the active variant (internal/filetxn/identity_test.go:21): identity_other.go (!unix && !windows) always returns ok=false by design, while TestDirectoryIdentity and lstatDirectoryIdentity fatal whenever the identity is unavailable. On js/wasm, plan9 and wasip1 the package therefore builds, and go test ./internal/filetxn/ fails unconditionally - the fail-closed variant is never asserted, only punished. Not a supported target and not an acceptance criterion of Windows build fails: internal/filetxn uses syscall.Stat_t without a Windows fallback #370, so non-blocking, but a build tag on the portable test (or a refusal assertion for the other variant) would make the platform matrix self-consistent with the claim that the remaining platforms compile and fail closed.

Question (3)

• Long-path precondition of the >MAX_PATH subtest is undocumented (and the blocking premise only holds off the named host) (internal/filetxn/identity_windows.go:22): The blocking finding at this head claims directoryIdentity has 'none of the long-path handling the os package applies' so that 'the same path succeeds for the rest of the transaction and fails only in the identity read' on stock Windows. Against the go1.26.5 sources that asymmetry only exists when the Go runtime does not set the PEB long-path bit: runtime/os_windows.go initLongPathSupport (present since go1.22, including go1.26.5) sets IsLongPathAwareProcess for Windows builds >= 15063, after which os's fixLongPath returns the path unmodified (os/path_windows.go:100-104), so os.Lstat/os.MkdirAll also hand raw >MAX_PATH paths to Win32 - on that host the subtest's own os.MkdirAll would fail before directoryIdentity is reached, which contradicts the recorded failure message. Go never reads the LongPathsEnabled registry value anywhere in its tree, its runtime long-path probe was removed with 'There hasn't been any report that the PEB update was not effective' (golang/go c098400548), and the author reports directoryIdentity succeeding for a 326-char path on Windows 11 with LongPathsEnabled=0. The real residual gap is documentation: Microsoft's contract (maximum-file-path-limitation.md) still requires registry value + longPathAware manifest, and on hosts where initLongPathSupport returns early (build < 15063, e.g. Windows 10 1607 / Server 2016 class) os adds \?\ while this read does not, so a >260-char target would fail only in the identity read and the subtest would go red there. Please record the assumed host conditions (Windows build, LongPathsEnabled value, exact command) for internal/filetxn/identity_windows_test.go:74 in the PR body or docs/verification.md so the precondition is auditable.
• On Windows the accept baseline compares two lazily resolved Lstat results, so accept cannot detect a directory swapped between creation and acceptance (internal/filetxn/directories.go:214): createdInfo is captured at directories.go:214 as a plain os.Lstat and is first compared only at directories.go:255 inside acceptCreatedDirectories. On Windows an Lstat result carries no file ID until os.SameFile triggers loadFileId, which re-opens the stored path (os/types_windows.go:287-362), so both sides of os.SameFile(directory.createdInfo, info) resolve the same current path at comparison time: if the directory is replaced between Mkdir and acceptance, the comparison passes and the snapshot records the replacement's identity as AcceptedDirectory - the unix guarantee behind 'accepted directory changed' degrades to path identity on Windows. The PR pins exactly this laziness in the test helper (replaceDirectoryWithDistinctIdentity) but not at the production capture point, while identity_test.go's comment asserts callers pin 'as callers do with os.SameFile'. This is not a regression (Windows did not build before), docs already state Windows filesystem transaction behavior is not yet verified, and it contradicts no acceptance criterion of Windows build fails: internal/filetxn uses syscall.Stat_t without a Windows fallback #370, so it is not blocking - but is this limitation recorded anywhere for the Feature: support PowerShell as a runtime target and a Windows platform for the CLI #332 Phase B work besides the PR thread?
• PR body test evidence still describes an earlier head and 'four subtests' while -run TestDirectoryIdentity matches nine at this head (internal/filetxn/identity_windows_test.go:32): The body's Focused Tests section records go test -run TestDirectoryIdentity ./internal/filetxn/ passing 'all four subtests' against branch b645cb4, but the current head is 4e3e08c, identity_windows_test.go did not exist at the cited heads, and -run patterns are unanchored regexes, so that command also runs the five subtests of TestDirectoryIdentityWindowsRefusals (nine total, not four). The current-head evidence exists only in the review thread (the guard-toggle table in the 10:15 reply, whose green rows imply the whole refusal suite including the MAX_PATH subtest passed). Please record the exact commands and subtest counts for the current head - including the Windows refusal suite and the Linux suite - in the PR body so the evidence is reproducible from the PR itself.

Verdict: ready to merge

Thanks for this round. I pushed 948448b and 1011b41, and updated the PR body with the current-head evidence.

Long paths (deepseek and glmflash, blocking; qwen38 and mimo). Fixed in code, so the test no longer depends on the host. directoryIdentity now opens every long clean absolute path with the \\?\ prefix (\\?\UNC\ for UNC paths), mirroring os.addExtendedPrefix. TestExtendedLengthPath pins the conversion, and disabling the prefix turns it red. The MAX_PATH subtest now exercises the prefixed open on any host.

On the premise: mimo's reading matches the Go sources. On Windows 10 1703 and later, runtime/os_windows.go initLongPathSupport sets the PEB long-path flag and canUseLongPaths, and os/path_windows.go fixLongPath then returns paths unchanged. So os and a raw CreateFile behave the same there, which is why the subtest passed with LongPathsEnabled=0 on this host. The real gap was builds before 15063, where the runtime skips the opt-in and only os adds the prefix. The prefix closes that gap too.

Test evidence and subtest counts (glmflash, mimo, qwen38). The -run TestDirectoryIdentity pattern is unanchored, as you say. The run on Windows 11, go1.26.5, LongPathsEnabled=0, at head 1011b41: go test -count=1 -v -run 'TestDirectoryIdentity|TestExtendedLengthPath' ./internal/filetxn/ passes TestDirectoryIdentity (4 subtests), TestDirectoryIdentityWindowsRefusals (5 subtests, including MAX_PATH), and TestExtendedLengthPath. The PR body now records this, along with the guard mutations and the Linux suite, for the current head.

Swap between creation and acceptance on Windows (glmflash, mimo). Agreed. createdInfo is first compared at acceptance, and on Windows its identity loads lazily from the path, so a swap in that window is compared by path rather than by the created object. I left the production path unchanged: it is transaction semantics on an unsupported platform, and the filetxn suites that exercise acceptance can't pass on Windows yet, so I couldn't verify a change there. It's now recorded in the PR body and in #389 (pinning the identity at capture with os.SameFile(createdInfo, createdInfo) is the candidate fix).

ReFS limitation (glm, qwen38). Recorded at the index derivation in identity_windows.go and in docs/verification.md, both pointing to #389.

identity_test.go on refuse-by-design platforms (deepseek, mimo). Now //go:build unix || windows. go vet ./internal/filetxn/ passes for js/wasm, wasip1/wasm, linux/amd64, darwin/arm64, and windows/amd64.

Windows gate (deepseek). go build ./... on windows/amd64 plus the identity tests above. go vet ./... and the full filetxn suite on Windows are tracked in #389.

Verification: Windows 11 as above; Linux in a golang:1.26 container (non-root, LF) with gofmt, go vet ./..., and go test ./... all clean; policy contracts pass.

Fleet review (glm)

Lens: requirements + adversarial. The PR satisfies issue #370's core requirement: internal/filetxn now builds on windows/amd64 by reading directory identity behind build-tagged variants (unix Stat_t read unchanged and bit-identical in behavior, Windows GetFileInformationByHandle, other platforms fail closed). The Windows variant is carefully hardened: non-directories refused, reparse points refused (so junctions are rejected even though the handle is opened with FILE_FLAG_OPEN_REPARSE_POINT), zero file index refused, and an os.SameFile cross-check between the caller's Lstat result and the opened handle; ?\ prefixing keeps long paths independent of the runtime long-path opt-in. New tests pin stability, sibling distinctness, replacement distinctness, stale-Lstat refusal, junction refusal, and >MAX_PATH behavior, and the guard-mutation evidence shows the tests actually bite. The docs now disclose the ReFS 64-bit-index limitation and the not-yet-verified Windows transaction behavior. No blocking defects found; three clarifications below, two of which the author already documents and defers.

Suggestion (1)

• Pre-existing assertEqual duplication makes the inventory count fragile (tests/test_community_policy.py:468): The hunk shows ~30 identical assertEqual(len(EXPECTED_FILES), ...) pairs already present on main; this PR faithfully updates every copy from 406 to 411 along with the five new files, so the counts are consistent. But the duplication itself means every future file addition must edit ~30 identical lines, and missing one silently desynchronizes the assertions. This predates the PR and does not block it; consider a small cleanup issue to collapse the repeated assertions into one.

Question (2)

• Windows lazy-identity TOCTOU: a swap before the first os.SameFile is compared by path (internal/filetxn/directories.go:258): The PR documents that on Windows an Lstat result loads its identity lazily at the first os.SameFile, so a directory swapped between creation and the first SameFile comparison is compared by path rather than by the object actually created — weakening the acceptance-time identity check on Windows specifically. The deferral to Feature: record Windows filetxn semantics decisions for the Phase B design #389 is reasonable (Windows is documented as not yet supported and this PR's scope is the build fix), but the ownership should be explicit: confirm that Feature: record Windows filetxn semantics decisions for the Phase B design #389 or the Phase B mode/backup design owns closing this window, so acceptance-time identity becomes an actual guarantee on Windows rather than a best-effort one. Unix behavior, the only supported platforms today, is unchanged.
• Scope narrowing from the author's own plan in Windows build fails: internal/filetxn uses syscall.Stat_t without a Windows fallback #370 (internal/filetxn/identity_windows.go:20): The author's plan in Windows build fails: internal/filetxn uses syscall.Stat_t without a Windows fallback #370 stated go test ./internal/filetxn/... would pass on Windows; this PR delivers the build fix and the identity tests only, deferring the remaining Windows test failures (directory fsync, POSIX permission bits, symlink privilege) to Feature: record Windows filetxn semantics decisions for the Phase B design #389. The issue's core blocker — cmd/cortex does not build on windows/amd64 — is demonstrably fixed, and docs/verification.md now describes the state accurately. Confirm the maintainer accepts this narrowing as satisfying Windows build fails: internal/filetxn uses syscall.Stat_t without a Windows fallback #370, or that Windows build fails: internal/filetxn uses syscall.Stat_t without a Windows fallback #370 stays open until the full package suite passes on Windows, so the issue is not closed on a narrower contract than announced.

Verdict: ready to merge

Fleet review (deepseek)

Lens: evidence + test design. I re-reviewed from scratch at head 1011b41 (my previous review of 4e3e08c blocked on the missing Windows long-path handling; extendedLengthPath now adds the \\?\/\\?\UNC\ prefix and TestExtendedLengthPath pins it, and identity_test.go now carries //go:build unix || windows, so both are resolved). The build fix is real and minimal: the syscall.Stat_t read now sits behind directoryIdentity with unix, windows and a fail-closed variant, all four call sites use it (two in production, two in restart_test.go), and the unix variant is the previous expression including the Ino == 0 refusal, so Linux and macOS behavior is unchanged. I read the raw Windows file at head instead of trusting the diff rendering: the prefix literals are correct, the 248-character threshold matches Go's own fixLongPath cutoff, the handle is opened with FILE_FLAG_BACKUP_SEMANTICS|FILE_FLAG_OPEN_REPARSE_POINT, the directory and reparse-point attributes are checked before the identity is used, the 64-bit index is assembled high-then-low, and os.SameFile is what turns a path-based read into a stale-Lstat refusal. Evidence is proportionate and attributed: the failing build is quoted from the issue, go build ./... plus go vet cover windows/amd64 and the js/wasm, wasip1/wasm, linux/amd64 and darwin/arm64 targets, the guard-mutation results name which guard each test protects, and the disclosed Windows gaps (path-based identity at accept time, ReFS 128-bit IDs, the rest of the package's Windows suite) are real pre-existing semantics tracked in #389, not scope hidden behind a green build. The two findings are test-design suggestions; neither has a concrete wrong-behavior scenario today.

Suggestion (2)

• extendedLengthPath boundary and device-path form are unpinned (internal/filetxn/identity_windows_test.go:32): extendedLengthPath exists to apply one threshold, and the table has a roughly 10-character absolute path and a roughly 270-character one, so the 247/248-character boundary on len(path) < maxShortDirectory has no case: changing < to <=, or 248 to another value, leaves every subtest green. The doc comment also claims the conversion follows the os package, but os/fixLongPath passes device paths (\\\\\\.\\\\...) through unchanged while this function would prepend \\?\ to a long one, producing an invalid name that makes directoryIdentity fail closed as identity is unavailable rather than reporting the directory's identity. Add cases at 247 and 248 characters and one long device path, so the function's contract is measured at the point where it decides.
• Windows evidence exercises the helper but not its production call sites (internal/filetxn/directories.go:258): The Windows gate you declare is go build ./... plus TestDirectoryIdentity* and TestExtendedLengthPath, and the rest of the package's Windows tests do not run for pre-existing reasons. That means the two places the new path argument matters are never executed on Windows: acceptCreatedDirectories (directories.go:258, directoryIdentity(directory.target, info)) and restartDirectoryMatchesAccepted (restart.go:404, called with the safepath.Resolve result at restart.go:388 and :484). A caller that passed the root-relative directory.path instead of the resolved directory.target would satisfy every Windows test and only fail at runtime, where the identity read would target the wrong path or fail closed. One Windows test that drives acceptCreatedDirectories or restartDirectoryMatchesAccepted over a real temporary directory would cover the wiring; if that is deferred with the rest of the Windows semantics, add it to the Feature: record Windows filetxn semantics decisions for the Phase B design #389 gate list so the omission is recorded rather than implicit.

Verdict: ready to merge

Thanks. I pushed adaec91.

extendedLengthPath boundary and device paths (deepseek). Fixed. Device paths (\\.\) now pass through unchanged, as os.addExtendedPrefix does. TestExtendedLengthPath pins 247 characters (unchanged), 248 (prefixed), and a long device path (unchanged).

Production wiring on Windows (deepseek). Added two tests:

• TestRestartDirectoryMatchesAcceptedWindows drives restartDirectoryMatchesAccepted with the resolved target. It matches the directory's own identity, refuses the root-relative path, and refuses a replacement.
• TestDirectoryIdentityOpensLongPathsExtended makes the prefix wiring observable even on hosts where the runtime long-path opt-in makes plain long paths work. It opens a long directory whose name ends with a dot, which only the \\?\ form preserves.

acceptCreatedDirectories on Windows still can't run end to end because of the fsync and mode issues, so it's on the #389 gate list.

Mutation check on Windows, each change applied alone with the same tests run:

Inventory duplication (glm). Agreed that the repeated asserts are fragile; that predates this PR. I can open a small cleanup issue if you want one.

Ownership of the lazy accept-time identity and the #370 scope (glm). #389 owns the accept-time identity, recorded in its comment with the candidate fix. Whether #370 closes on this PR or stays open until the full Windows suite passes is your call.

Verification at adaec91: Windows 11 (identity, prefix, and restart tests; mutations as above); go vet for js/wasm, wasip1, linux, and darwin; Linux container with go vet ./... and go test ./... clean.

Fleet review (deepseek)

Lens: evidence + test design. Reviewed at head adaec91; my earlier review of 1011b41 asked for the extendedLengthPath boundary and device-path cases, and for coverage of the production wiring. Both are now answered: TestExtendedLengthPath carries the 247/248 threshold, a long UNC path, a long device path, an already-extended path, a long relative path and a long unclean path, and TestRestartDirectoryMatchesAcceptedWindows drives the restart check through a resolved target, including the root-relative mismatch and the replaced directory. The build fix itself is unchanged in shape and still correct: the syscall.Stat_t read sits behind directoryIdentity with unix, windows and fail-closed variants, the unix variant is the previous expression including the Ino == 0 refusal, the Windows variant opens with FILE_FLAG_BACKUP_SEMANTICS|FILE_FLAG_OPEN_REPARSE_POINT, refuses non-directories, reparse points and a zero index, and uses os.SameFile to turn a path-based read into a stale-Lstat refusal; the persisted AcceptedDirectory shape is untouched. The inventory arithmetic in tests/test_community_policy.py is honest: five new files against 406 -> 411, and the new paths are added to EXPECTED_FILES. No blocking defect. What remains is one production call site that the declared Windows gate still does not execute, and a pre-existing duplicated assertion block this PR renumbers but does not clean up.

Suggestion (2)

• The acceptCreatedDirectories wiring is still the only new call site with no Windows execution (internal/filetxn/directories.go:258): The restart path is now covered by TestRestartDirectoryMatchesAcceptedWindows, which is the right shape: it proves the resolved target is what reaches directoryIdentity and that a root-relative path does not match. acceptCreatedDirectories at this line has no equivalent: its Windows tests do not run (the package's other Windows tests fail for pre-existing fsync and permission reasons, per the PR body), and the declared gate is go build ./... plus the identity tests, so directoryIdentity(directory.target, info) in the accept path is never executed on Windows. The risk is specific to this change, because the path argument is ignored on unix and only matters on Windows: a caller that passed the root-relative directory.path instead of directory.target would keep every current Windows test green and only fail at runtime, as accepted directory identity is unavailable or as an identity read against the wrong directory. Either add one Windows test that drives acceptCreatedDirectories over a real temporary directory, or list this call site in the Feature: record Windows filetxn semantics decisions for the Phase B design #389 gate so the omission is recorded instead of implied.
• The count-block duplication is pre-existing but this PR renumbers 30 copies of it (tests/test_community_policy.py:465): The inventory test asserts len(EXPECTED_FILES) and len(actual_files) 30 times in a row, and this PR edits the same constant in all 30 pairs (406 -> 411). The duplication is not introduced here, so it is not a merge blocker, but the next count change has to be applied 30 times by hand and a mangled edit in one copy stays invisible next to the other 29 passing assertions. Collapsing the block to one assertion pair while you are already editing every line of it makes the next inventory update a one-line diff.

Verdict: ready to merge

Fleet review (glm)

Lens: requirements + adversarial. The PR satisfies issue #370's core requirement: internal/filetxn now builds on windows/amd64 by reading directory identity behind build-tagged variants (unix Stat_t read unchanged and bit-identical in behavior, Windows GetFileInformationByHandle, other platforms fail closed). The Windows variant is carefully hardened: non-directories refused, reparse points refused (so junctions are rejected even though the handle is opened with FILE_FLAG_OPEN_REPARSE_POINT), zero file index refused, and an os.SameFile cross-check between the caller's Lstat result and the opened handle; ?\ prefixing keeps long paths independent of the runtime long-path opt-in. New tests pin stability, sibling distinctness, replacement distinctness, stale-Lstat refusal, junction refusal, and >MAX_PATH behavior, and the guard-mutation evidence shows the tests actually bite. The docs now disclose the ReFS 64-bit-index limitation and the not-yet-verified Windows transaction behavior. No blocking defects found; three clarifications below, two of which the author already documents and defers.

Suggestion (1)

• Pre-existing assertEqual duplication makes the inventory count fragile (tests/test_community_policy.py:468): The hunk shows ~30 identical assertEqual(len(EXPECTED_FILES), ...) pairs already present on main; this PR faithfully updates every copy from 406 to 411 along with the five new files, so the counts are consistent. But the duplication itself means every future file addition must edit ~30 identical lines, and missing one silently desynchronizes the assertions. This predates the PR and does not block it; consider a small cleanup issue to collapse the repeated assertions into one.

Question (2)

• Windows lazy-identity TOCTOU: a swap before the first os.SameFile is compared by path (internal/filetxn/directories.go:258): The PR documents that on Windows an Lstat result loads its identity lazily at the first os.SameFile, so a directory swapped between creation and the first SameFile comparison is compared by path rather than by the object actually created — weakening the acceptance-time identity check on Windows specifically. The deferral to Feature: record Windows filetxn semantics decisions for the Phase B design #389 is reasonable (Windows is documented as not yet supported and this PR's scope is the build fix), but the ownership should be explicit: confirm that Feature: record Windows filetxn semantics decisions for the Phase B design #389 or the Phase B mode/backup design owns closing this window, so acceptance-time identity becomes an actual guarantee on Windows rather than a best-effort one. Unix behavior, the only supported platforms today, is unchanged.
• Scope narrowing from the author's own plan in Windows build fails: internal/filetxn uses syscall.Stat_t without a Windows fallback #370 (internal/filetxn/identity_windows.go:20): The author's plan in Windows build fails: internal/filetxn uses syscall.Stat_t without a Windows fallback #370 stated go test ./internal/filetxn/... would pass on Windows; this PR delivers the build fix and the identity tests only, deferring the remaining Windows test failures (directory fsync, POSIX permission bits, symlink privilege) to Feature: record Windows filetxn semantics decisions for the Phase B design #389. The issue's core blocker — cmd/cortex does not build on windows/amd64 — is demonstrably fixed, and docs/verification.md now describes the state accurately. Confirm the maintainer accepts this narrowing as satisfying Windows build fails: internal/filetxn uses syscall.Stat_t without a Windows fallback #370, or that Windows build fails: internal/filetxn uses syscall.Stat_t without a Windows fallback #370 stays open until the full package suite passes on Windows, so the issue is not closed on a narrower contract than announced.

Verdict: ready to merge

The blocking finding tracked in correction #391 (long-path handling) is fixed in 1011b41 and adaec91. The re-run on a default Windows 11 host is in #391.