sessions: a Codex archived copy rewritten at the same size in the clock tick of its last comparison is compared again, not taken from the cache, on Windows too (#1299, wizzy-yang)

The copy cache trusted a change time it had seen as proof the file had not
changed. NTFS stamps ChangeTime from Windows' clock tick, not the precise
clock: on the Windows box, 300 ms of back-to-back same-size writes got a
new ChangeTime only every ~0.5 ms (2147 of 2691 writes kept the previous
stamp; diffs 0.5/1.0/1.5 ms). A rewrite in the tick of the comparison kept
the stamp the cache saw, so the cached relation was reused. That, not write
merging, is why TestCodexArchivedSameSizeRewriteInvalidatesDiscovery failed
19 of 300 runs on Windows, alone as well as in the full package.

A comparison is now cached only when both files' change times are more than
a second older than the moment it started (codexStampSettle); a file changed
more recently is compared again next time. A later rewrite then lands in a
later tick and changes the stamp. Change times are compared as times
(Windows FILETIME converted).

TestCodexSameTickRewriteIsNotCached gives every write one change time and a
clock 10 ms into that tick. With the settle rule taken out (cacheable = both
stamps present), it fails: "a rewrite in the tick of the comparison reused
its stale relation". With it, it passes, and the second half checks settled
copies are still compared once, not every time.
TestCodexArchivedDiscoveryCacheAndAppend moves its clock a minute on so the
just-written copies count as settled.

The Windows CI step now runs the Codex copy tests (CodexArchived,
CodexSameTick, CodexPlainTwin), which it had never run.

Run: Windows 11 box (cross-built sessions.test.exe, sandbox USERPROFILE/
APPDATA/TEMP): old binary 19/300 failures of the reported test; new binary
0/1000 for the three Codex copy tests, and the Windows CI regex x5 with
testdata passes. macOS: go test -race ./internal/sessions, -race -count=30 of
the Codex copy tests, go vet (darwin, GOOS=linux, GOOS=windows), gofmt -l.
Not run: Linux with a coarse ctime clock (kernels before 6.13); the rule
covers it the same way.
This commit is contained in:
yetone committed 2026-10-08 19:04:18 +08:00
1 parent 6ab4c16f68
commit 79f9b3752b
4 files changed
+92 -13

No files matched your search

+3 -2
View File
@@ -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.
+59 -1
View File
@@ -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"
+27 -8
View File
@@ -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()
+3 -2
View File
@@ -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
}