From 0198e8990b45dd3f6cf34c3dde75bf29c66bd9b5 Mon Sep 17 00:00:00 2001 From: s1d3sw1ped_bot <12+s1d3sw1ped_bot@git.s1d3sw1ped.com> Date: Tue, 1 Sep 2026 13:37:56 -0500 Subject: [PATCH 1/3] docs: Add CONTRIBUTING.md Contributors need a short guide for develop-targeted PRs, commit subject form, and Gitea issue-closing rules. --- CONTRIBUTING.md | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 CONTRIBUTING.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..ca38dc7 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,40 @@ +# Contributing + +## Propose changes + +Open a pull request against `develop`. Keep the default branch for releases and +stable tips; land work on `develop` first. + +Point at an existing issue when one fits. Prefer a short issue that states the +symptom or request before a large PR. + +## Commits + +Subject form: + +``` +area: Imperative summary +``` + +- **Area** is a real package, directory, or subsystem token (`ci:`, `docs:`, + Go package name). Not a lone filename. +- **Imperative** mood: Fix, Add, Remove — not "Fixed" or "This patch…". +- No trailing period. Aim ≤ ~70–75 characters for the whole subject. +- Not conventional-commits (`feat:` / `fix:` / `chore:` as types). + +Body explains **why**. Establish the problem, then say what you are doing. +One logical change per commit; split fix and cleanup. + +## Pull requests + +Title matches the primary commit subject. + +- **What** changed +- **Why** (problem and impact) +- **Test** (concrete steps; "CI green" alone is weak) + +## Issues and closing + +Cite leftover issues by **full URL**. Gitea closes issues when `#N` appears in +merge text, so do not put `#N` in the merge message unless that issue is actually +done. Use `Fixes #N` / `Closes #N` only when the leftover work is finished. From 30a695458ec281c9bbbc957a152480e48e2e1953 Mon Sep 17 00:00:00 2001 From: s1d3sw1ped_bot <12+s1d3sw1ped_bot@git.s1d3sw1ped.com> Date: Tue, 1 Sep 2026 20:19:14 +0000 Subject: [PATCH 2/3] vfs/disk: Fix EvictDiskVisibilityAndRecreateSafety flake New() launches background calculateSizeAndPopulateIndex which scans disk and calls insertBatch. Create does not wait on initDone, so a file can be written and indexed, discovered by the scan, then removed from d.info and disk by EvictLRU/EvictBySize. insertBatch then re-inserted the stale discoveredFile without checking the path still existed, so Stat succeeded from the index while os.Stat failed. Re-stat under the lock and skip gone files so evicted keys are not resurrected. Fixes #18. --- vfs/disk/disk.go | 20 ++++++-- vfs/disk/disk_test.go | 115 +++++++++++++++++++++++++++--------------- 2 files changed, 89 insertions(+), 46 deletions(-) diff --git a/vfs/disk/disk.go b/vfs/disk/disk.go index 8dd163c..35e4cba 100644 --- a/vfs/disk/disk.go +++ b/vfs/disk/disk.go @@ -248,15 +248,25 @@ func (d *DiskFS) calculateSizeAndPopulateIndex() { // insertBatch populates info/LRU under lock for a bounded batch (follows maxEvictBatch pattern for short critical sections). // Size is incremented here only for files actually added (prevents double-count vs. concurrent Create during window). +// Fail-closed: re-stat each path under d.mu and skip if the file is gone. Create does not wait on +// initDone, so a file the scanner observed can be Evict/Delete'd (info + os.Remove) before this +// insert runs. Inserting without a live-file check would resurrect the key in d.info and make +// Stat succeed while os.Stat fails. func (d *DiskFS) insertBatch(batch []discoveredFile) { d.mu.Lock() for _, df := range batch { - if _, exists := d.info[df.key]; !exists { - fi := vfs.NewFileInfoFromOS(df.osInfo, df.key) - d.info[df.key] = fi - d.LRU.Add(df.key, fi) - d.size += df.size + if _, exists := d.info[df.key]; exists { + continue } + path := d.pathForKey(df.key) + st, err := os.Stat(path) + if err != nil { + continue + } + fi := vfs.NewFileInfoFromOS(st, df.key) + d.info[df.key] = fi + d.LRU.Add(df.key, fi) + d.size += st.Size() } d.mu.Unlock() } diff --git a/vfs/disk/disk_test.go b/vfs/disk/disk_test.go index 287eb51..f77fc7a 100644 --- a/vfs/disk/disk_test.go +++ b/vfs/disk/disk_test.go @@ -371,7 +371,8 @@ func testKey(i int) string { // artifacts for victims are immediately gone (no resurrection via lazy discovery in Stat/Open), // and that recreating the same key produces independent content that is not subject to any // stale eviction unlinks. This exercises the coordinated WLock remove path for DiskFS. -// Uses tolerant checks suitable for raw DiskFS lazy discovery + bg size. +// Create does not wait on initDone, so this also covers insertBatch racing with eviction: +// gone files must not be re-indexed (Stat present / disk missing). func TestDiskFS_EvictDiskVisibilityAndRecreateSafety(t *testing.T) { t.Parallel() td := t.TempDir() @@ -400,47 +401,21 @@ func TestDiskFS_EvictDiskVisibilityAndRecreateSafety(t *testing.T) { _ = d.EvictBySize(1024*1024, true) } - // Consistency check: never have a key absent from Stat but with a file on disk (would indicate - // either resurrection risk or orphan). If Stat succeeds, file should exist. - // A few retries tolerate the documented lazy discovery + eviction coordination windows under - // artificial "force massive eviction then immediate audit" load (especially visible under -race). - for attempt := 0; attempt < 3; attempt++ { - bad := false - for _, k := range created { - p := d.pathForKey(k) - _, statErr := d.Stat(k) - _, diskErr := os.Stat(p) - if statErr != nil { - if !os.IsNotExist(diskErr) { - bad = true - } - } else { - if diskErr != nil { - bad = true - } - } - } - if !bad { - break - } - if attempt < 2 { - time.Sleep(10 * time.Millisecond) - } else { - // On final attempt, report the last observed state for the keys - for _, k := range created { - p := d.pathForKey(k) - _, statErr := d.Stat(k) - _, diskErr := os.Stat(p) - if statErr != nil { - if !os.IsNotExist(diskErr) { - t.Errorf("key %s absent via Stat but file lingers on disk at %s (resurrection risk)", k, p) - } - } else { - if diskErr != nil { - t.Errorf("key %s present via Stat but missing on disk: %v", k, diskErr) - } - } + // Drain bg population so insertBatch cannot still be in flight when we audit. + _ = d.Size() + + // Consistency: Stat success iff the file exists on disk. insertBatch must not resurrect + // keys whose backing files were already evicted. + for _, k := range created { + p := d.pathForKey(k) + _, statErr := d.Stat(k) + _, diskErr := os.Stat(p) + if statErr != nil { + if !os.IsNotExist(diskErr) { + t.Errorf("key %s absent via Stat but file lingers on disk at %s (resurrection risk)", k, p) } + } else if diskErr != nil { + t.Errorf("key %s present via Stat but missing on disk: %v", k, diskErr) } } @@ -464,6 +439,64 @@ func TestDiskFS_EvictDiskVisibilityAndRecreateSafety(t *testing.T) { } } +// TestDiskFS_InsertBatchSkipsGoneFiles is the fail-closed contract for bg/lazy index +// insert: a discoveredFile whose path was removed (evicted) must not be re-inserted +// into d.info. That resurrection is what made Stat succeed while os.Stat failed. +func TestDiskFS_InsertBatchSkipsGoneFiles(t *testing.T) { + t.Parallel() + td := t.TempDir() + d, err := New(td, 10*1024*1024, nil) + if err != nil { + t.Fatal(err) + } + _ = d.Size() // finish constructor scan so it cannot also index these keys + + liveKey := "live" + goneKey := "gone" + writeKey := func(key, body string) os.FileInfo { + t.Helper() + p := d.pathForKey(key) + if err := os.MkdirAll(filepath.Dir(p), 0700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, []byte(body), 0600); err != nil { + t.Fatal(err) + } + st, err := os.Stat(p) + if err != nil { + t.Fatal(err) + } + return st + } + liveInfo := writeKey(liveKey, "still-here") + goneInfo := writeKey(goneKey, "about-to-vanish") + if err := os.Remove(d.pathForKey(goneKey)); err != nil { + t.Fatal(err) + } + + d.insertBatch([]discoveredFile{ + {key: liveKey, size: liveInfo.Size(), osInfo: liveInfo}, + {key: goneKey, size: goneInfo.Size(), osInfo: goneInfo}, + }) + + d.mu.RLock() + _, liveExists := d.info[liveKey] + _, goneExists := d.info[goneKey] + d.mu.RUnlock() + if !liveExists { + t.Errorf("insertBatch skipped live key %s", liveKey) + } + if goneExists { + t.Errorf("insertBatch resurrected gone key %s", goneKey) + } + if _, err := d.Stat(goneKey); err == nil { + t.Errorf("Stat succeeded for gone key %s", goneKey) + } + if _, err := os.Stat(d.pathForKey(liveKey)); err != nil { + t.Errorf("live key %s missing on disk: %v", liveKey, err) + } +} + // TestDiskFS_EvictBoundedLargeN exercises the maxEvictBatch early-break logic (Idea #2) // under a map size >> batch limit. Forces repeated eviction rounds via GC-style pressure // and asserts progress + consistency (no resurrection/orphans). Covers bounded collection From acd006d4a2a614ec884b8df294137271131a8c70 Mon Sep 17 00:00:00 2001 From: s1d3sw1ped_bot <12+s1d3sw1ped_bot@git.s1d3sw1ped.com> Date: Tue, 1 Sep 2026 20:19:19 +0000 Subject: [PATCH 3/3] ci: Skip test workflow on markdown-only pushes Docs-only pushes to main (**.md / CONTRIBUTING.md) do not need the full check-and-test job. Pull requests are unchanged. --- .gitea/workflows/test-pr.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitea/workflows/test-pr.yaml b/.gitea/workflows/test-pr.yaml index bb925aa..00ab8f2 100644 --- a/.gitea/workflows/test-pr.yaml +++ b/.gitea/workflows/test-pr.yaml @@ -4,6 +4,9 @@ on: push: branches: - main + paths-ignore: + - '**.md' + - 'CONTRIBUTING.md' jobs: check-and-test: