Cordy/cairn-desktop

Fork 0

Task 5: Filesystem interface and in-memory implementation #5

New issue

Closed

opened 2026-09-10 17:15:16 +00:00 by Cordy · 2 comments

Cordy commented

2026-09-10 17:15:16 +00:00

Owner

Depends on Task 4.

Goal

Abstract the filesystem so the engine can be driven entirely in memory. The real OS filesystem arrives in Plan 2.

Files

Create: internal/vfs/vfs.go, internal/vfs/memfs.go, internal/vfs/memfs_test.go

Produces

type FileInfo struct {
	Path    string
	Size    int64
	ModTime time.Time
	IsDir   bool
	FileID  string // inode (Unix) / file index (Windows); survives renames; "" = unknown
}

type FS interface {
	Walk() ([]FileInfo, error)
	Open(path string) (io.ReadCloser, error)
	Write(path string, r io.Reader, mod time.Time) error
	Mkdir(path string) error
	Remove(path string) error
	Move(from, to string) error
	Stat(path string) (FileInfo, bool, error)
}

func NewMemFS() *MemFS

Contract implementations must honour

Walk returns sorted paths, all canonical (see sync.Normalise).
Write creates parent directories implicitly.
Move preserves FileID — local rename detection (Task 11) depends on this.
Remove on a directory removes its children too.
Stat returns (_, false, nil) for a missing path — missing is not an error.
An empty FileID means "unknown" and must never be treated as a match.

Steps

Write the failing tests. Cover, at minimum:
- write → stat → open round trip, with size, mtime and a non-empty FileID
- Write("x/y/z.txt", …) creates x/y
- Move preserves FileID, target exists, source is gone
- Walk output is sorted

func TestMemFSMovePreservesFileID(t *testing.T) {
	fs := NewMemFS()
	fs.Write("a.txt", strings.NewReader("v"), time.Now())
	before, _, _ := fs.Stat("a.txt")

	if err := fs.Move("a.txt", "b.txt"); err != nil {
		t.Fatalf("move: %v", err)
	}
	after, ok, _ := fs.Stat("b.txt")
	if !ok {
		t.Fatal("b.txt missing after move")
	}
	if after.FileID != before.FileID {
		t.Error("FileID must survive a move — rename detection depends on it")
	}
	if _, ok, _ := fs.Stat("a.txt"); ok {
		t.Error("a.txt should be gone after move")
	}
}

Run them, confirm they fail.
Implement MemFS. Back it with a map[string]*node plus a mutex; assign FileID from a monotonic counter. Open must return a copy of the data so callers cannot mutate stored content.
Assert the interface is satisfied: var _ FS = (*MemFS)(nil)
Run tests, confirm they pass.
Commit: git commit -s -m "feat(vfs): FS interface and in-memory implementation"

Acceptance criteria

All contract points above are covered by a named test.
MemFS is safe for concurrent use.

Depends on Task 4. ## Goal Abstract the filesystem so the engine can be driven entirely in memory. The real OS filesystem arrives in Plan 2. ## Files - Create: `internal/vfs/vfs.go`, `internal/vfs/memfs.go`, `internal/vfs/memfs_test.go` ## Produces ```go type FileInfo struct { Path string Size int64 ModTime time.Time IsDir bool FileID string // inode (Unix) / file index (Windows); survives renames; "" = unknown } type FS interface { Walk() ([]FileInfo, error) Open(path string) (io.ReadCloser, error) Write(path string, r io.Reader, mod time.Time) error Mkdir(path string) error Remove(path string) error Move(from, to string) error Stat(path string) (FileInfo, bool, error) } func NewMemFS() *MemFS ``` ## Contract implementations must honour - `Walk` returns **sorted** paths, all canonical (see `sync.Normalise`). - `Write` **creates parent directories** implicitly. - `Move` **preserves `FileID`** — local rename detection (Task 11) depends on this. - `Remove` on a directory removes its children too. - `Stat` returns `(_, false, nil)` for a missing path — missing is not an error. - An empty `FileID` means "unknown" and must never be treated as a match. ## Steps - [ ] **Write the failing tests.** Cover, at minimum: - write → stat → open round trip, with size, mtime and a non-empty `FileID` - `Write("x/y/z.txt", …)` creates `x/y` - `Move` preserves `FileID`, target exists, source is gone - `Walk` output is sorted ```go func TestMemFSMovePreservesFileID(t *testing.T) { fs := NewMemFS() fs.Write("a.txt", strings.NewReader("v"), time.Now()) before, _, _ := fs.Stat("a.txt") if err := fs.Move("a.txt", "b.txt"); err != nil { t.Fatalf("move: %v", err) } after, ok, _ := fs.Stat("b.txt") if !ok { t.Fatal("b.txt missing after move") } if after.FileID != before.FileID { t.Error("FileID must survive a move — rename detection depends on it") } if _, ok, _ := fs.Stat("a.txt"); ok { t.Error("a.txt should be gone after move") } } ``` - [ ] **Run them, confirm they fail.** - [ ] **Implement `MemFS`.** Back it with a `map[string]*node` plus a mutex; assign `FileID` from a monotonic counter. `Open` must return a **copy** of the data so callers cannot mutate stored content. - [ ] **Assert the interface is satisfied:** `var _ FS = (*MemFS)(nil)` - [ ] **Run tests, confirm they pass.** - [ ] **Commit:** `git commit -s -m "feat(vfs): FS interface and in-memory implementation"` ## Acceptance criteria - All contract points above are covered by a named test. - `MemFS` is safe for concurrent use.

Cordy added this to the phase-1-engine milestone

2026-09-10 17:15:16 +00:00

Cordy added the

engine

label

2026-09-10 17:15:16 +00:00

Cordy commented

2026-09-10 18:08:02 +00:00

Author

Owner

Amendment — 2026-09-10: files-on-demand is now in scope

Virtual files moved from "decided against for v1" to a planned phase (phase-4-virtual-files).
That changes this task, so read this before implementing.

Add a hydration flag to `FileInfo`

type FileInfo struct {
	Path    string
	Size    int64
	ModTime time.Time
	IsDir   bool
	FileID  string
	// Hydrated reports whether the file's content is actually present locally.
	// A placeholder (files-on-demand) has full metadata — name, size, mtime —
	// but no bytes on disk. Always true for MemFS and OSFS; only a
	// placeholder-aware filesystem (phase 4) ever reports false.
	Hydrated bool
}

Why it has to go in now

Without it, the engine has no way to distinguish "a file whose content I can read" from "a
file whose content would have to be downloaded to read". It would then hash placeholders to
detect changes — hydrating the entire tree on the first sync pass and defeating the whole
feature.

Adding the field now costs one line. Adding it later means changing an interface that four
packages depend on, plus every test that constructs a FileInfo.

For this task specifically

MemFS sets Hydrated: true unconditionally. It has no placeholder concept.
Add a test asserting that, so the contract is explicit rather than incidental.
Everything else in this issue is unchanged.

OSFS (P2-5) will do the same. Only the phase-4 providers ever return false.

## Amendment — 2026-09-10: files-on-demand is now in scope Virtual files moved from "decided against for v1" to a planned phase (`phase-4-virtual-files`). That changes **this task**, so read this before implementing. ### Add a hydration flag to `FileInfo` ```go type FileInfo struct { Path string Size int64 ModTime time.Time IsDir bool FileID string // Hydrated reports whether the file's content is actually present locally. // A placeholder (files-on-demand) has full metadata — name, size, mtime — // but no bytes on disk. Always true for MemFS and OSFS; only a // placeholder-aware filesystem (phase 4) ever reports false. Hydrated bool } ``` ### Why it has to go in now Without it, the engine has no way to distinguish "a file whose content I can read" from "a file whose content would have to be downloaded to read". It would then hash placeholders to detect changes — **hydrating the entire tree on the first sync pass and defeating the whole feature.** Adding the field now costs one line. Adding it later means changing an interface that four packages depend on, plus every test that constructs a `FileInfo`. ### For this task specifically - `MemFS` sets `Hydrated: true` unconditionally. It has no placeholder concept. - Add a test asserting that, so the contract is explicit rather than incidental. - Everything else in this issue is unchanged. `OSFS` (P2-5) will do the same. Only the phase-4 providers ever return `false`.

Cordy commented

2026-09-10 21:00:18 +00:00

Author

Owner

Done

e39dc13c46 — feat(vfs): FS interface and in-memory implementation
5d99e3fe78 — fix(vfs): MemFS.Move rejects unsafe destinations

What was built

internal/vfs/vfs.go: FileInfo and the FS interface (Walk, Open, Write, Mkdir, Remove, Move, Stat), verbatim to the issue's signatures.
internal/vfs/memfs.go: MemFS, a mutex-guarded in-memory FS with monotonic FileIDs; Open returns a copy of stored bytes.
internal/vfs/memfs_test.go: 15 named tests covering every contract point.
Fix round 1: Move now rejects the three unsafe destinations a real filesystem would reject (self-move no-op; into-own-descendant wraps fs.ErrInvalid; onto an existing path where either side is a directory wraps fs.ErrExist) instead of silently corrupting the tree.

Tests

go vet ./... and go test -count=1 ./... green locally (darwin) and in CI (linux).
CI run 5: http://192.168.10.245/Cordy/cairn-desktop/actions/runs/5 — linux/arm64, Go 1.25.5, green.
No coverage percentage emitted (test step runs without -cover).

Acceptance criteria

"All contract points above are covered by a named test" — met: Walk sorted+canonical, Write creates parent dirs, Move preserves FileID (issue's verbatim test), Remove recursive, Stat missing-path non-error, empty FileID never assigned.
"MemFS is safe for concurrent use" — met: every method takes the mutex; TestMemFSConcurrentUse passes under -race locally; CI's plain run still catches an unlocked map via a runtime panic.
var _ FS = (*MemFS)(nil) — present.

Rulings

R2 (brief): MemFS wraps io/fs.ErrNotExist for Open/Remove/Move on a missing path — implemented via &fs.PathError.
R12 (brief): commit subjects verbatim from the issue, git commit -s, Co-Authored-By trailer — both commits comply.
CV1 (progress.md): CI on Go 1.25.5/linux was the post-push gate — satisfied by CI run 5, green.
CV2 (progress.md): red step reproduced independently (undefined: NewMemFS) — satisfied.
CV3 (progress.md): -race evidence is local-only (CI can't run -race without cgo on linux); the plain CI run still catches an unlocked map — satisfied for #5.
CV4 (progress.md): "empty FileID never a match" — MemFS side satisfied (never assigns an empty ID); the matching-side check carries forward to #11.

Deferred

F3 (minor): "Open returns a copy" has no test.
F4 (minor): Write-keeps-FileID and Mkdir-no-op-on-existing-directory behaviours untested.
F5 (minor): kind-mismatch errors are bare fmt.Errorf strings wrapping no sentinel.
F3–F5 were explicitly out of scope for this fix round and are untouched by the diff.
Task 11 (rename propagation) and R4 (conflict handling) impact from a corrupt MemFS tree — outside this task, not assessed here.

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

**Done** - http://192.168.10.245/Cordy/cairn-desktop/commit/e39dc13c46b7d8e0fa28799f701b4d20ba0182dc — feat(vfs): FS interface and in-memory implementation - http://192.168.10.245/Cordy/cairn-desktop/commit/5d99e3fe78db9e6c3bbb5c4f608127280183359c — fix(vfs): MemFS.Move rejects unsafe destinations **What was built** - `internal/vfs/vfs.go`: `FileInfo` and the `FS` interface (Walk, Open, Write, Mkdir, Remove, Move, Stat), verbatim to the issue's signatures. - `internal/vfs/memfs.go`: `MemFS`, a mutex-guarded in-memory `FS` with monotonic `FileID`s; `Open` returns a copy of stored bytes. - `internal/vfs/memfs_test.go`: 15 named tests covering every contract point. - Fix round 1: `Move` now rejects the three unsafe destinations a real filesystem would reject (self-move no-op; into-own-descendant wraps `fs.ErrInvalid`; onto an existing path where either side is a directory wraps `fs.ErrExist`) instead of silently corrupting the tree. **Tests** - `go vet ./...` and `go test -count=1 ./...` green locally (darwin) and in CI (linux). - CI run 5: http://192.168.10.245/Cordy/cairn-desktop/actions/runs/5 — linux/arm64, Go 1.25.5, green. - No coverage percentage emitted (test step runs without `-cover`). **Acceptance criteria** - "All contract points above are covered by a named test" — met: Walk sorted+canonical, Write creates parent dirs, Move preserves FileID (issue's verbatim test), Remove recursive, Stat missing-path non-error, empty FileID never assigned. - "MemFS is safe for concurrent use" — met: every method takes the mutex; `TestMemFSConcurrentUse` passes under `-race` locally; CI's plain run still catches an unlocked map via a runtime panic. - `var _ FS = (*MemFS)(nil)` — present. **Rulings** - R2 (brief): MemFS wraps `io/fs.ErrNotExist` for Open/Remove/Move on a missing path — implemented via `&fs.PathError`. - R12 (brief): commit subjects verbatim from the issue, `git commit -s`, `Co-Authored-By` trailer — both commits comply. - CV1 (progress.md): CI on Go 1.25.5/linux was the post-push gate — satisfied by CI run 5, green. - CV2 (progress.md): red step reproduced independently (`undefined: NewMemFS`) — satisfied. - CV3 (progress.md): `-race` evidence is local-only (CI can't run `-race` without cgo on linux); the plain CI run still catches an unlocked map — satisfied for #5. - CV4 (progress.md): "empty FileID never a match" — MemFS side satisfied (never assigns an empty ID); the matching-side check carries forward to #11. **Deferred** - F3 (minor): "Open returns a copy" has no test. - F4 (minor): Write-keeps-FileID and Mkdir-no-op-on-existing-directory behaviours untested. - F5 (minor): kind-mismatch errors are bare `fmt.Errorf` strings wrapping no sentinel. - F3–F5 were explicitly out of scope for this fix round and are untouched by the diff. - Task 11 (rename propagation) and R4 (conflict handling) impact from a corrupt MemFS tree — outside this task, not assessed here. _Implemented and reviewed by Claude (subagent-driven), landed on main after review and green CI._

Cordy closed this issue

2026-09-10 21:00:24 +00:00