diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 66f30c3a..90e6de91 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -122,10 +122,11 @@ jobs: run: go test -v -count=1 -timeout 2m ./internal/update -run '^(TestInstallBinary|TestMoveAside)' # These tests use temporary paths, without the Unix HOME assumptions - # of the full suite. Run the native link-kind and sharing-error checks. + # of the full suite. Run the native link-kind and sharing-error checks, + # and the Codex copy checks that read NTFS ChangeTime (#1299). - name: Windows session moves if: runner.os == 'Windows' - run: go test -v -count=1 ./internal/sessions -run '^Test(WindowsSession|Move|CopyAll)' + run: go test -v -count=1 ./internal/sessions -run '^Test(WindowsSession|Move|CopyAll|CodexArchived|CodexSameTick|CodexPlainTwin)' # dsh's layered config is read and edited in temporary homes. Run # these checks on Windows too, including Desktop's home patch layer. diff --git a/internal/sessions/codex_archive_test.go b/internal/sessions/codex_archive_test.go index 1470b1ea..4f27bc8e 100644 --- a/internal/sessions/codex_archive_test.go +++ b/internal/sessions/codex_archive_test.go @@ -109,9 +109,14 @@ func TestCodexArchivedDiscoveryCacheAndAppend(t *testing.T) { if err != nil { t.Fatal(err) } - if codexChangeStamp(info) == "" { + if codexChangeTime(info).IsZero() { t.Skip("filesystem exposes no change-time stamp; safe fallback reads content") } + // The copies were written a moment ago; a minute on, their stamps have + // settled and the comparison is cached. + now := codexDiscoveryNow + codexDiscoveryNow = func() time.Time { return time.Now().Add(time.Minute) } + t.Cleanup(func() { codexDiscoveryNow = now }) reads, compare := 0, compareCodexCopies compareCodexCopies = func(a, b string) (int, error) { reads++ @@ -158,6 +163,59 @@ func TestCodexArchivedSameSizeRewriteInvalidatesDiscovery(t *testing.T) { } } +// #1299: NTFS stamps ChangeTime from Windows' clock tick (0.5 ms here, up +// to 15.6 ms), so a rewrite in the tick of the comparison keeps the stamp +// the cache saw. Here every write in the test lands in one 15.6 ms tick and +// gets the same change time, and the rewrite must still be seen. +func TestCodexSameTickRewriteIsNotCached(t *testing.T) { + d := setupCalls(t) + name := "rollout-2026-09-20T10-00-00-0190aaaa-1111-7222-8333-444455556666.jsonl" + live, archived := filepath.Join(d.codex, "sessions", name), filepath.Join(d.codex, "archived_sessions", name) + tick := time.Now().Truncate(time.Second) + stat, now := codexDiscoveryStat, codexDiscoveryNow + codexDiscoveryStat = func(path string) (os.FileInfo, error) { + info, err := os.Stat(path) + if err != nil { + return nil, err + } + return codexDiscoveryInfo{info, tick}, nil + } + codexDiscoveryNow = func() time.Time { return tick.Add(10 * time.Millisecond) } + t.Cleanup(func() { codexDiscoveryStat, codexDiscoveryNow = stat, now }) + writeLines(t, live, `{"x":1}`) + writeLines(t, archived, `{"x":1}`) + info, err := os.Stat(archived) + if err != nil { + t.Fatal(err) + } + if len(codexFiles()) != 1 { + t.Fatal("initial copies were not deduplicated") + } + writeLines(t, archived, `{"x":2}`) + if err := os.Chtimes(archived, info.ModTime(), info.ModTime()); err != nil { + t.Fatal(err) + } + if fs := codexFiles(); len(fs) != 2 { + t.Fatalf("a rewrite in the tick of the comparison reused its stale relation: %+v", fs) + } + // Once the tick is long past, the same stamps are cached. + codexDiscoveryNow = func() time.Time { return tick.Add(time.Hour) } + reads, compare := 0, compareCodexCopies + compareCodexCopies = func(a, b string) (int, error) { + reads++ + return compare(a, b) + } + t.Cleanup(func() { compareCodexCopies = compare }) + for range 3 { + if len(codexFiles()) != 2 { + t.Fatal("settled divergent copies were deduplicated") + } + } + if reads != 1 { + t.Fatalf("settled copies were compared %d times, want 1", reads) + } +} + func TestCodexArchivedReadErrorIsNotCached(t *testing.T) { d := setupCalls(t) name := "rollout-2026-09-20T10-00-00-0190aaaa-1111-7222-8333-444455556666.jsonl" diff --git a/internal/sessions/codex_discovery.go b/internal/sessions/codex_discovery.go index f1143c1f..c7984a24 100644 --- a/internal/sessions/codex_discovery.go +++ b/internal/sessions/codex_discovery.go @@ -2,13 +2,13 @@ package sessions import ( "bytes" - "fmt" "io" "os" "path/filepath" "reflect" "strings" "sync" + "time" ) const ( @@ -33,13 +33,24 @@ var codexCopyCache = struct { // Windows replaces this with a same-handle stat carrying ChangeTime. var codexDiscoveryStat = os.Stat var compareCodexCopies = readCodexCopies +var codexDiscoveryNow = time.Now + +// A file system stamps change times from the clock tick, not the precise +// clock: NTFS from Windows' system time (0.5 ms to 15.6 ms), Linux from the +// coarse kernel clock. Two writes within one tick get the same change time, +// so a comparison is cached only for files whose change time is older than +// this when it starts. A later rewrite is then in a later tick and changes +// the stamp; a file changed more recently is compared again. +const codexStampSettle = time.Second type codexDiscoveryInfo struct { os.FileInfo - ChangeTime string + ChangeTime time.Time } -func codexChangeStamp(info os.FileInfo) string { +// codexChangeTime is the file's change time, zero when the file system +// doesn't expose one. +func codexChangeTime(info os.FileInfo) time.Time { if wrapped, ok := info.(codexDiscoveryInfo); ok { return wrapped.ChangeTime } @@ -51,22 +62,29 @@ func codexChangeStamp(info os.FileInfo) string { v = v.Elem() } if v.Kind() != reflect.Struct { - return "" + return time.Time{} } for _, name := range []string{"Ctimespec", "Ctim"} { c := v.FieldByName(name) if c.IsValid() && c.Kind() == reflect.Struct { sec, nsec := c.FieldByName("Sec"), c.FieldByName("Nsec") if sec.IsValid() && nsec.IsValid() && sec.CanInt() && nsec.CanInt() { - return fmt.Sprintf("%d:%d", sec.Int(), nsec.Int()) + return time.Unix(sec.Int(), nsec.Int()) } } } - return "" + return time.Time{} +} + +// settledCodexStamp reports whether a comparison of this file, started at +// start, can be cached: it has a change time from before the current tick. +func settledCodexStamp(info os.FileInfo, start time.Time) bool { + c := codexChangeTime(info) + return !c.IsZero() && c.Before(start.Add(-codexStampSettle)) } func sameCodexSnapshot(a, b os.FileInfo) bool { - if a == nil || b == nil || a.Size() != b.Size() || !a.ModTime().Equal(b.ModTime()) || codexChangeStamp(a) != codexChangeStamp(b) { + if a == nil || b == nil || a.Size() != b.Size() || !a.ModTime().Equal(b.ModTime()) || !codexChangeTime(a).Equal(codexChangeTime(b)) { return false } if wrapped, ok := a.(codexDiscoveryInfo); ok { @@ -112,13 +130,14 @@ func distinctCodexFiles(files []file) []file { } func codexCopyRelation(first, second string) int { + start := codexDiscoveryNow() a, errA := codexDiscoveryStat(first) b, errB := codexDiscoveryStat(second) if errA != nil || errB != nil { return codexCopiesDiffer } key := codexCopyPair{first, second} - cacheable := codexChangeStamp(a) != "" && codexChangeStamp(b) != "" + cacheable := settledCodexStamp(a, start) && settledCodexStamp(b, start) codexCopyCache.Lock() saved, found := codexCopyCache.Pairs[key] codexCopyCache.Unlock() diff --git a/internal/sessions/codex_discovery_windows.go b/internal/sessions/codex_discovery_windows.go index 707d3014..f8727396 100644 --- a/internal/sessions/codex_discovery_windows.go +++ b/internal/sessions/codex_discovery_windows.go @@ -1,8 +1,8 @@ package sessions import ( - "fmt" "os" + "time" "unsafe" "golang.org/x/sys/windows" @@ -30,5 +30,6 @@ func statCodexDiscovery(path string) (os.FileInfo, error) { (*byte)(unsafe.Pointer(&basic)), uint32(unsafe.Sizeof(basic))); err != nil || basic.Change == 0 { return info, nil // no stamp means content comparisons are not cached } - return codexDiscoveryInfo{info, fmt.Sprint(basic.Change)}, nil + change := windows.Filetime{LowDateTime: uint32(basic.Change), HighDateTime: uint32(basic.Change >> 32)} + return codexDiscoveryInfo{info, time.Unix(0, change.Nanoseconds())}, nil }