Cordy/cairn-desktop

Fork 0

P2-5: Real OS filesystem — vfs.FS with per-platform FileID #25

New issue

Closed

opened 2026-09-10 17:41:47 +00:00 by Cordy · 2 comments

Cordy commented

2026-09-10 17:41:47 +00:00

Owner

Depends on P2-4. Can be done in parallel with the remote tasks.

Goal

Implement vfs.FS against the real filesystem, satisfying exactly the contract MemFS established in Task 5 — so the engine cannot tell them apart.

Files

Create: internal/vfs/osfs.go, internal/vfs/fileid_unix.go, internal/vfs/fileid_windows.go, internal/vfs/osfs_test.go

Note: this package may use os, path/filepath and syscall. Only internal/sync is forbidden from doing so.

FileID — the whole reason for the build tags

Rename detection (Task 11) depends on an identifier that survives a rename.

Unix (//go:build !windows): the inode from syscall.Stat_t.Ino, plus the device number to be safe across mounts. Format as "dev:ino".
Windows (//go:build windows): the file index from GetFileInformationByHandle — nFileIndexHigh/nFileIndexLow, combined with the volume serial number.
If either lookup fails, return "". Empty means unknown, and the engine already treats unknown as "never matches". Silently substituting something else would cause false rename detection, which moves the wrong file.

Path handling

The engine speaks /-separated, NFC, root-relative paths. osfs owns translation to and from native paths — filepath.FromSlash outward, filepath.ToSlash plus sync.Normalise inward.
Windows long paths: prefix with \\?\ for absolute paths to escape the 260-character MAX_PATH limit.
Skip symlinks in v1. Do not follow, do not copy, do not error the run. Log and continue. Guessing here creates loops and duplicated trees.

Atomic writes

Write must not leave a truncated file if the process dies mid-write. Write to a temporary file in the same directory, then os.Rename over the target — rename within a filesystem is atomic. A half-written file that the engine then hashes and records as "synced" is silent corruption.

Steps

Write failing tests using t.TempDir():
- the full MemFS contract from Task 5, re-run against osfs (extract those assertions into a shared test helper so both implementations are verified against the same contract)
- FileID is non-empty on the test platform and survives a rename
- Write is atomic: no partial file is observable, and an interrupted write leaves the original intact
- a symlink is skipped, not followed
- a path with non-ASCII characters round-trips (this is where NFD/NFC bites on macOS)
Run; confirm failure.
Implement, with build-tagged FileID helpers.
var _ FS = (*OSFS)(nil)
Run tests; confirm they pass.
Commit: git commit -s -m "feat(vfs): real filesystem with per-platform FileID"

Acceptance criteria

The shared contract suite passes against both MemFS and OSFS.
Writes are atomic.
FileID survives renames on the platform under test, and is "" rather than wrong when unavailable.

Depends on P2-4. Can be done in parallel with the `remote` tasks. ## Goal Implement `vfs.FS` against the real filesystem, satisfying exactly the contract `MemFS` established in Task 5 — so the engine cannot tell them apart. ## Files - Create: `internal/vfs/osfs.go`, `internal/vfs/fileid_unix.go`, `internal/vfs/fileid_windows.go`, `internal/vfs/osfs_test.go` **Note:** this package may use `os`, `path/filepath` and `syscall`. Only `internal/sync` is forbidden from doing so. ## FileID — the whole reason for the build tags Rename detection (Task 11) depends on an identifier that survives a rename. - **Unix** (`//go:build !windows`): the inode from `syscall.Stat_t.Ino`, plus the device number to be safe across mounts. Format as `"dev:ino"`. - **Windows** (`//go:build windows`): the file index from `GetFileInformationByHandle` — `nFileIndexHigh`/`nFileIndexLow`, combined with the volume serial number. - **If either lookup fails, return `""`.** Empty means unknown, and the engine already treats unknown as "never matches". Silently substituting something else would cause false rename detection, which moves the wrong file. ## Path handling - The engine speaks `/`-separated, NFC, root-relative paths. `osfs` owns translation to and from native paths — `filepath.FromSlash` outward, `filepath.ToSlash` plus `sync.Normalise` inward. - **Windows long paths:** prefix with `\\?\` for absolute paths to escape the 260-character `MAX_PATH` limit. - **Skip symlinks in v1.** Do not follow, do not copy, do not error the run. Log and continue. Guessing here creates loops and duplicated trees. ## Atomic writes `Write` must **not** leave a truncated file if the process dies mid-write. Write to a temporary file in the same directory, then `os.Rename` over the target — rename within a filesystem is atomic. A half-written file that the engine then hashes and records as "synced" is silent corruption. ## Steps - [ ] Write failing tests using `t.TempDir()`: - the full `MemFS` contract from Task 5, re-run against `osfs` (extract those assertions into a shared test helper so both implementations are verified against the *same* contract) - `FileID` is non-empty on the test platform and survives a rename - `Write` is atomic: no partial file is observable, and an interrupted write leaves the original intact - a symlink is skipped, not followed - a path with non-ASCII characters round-trips (this is where NFD/NFC bites on macOS) - [ ] Run; confirm failure. - [ ] Implement, with build-tagged `FileID` helpers. - [ ] `var _ FS = (*OSFS)(nil)` - [ ] Run tests; confirm they pass. - [ ] Commit: `git commit -s -m "feat(vfs): real filesystem with per-platform FileID"` ## Acceptance criteria - The shared contract suite passes against **both** `MemFS` and `OSFS`. - Writes are atomic. - `FileID` survives renames on the platform under test, and is `""` rather than wrong when unavailable.

Cordy added this to the phase-2-integration milestone

2026-09-10 17:41:47 +00:00

Cordy added the

data-integrity

platform

labels

2026-09-10 17:41:47 +00:00

Cordy referenced this issue

2026-09-10 20:19:41 +00:00

Task 3: Path normalisation (NFC, separators) #3

Cordy commented

2026-09-11 01:50:34 +00:00

Author

Owner

Amendment — 2026-09-11: phase-1 hand-off (binding rulings Task 3 F1 and F5; final review X1, X8, X13)

These points amend "Path handling" above (filepath.ToSlash plus sync.Normalise inward).

Task 3 F1 — names that cannot be normalised safely

(a) On a non-Windows OSFS, a native name containing \ is never passed to
Normalise. Normalise would turn it into a different path. Skip the name and report it per
file with a legible reason.
(b) Every Walk checks that no two distinct native names normalise to the same engine
path. This covers backslash twins, NFC/NFD twins on ext4, and NFC compatibility singletons.
Skip every member of such a group with a reason, as #14 does for case collisions.
(c) A refused name must never count as missing, so it can never feed a deletion.
Leaving it out of Walk while it has a state row reads as a local delete, and the engine would
then delete the server copy. Since commit e7cef2d
(fix(sync): refuse non-canonical paths and their canonical twins), the engine refuses a
listed path that is not canonical, together with its canonical twin. Listing the raw name therefore
meets (c). Omitting it does not.

Task 3 F5 — keep the native name

Keep the native name observed for every entry, using a per-walk engine-path → native-name map or
the equivalent. Use it for Open, Write, Remove and Move on an existing entry.
filepath.FromSlash(enginePath) is only for names the client itself creates. On ext4, an
NFC-rebuilt path for an NFD-stored name returns ErrNotExist, which under R2 reads as a normal
race one step from a deletion.

Final review X1 — root containment (defence in depth)

The engine now refuses paths with an empty, . or .. segment, or a leading /, as Skips. It
does this on every platform (commit 6a9a06b, fix(sync): refuse paths that could resolve outside the sync root). OSFS must not rely on that alone:

Every method refuses an engine path that would resolve outside the root.
Use filepath.IsLocal on the relative path, and refuse volume names such as C: and UNC
forms on Windows.
Perform all operations through os.Root (Go ≥ 1.24, so available under the go 1.25
directive). A symlinked parent directory then cannot carry a write out of the sync folder
either.
Add a test for each of these.

Final review X13 — report the on-disk casing on a case-insensitive volume

The problem: the server has Docs/a.txt, and it is downloaded into an existing local docs/.
The engine records Docs/a.txt, while Walk later returns docs/a.txt. The case-collision
check then freezes both paths on every pass, so the subtree never syncs again. No bytes are lost.

The requirement: Walk and Stat report the name as stored on disk. A Write whose existing
ancestor differs from the engine path only in case must not silently land under the other
casing. Either refuse it with a distinct error, which becomes a Skip, or report the path actually
written, so the engine never records a path that Walk will not return. Related deferred
findings: Task 14 F3 (case-only renames) and F4 (folder twins).

Final review X8 — conflict names within NAME_MAX

ConflictName adds 37 bytes ( (conflicted copy YYYY-MM-DD HH-MM-SS)), plus N under R4. A
legal 244-byte name therefore becomes 282 bytes, beyond the 255-byte limit of ext4 and APFS and
the 255 UTF-16 units of NTFS. The conflict copy then fails on every pass. That is a permanent
Skip; both versions stay where they are.

The first task that writes conflict copies to a real disk (this one) truncates the conflict
name's stem, on a UTF-8 character boundary, so the whole segment fits. It keeps the extension,
#10's format and the R4 numbering, and adds a test with a 244-byte name. The change itself is in
internal/sync/conflict.go/conflictPath; the limit is a property of this filesystem.

## Amendment — 2026-09-11: phase-1 hand-off (binding rulings Task 3 F1 and F5; final review X1, X8, X13) These points amend "Path handling" above (`filepath.ToSlash` plus `sync.Normalise` inward). ### Task 3 F1 — names that cannot be normalised safely - **(a)** On a non-Windows OSFS, a native name containing `\` is **never passed to `Normalise`**. `Normalise` would turn it into a different path. Skip the name and report it per file with a legible reason. - **(b)** Every `Walk` checks that **no two distinct native names normalise to the same engine path**. This covers backslash twins, NFC/NFD twins on ext4, and NFC compatibility singletons. Skip every member of such a group with a reason, as #14 does for case collisions. - **(c)** A refused name must **never count as missing**, so it can never feed a deletion. Leaving it out of `Walk` while it has a state row reads as a local delete, and the engine would then delete the server copy. Since commit e7cef2d (`fix(sync): refuse non-canonical paths and their canonical twins`), the engine refuses a listed path that is not canonical, together with its canonical twin. Listing the raw name therefore meets (c). Omitting it does not. ### Task 3 F5 — keep the native name Keep the native name observed for every entry, using a per-walk engine-path → native-name map or the equivalent. Use it for `Open`, `Write`, `Remove` and `Move` on an **existing** entry. `filepath.FromSlash(enginePath)` is only for names the client itself creates. On ext4, an NFC-rebuilt path for an NFD-stored name returns `ErrNotExist`, which under R2 reads as a normal race one step from a deletion. ### Final review X1 — root containment (defence in depth) The engine now refuses paths with an empty, `.` or `..` segment, or a leading `/`, as Skips. It does this on every platform (commit 6a9a06b, `fix(sync): refuse paths that could resolve outside the sync root`). OSFS must not rely on that alone: - Every method refuses an engine path that would resolve outside the root. - Use `filepath.IsLocal` on the relative path, and refuse volume names such as `C:` and UNC forms on Windows. - Perform all operations through `os.Root` (Go ≥ 1.24, so available under the `go 1.25` directive). A symlinked parent directory then cannot carry a write out of the sync folder either. - Add a test for each of these. ### Final review X13 — report the on-disk casing on a case-insensitive volume The problem: the server has `Docs/a.txt`, and it is downloaded into an existing local `docs/`. The engine records `Docs/a.txt`, while `Walk` later returns `docs/a.txt`. The case-collision check then freezes both paths on every pass, so the subtree never syncs again. No bytes are lost. The requirement: `Walk` and `Stat` report the name as stored on disk. A `Write` whose existing ancestor differs from the engine path only in case must not silently land under the other casing. Either refuse it with a distinct error, which becomes a Skip, or report the path actually written, so the engine never records a path that `Walk` will not return. Related deferred findings: Task 14 F3 (case-only renames) and F4 (folder twins). ### Final review X8 — conflict names within NAME_MAX `ConflictName` adds 37 bytes (` (conflicted copy YYYY-MM-DD HH-MM-SS)`), plus ` N` under R4. A legal 244-byte name therefore becomes 282 bytes, beyond the 255-byte limit of ext4 and APFS and the 255 UTF-16 units of NTFS. The conflict copy then fails on every pass. That is a permanent Skip; both versions stay where they are. The first task that writes conflict copies to a real disk (this one) truncates the conflict name's stem, on a UTF-8 character boundary, so the whole segment fits. It keeps the extension, #10's format and the R4 numbering, and adds a test with a 244-byte name. The change itself is in `internal/sync/conflict.go`/`conflictPath`; the limit is a property of this filesystem.

Cordy referenced this issue from a commit

2026-09-11 01:55:32 +00:00

docs: CLAUDE.md lists the phase-1 hand-off amendments on #22, #23, #25, #26

Cordy commented

2026-09-11 07:59:23 +00:00

Author

Owner

Done

What was built

OSFS: real-filesystem vfs.FS with per-platform FileID (dev:ino Unix, vol:index Windows), atomic Write (temp file + fsync + rename), root containment via os.Root + inside()/IsLocal.
Shared 19-case contract suite run identically against MemFS and OSFS.
Safe handling of names that can't be normalised: backslash names and NFC/NFD/compat twins listed raw, never silently merged or dropped.
Fix round 1: symlinks/junctions/pipes/sockets are now listed Unsynced (not omitted); the engine Skips them and their subtree instead of reading them as deletes — closes the review's F1 data-integrity gap.
Conflict-copy names truncated on a UTF-8 boundary to fit the 255-byte filesystem limit (amendment X8).

Tests
CI run #23, linux/arm64, green. Coverage total: 90.5%. 19 contract cases × 2 FS impls, plus OSFS-specific tests (atomic write, FileID survival, symlink/pipe skip, non-ASCII round trip, on-disk casing, root-escape refusal, twin grouping), 19 mutation kills across both rounds, cross-target go vet clean for windows/amd64 and linux/arm64.

Acceptance criteria

Contract suite passes against both MemFS and OSFS — TestMemFS/TestOSFS.
Writes atomic — TestOSFSWriteIsAtomic; mutants M1/M11 killed.
FileID survives renames, "" when unavailable — TestOSFSFileIDSurvivesRenames/...IsEmptyWhenUnknown.
Symlinks skipped, not followed — refined by F1: listed Unsynced, engine Skips instead of deleting.
Non-ASCII round trip — TestOSFSNonASCIIRoundTrip.
F1(a)-(c)/F5/X1/X13/X8 amendments — each met (enginePaths, names/nativeOf, inside+os.Root, ErrDifferentSpelling, conflictName/truncateUTF8); see review for full mapping.

Rulings

P2-R1: push after clean review; close only on green CI; comment+close, never edit body; -s + Co-Authored-By.
P2-R6: Windows FileID/os.Root paths untested here; vet stays clean for windows/amd64 & linux/arm64; recorded below.
Ruling F1: add Unsynced string to FileInfo; Walk lists non-file/folder entries as Unsynced; engine Skips path+subtree before Decide — CLAUDE.md "never delete on ambiguity" and amendment F1(c) outrank the issue's plain "skip" wording.
Ruling CV2: the on-disk twin test t.Fatalfs (not Skip) on Linux, so the green CI run is real evidence it ran.
Parked CV1/CV3/CV4/CV5/CV6: Windows runtime, case-insensitive casing test, push timing, evidence regeneration, and write-durability all reasoned through or regenerated locally; no code changes required.

Deferred

F2 hard-link sibling in nameOnDisk (osfs.go:451); F3 Write drops replaced file's permission bits (osfs.go:542); F4 deleteLocal has no re-check before Remove, carried to #27; F5 interrupted-write temp files never reclaimed (osfs.go:140).
Write over a pipe still uses errLink's symlink-wording text; Windows junctions untested until #19; a new server file under a linked folder isn't downloaded while the link stands (ruling accepts as noise, not a delete).

Implemented and reviewed by Claude (subagent-driven), landed on main after review and green CI.

**Done** - [feat(vfs): real filesystem with per-platform FileID](http://192.168.10.245/Cordy/cairn-desktop/commit/c5949e9e4e2f48109cd75fd6371fb4efdf2f4d4e) - [fix(vfs): list symbolic links as unsynced so they never read as deleted](http://192.168.10.245/Cordy/cairn-desktop/commit/943bd02e01a6f9d4d3329ecef9dc864ff478ec15) **What was built** - `OSFS`: real-filesystem `vfs.FS` with per-platform `FileID` (`dev:ino` Unix, `vol:index` Windows), atomic `Write` (temp file + fsync + rename), root containment via `os.Root` + `inside()`/`IsLocal`. - Shared 19-case contract suite run identically against `MemFS` and `OSFS`. - Safe handling of names that can't be normalised: backslash names and NFC/NFD/compat twins listed raw, never silently merged or dropped. - Fix round 1: symlinks/junctions/pipes/sockets are now listed `Unsynced` (not omitted); the engine Skips them and their subtree instead of reading them as deletes — closes the review's F1 data-integrity gap. - Conflict-copy names truncated on a UTF-8 boundary to fit the 255-byte filesystem limit (amendment X8). **Tests** CI run [#23](http://192.168.10.245/Cordy/cairn-desktop/actions/runs/23), linux/arm64, green. Coverage `total: 90.5%`. 19 contract cases × 2 FS impls, plus OSFS-specific tests (atomic write, FileID survival, symlink/pipe skip, non-ASCII round trip, on-disk casing, root-escape refusal, twin grouping), 19 mutation kills across both rounds, cross-target `go vet` clean for windows/amd64 and linux/arm64. **Acceptance criteria** - Contract suite passes against both MemFS and OSFS — `TestMemFS`/`TestOSFS`. - Writes atomic — `TestOSFSWriteIsAtomic`; mutants M1/M11 killed. - FileID survives renames, `""` when unavailable — `TestOSFSFileIDSurvivesRenames`/`...IsEmptyWhenUnknown`. - Symlinks skipped, not followed — refined by F1: listed Unsynced, engine Skips instead of deleting. - Non-ASCII round trip — `TestOSFSNonASCIIRoundTrip`. - F1(a)-(c)/F5/X1/X13/X8 amendments — each met (`enginePaths`, `names`/`nativeOf`, `inside`+`os.Root`, `ErrDifferentSpelling`, `conflictName`/`truncateUTF8`); see review for full mapping. **Rulings** - P2-R1: push after clean review; close only on green CI; comment+close, never edit body; `-s` + Co-Authored-By. - P2-R6: Windows FileID/`os.Root` paths untested here; vet stays clean for windows/amd64 & linux/arm64; recorded below. - Ruling F1: add `Unsynced string` to `FileInfo`; Walk lists non-file/folder entries as Unsynced; engine Skips path+subtree before Decide — CLAUDE.md "never delete on ambiguity" and amendment F1(c) outrank the issue's plain "skip" wording. - Ruling CV2: the on-disk twin test `t.Fatalf`s (not Skip) on Linux, so the green CI run is real evidence it ran. - Parked CV1/CV3/CV4/CV5/CV6: Windows runtime, case-insensitive casing test, push timing, evidence regeneration, and write-durability all reasoned through or regenerated locally; no code changes required. **Deferred** - F2 hard-link sibling in `nameOnDisk` (osfs.go:451); F3 `Write` drops replaced file's permission bits (osfs.go:542); F4 `deleteLocal` has no re-check before `Remove`, carried to #27; F5 interrupted-write temp files never reclaimed (osfs.go:140). - `Write` over a pipe still uses errLink's symlink-wording text; Windows junctions untested until #19; a new server file under a linked folder isn't downloaded while the link stands (ruling accepts as noise, not a delete). _Implemented and reviewed by Claude (subagent-driven), landed on main after review and green CI._

Cordy closed this issue

2026-09-11 07:59:27 +00:00